preserve supplemental codecs in publish mode

[orgads]

Add RED (RFC 2198) codec definition to codeclib as a supplemental codec. Modify codec_store_accept_one() to preserve supplemental codecs whose clock rate matches the accepted primary codec. Skip supplemental codecs in codec_store_is_full_answer() since they are optional in SDP answers.

[orgads]

Rebased on top of #2101 to fix CI.

[orgads]

@rfuchs ping

[rfuchs]

Seems like this is a little misbehaved in that it accepts all supplemental codecs, and not just the ones matching the accepted primary codec.

When I publish with

m=audio 21326 RTP/AVP 111 9 0 8 110 101 96 97
a=rtpmap:111 opus/48000/2
a=rtcp-fb:111 transport-cc
a=fmtp:111 minptime=10;useinbandfec=1
a=rtpmap:9 G722/8000
a=rtpmap:0 PCMU/8000
a=rtpmap:8 PCMA/8000
a=rtpmap:110 telephone-event/48000
a=rtpmap:101 telephone-event/8000
a=rtpmap:96 RED/8000
a=rtpmap:97 RED/48000

it answers with

m=audio 34470 RTP/AVP 111 110 101 96 97
c=IN IP4 192.168.1.66
a=rtpmap:111 opus/48000/2
a=fmtp:111 useinbandfec=1; minptime=10
a=rtcp-fb:111 transport-cc
a=rtpmap:110 telephone-event/48000
a=rtpmap:101 telephone-event/8000
a=rtpmap:96 RED/8000
a=rtpmap:97 RED/48000

The 48000 ones aren't supposed to be there.

Also the test outputs should be exact matches as much as possible, with regex matches only where variable output is expected (e.g. port numbers).

[orgads]

Good catch! Should be fixed now.

[orgads]

hmm... Turns out our SBC sends hardcoded RED/8000, which doesn't always match the primary codec. Does it make sense to keep "single" RED even if it doesn't match the sample rate?

We will fix the SBC, but this will take time. So we can either keep using a patched version of rtpengine, or work out a compatible solution...

[rfuchs]

Pff... Does it provide a a=fmtp for the red encoding, indicating which other codecs it applies to?

Does it make sense to keep "single" RED even if it doesn't match the sample rate?

I suppose so? If the offerer doesn't agree with it, then technically they should just ignore it. OTOH I wouldn't be surprised if this lead to failures in some other fringe cases 🤦

[orgads]

Pff... Does it provide a a=fmtp for the red encoding, indicating which other codecs it applies to?

Unfortunately not. Sample SDP:

v=0
o=AudiocodesGW 870945942 1165690303 IN IP4 10.2.3.4
s=SBC-Call
c=IN IP4 10.2.3.4
t=0 0
m=audio 24400 RTP/AVP 104 97 101
c=IN IP4 10.2.3.4
a=ptime:20
a=sendonly
a=rtpmap:104 SILK/16000
a=fmtp:104 maxaveragebitrate=50000; useinbandfec=0; minptime=20
a=rtpmap:97 RED/8000
a=rtpmap:101 telephone-event/16000
a=fmtp:101 0-15,16

Does it make sense to keep "single" RED even if it doesn't match the sample rate?

I suppose so? If the offerer doesn't agree with it, then technically they should just ignore it. OTOH I wouldn't be surprised if this lead to failures in some other fringe cases 🤦

I'll need to discuss this internally first. Since we won't be updating to upstream rtpengine soon (and when we will, I hope to get the SBC fix along with that), I suppose you can just merge it as it is now (if it is acceptable of course). We'll use the patched version until then.

Thank you!

[orgads]

Rebased to resolve the conflict.

[rfuchs]

All right, thank you.

We could use more/proper support for RED anyway, but this is a wishlist item for now.