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 <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 08 Sep 2023 16:39:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment