Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11060 )
Change subject: codec_pref: handle S0-S15 in ASSIGNMENT REQUEST ...................................................................... Patch Set 8: (9 comments) I must admit that I see the general idea, which looks good, but I don't fully grok the fixme-aspects. So I just found a few typos while trying to figure those out... not important. https://gerrit.osmocom.org/#/c/11060/8//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/11060/8//COMMIT_MSG@15 PS8, Line 15: in the ASSIGNMENT COMPLETE speech codec (choosen) field are hardcoded. "chosen" (yes, "to choose", but "chosen") https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c File src/osmo-bsc/codec_pref.c: https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@177 PS8, Line 177: /* Helper function to generate the bss supported amr configuration bits (S0-S15) */ (I prefer not writing "Helper function" everywhere, all functions except main() could be seen as "helpers"; just say what the function does.) https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@185 PS8, Line 185: /* Lookup the BTS specific AMR rate configuration. Thsi config is set "This" https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@286 PS8, Line 286: { (unusual position for '{') https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@288 PS8, Line 288: * configuration that we did not previously advertised as "did .. advertise" https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@290 PS8, Line 290: * configuration enters the further processing steps we agein "again" https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c File src/osmo-bsc/lchan_fsm.c: https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c@435 PS8, Line 435: /* FIXME: At the moment we can not support multiple codc rates in one "codec" https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c@437 PS8, Line 437: * for Threshold and Hysteresis. Those fiels are not needed when only "fields" https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c@440 PS8, Line 440: * struct would then also be different because each rate neads its "needs" -- To view, visit https://gerrit.osmocom.org/11060 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d8ded51b3eb4c003fe2da6f2d6f48d001b73737 Gerrit-Change-Number: 11060 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <[email protected]> Gerrit-Assignee: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-CC: Neels Hofmeyr <[email protected]> Gerrit-Comment-Date: Tue, 02 Oct 2018 13:56:34 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
