From 1055a1e71e3b5d63912f5adc1e8e58b8a10a4f58 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 7 Apr 2020 12:06:58 -0400 Subject: [PATCH] TT#79351 detect and fix payload type collisions fixes #963 Change-Id: I938b1c4b9baed37caf718f7537bbb9c11f7b91e5 --- daemon/codec.c | 65 +++++++++++++++++++++++++++++++----------- lib/rtplib.c | 12 ++++++-- lib/rtplib.h | 1 + t/auto-daemon-tests.pl | 60 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 116 insertions(+), 22 deletions(-) diff --git a/daemon/codec.c b/daemon/codec.c index dc047186e..6b1260b43 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -430,6 +430,21 @@ static int __dtmf_payload_type(GHashTable *dtmf_sinks, struct rtp_payload_type * return dtmf_payload_type; } +static int __unused_pt_number(struct call_media *media, int num) { + if (num < 0) + num = 96; // default first dynamic payload type number + while (1) { + if (!g_hash_table_lookup(media->codecs_recv, &num)) + break; // OK + num++; + if (num < 96) // if an RFC type was taken already + num = 96; + else if (num >= 128) + return -1; + } + return num; +} + static void __accept_transcode_codecs(struct call_media *receiver, struct call_media *sink) { // if the other side is transcoding, we need to accept codecs that were // originally offered (recv->send) if we support them, even if the @@ -440,7 +455,9 @@ static void __accept_transcode_codecs(struct call_media *receiver, struct call_m ensure_codec_def(pt, receiver); if (!pt->codec_def) continue; - if (g_hash_table_lookup(receiver->codecs_recv, &pt->payload_type)) { + struct rtp_payload_type *existing_pt + = g_hash_table_lookup(receiver->codecs_recv, &pt->payload_type); + if (existing_pt && !rtp_payload_type_cmp_nf(existing_pt, pt)) { // already present. // to keep the order intact, we seek the list for the position // of this codec entry. all newly added codecs must come after @@ -458,6 +475,28 @@ static void __accept_transcode_codecs(struct call_media *receiver, struct call_m continue; } + if (existing_pt) { + // PT collision. We must renumber one of the entries. `pt` is taken + // from the send list, so the PT should remain the same. Renumber + // the existing entry. + int new_pt = __unused_pt_number(receiver, existing_pt->payload_type); + if (new_pt < 0) { + ilog(LOG_WARN, "Ran out of RTP payload type numbers while accepting '" + STR_FORMAT "' due to '" STR_FORMAT "'", + STR_FMT(&pt->encoding_with_params), + STR_FMT(&existing_pt->encoding_with_params)); + continue; + } + ilog(LOG_DEBUG, "Renumbering '" STR_FORMAT "' from PT %i to %i due to '" STR_FORMAT "'", + STR_FMT(&existing_pt->encoding_with_params), + existing_pt->payload_type, + new_pt, + STR_FMT(&pt->encoding_with_params)); + g_hash_table_steal(receiver->codecs_recv, &existing_pt->payload_type); + existing_pt->payload_type = new_pt; + g_hash_table_insert(receiver->codecs_recv, &existing_pt->payload_type, existing_pt); + } + ilog(LOG_DEBUG, "Accepting offered codec " STR_FORMAT " due to transcoding", STR_FMT(&pt->encoding_with_params)); MEDIA_SET(receiver, TRANSCODE); @@ -1996,23 +2035,15 @@ static struct rtp_payload_type *codec_add_payload_type(const str *codec, struct if (pt == (void *) 0x1) return NULL; - // find an unused payload type number - if (pt->payload_type < 0) - pt->payload_type = 96; // default first dynamic payload type number - while (1) { - if (!g_hash_table_lookup(media->codecs_recv, &pt->payload_type)) - break; // OK - pt->payload_type++; - if (pt->payload_type < 96) // if an RFC type was taken already - pt->payload_type = 96; - else if (pt->payload_type >= 128) { - ilog(LOG_WARN, "Ran out of RTP payload type numbers while adding codec '" - STR_FORMAT "' for transcoding", - STR_FMT(&pt->encoding_with_params)); - payload_type_free(pt); - return NULL; - } + pt->payload_type = __unused_pt_number(media, pt->payload_type); + if (pt->payload_type < 0) { + ilog(LOG_WARN, "Ran out of RTP payload type numbers while adding codec '" + STR_FORMAT "' for transcoding", + STR_FMT(&pt->encoding_with_params)); + payload_type_free(pt); + return NULL; } + return pt; } diff --git a/lib/rtplib.c b/lib/rtplib.c index cab180bc8..1c5c6dec6 100644 --- a/lib/rtplib.c +++ b/lib/rtplib.c @@ -143,15 +143,21 @@ const struct rtp_payload_type *rtp_get_rfc_codec(const str *codec) { } int rtp_payload_type_cmp(const struct rtp_payload_type *a, const struct rtp_payload_type *b) { + if (rtp_payload_type_cmp_nf(a, b)) + return 1; + if (str_cmp_str(&a->format_parameters, &b->format_parameters)) + return 1; + return 0; +} + +int rtp_payload_type_cmp_nf(const struct rtp_payload_type *a, const struct rtp_payload_type *b) { if (a->payload_type != b->payload_type) return 1; if (a->clock_rate != b->clock_rate) return 1; if (a->channels != b->channels) return 1; - if (str_cmp_str(&a->encoding_with_params, &b->encoding_with_params)) - return 1; - if (str_cmp_str(&a->format_parameters, &b->format_parameters)) + if (str_casecmp_str(&a->encoding_with_params, &b->encoding_with_params)) return 1; return 0; } diff --git a/lib/rtplib.h b/lib/rtplib.h index 2b6e0cee7..31a8ac6e8 100644 --- a/lib/rtplib.h +++ b/lib/rtplib.h @@ -42,6 +42,7 @@ const struct rtp_payload_type *rtp_get_rfc_payload_type(unsigned int type); const struct rtp_payload_type *rtp_get_rfc_codec(const str *codec); int rtp_payload_type_cmp(const struct rtp_payload_type *, const struct rtp_payload_type *); +int rtp_payload_type_cmp_nf(const struct rtp_payload_type *, const struct rtp_payload_type *); #endif diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index 37e780c9d..8a71063d7 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -18,6 +18,64 @@ my ($sock_a, $sock_b, $port_a, $port_b, $ssrc, $resp, $srtp_ctx_a, $srtp_ctx_b, +# PT collisions (GH 963) + +new_call(); + +offer('gh 963', { ICE => 'remove', codec => { mask => ['all'], transcode => ['PCMA','telephone-event'] } }, < 'remove', }, < ft() }); -done_testing(); -exit; # github issue 850