[asterisk-commits] file: branch 11 r373413 - in /branches/11: channels/ res/

[SVN commits to the Asterisk project]

Author: file
Date: Mon Sep 24 09:25:43 2012
New Revision: 373413

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=373413
Log:
Fix an issue with H.264 format attribute comparison and fix an issue with improper SDP being produced.

The H.264 format attribute module compares two format attribute structures to determine if they are
compatible or not. In some instances it was possible for this check to determine that both structures
were incompatible when they actually should be considered compatible. This check has now been made even
more permissive by assuming that if no attribute information is available the two structures are compatible.
If both structures contain attribute information a base level comparison of the H.264 IDC value is done to
see if they are compatible or not.

The above issue uncovered a secondary issue in chan_sip where the SDP being produced would be incorrect if
the formats were considered incompatible. This has now been fixed by checking that all information required
to produce the SDP is available instead of assuming it is.

(closes issue ASTERISK-20464)
Reported by: Leif Madsen

Modified:
    branches/11/channels/chan_sip.c
    branches/11/res/res_format_attr_h264.c

Modified: branches/11/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/channels/chan_sip.c?view=diff&rev=373413&r1=373412&r2=373413
==============================================================================
--- branches/11/channels/chan_sip.c (original)
+++ branches/11/channels/chan_sip.c Mon Sep 24 09:25:43 2012
@@ -12227,11 +12227,17 @@
 {
 	int rtp_code;
 	struct ast_format_list fmt;
+	const char *mime;
+	unsigned int rate;
 
 	if (debug)
 		ast_verbose("Adding codec %d (%s) to SDP\n", format->id, ast_getformatname(format));
-	if ((rtp_code = ast_rtp_codecs_payload_code(ast_rtp_instance_get_codecs(p->rtp), 1, format, 0)) == -1)
+
+	if (((rtp_code = ast_rtp_codecs_payload_code(ast_rtp_instance_get_codecs(p->rtp), 1, format, 0)) == -1) ||
+	    !(mime = ast_rtp_lookup_mime_subtype2(1, format, 0, ast_test_flag(&p->flags[0], SIP_G726_NONSTANDARD) ? AST_RTP_OPT_G726_NONSTANDARD : 0)) ||
+	    !(rate = ast_rtp_lookup_sample_rate2(1, format, 0))) {
 		return;
+	}
 
 	if (p->rtp) {
 		struct ast_codec_pref *pref = &ast_rtp_instance_get_codecs(p->rtp)->pref;
@@ -12239,10 +12245,7 @@
 	} else /* I don't see how you couldn't have p->rtp, but good to check for and error out if not there like earlier code */
 		return;
 	ast_str_append(m_buf, 0, " %d", rtp_code);
-	ast_str_append(a_buf, 0, "a=rtpmap:%d %s/%d\r\n",
-		rtp_code,
-		ast_rtp_lookup_mime_subtype2(1, format, 0, ast_test_flag(&p->flags[0], SIP_G726_NONSTANDARD) ? AST_RTP_OPT_G726_NONSTANDARD : 0),
-		ast_rtp_lookup_sample_rate2(1, format, 0));
+	ast_str_append(a_buf, 0, "a=rtpmap:%d %s/%d\r\n", rtp_code, mime, rate);
 
 	ast_format_sdp_generate(format, rtp_code, a_buf);
 
@@ -12288,6 +12291,8 @@
 			     int debug, int *min_packet_size)
 {
 	int rtp_code;
+	const char *subtype;
+	unsigned int rate;
 
 	if (!p->vrtp)
 		return;
@@ -12295,13 +12300,14 @@
 	if (debug)
 		ast_verbose("Adding video codec %d (%s) to SDP\n", format->id, ast_getformatname(format));
 
-	if ((rtp_code = ast_rtp_codecs_payload_code(ast_rtp_instance_get_codecs(p->vrtp), 1, format, 0)) == -1)
+	if (((rtp_code = ast_rtp_codecs_payload_code(ast_rtp_instance_get_codecs(p->vrtp), 1, format, 0)) == -1) ||
+	    !(subtype = ast_rtp_lookup_mime_subtype2(1, format, 0, 0)) ||
+	    !(rate = ast_rtp_lookup_sample_rate2(1, format, 0))) {
 		return;
+	}
 
 	ast_str_append(m_buf, 0, " %d", rtp_code);
-	ast_str_append(a_buf, 0, "a=rtpmap:%d %s/%d\r\n", rtp_code,
-		       ast_rtp_lookup_mime_subtype2(1, format, 0, 0),
-		       ast_rtp_lookup_sample_rate2(1, format, 0));
+	ast_str_append(a_buf, 0, "a=rtpmap:%d %s/%d\r\n", rtp_code, subtype, rate);
 
 	ast_format_sdp_generate(format, rtp_code, a_buf);
 }

Modified: branches/11/res/res_format_attr_h264.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/res/res_format_attr_h264.c?view=diff&rev=373413&r1=373412&r2=373413
==============================================================================
--- branches/11/res/res_format_attr_h264.c (original)
+++ branches/11/res/res_format_attr_h264.c Mon Sep 24 09:25:43 2012
@@ -74,11 +74,12 @@
 
 static enum ast_format_cmp_res h264_format_attr_cmp(const struct ast_format_attr *fattr1, const struct ast_format_attr *fattr2)
 {
-	unsigned int idc1 = fattr1->format_attr[H264_ATTR_KEY_PROFILE_IDC] ? fattr1->format_attr[H264_ATTR_KEY_PROFILE_IDC] : 0x42;
-	unsigned int idc2 = fattr2->format_attr[H264_ATTR_KEY_PROFILE_IDC] ? fattr2->format_attr[H264_ATTR_KEY_PROFILE_IDC] : 0x42;
-
-	/* We are as permissive as possible to ensure the maximum number of calls succeed */
-	return (idc1 == idc2) ? AST_FORMAT_CMP_EQUAL : AST_FORMAT_CMP_NOT_EQUAL;
+	if (!fattr1->format_attr[H264_ATTR_KEY_PROFILE_IDC] || !fattr2->format_attr[H264_ATTR_KEY_PROFILE_IDC] ||
+	    (fattr1->format_attr[H264_ATTR_KEY_PROFILE_IDC] == fattr2->format_attr[H264_ATTR_KEY_PROFILE_IDC])) {
+		return AST_FORMAT_CMP_EQUAL;
+	}
+
+	return AST_FORMAT_CMP_NOT_EQUAL;
 }
 
 static int h264_format_attr_get_joint(const struct ast_format_attr *fattr1, const struct ast_format_attr *fattr2, struct ast_format_attr *result)