From bb38010c6758b9f2824285461f3fb785bfce4389 Mon Sep 17 00:00:00 2001 From: Joshua Colp Date: Fri, 28 Aug 2015 22:22:45 -0300 Subject: [PATCH] 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 --- include/asterisk/res_pjsip_session.h | 6 ++++++ res/res_pjsip_sdp_rtp.c | 19 ++++++++++++++++--- res/res_pjsip_session.c | 7 +++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h index 734a887b31..6139847130 100644 --- a/include/asterisk/res_pjsip_session.h +++ b/include/asterisk/res_pjsip_session.h @@ -356,6 +356,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 f7fd5b85fd..a66aebb364 100644 --- a/res/res_pjsip_sdp_rtp.c +++ b/res/res_pjsip_sdp_rtp.c @@ -1317,6 +1317,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); } @@ -1368,13 +1369,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; @@ -1387,6 +1398,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, }; @@ -1397,6 +1409,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 801746459b..1dcac7e972 100644 --- 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; }