res_pjsip_sdp_rtp: Fix multiple keepalive scheduled items.

The keepalive support in res_pjsip_sdp_rtp currently assumes
that a stream will only be negotiated once. This is false.
If the stream is replaced and later added back it can be
negotiated again causing multiple keepalive scheduled items
to exist. This change explicitly deletes the existing
keepalive scheduled item before adding the new one.

The res_pjsip_sdp_rtp module also does not stop RTP
keepalives or timeout timer if the stream has been
replaced. This change adds a callback to the session media
interface to allow a media stream to be stopped without
the resources being destroyed. This allows the scheduled
items and RTP to be stopped when the stream no longer
exists.

ASTERISK-25356 #close

Change-Id: Ibe6a7cc0927c87326fd5f1c0d4ad889dbfbea1de

diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h
index 2893f66ab8d6d9d59c1c9db6c3ed29b8467bb799..ddc87b024b51c7dd5091b02b8346dbc2c30b3307 100644 (file)
--- a/include/asterisk/res_pjsip_session.h
+++ b/include/asterisk/res_pjsip_session.h
@@ -354,6 +354,12 @@ struct ast_sip_session_sdp_handler {
         */
        int (*apply_negotiated_sdp_stream)(struct ast_sip_session *session, struct ast_sip_session_media *session_media, const struct pjmedia_sdp_session *local, const struct pjmedia_sdp_media *local_stream,
                const struct pjmedia_sdp_session *remote, const struct pjmedia_sdp_media *remote_stream);
+       /*!
+        * \brief Stop a session_media created by this handler but do not destroy resources
+        * \param session The session for which media is being stopped
+        * \param session_media The media to destroy
+        */
+       void (*stream_stop)(struct ast_sip_session_media *session_media);
        /*!
         * \brief Destroy a session_media created by this handler
         * \param session The session for which media is being destroyed

diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index f6c300f1fdf69d20ce89dd6547f96d1e24bb8d22..df2820479e16769d427e6f1d8887435c631d03e3 100644 (file)
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1307,6 +1307,7 @@ static int apply_negotiated_sdp_stream(struct ast_sip_session *session, struct a
                 * a NAT. This way there won't be an awkward delay before media starts flowing in some
                 * scenarios.
                 */
+               AST_SCHED_DEL(sched, session_media->keepalive_sched_id);
                session_media->keepalive_sched_id = ast_sched_add_variable(sched, 500, send_keepalive,
                        session_media, 1);
        }
@@ -1358,13 +1359,23 @@ static void change_outgoing_sdp_stream_media_address(pjsip_tx_data *tdata, struc
        pj_strdup2(tdata->pool, &stream->conn->addr, transport->external_media_address);
 }
 
+/*! \brief Function which stops the RTP instance */
+static void stream_stop(struct ast_sip_session_media *session_media)
+{
+       if (!session_media->rtp) {
+               return;
+       }
+
+       AST_SCHED_DEL(sched, session_media->keepalive_sched_id);
+       AST_SCHED_DEL(sched, session_media->timeout_sched_id);
+       ast_rtp_instance_stop(session_media->rtp);
+}
+
 /*! \brief Function which destroys the RTP instance when session ends */
 static void stream_destroy(struct ast_sip_session_media *session_media)
 {
        if (session_media->rtp) {
-               AST_SCHED_DEL(sched, session_media->keepalive_sched_id);
-               AST_SCHED_DEL(sched, session_media->timeout_sched_id);
-               ast_rtp_instance_stop(session_media->rtp);
+               stream_stop(session_media);
                ast_rtp_instance_destroy(session_media->rtp);
        }
        session_media->rtp = NULL;
@@ -1377,6 +1388,7 @@ static struct ast_sip_session_sdp_handler audio_sdp_handler = {
        .create_outgoing_sdp_stream = create_outgoing_sdp_stream,
        .apply_negotiated_sdp_stream = apply_negotiated_sdp_stream,
        .change_outgoing_sdp_stream_media_address = change_outgoing_sdp_stream_media_address,
+       .stream_stop = stream_stop,
        .stream_destroy = stream_destroy,
 };
 
@@ -1387,6 +1399,7 @@ static struct ast_sip_session_sdp_handler video_sdp_handler = {
        .create_outgoing_sdp_stream = create_outgoing_sdp_stream,
        .apply_negotiated_sdp_stream = apply_negotiated_sdp_stream,
        .change_outgoing_sdp_stream_media_address = change_outgoing_sdp_stream_media_address,
+       .stream_stop = stream_stop,
        .stream_destroy = stream_destroy,
 };
 

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 16ac640d9c363da4fc163691ecae1dc07211dab5..16ed38dd06ec2ec69a4aa6f5e1bd633c47447838 100644 (file)
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -362,6 +362,13 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags)
                        }
                }
        }
+
+       if (session_media->handler && session_media->handler->stream_stop) {
+               ast_debug(1, "Stopping SDP media stream '%s' as it is not currently negotiated\n",
+                       session_media->stream_type);
+               session_media->handler->stream_stop(session_media);
+       }
+
        return CMP_MATCH;
 }