emiago
Diago doesn't support dynamic payload types for incoming offers with Opus on ptypes != 96
If I were to have an an endpoint send an INVITE w/ SDP to a diago UAS offering only opus on a ptype other than 96, updateRemoteCodecs() in MediaSession.go will not be able to match the codec, since it currently compares all fields of rc vs c.
The first part of this is to loosen the codec match to be just codec name, sample rate, and channel count. Ptypes are dynamic, so it shouldn't be compared. Ptimes are generally a suggestion, not a mandate, so that ideally shouldn't be a comparison point here, though as currently implemented, the comparisons are always true there.
Beyond that, if the peer offers opus on a different payload type, let's say 127, you've got two choices.
Option 1: you respond back with ptype 96 in your answer. Here, you expect to receive media on with ptype 96, but you send media to your peer on ptype 127. You mainly need to maintain a local vs remote ptype value to track this. There's a few places that do a lookup on ptypes, that'll need a fixup, but it's simple. Same code should be sufficient if Diago is a UAC offering opus on PT 96, and the peer responds with PT 127 (you expect to receive on PT 96, still send using 127).
Option 2: You renumber your ptypes to match the offer, so on an incoming invite, you'd change the local ptype to 127. This is a SHOULD item from RFC 3264, section 6.1 (though option 1 is still completely legal). It still requires much of the same work as Option 1, but you have to track an offer SDP vs an answer SDP, as you don't want to accidentally renumber your local PayloadTypes in response to an answer SDP, plus you need to have a dynamic look of of PayloadType to Codec everywhere in the code.
Also, something to think about for the future: you might need to have special comparison code to compare codecs by FMTP params (eg. H.264's packetization-mode param and G.722.1's bitrate param would mandate a distinct ptype). Of course with just G.711 and Opus supported, that's something for later.
I have a patch that'll cover option 1. The simpler implementation is good enough for what I'm doing, where diago acts as a UAS in a "glue" service. It's based on the v.0.20.0 tag.
Hi @jeffwallace-media thanks for detailed response. Yes Codec matching and this support needs more work. I kind agree opus here was more exception and mostly should not have some fixed pt. I think usage or requirements were more on client side so not so much server is checked.
We would probably lean to RFC recommendation. There is somewhere open more topic to have this codec transcoding unrelated to static values in code. So point taken, will be fixed.
