[asterisk-commits] file: trunk r413120 - in /trunk: ./ res/res_pjsip_sdp_rtp.c

[SVN commits to the Asterisk project]

Author: file
Date: Wed Apr 30 07:39:11 2014
New Revision: 413120

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=413120
Log:
res_pjsip_sdp_rtp: Fix issue where sending a hold SDP twice could cause an unhold.

This change fixes a bug where if an SDP with media address and sendonly was
received twice the underlying call would go off hold, instead of remaining on hold.
This occured because the code did not properly take into account that the SDP
may contain both a valid media address and the sendonly attribute.

The code now examines the sendonly attribute and media address first, so if the
SDP is received again no change will occur.

ASTERISK-23558 #comment Reported by: John Bigelow

Review: https://reviewboard.asterisk.org/r/3472/
........

Merged revisions 413119 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/res/res_pjsip_sdp_rtp.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.

Modified: trunk/res/res_pjsip_sdp_rtp.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_pjsip_sdp_rtp.c?view=diff&rev=413120&r1=413119&r2=413120
==============================================================================
--- trunk/res/res_pjsip_sdp_rtp.c (original)
+++ trunk/res/res_pjsip_sdp_rtp.c Wed Apr 30 07:39:11 2014
@@ -1077,30 +1077,29 @@
 	/* If ICE support is enabled find all the needed attributes */
 	process_ice_attributes(session, session_media, remote, remote_stream);
 
+	/* Ensure the RTP instance is active */
+	ast_rtp_instance_activate(session_media->rtp);
+
 	/* audio stream handles music on hold */
 	if (media_type != AST_FORMAT_TYPE_AUDIO) {
 		return 1;
 	}
 
-	/* Music on hold for audio streams only */
-	if (session_media->held &&
-	    (!ast_sockaddr_isnull(addrs) ||
-	     !pjmedia_sdp_media_find_attr2(remote_stream, "sendonly", NULL))) {
+	if (ast_sockaddr_isnull(addrs) ||
+		ast_sockaddr_is_any(addrs) ||
+		pjmedia_sdp_media_find_attr2(remote_stream, "sendonly", NULL)) {
+		if (!session_media->held) {
+			/* The remote side has put us on hold */
+			ast_queue_hold(session->channel, session->endpoint->mohsuggest);
+			ast_rtp_instance_stop(session_media->rtp);
+			ast_queue_frame(session->channel, &ast_null_frame);
+			session_media->held = 1;
+		}
+	} else if (session_media->held) {
 		/* The remote side has taken us off hold */
 		ast_queue_unhold(session->channel);
 		ast_queue_frame(session->channel, &ast_null_frame);
 		session_media->held = 0;
-	} else if (ast_sockaddr_isnull(addrs) ||
-		   ast_sockaddr_is_any(addrs) ||
-		   pjmedia_sdp_media_find_attr2(remote_stream, "sendonly", NULL)) {
-		/* The remote side has put us on hold */
-		ast_queue_hold(session->channel, session->endpoint->mohsuggest);
-		ast_rtp_instance_stop(session_media->rtp);
-		ast_queue_frame(session->channel, &ast_null_frame);
-		session_media->held = 1;
-	} else {
-		/* The remote side has not changed state, but make sure the instance is active */
-		ast_rtp_instance_activate(session_media->rtp);
 	}
 
 	return 1;