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/

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

diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 17aa746c941ab767d5e41550d409360bbb744d16..0e3557b990198ea79da9e368758b845669211f95 100644 (file)
--- 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;