TT#79351 detect and fix payload type collisions

fixes #963

Change-Id: I938b1c4b9baed37caf718f7537bbb9c11f7b91e5

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;
 }
 

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;
 }

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

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'] } }, <<SDP);
+v=0
+o=- 1822058533 1822058533 IN IP4 1.2.3.4
+s=Asterisk
+c=IN IP4 1.2.3.4
+t=0 0
+m=audio 27998 RTP/AVP 96 120
+a=rtpmap:96 opus/48000/2
+a=fmtp:96 useinbandfec=1;maxplaybackrate=16000;sprop-maxcapturerate=16000;maxaveragebitrate=12000;cbr=1
+a=rtpmap:120 telephone-event/48000
+a=fmtp:120 0-16
+----------------------------------
+v=0
+o=- 1822058533 1822058533 IN IP4 1.2.3.4
+s=Asterisk
+c=IN IP4 203.0.113.1
+t=0 0
+m=audio PORT RTP/AVP 8 96
+a=rtpmap:8 PCMA/8000
+a=rtpmap:96 telephone-event/8000
+a=fmtp:96 0-15
+a=sendrecv
+a=rtcp:PORT
+SDP
+
+answer('gh 963', { ICE => 'remove', }, <<SDP);
+v=0
+o=- 3793596600 3793596601 IN IP4 172.17.0.2
+s=pjmedia
+t=0 0
+m=audio 40935 RTP/AVP 8 96
+c=IN IP4 172.17.0.2
+a=rtpmap:8 PCMA/8000
+a=rtpmap:96 telephone-event/8000
+a=fmtp:96 0-15
+----------------------------------
+v=0
+o=- 3793596600 3793596601 IN IP4 172.17.0.2
+s=pjmedia
+t=0 0
+m=audio PORT RTP/AVP 96 120
+c=IN IP4 203.0.113.1
+a=rtpmap:96 opus/48000/2
+a=rtpmap:120 telephone-event/48000
+a=fmtp:96 useinbandfec=1;maxplaybackrate=16000;sprop-maxcapturerate=16000;maxaveragebitrate=12000;cbr=1
+a=fmtp:120 0-16
+a=sendrecv
+a=rtcp:PORT
+SDP
+
+
+
+
+
 # symmetric-codec flag (GH 953)
 
 new_call();
@@ -948,8 +1006,6 @@ rtpe_req('delete', "delete", { 'from-tag' => ft() });
 
 
 
-done_testing();
-exit;
 
 
 # github issue 850