Patch Set 2: Code-Review-1 (7 comments)
https://gerrit.osmocom.org/#/c/7793/2/openbsc/include/openbsc/mgcp_internal.h 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. https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/libmgcp/mgcp_protocol.c 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? https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/libmgcp/mgcp_vty.c 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. https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c 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) https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c 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 https://gerrit.osmocom.org/7793 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: daniel <dwillm...@sysmocom.de> Gerrit-HasComments: Yes
