res_pjsip_sdp_rtp: Improve detecting of lack of RTP activity

Change RTP timer behavior for detecting RTP only after two-way
SDP channel establishment. Ignore detecting after receiving 183
with SDP or while direct media is used.
Make rtp_timeout and rtp_timeout_hold options consistent to rtptimeout
and rtpholdtimeout options in chan_sip.

ASTERISK-26689 #close
ASTERISK-29929 #close

Change-Id: I07326d5b9c40f25db717fd6075f6f3a8d77279eb

diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index e8fbb3d79ad56a93b178b8aa559d1fb0c85cf27b..61c4cfb7e6a4b8d116c675900a8eb173defe4f1e 100644 (file)
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -337,14 +337,6 @@ static int check_for_rtp_changes(struct ast_channel *chan, struct ast_rtp_instan
                ast_sockaddr_setnull(&media->direct_media_addr);
                changed = 1;
                if (media->rtp) {
-                       /* Direct media has ended - reset time of last received RTP packet
-                        * to avoid premature RTP timeout. Synchronisation between the
-                        * modification of direct_mdedia_addr+last_rx here and reading the
-                        * values in res_pjsip_sdp_rtp.c:rtp_check_timeout() is provided
-                        * by the channel's lock (which is held while this function is
-                        * executed).
-                        */
-                       ast_rtp_instance_set_last_rx(media->rtp, time(NULL));
                        ast_rtp_instance_set_prop(media->rtp, AST_RTP_PROPERTY_RTCP, 1);
                        if (position != -1) {
                                ast_channel_set_fd(chan, position + AST_EXTENDED_FDS, ast_rtp_instance_fd(media->rtp, 1));

diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index e1d1701411c25d0f282cdbe15f613d2d7eb6d878..3e10cfdb0410eb28839c616a2d741ed90a681ccd 100644 (file)
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -106,9 +106,10 @@ static int rtp_check_timeout(const void *data)
 {
        struct ast_sip_session_media *session_media = (struct ast_sip_session_media *)data;
        struct ast_rtp_instance *rtp = session_media->rtp;
+       struct ast_channel *chan;
        int elapsed;
+       int now;
        int timeout;
-       struct ast_channel *chan;
 
        if (!rtp) {
                return 0;
@@ -119,41 +120,37 @@ static int rtp_check_timeout(const void *data)
                return 0;
        }
 
-       /* Get channel lock to make sure that we access a consistent set of values
-        * (last_rx and direct_media_addr) - the lock is held when values are modified
-        * (see send_direct_media_request()/check_for_rtp_changes() in chan_pjsip.c). We
-        * are trying to avoid a situation where direct_media_addr has been reset but the
-        * last-rx time was not set yet.
-        */
-       ast_channel_lock(chan);
-
-       elapsed = time(NULL) - ast_rtp_instance_get_last_rx(rtp);
+       /* Store these values locally to avoid multiple function calls */
+       now = time(NULL);
        timeout = ast_rtp_instance_get_timeout(rtp);
-       if (elapsed < timeout) {
-               ast_channel_unlock(chan);
+
+       /* If the channel is not in UP state or call is redirected
+        * outside Asterisk return for later check.
+        */
+       if (ast_channel_state(chan) != AST_STATE_UP || !ast_sockaddr_isnull(&session_media->direct_media_addr)) {
+               /* Avoiding immediately disconnect after channel up or direct media has been stopped */
+               ast_rtp_instance_set_last_rx(rtp, now);
                ast_channel_unref(chan);
-               return (timeout - elapsed) * 1000;
+               /* Recheck after half timeout for avoiding possible races
+               * and faster reacting to cases while there is no an RTP at all.
+               */
+               return timeout * 500;
        }
 
-       /* Last RTP packet was received too long ago
-        * - disconnect channel unless direct media is in use.
-        */
-       if (!ast_sockaddr_isnull(&session_media->direct_media_addr)) {
-               ast_debug_rtp(3, "(%p) RTP not disconnecting channel '%s' for lack of %s RTP activity in %d seconds "
-                       "since direct media is in use\n", rtp, ast_channel_name(chan),
-                       ast_codec_media_type2str(session_media->type), elapsed);
-               ast_channel_unlock(chan);
+       elapsed = now - ast_rtp_instance_get_last_rx(rtp);
+       if (elapsed < timeout) {
                ast_channel_unref(chan);
-               return timeout * 1000; /* recheck later, direct media may have ended then */
+               return (timeout - elapsed) * 1000;
        }
 
        ast_log(LOG_NOTICE, "Disconnecting channel '%s' for lack of %s RTP activity in %d seconds\n",
                ast_channel_name(chan), ast_codec_media_type2str(session_media->type), elapsed);
 
+       ast_channel_lock(chan);
        ast_channel_hangupcause_set(chan, AST_CAUSE_REQUESTED_CHAN_UNAVAIL);
-       ast_softhangup(chan, AST_SOFTHANGUP_DEV);
-
        ast_channel_unlock(chan);
+
+       ast_softhangup(chan, AST_SOFTHANGUP_DEV);
        ast_channel_unref(chan);
 
        return 0;
@@ -2234,8 +2231,7 @@ static int apply_negotiated_sdp_stream(struct ast_sip_session *session,
        }
 
        if (ast_rtp_instance_get_timeout(session_media->rtp)) {
-               session_media->timeout_sched_id = ast_sched_add_variable(sched,
-                       ast_rtp_instance_get_timeout(session_media->rtp) * 1000, rtp_check_timeout,
+               session_media->timeout_sched_id = ast_sched_add_variable(sched, 500, rtp_check_timeout,
                        session_media, 1);
        }