osmo-mgw "decides" for a codec during MGCP handling; instead it should handle all configured payload type numbers as they arrive in RTP
This is how it should be:
- CRCX/MDCX sets up two conns of an endpoint, each with one or more codecs listed.
- At some point, RTP packets arrive, and the payload type number indicates the RTP payload's actual codec, as was listed in CRCX/MDCX.
When RTP packets arrive, that is when osmo-mgw needs to decide which codec to forward to the other side,
and that is when errors can happen from not being able to transcode. Not before.
However, working with mgcp_test.c, I notice that osmo-mgw already does some sort of "codec decision" when handling the CRCX message.
By adding another "CRCX" test with AMR#112 and GSM#3, I suddenly get this failure during CRCX handling:
mgcp_protocol.c:833 endpoint:rtpbridge/1@mgw CI:91D7CDD4 CRCX: codec negotiation failure
mgcp_protocol.c:1149 endpoint:rtpbridge/1@mgw CRCX: unable to create connection
resulting in an MGCP protocol error:
534 2 FAIL
Investigation shows that in mgcp_tests.c, there somehow already is another conn on the endpoint, configured as EFR only,
meaning there is no match with [AMR,GSM]; figured out from this dbg output of conn_src and conn_dst:
mgcp_codec.c:443 src 112 AMR/8000 AMR
mgcp_codec.c:443 src 3 GSM/8000/1 GSM
mgcp_codec.c:449 dst 97 GSM-EFR/8000 GSM-EFR
So this fails only on the basis of the two codecs lists on two sides of an endpoint, at CRCX time.
Even if a client sets up two sides of an endpoint with unresolvable codecs (for TFO), the CRCX/MDCX to do so must be allowed.
The client may still MDCX later with better codecs, for example.
Also the client is free to choose codecs that don't match,
and generally do as much foot shooting as they like -- it's completely fine
to pick stupid codec lists that are suboptimal or not transcodable, as far as MGCP is concerned.
The only reason to completely fail CRCX/MDCX because of codecs would be: not a single one of the listed codecs is known to osmo-mgw.
Yes, we should reject codecs unknown to osmo-mgw, but we should do so only by returning a reduced list in the MGCP OK response,
so that only the known codecs are returned. Not by failing the MGCP protocol.
During MGCP handling, osmo-mgw has absolutely no need and no business to decide for a specific codec, or even check whether there is a common codec.
In code, these are the problematic places:
mgcp_codec_decide() called from handle_codec_info() called from handle_create_con()/_modify_con() in mgcp_protocol.c.
These are the handlers for CRCX / MDCX, resp.
That is the only place mgcp_codec_decide() is called.
There should not be a "codec" member in struct mgcp_rtp_end:
/* currently selected audio codec */
struct mgcp_rtp_codec *codec;
It suggests that an endpoint needs to pick one codec, which it never should do. Instead, an endpoint should have a list of codecs with corresponding payload type numbers, and should figure out what to do with incoming RTP payload type numbers, when they come.
It looks like mgcp_codec_decide() figures out how to process incoming RTP packets already at CRCX/MDCX, stores the decision in end.codec, and then handles all RTP the same...?
Which so far has never been a problem because our clients assign only one codec,
and no-one is testing dynamically switching pt numbers in an active RTP stream on osmo-mgw?
(I think actually it is a good idea to do all possible payload type conversion decisions early at CRCX/MDCX time and cache the results, so that an ongoing RTP stream doesn't repeat the same calculations for each and every RTP packet. But do not only choose one single codec, rather cache the best match for every configured PT, in both directions across the endp.)