Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34349?usp=email )
Change subject: mgcp_client_fsm: allow the same codec multiple times in ptmap ...................................................................... Patch Set 2: Code-Review-1 (4 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/34349/comment/f91b30ef_42706e8e PS2, Line 10: he which to used and optionally a payload type map (ptmap) that is used "he which" makes no sense :) Patchset: PS2: After looking at the whole patch, I have the feeling this patch is a HACK and not a real fix. Looks like you should be looking at more information and not only "codec" in order to find the correct payload type? I'm not sure this should be merged as it is, specially because it adds a new public API which basically still has the same/similar problem as the previous one. File include/osmocom/mgcp_client/mgcp_client_fsm.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34349/comment/266edcde_ea066701 PS2, Line 39: * codecs array above). In case the same codec type (enum mgcp_codecs) is appearing multiple times in codecs. This sentence is wrong and I'm not following it: "In case ..." you are missing a"then" clause in the same sentence. File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34349/comment/94c8ff42_4a55f20d PS2, Line 152: unsigned int map_codec_to_pt2(bool *ptmap_used, const struct ptmap *ptmap, I'd say better move the "ptmap_used" arg to the end, since it's the only in-out param (so when reading from left to right you see input then output). Maybe not to the end, but after "ptmap" argument (because they share ptmap_len). Since it's called map_codec_to_pt, maybe it even makes sense to have them this way: map_codec_to_pt2(enum mgcp_codecs code, const struct ptmap *ptmap, bool *ptmap_used, unsigned int ptmap_len); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34349?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ie13ce59d3165936a16e16588b4d58f0ce7e0ae67 Gerrit-Change-Number: 34349 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: dexter <[email protected]> Gerrit-Comment-Date: Fri, 08 Sep 2023 16:39:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
