TT#104500 better support a=rtcp-fb

Link the attribute value to each RTP payload type and only output it if
PT is still present

Also merge PT attribute printing into a single loop

closes #1128

Change-Id: I494498ca806b795045e9e5bbb82174fa4aef8b2b
(cherry picked from commit 54e68942be)

daemon/codec.c

@@ -2270,6 +2270,7 @@ static void __rtp_payload_type_dup(struct call *call, struct rtp_payload_type *p
 	call_str_cpy(call, &pt->encoding_parameters, &pt->encoding_parameters);
 	call_str_cpy(call, &pt->format_parameters, &pt->format_parameters);
 	call_str_cpy(call, &pt->codec_opts, &pt->codec_opts);
+	call_str_cpy(call, &pt->rtcp_fb, &pt->rtcp_fb);
 }
 static struct rtp_payload_type *__rtp_payload_type_copy(const struct rtp_payload_type *pt) {
 	struct rtp_payload_type *pt_copy = g_slice_alloc(sizeof(*pt));

daemon/sdp.c

@@ -164,6 +164,13 @@ struct attribute_rtpmap {
 	struct rtp_payload_type rtp_pt;
 };
 
+struct attribute_rtcp_fb {
+	str payload_type_str;
+	str value;
+
+	unsigned int payload_type;
+};
+
 struct attribute_fmtp {
 	str payload_type_str;
 	str format_parms_str;
@@ -253,6 +260,7 @@ struct sdp_attribute {	/* example: a=rtpmap:8 PCMA/8000 */
 		struct attribute_fingerprint fingerprint;
 		struct attribute_setup setup;
 		struct attribute_rtpmap rtpmap;
+		struct attribute_rtcp_fb rtcp_fb;
 		struct attribute_fmtp fmtp;
 		struct attribute_t38faxudpec t38faxudpec;
 		int i;
@@ -744,6 +752,28 @@ static int parse_attribute_setup(struct sdp_attribute *output) {
 	return 0;
 }
 
+static int parse_attribute_rtcp_fb(struct sdp_attribute *output) {
+	PARSE_DECL;
+	struct attribute_rtcp_fb *a;
+
+	output->attr = ATTR_RTCP_FB;
+	a = &output->u.rtcp_fb;
+
+	PARSE_INIT;
+	EXTRACT_TOKEN(u.rtcp_fb.payload_type_str);
+	a->value = *value_str;
+
+	if (!str_cmp(&output->u.rtcp_fb.value, "*"))
+		a->payload_type = -1;
+	else {
+		a->payload_type = str_to_i(&a->payload_type_str, -1);
+		if (a->payload_type == -1)
+			return -1;
+	}
+
+	return 0;
+}
+
 static int parse_attribute_rtpmap(struct sdp_attribute *output) {
 	PARSE_DECL;
 	char *ep;
@@ -803,7 +833,7 @@ static int parse_attribute_fmtp(struct sdp_attribute *output) {
 
 	PARSE_INIT;
 	EXTRACT_TOKEN(u.fmtp.payload_type_str);
-	output->u.fmtp.format_parms_str = *value_str;
+	a->format_parms_str = *value_str;
 
 	a->payload_type = str_to_i(&a->payload_type_str, -1);
 	if (a->payload_type == -1)
@@ -979,7 +1009,7 @@ static int parse_attribute(struct sdp_attribute *a) {
 			a->attr = ATTR_END_OF_CANDIDATES;
 			break;
 		case CSH_LOOKUP("rtcp-fb"):
-			a->attr = ATTR_RTCP_FB;
+			ret = parse_attribute_rtcp_fb(a);
 			break;
 		case CSH_LOOKUP("T38FaxVersion"):
 			ret = parse_attribute_int(a, ATTR_T38FAXVERSION, -1);
@@ -1260,7 +1290,7 @@ static int fill_endpoint(struct endpoint *ep, const struct sdp_media *media, str
 
 static int __rtp_payload_types(struct stream_params *sp, struct sdp_media *media)
 {
-	GHashTable *ht_rtpmap, *ht_fmtp;
+	GHashTable *ht_rtpmap, *ht_fmtp, *ht_rtcp_fb;
 	GQueue *q;
 	GList *ql;
 	struct sdp_attribute *attr;
@@ -1285,6 +1315,15 @@ static int __rtp_payload_types(struct stream_params *sp, struct sdp_media *media
 		attr = ql->data;
 		g_hash_table_insert(ht_fmtp, &attr->u.fmtp.payload_type, &attr->u.fmtp.format_parms_str);
 	}
+	// do the same for a=rtcp-fb
+	ht_rtcp_fb = g_hash_table_new(g_int_hash, g_int_equal);
+	q = attr_list_get_by_id(&media->attributes, ATTR_RTCP_FB);
+	for (ql = q ? q->head : NULL; ql; ql = ql->next) {
+		attr = ql->data;
+		if (attr->u.rtcp_fb.payload_type == -1)
+			continue;
+		g_hash_table_insert(ht_rtcp_fb, &attr->u.rtcp_fb.payload_type, &attr->u.rtcp_fb.value);
+	}
 
 	/* then go through the format list and associate */
 	for (ql = media->format_list.head; ql; ql = ql->next) {
@@ -1315,6 +1354,9 @@ static int __rtp_payload_types(struct stream_params *sp, struct sdp_media *media
 		s = g_hash_table_lookup(ht_fmtp, &i);
 		if (s)
 			pt->format_parameters = *s;
+		s = g_hash_table_lookup(ht_rtcp_fb, &i);
+		if (s)
+			pt->rtcp_fb = *s;
 
 		// fill in ptime
 		if (sp->ptime)
@@ -1333,6 +1375,7 @@ error:
 out:
 	g_hash_table_destroy(ht_rtpmap);
 	g_hash_table_destroy(ht_fmtp);
+	g_hash_table_destroy(ht_rtcp_fb);
 	return ret;
 }
 
@@ -1721,14 +1764,16 @@ static void insert_codec_parameters(struct sdp_chopper *chop, struct call_media
 		chopper_append_printf(chop, "a=rtpmap:%u " STR_FORMAT "\r\n",
 				pt->payload_type,
 				STR_FMT(&pt->encoding_with_params));
-	}
-	for (GList *l = cm->codecs_prefs_recv.head; l; l = l->next) {
-		struct rtp_payload_type *pt = l->data;
-		if (!pt->format_parameters.len)
-			continue;
-		chopper_append_printf(chop, "a=fmtp:%u " STR_FORMAT "\r\n",
-				pt->payload_type,
-				STR_FMT(&pt->format_parameters));
+		if (pt->format_parameters.len) {
+			chopper_append_printf(chop, "a=fmtp:%u " STR_FORMAT "\r\n",
+					pt->payload_type,
+					STR_FMT(&pt->format_parameters));
+		}
+		if (pt->rtcp_fb.len) {
+			chopper_append_printf(chop, "a=rtcp-fb:%u " STR_FORMAT "\r\n",
+					pt->payload_type,
+					STR_FMT(&pt->rtcp_fb));
+		}
 	}
 }
 
@@ -2022,6 +2067,12 @@ static int process_media_attributes(struct sdp_chopper *chop, struct sdp_media *
 				if (media->codecs_prefs_recv.length > 0)
 					goto strip;
 				break;
+			case ATTR_RTCP_FB:
+				if (attr->u.rtcp_fb.payload_type == -1)
+					break; // leave this one alone
+				if (media->codecs_prefs_recv.length > 0)
+					goto strip;
+				break;
 
 			case ATTR_EXTMAP:
 			case ATTR_CRYPTO:

lib/rtplib.h

@@ -25,6 +25,7 @@ struct rtp_payload_type {
 	int channels; // 2
 	str format_parameters; // value of a=fmtp
 	str codec_opts; // extra codec-specific options
+	str rtcp_fb; // a=rtcp-fb:...
 
 	int ptime; // default from RFC
 	int bitrate;

t/auto-daemon-tests.pl

@@ -1924,8 +1924,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=setup:actpass
@@ -1959,8 +1959,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=setup:actpass
@@ -1994,8 +1994,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=setup:actpass
@@ -2034,8 +2034,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:CRYPTO128
@@ -2077,8 +2077,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:CRYPTO128
@@ -2126,8 +2126,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:CRYPTO128
@@ -2201,8 +2201,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:CRYPTO128
@@ -2277,8 +2277,8 @@ t=0 0
 m=audio PORT RTP/SAVP 0 101 8
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:101 0-16
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:CRYPTO128
@@ -4160,8 +4160,8 @@ 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=rtpmap:120 telephone-event/48000
 a=fmtp:120 0-16
 a=sendrecv
 a=rtcp:PORT
@@ -4215,8 +4215,8 @@ 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=rtpmap:120 telephone-event/48000
 a=fmtp:120 0-16
 a=sendrecv
 a=rtcp:PORT
@@ -4293,8 +4293,8 @@ b=TIAS:96000
 a=ssrc:243811319 cname:04389d431bdd5c52
 a=rtpmap:8 PCMA/8000
 a=rtpmap:107 opus/48000/2
-a=rtpmap:101 telephone-event/8000
 a=fmtp:107 useinbandfec=1
+a=rtpmap:101 telephone-event/8000
 a=fmtp:101 0-16
 a=sendrecv
 a=rtcp:PORT
@@ -4367,8 +4367,8 @@ c=IN IP4 203.0.113.1
 b=TIAS:96000
 a=ssrc:243811319 cname:04389d431bdd5c52
 a=rtpmap:107 opus/48000/2
-a=rtpmap:101 telephone-event/8000
 a=fmtp:107 useinbandfec=1
+a=rtpmap:101 telephone-event/8000
 a=fmtp:101 0-16
 a=sendrecv
 a=rtcp:PORT
@@ -4442,8 +4442,8 @@ b=TIAS:96000
 a=ssrc:243811319 cname:04389d431bdd5c52
 a=rtpmap:8 PCMA/8000
 a=rtpmap:107 opus/48000/2
-a=rtpmap:101 telephone-event/8000
 a=fmtp:107 useinbandfec=1
+a=rtpmap:101 telephone-event/8000
 a=fmtp:101 0-16
 a=sendrecv
 a=rtcp:PORT
@@ -4516,9 +4516,9 @@ c=IN IP4 203.0.113.1
 b=TIAS:96000
 a=ssrc:243811319 cname:04389d431bdd5c52
 a=rtpmap:107 opus/48000/2
+a=fmtp:107 useinbandfec=1
 a=rtpmap:8 PCMA/8000
 a=rtpmap:101 telephone-event/8000
-a=fmtp:107 useinbandfec=1
 a=fmtp:101 0-16
 a=sendrecv
 a=rtcp:PORT
@@ -4590,8 +4590,8 @@ a=ssrc:243811319 cname:04389d431bdd5c52
 a=rtpmap:8 PCMA/8000
 a=rtpmap:107 opus/48000/2
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:96 telephone-event/48000
 a=fmtp:101 0-16
+a=rtpmap:96 telephone-event/48000
 a=fmtp:96 0-15
 a=sendrecv
 a=rtcp:PORT
@@ -4662,8 +4662,8 @@ a=ssrc:243811319 cname:04389d431bdd5c52
 a=rtpmap:8 PCMA/8000
 a=rtpmap:107 opus/48000/2
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:96 telephone-event/48000
 a=fmtp:101 0-16
+a=rtpmap:96 telephone-event/48000
 a=fmtp:96 0-15
 a=sendrecv
 a=rtcp:PORT
@@ -5254,13 +5254,14 @@ a=extmap:4 urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id
 a=extmap:5 urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id
 a=msid:qDSKVQw0XQOFzGhek25Kn3RLxyHTM2ooxMUY 7d669de6-65e9-4fbe-829e-e89dc4baf81c
 a=rtcp-mux
-a=rtcp-fb:111 transport-cc
 a=ssrc:2628106563 cname:wMyHbPOf/cCq2tup
 a=ssrc:2628106563 msid:qDSKVQw0XQOFzGhek25Kn3RLxyHTM2ooxMUY 7d669de6-65e9-4fbe-829e-e89dc4baf81c
 a=ssrc:2628106563 mslabel:qDSKVQw0XQOFzGhek25Kn3RLxyHTM2ooxMUY
 a=ssrc:2628106563 label:7d669de6-65e9-4fbe-829e-e89dc4baf81c
 a=mid:0
 a=rtpmap:111 opus/48000/2
+a=fmtp:111 minptime=10;useinbandfec=1
+a=rtcp-fb:111 transport-cc
 a=rtpmap:103 ISAC/16000
 a=rtpmap:104 ISAC/32000
 a=rtpmap:9 G722/8000
@@ -5273,7 +5274,6 @@ a=rtpmap:110 telephone-event/48000
 a=rtpmap:112 telephone-event/32000
 a=rtpmap:113 telephone-event/16000
 a=rtpmap:126 telephone-event/8000
-a=fmtp:111 minptime=10;useinbandfec=1
 a=sendrecv
 a=candidate:ICEBASE 1 UDP 16777215 203.0.113.1 PORT typ relay raddr 203.0.113.1 rport PORT
 a=candidate:ICEBASE 1 UDP 16776959 2001:db8:4321::1 PORT typ relay raddr 2001:db8:4321::1 rport PORT
@@ -5364,10 +5364,11 @@ a=extmap:4 urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id
 a=extmap:5 urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id
 a=msid:9z51ZTKhoszc7zqj5gxEX309ODe940YpMplv 8a622ecc-1fff-4675-8bf4-7b924845b3fd
 a=rtcp-mux
-a=rtcp-fb:111 transport-cc
 a=ssrc:97254339 cname:d7zRWvteaW9fc2Yu
 a=mid:0
 a=rtpmap:111 opus/48000/2
+a=fmtp:111 minptime=10;useinbandfec=1
+a=rtcp-fb:111 transport-cc
 a=rtpmap:103 ISAC/16000
 a=rtpmap:104 ISAC/32000
 a=rtpmap:9 G722/8000
@@ -5380,7 +5381,6 @@ a=rtpmap:110 telephone-event/48000
 a=rtpmap:112 telephone-event/32000
 a=rtpmap:113 telephone-event/16000
 a=rtpmap:126 telephone-event/8000
-a=fmtp:111 minptime=10;useinbandfec=1
 a=sendrecv
 a=rtcp:PORT
 a=rtcp-mux
@@ -5420,8 +5420,8 @@ t=0 0
 m=audio PORT RTP/AVP 96 8
 c=IN IP4 203.0.113.1
 a=rtpmap:96 iLBC/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:96 mode=30
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=ptime:30
@@ -5484,8 +5484,8 @@ t=0 0
 m=audio PORT RTP/AVP 96 8
 c=IN IP4 203.0.113.1
 a=rtpmap:96 iLBC/8000
-a=rtpmap:8 PCMA/8000
 a=fmtp:96 mode=20
+a=rtpmap:8 PCMA/8000
 a=sendrecv
 a=rtcp:PORT
 a=ptime:20
@@ -7562,12 +7562,12 @@ c=IN IP4 203.0.113.1
 t=0 0
 m=audio PORT RTP/AVP 120 8 0 101 96
 a=rtpmap:120 opus/48000/2
+a=fmtp:120 useinbandfec=1; usedtx=1; maxaveragebitrate=64000
 a=rtpmap:8 PCMA/8000
 a=rtpmap:0 PCMU/8000
 a=rtpmap:101 telephone-event/8000
-a=rtpmap:96 telephone-event/48000
-a=fmtp:120 useinbandfec=1; usedtx=1; maxaveragebitrate=64000
 a=fmtp:101 0-15
+a=rtpmap:96 telephone-event/48000
 a=fmtp:96 0-15
 a=sendrecv
 a=rtcp:PORT
@@ -9017,10 +9017,10 @@ m=audio PORT RTP/AVP 0 106 101 98
 c=IN IP4 203.0.113.1
 a=rtpmap:0 PCMU/8000
 a=rtpmap:106 opus/48000/2
-a=rtpmap:101 telephone-event/8000
-a=rtpmap:98 telephone-event/48000
 a=fmtp:106 maxplaybackrate=16000; sprop-maxcapturerate=16000; minptime=20; cbr=1; maxaveragebitrate=20000; useinbandfec=1
+a=rtpmap:101 telephone-event/8000
 a=fmtp:101 0-16
+a=rtpmap:98 telephone-event/48000
 a=fmtp:98 0-16
 a=sendrecv
 a=rtcp:PORT
@@ -9104,8 +9104,8 @@ a=maxptime:20
 a=rtpmap:0 PCMU/8000
 a=rtpmap:8 PCMA/8000
 a=rtpmap:18 G729/8000
-a=rtpmap:101 telephone-event/8000
 a=fmtp:18 annexb=no
+a=rtpmap:101 telephone-event/8000
 a=fmtp:101 0-15
 a=sendrecv
 a=rtcp:PORT
@@ -9545,10 +9545,10 @@ a=rtpmap:8 PCMA/8000
 a=rtpmap:97 speex/16000
 a=rtpmap:9 G722/8000
 a=rtpmap:98 telephone-event/8000
-a=rtpmap:99 telephone-event/16000
-a=rtpmap:100 telephone-event/48000
 a=fmtp:98 0-15
+a=rtpmap:99 telephone-event/16000
 a=fmtp:99 0-15
+a=rtpmap:100 telephone-event/48000
 a=fmtp:100 0-15
 a=sendrecv
 a=rtcp:PORT
@@ -9866,8 +9866,8 @@ c=IN IP4 203.0.113.1
 a=rtpmap:100 PCMU/16000
 a=rtpmap:8 PCMA/8000
 a=rtpmap:101 telephone-event/16000
-a=rtpmap:96 telephone-event/8000
 a=fmtp:101 0-15
+a=rtpmap:96 telephone-event/8000
 a=fmtp:96 0-15
 a=sendrecv
 a=rtcp:PORT

t/transcode-test.c

@@ -99,7 +99,7 @@ static void codec_set(char *c) {
 static void __sdp_pt_fmt(int num, str codec, int clockrate, str full_codec, str fmt) {
 	struct rtp_payload_type *pt = g_slice_alloc(sizeof(*pt));
 	*pt = (struct rtp_payload_type) { num, full_codec, codec,
-		clockrate, STR_CONST_INIT(""), 1, fmt, {0,0}, 0, 0, NULL };
+		clockrate, STR_CONST_INIT(""), 1, fmt, {0,0}, {0,0}, 0, 0, NULL };
 	g_queue_push_tail(&rtp_types, pt);
 }