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


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@413120 65c4cc65-6c06-0410-ace0-fbb531ad65f3

1 files changed, 14 insertions, 15 deletions

diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 17aa746c9..0e3557b99 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1077,30 +1077,29 @@ static int apply_negotiated_sdp_stream(struct ast_sip_session *session, struct a
 	/* 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;