[Asterisk-code-review] chan sip: Better ICE handling for RTCP-MUX (asterisk[14])

Sean Bright asteriskteam at digium.com
Fri May 19 10:11:08 CDT 2017


Sean Bright has uploaded a new change for review. ( https://gerrit.asterisk.org/5651 )

Change subject: chan_sip: Better ICE handling for RTCP-MUX
......................................................................

chan_sip: Better ICE handling for RTCP-MUX

If we are offered or are offering RTCP-MUX, don't consider or emit RTCP
ICE candidates. This confuses certain browsers (current Firefox for
example) and causes intial audio setup delays.

ASTERISK-26982 #close

Change-Id: Ifeaf47e83972fe8dbe58b7fb3d6d1823400cfb91
---
M channels/chan_sip.c
1 file changed, 50 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/51/5651/1

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index ff2e5ba..6d14f59 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -1211,14 +1211,14 @@
 static int process_sdp_o(const char *o, struct sip_pvt *p);
 static int process_sdp_c(const char *c, struct ast_sockaddr *addr);
 static int process_sdp_a_sendonly(const char *a, int *sendonly);
-static int process_sdp_a_ice(const char *a, struct sip_pvt *p, struct ast_rtp_instance *instance);
+static int process_sdp_a_ice(const char *a, struct sip_pvt *p, struct ast_rtp_instance *instance, int rtcp_mux);
 static int process_sdp_a_rtcp_mux(const char *a, struct sip_pvt *p, int *requested);
 static int process_sdp_a_dtls(const char *a, struct sip_pvt *p, struct ast_rtp_instance *instance);
 static int process_sdp_a_audio(const char *a, struct sip_pvt *p, struct ast_rtp_codecs *newaudiortp, int *last_rtpmap_codec);
 static int process_sdp_a_video(const char *a, struct sip_pvt *p, struct ast_rtp_codecs *newvideortp, int *last_rtpmap_codec);
 static int process_sdp_a_text(const char *a, struct sip_pvt *p, struct ast_rtp_codecs *newtextrtp, char *red_fmtp, int *red_num_gen, int *red_data_pt, int *last_rtpmap_codec);
 static int process_sdp_a_image(const char *a, struct sip_pvt *p);
-static void add_ice_to_sdp(struct ast_rtp_instance *instance, struct ast_str **a_buf);
+static void add_ice_to_sdp(struct ast_rtp_instance *instance, struct ast_str **a_buf, int rtcp_mux);
 static void add_dtls_to_sdp(struct ast_rtp_instance *instance, struct ast_str **a_buf);
 static void start_ice(struct ast_rtp_instance *instance, int offer);
 static void add_codec_to_sdp(const struct sip_pvt *p, struct ast_format *codec,
@@ -10135,6 +10135,24 @@
 	}
 }
 
+static int has_media_level_attribute(int start, struct sip_request *req, const char *attr)
+{
+	int next = start;
+	char type;
+	const char *value;
+
+	/* We don't care about the return result here */
+	get_sdp_iterate(&next, req, "m");
+
+	while ((type = get_sdp_line(&start, next, req, &value)) != '\0') {
+		if (type == 'a' && !strcasecmp(value, attr)) {
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 /*! \brief Process SIP SDP offer, select formats and activate media channels
 	If offer is rejected, we will not change any properties of the call
  	Return 0 on success, a negative value on errors.
@@ -10277,13 +10295,13 @@
 			else if (process_sdp_a_image(value, p))
 				processed = TRUE;
 
-			if (process_sdp_a_ice(value, p, p->rtp)) {
+			if (process_sdp_a_ice(value, p, p->rtp, 0)) {
 				processed = TRUE;
 			}
-			if (process_sdp_a_ice(value, p, p->vrtp)) {
+			if (process_sdp_a_ice(value, p, p->vrtp, 0)) {
 				processed = TRUE;
 			}
-			if (process_sdp_a_ice(value, p, p->trtp)) {
+			if (process_sdp_a_ice(value, p, p->trtp, 0)) {
 				processed = TRUE;
 			}
 
@@ -10323,6 +10341,7 @@
 		int image = FALSE;
 		int text = FALSE;
 		int processed_crypto = FALSE;
+		int rtcp_mux_offered = 0;
 		char protocol[18] = {0,};
 		unsigned int x;
 		struct ast_rtp_engine_dtls *dtls;
@@ -10341,6 +10360,9 @@
 		}
 		AST_LIST_INSERT_TAIL(&p->offered_media, offer, next);
 		offer->type = SDP_UNKNOWN;
+
+		/* We need to check for this ahead of time */
+		rtcp_mux_offered = has_media_level_attribute(iterator, req, "rtcp-mux");
 
 		/* Check for 'audio' media offer */
 		if (strncmp(m, "audio ", 6) == 0) {
@@ -10708,7 +10730,7 @@
 			case 'a':
 				/* Audio specific scanning */
 				if (audio) {
-					if (process_sdp_a_ice(value, p, p->rtp)) {
+					if (process_sdp_a_ice(value, p, p->rtp, rtcp_mux_offered)) {
 						processed = TRUE;
 					} else if (process_sdp_a_dtls(value, p, p->rtp)) {
 						processed_crypto = TRUE;
@@ -10729,7 +10751,7 @@
 				}
 				/* Video specific scanning */
 				else if (video) {
-					if (process_sdp_a_ice(value, p, p->vrtp)) {
+					if (process_sdp_a_ice(value, p, p->vrtp, rtcp_mux_offered)) {
 						processed = TRUE;
 					} else if (process_sdp_a_dtls(value, p, p->vrtp)) {
 						processed_crypto = TRUE;
@@ -10748,7 +10770,7 @@
 				}
 				/* Text (T.140) specific scanning */
 				else if (text) {
-					if (process_sdp_a_ice(value, p, p->trtp)) {
+					if (process_sdp_a_ice(value, p, p->trtp, rtcp_mux_offered)) {
 						processed = TRUE;
 					} else if (process_sdp_a_text(value, p, &newtextrtp, red_fmtp, &red_num_gen, red_data_pt, &last_rtpmap_codec)) {
 						processed = TRUE;
@@ -11270,7 +11292,7 @@
 	return found;
 }
 
-static int process_sdp_a_ice(const char *a, struct sip_pvt *p, struct ast_rtp_instance *instance)
+static int process_sdp_a_ice(const char *a, struct sip_pvt *p, struct ast_rtp_instance *instance, int rtcp_mux_offered)
 {
 	struct ast_rtp_engine_ice *ice;
 	int found = FALSE;
@@ -11290,6 +11312,12 @@
 		found = TRUE;
 	} else if (sscanf(a, "candidate: %31s %30u %3s %30u %23s %30u typ %5s %*s %23s %*s %30u", foundation, &candidate.id, transport, (unsigned *)&candidate.priority,
 			  address, &port, cand_type, relay_address, &relay_port) >= 7) {
+
+		if (rtcp_mux_offered && ast_test_flag(&p->flags[2], SIP_PAGE3_RTCP_MUX) && candidate.id > 1) {
+			/* If we support RTCP-MUX and they offered it, don't consider RTCP candidates */
+			return TRUE;
+		}
+
 		candidate.foundation = foundation;
 		candidate.transport = transport;
 
@@ -12959,7 +12987,7 @@
 }
 
 /*! \brief Add ICE attributes to SDP */
-static void add_ice_to_sdp(struct ast_rtp_instance *instance, struct ast_str **a_buf)
+static void add_ice_to_sdp(struct ast_rtp_instance *instance, struct ast_str **a_buf, int rtcp_mux)
 {
 	struct ast_rtp_engine_ice *ice = ast_rtp_instance_get_ice(instance);
 	const char *username, *password;
@@ -12982,6 +13010,12 @@
 	i = ao2_iterator_init(candidates, 0);
 
 	while ((candidate = ao2_iterator_next(&i))) {
+		/* Don't emit RTCP candidates if we are offering RTCP-MUX */
+		if (rtcp_mux && candidate->id > 1) {
+			ao2_ref(candidate, -1);
+			continue;
+		}
+
 		ast_str_append(a_buf, 0, "a=candidate:%s %u %s %d ", candidate->foundation, candidate->id, candidate->transport, candidate->priority);
 		ast_str_append(a_buf, 0, "%s ", ast_sockaddr_stringify_host(&candidate->address));
 
@@ -13419,6 +13453,8 @@
 	int min_video_packet_size = 0;
 	int min_text_packet_size = 0;
 
+	int rtcp_mux = ast_test_flag(&p->flags[2], SIP_PAGE3_RTCP_MUX);
+
 	struct ast_str *codec_buf = ast_str_alloca(AST_FORMAT_CAP_NAMES_LEN);
 
 	/* Set the SDP session name */
@@ -13544,7 +13580,7 @@
 
 			if (!doing_directmedia) {
 				if (ast_test_flag(&p->flags[2], SIP_PAGE3_ICE_SUPPORT)) {
-					add_ice_to_sdp(p->vrtp, &a_video);
+					add_ice_to_sdp(p->vrtp, &a_video, rtcp_mux);
 				}
 
 				add_dtls_to_sdp(p->vrtp, &a_video);
@@ -13568,7 +13604,7 @@
 
 			if (!doing_directmedia) {
 				if (ast_test_flag(&p->flags[2], SIP_PAGE3_ICE_SUPPORT)) {
-					add_ice_to_sdp(p->trtp, &a_text);
+					add_ice_to_sdp(p->trtp, &a_text, rtcp_mux);
 				}
 
 				add_dtls_to_sdp(p->trtp, &a_text);
@@ -13694,7 +13730,7 @@
 
 		if (!doing_directmedia) {
 			if (ast_test_flag(&p->flags[2], SIP_PAGE3_ICE_SUPPORT)) {
-				add_ice_to_sdp(p->rtp, &a_audio);
+				add_ice_to_sdp(p->rtp, &a_audio, rtcp_mux);
 				/* Start ICE negotiation, and setting that we are controlled agent,
 				   as this is response to offer */
 				if (resp->method == SIP_RESPONSE) {
@@ -13706,7 +13742,7 @@
 		}
 
 		/* If we've got rtcp-mux enabled, just unconditionally offer it in all SDPs */
-		if (ast_test_flag(&p->flags[2], SIP_PAGE3_RTCP_MUX)) {
+		if (rtcp_mux) {
 			ast_str_append(&a_audio, 0, "a=rtcp-mux\r\n");
 			ast_str_append(&a_video, 0, "a=rtcp-mux\r\n");
 		}

-- 
To view, visit https://gerrit.asterisk.org/5651
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifeaf47e83972fe8dbe58b7fb3d6d1823400cfb91
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>



More information about the asterisk-code-review mailing list