MT#55283 Treat missing a=fmtp as defaults for AMR

Per RFC 4867 section 8.1, absent fmtp parameters imply defaults (bandwidth-efficient mode, no CRC,
no robust-sorting, no interleaving). Previously, codec matching was rejected when either side had no
``a=fmtp`` attribute for the AMR payload type, causing AMR to be dropped as a "stray answer codec".

This patch changes this behavior to instead "parse" fmtp even when absent so that the format struct
is populated with zero-initialized defaults and fmtp_parsed is set.

Closes #2141

Change-Id: I1371cb1fdb9354746a4019477845d777e619a2f6
pull/2142/head
Alexander Bakker 3 weeks ago committed by Richard Fuchs
parent ef39a3e8ae
commit 702ccec969

@ -122,7 +122,11 @@ static void amr_parse_format_cb(str *key, str *token, void *data) {
}
}
static bool amr_format_parse(struct rtp_codec_format *f, const str *fmtp) {
codeclib_key_value_parse(fmtp, true, amr_parse_format_cb, f);
// Per RFC 4867 section 8.1, missing fmtp parameters use defaults (bandwidth-efficient mode, no
// CRC, no robust-sorting, no interleaving). Parse whatever is present. Absent fields keep their
// defaults.
if (fmtp && fmtp->len)
codeclib_key_value_parse(fmtp, true, amr_parse_format_cb, f);
return true;
}
static void amr_set_encdec_options(codec_options_t *opts, codec_def_t *def) {

@ -149,8 +149,6 @@ bool codec_parse_fmtp(codec_def_t *def, struct rtp_codec_format *fmtp, const str
return false;
if (!def->format_parse)
return true;
if (!fmtp_string)
return true;
if (!fmtp) {
ZERO(fmtp_store);
fmtp = &fmtp_store;
@ -160,6 +158,8 @@ bool codec_parse_fmtp(codec_def_t *def, struct rtp_codec_format *fmtp, const str
*copy = fmtp->parsed;
return true;
}
// Call format_parse even without fmtp_string so that codecs can populate defaults and set
// fmtp_parsed = true (e.g. AMR with RFC 4867 defaults).
bool ret = def->format_parse(fmtp, fmtp_string);
if (ret) {
fmtp->fmtp_parsed = true;

@ -3,6 +3,8 @@
static bool ilbc_format_parse(struct rtp_codec_format *f, const str *fmtp) {
if (!fmtp || !fmtp->len)
return false;
switch (__csh_lookup(fmtp)) {
case CSH_LOOKUP("mode=20"):
f->parsed.ilbc.mode = 20;

@ -207,4 +207,196 @@ a=rtcp:PORT
SDP
#done_testing;NGCP::Rtpengine::AutoTest::terminate('f00');exit;
# AMR-WB with octet-align=0 in offer, answer without fmtp (defaults match)
new_call;
offer('AMR-WB octet-align=0 offer, answer missing fmtp - should match', {
'rtpp-flags' => 'replace-origin address-family=IP4 transport-protocol=RTP/AVP',
'from-tag' => ft(),
}, <<SDP);
v=0
o=- 1545997027 1 IN IP4 198.51.100.1
s=tester
t=0 0
m=audio 2000 RTP/AVP 96 101
a=rtpmap:96 AMR-WB/16000
a=fmtp:96 octet-align=0
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
c=IN IP4 198.51.100.1
a=sendrecv
----------------------------------
v=0
o=- 1545997027 1 IN IP4 203.0.113.1
s=tester
t=0 0
m=audio PORT RTP/AVP 96 101
c=IN IP4 203.0.113.1
a=rtpmap:96 AMR-WB/16000
a=fmtp:96 octet-align=0
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=sendrecv
a=rtcp:PORT
SDP
# Answer without fmtp: per RFC 4867 s8.1, missing fmtp = defaults = octet-align=0
# This should match the offer's octet-align=0
answer('AMR-WB octet-align=0 offer, answer missing fmtp - should match', {
'rtpp-flags' => 'replace-origin address-family=IP4 transport-protocol=RTP/AVP',
'from-tag' => ft(),
'to-tag' => tt(),
}, <<SDP);
v=0
o=- 1545997027 1 IN IP4 198.51.100.3
s=tester
t=0 0
m=audio 2002 RTP/AVP 96 101
a=rtpmap:96 AMR-WB/16000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
c=IN IP4 198.51.100.3
a=sendrecv
--------------------------------------
v=0
o=- 1545997027 1 IN IP4 203.0.113.1
s=tester
t=0 0
m=audio PORT RTP/AVP 96 101
c=IN IP4 203.0.113.1
a=rtpmap:96 AMR-WB/16000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=sendrecv
a=rtcp:PORT
SDP
# AMR-WB with octet-align=1 in offer, answer without fmtp (incompatible)
new_call;
offer('AMR-WB octet-align=1 offer, answer missing fmtp - incompatible', {
'rtpp-flags' => 'replace-origin address-family=IP4 transport-protocol=RTP/AVP',
'from-tag' => ft(),
}, <<SDP);
v=0
o=- 1545997027 1 IN IP4 198.51.100.1
s=tester
t=0 0
m=audio 2000 RTP/AVP 96 0 101
a=rtpmap:96 AMR-WB/16000
a=fmtp:96 octet-align=1
a=rtpmap:0 PCMU/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
c=IN IP4 198.51.100.1
a=sendrecv
----------------------------------
v=0
o=- 1545997027 1 IN IP4 203.0.113.1
s=tester
t=0 0
m=audio PORT RTP/AVP 96 0 101
c=IN IP4 203.0.113.1
a=rtpmap:96 AMR-WB/16000
a=fmtp:96 octet-align=1
a=rtpmap:0 PCMU/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=sendrecv
a=rtcp:PORT
SDP
# Answer without fmtp: missing = defaults = octet-align=0, which != octet-align=1
# AMR-WB should be rejected, PCMU should remain
answer('AMR-WB octet-align=1 offer, answer missing fmtp - incompatible', {
'rtpp-flags' => 'replace-origin address-family=IP4 transport-protocol=RTP/AVP',
'from-tag' => ft(),
'to-tag' => tt(),
}, <<SDP);
v=0
o=- 1545997027 1 IN IP4 198.51.100.3
s=tester
t=0 0
m=audio 2002 RTP/AVP 96 0 101
a=rtpmap:96 AMR-WB/16000
a=rtpmap:0 PCMU/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
c=IN IP4 198.51.100.3
a=sendrecv
--------------------------------------
v=0
o=- 1545997027 1 IN IP4 203.0.113.1
s=tester
t=0 0
m=audio PORT RTP/AVP 0 101
c=IN IP4 203.0.113.1
a=rtpmap:0 PCMU/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=sendrecv
a=rtcp:PORT
SDP
# AMR-WB with no fmtp in both offer and answer (both use defaults, should match)
new_call;
offer('AMR-WB no fmtp in offer or answer - defaults match', {
'rtpp-flags' => 'replace-origin address-family=IP4 transport-protocol=RTP/AVP',
'from-tag' => ft(),
}, <<SDP);
v=0
o=- 1545997027 1 IN IP4 198.51.100.1
s=tester
t=0 0
m=audio 2000 RTP/AVP 96 101
a=rtpmap:96 AMR-WB/16000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
c=IN IP4 198.51.100.1
a=sendrecv
----------------------------------
v=0
o=- 1545997027 1 IN IP4 203.0.113.1
s=tester
t=0 0
m=audio PORT RTP/AVP 96 101
c=IN IP4 203.0.113.1
a=rtpmap:96 AMR-WB/16000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=sendrecv
a=rtcp:PORT
SDP
answer('AMR-WB no fmtp in offer or answer - defaults match', {
'rtpp-flags' => 'replace-origin address-family=IP4 transport-protocol=RTP/AVP',
'from-tag' => ft(),
'to-tag' => tt(),
}, <<SDP);
v=0
o=- 1545997027 1 IN IP4 198.51.100.3
s=tester
t=0 0
m=audio 2002 RTP/AVP 96 101
a=rtpmap:96 AMR-WB/16000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
c=IN IP4 198.51.100.3
a=sendrecv
--------------------------------------
v=0
o=- 1545997027 1 IN IP4 203.0.113.1
s=tester
t=0 0
m=audio PORT RTP/AVP 96 101
c=IN IP4 203.0.113.1
a=rtpmap:96 AMR-WB/16000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=sendrecv
a=rtcp:PORT
SDP
done_testing();

@ -119,6 +119,7 @@ static void __start(const char *file, int line) {
}
#define transcode(codec) t_queue_push_tail(&flags.codec_transcode, sdup(#codec))
#define transcode_s(s) t_queue_push_tail(&flags.codec_transcode, sdup(s))
#define c_accept(codec) t_queue_push_tail(&flags.codec_accept, sdup(#codec))
#define c_consume(codec) t_queue_push_tail(&flags.codec_consume, sdup(#codec))
#define c_mask(codec) t_queue_push_tail(&flags.codec_mask, sdup(#codec))
@ -621,6 +622,49 @@ int main(void) {
packet_seq(A, 96, AMR_WB_payload_noe, 0, 0, -1, ""); // nothing due to resampling/decoding buffer
packet_seq_nf(A, 96, AMR_WB_payload_noe, 320, 1, 0, PCMU_payload);
end();
// forward AMR-WB, answer without fmtp (missing = defaults = octet-align=0)
// offer has octet-align=0, answer omits fmtp -> should match
start();
sdp_pt(0, PCMU, 8000);
transcode_s("AMR-WB/16000/1///octet-align=0");
offer();
expect(A, "0/PCMU/8000");
expect(B, "0/PCMU/8000 96/AMR-WB/16000/octet-align=0");
sdp_pt(96, AMR-WB, 16000); // answer without fmtp
answer();
expect(A, "0/PCMU/8000");
expect(B, "96/AMR-WB/16000");
end();
// forward AMR-WB, answer without fmtp but offer has octet-align=1
// these are incompatible
start();
sdp_pt(0, PCMU, 8000);
transcode(AMR-WB);
offer();
expect(A, "0/PCMU/8000");
expect(B, "0/PCMU/8000 96/AMR-WB/16000/octet-align=1;mode-change-capability=2");
sdp_pt(0, PCMU, 8000);
sdp_pt(96, AMR-WB, 16000); // answer with PCMU + AMR-WB (no fmtp implies octet-align=0)
answer();
// AMR-WB should be rejected as incompatible (octet-align=0 != octet-align=1), only PCMU remains
expect(A, "0/PCMU/8000");
expect(B, "0/PCMU/8000");
end();
// forward AMR-WB, answer with matching octet-align=1
start();
sdp_pt(0, PCMU, 8000);
transcode(AMR-WB);
offer();
expect(A, "0/PCMU/8000");
expect(B, "0/PCMU/8000 96/AMR-WB/16000/octet-align=1;mode-change-capability=2");
sdp_pt_fmt(96, AMR-WB, 16000, "octet-align=1");
answer();
expect(A, "0/PCMU/8000");
expect(B, "96/AMR-WB/16000/octet-align=1");
end();
}
}

Loading…
Cancel
Save