Attention is currently required from: dexter.
Patch set 2:Code-Review -1
4 comments:
Commit Message:
Patch Set #2, Line 10: he which to used and optionally a payload type map (ptmap) that is used
"he which" makes no sense :)
Patchset:
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:
Patch Set #2, 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:
Patch Set #2, 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 change 34349. To unsubscribe, or for help writing mail filters, visit settings.