Patch Set 2: Code-Review-1

File openbsc/include/openbsc/mgcp_internal.h:

Line 215:       uint32_t bts_jitter_delay_min;
what units is this? ms? us? bytes? codec-frames?  If the varioable name doesn't 
indicate this (like delay_ms_min) the comment should at least state it.
File openbsc/src/libmgcp/mgcp_protocol.c:

Line 872:                       if(endp->bts_use_jibuf) {
"if" is not a function (space needed)

Line 887:       if(endp->bts_use_jibuf) {
same here

Line 1357:      if(endp->bts_jb)
and again?
File openbsc/src/libmgcp/mgcp_vty.c:

Line 1371:       DEJITTER_STR " Minimum Delay\n" "Minimum Delay\n")
see my other comment at the struct definition.  The VTY user has no idea about 
the unit of the value he's supposed to enter here.  the "?" help should make 
that clear.
File openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c:

Line 589:               if(bsc_endp->bsc->cfg->bts_use_jibuf_override)
if is not a function (below two more instances)
File openbsc/src/osmo-bsc_nat/bsc_nat_vty.c:

Line 1266:       "bts-jitter-buffer-delay-min <1-65535>",
same comment related to "unit" of the value

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <>
Gerrit-Reviewer: Harald Welte <>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <>
Gerrit-Reviewer: daniel <>
Gerrit-HasComments: Yes

Reply via email to