<p>Joshua Colp <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/18230">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
Kevin Harwell: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_sdp_rtp: Improve detecting of lack of RTP activity<br><br>Change RTP timer behavior for detecting RTP only after two-way<br>SDP channel establishment. Ignore detecting after receiving 183<br>with SDP or while direct media is used.<br>Make rtp_timeout and rtp_timeout_hold options consistent to rtptimeout<br>and rtpholdtimeout options in chan_sip.<br><br>ASTERISK-26689 #close<br>ASTERISK-29929 #close<br><br>Change-Id: I07326d5b9c40f25db717fd6075f6f3a8d77279eb<br>---<br>M channels/chan_pjsip.c<br>M res/res_pjsip_sdp_rtp.c<br>2 files changed, 23 insertions(+), 35 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c</span><br><span>index e8fbb3d..61c4cfb 100644</span><br><span>--- a/channels/chan_pjsip.c</span><br><span>+++ b/channels/chan_pjsip.c</span><br><span>@@ -337,14 +337,6 @@</span><br><span> ast_sockaddr_setnull(&media->direct_media_addr);</span><br><span> changed = 1;</span><br><span> if (media->rtp) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Direct media has ended - reset time of last received RTP packet</span><br><span style="color: hsl(0, 100%, 40%);">- * to avoid premature RTP timeout. Synchronisation between the</span><br><span style="color: hsl(0, 100%, 40%);">- * modification of direct_mdedia_addr+last_rx here and reading the</span><br><span style="color: hsl(0, 100%, 40%);">- * values in res_pjsip_sdp_rtp.c:rtp_check_timeout() is provided</span><br><span style="color: hsl(0, 100%, 40%);">- * by the channel's lock (which is held while this function is</span><br><span style="color: hsl(0, 100%, 40%);">- * executed).</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- ast_rtp_instance_set_last_rx(media->rtp, time(NULL));</span><br><span> ast_rtp_instance_set_prop(media->rtp, AST_RTP_PROPERTY_RTCP, 1);</span><br><span> if (position != -1) {</span><br><span> ast_channel_set_fd(chan, position + AST_EXTENDED_FDS, ast_rtp_instance_fd(media->rtp, 1));</span><br><span>diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c</span><br><span>index e1d1701..3e10cfd 100644</span><br><span>--- a/res/res_pjsip_sdp_rtp.c</span><br><span>+++ b/res/res_pjsip_sdp_rtp.c</span><br><span>@@ -106,9 +106,10 @@</span><br><span> {</span><br><span> struct ast_sip_session_media *session_media = (struct ast_sip_session_media *)data;</span><br><span> struct ast_rtp_instance *rtp = session_media->rtp;</span><br><span style="color: hsl(0, 100%, 40%);">- int elapsed;</span><br><span style="color: hsl(0, 100%, 40%);">- int timeout;</span><br><span> struct ast_channel *chan;</span><br><span style="color: hsl(120, 100%, 40%);">+ int elapsed;</span><br><span style="color: hsl(120, 100%, 40%);">+ int now;</span><br><span style="color: hsl(120, 100%, 40%);">+ int timeout;</span><br><span> </span><br><span> if (!rtp) {</span><br><span> return 0;</span><br><span>@@ -119,41 +120,37 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Get channel lock to make sure that we access a consistent set of values</span><br><span style="color: hsl(0, 100%, 40%);">- * (last_rx and direct_media_addr) - the lock is held when values are modified</span><br><span style="color: hsl(0, 100%, 40%);">- * (see send_direct_media_request()/check_for_rtp_changes() in chan_pjsip.c). We</span><br><span style="color: hsl(0, 100%, 40%);">- * are trying to avoid a situation where direct_media_addr has been reset but the</span><br><span style="color: hsl(0, 100%, 40%);">- * last-rx time was not set yet.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- ast_channel_lock(chan);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- elapsed = time(NULL) - ast_rtp_instance_get_last_rx(rtp);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Store these values locally to avoid multiple function calls */</span><br><span style="color: hsl(120, 100%, 40%);">+ now = time(NULL);</span><br><span> timeout = ast_rtp_instance_get_timeout(rtp);</span><br><span style="color: hsl(0, 100%, 40%);">- if (elapsed < timeout) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_channel_unlock(chan);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If the channel is not in UP state or call is redirected</span><br><span style="color: hsl(120, 100%, 40%);">+ * outside Asterisk return for later check.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_channel_state(chan) != AST_STATE_UP || !ast_sockaddr_isnull(&session_media->direct_media_addr)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Avoiding immediately disconnect after channel up or direct media has been stopped */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rtp_instance_set_last_rx(rtp, now);</span><br><span> ast_channel_unref(chan);</span><br><span style="color: hsl(0, 100%, 40%);">- return (timeout - elapsed) * 1000;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Recheck after half timeout for avoiding possible races</span><br><span style="color: hsl(120, 100%, 40%);">+ * and faster reacting to cases while there is no an RTP at all.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ return timeout * 500;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Last RTP packet was received too long ago</span><br><span style="color: hsl(0, 100%, 40%);">- * - disconnect channel unless direct media is in use.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!ast_sockaddr_isnull(&session_media->direct_media_addr)) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_debug_rtp(3, "(%p) RTP not disconnecting channel '%s' for lack of %s RTP activity in %d seconds "</span><br><span style="color: hsl(0, 100%, 40%);">- "since direct media is in use\n", rtp, ast_channel_name(chan),</span><br><span style="color: hsl(0, 100%, 40%);">- ast_codec_media_type2str(session_media->type), elapsed);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_channel_unlock(chan);</span><br><span style="color: hsl(120, 100%, 40%);">+ elapsed = now - ast_rtp_instance_get_last_rx(rtp);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (elapsed < timeout) {</span><br><span> ast_channel_unref(chan);</span><br><span style="color: hsl(0, 100%, 40%);">- return timeout * 1000; /* recheck later, direct media may have ended then */</span><br><span style="color: hsl(120, 100%, 40%);">+ return (timeout - elapsed) * 1000;</span><br><span> }</span><br><span> </span><br><span> ast_log(LOG_NOTICE, "Disconnecting channel '%s' for lack of %s RTP activity in %d seconds\n",</span><br><span> ast_channel_name(chan), ast_codec_media_type2str(session_media->type), elapsed);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ ast_channel_lock(chan);</span><br><span> ast_channel_hangupcause_set(chan, AST_CAUSE_REQUESTED_CHAN_UNAVAIL);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_softhangup(chan, AST_SOFTHANGUP_DEV);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> ast_channel_unlock(chan);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_softhangup(chan, AST_SOFTHANGUP_DEV);</span><br><span> ast_channel_unref(chan);</span><br><span> </span><br><span> return 0;</span><br><span>@@ -2234,8 +2231,7 @@</span><br><span> }</span><br><span> </span><br><span> if (ast_rtp_instance_get_timeout(session_media->rtp)) {</span><br><span style="color: hsl(0, 100%, 40%);">- session_media->timeout_sched_id = ast_sched_add_variable(sched,</span><br><span style="color: hsl(0, 100%, 40%);">- ast_rtp_instance_get_timeout(session_media->rtp) * 1000, rtp_check_timeout,</span><br><span style="color: hsl(120, 100%, 40%);">+ session_media->timeout_sched_id = ast_sched_add_variable(sched, 500, rtp_check_timeout,</span><br><span> session_media, 1);</span><br><span> }</span><br><span> </span><br><span></span><br></pre><div style="white-space:pre-wrap"></div><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18230">change 18230</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/18230"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: I07326d5b9c40f25db717fd6075f6f3a8d77279eb </div>
<div style="display:none"> Gerrit-Change-Number: 18230 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Boris P. Korzun <drtr0jan@yandex.ru> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>