[Asterisk-code-review] chan_psip, res_pjsip_sdp_rtp: ignore rtptimeout if direct-media is ac... (asterisk[13])

[George Joseph]

George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13976 )

Change subject: chan_psip, res_pjsip_sdp_rtp: ignore rtptimeout if direct-media is active
......................................................................

chan_psip, res_pjsip_sdp_rtp: ignore rtptimeout if direct-media is active

Do not hang up a PJSIP channel on RTP timeout if that channel is in
a direct-media bridge. Also reset the time of the last received RTP packet when
direct-media ends (wait full rtp_timeout period before checking first time after
audio came back to Asterisk).

ASTERISK-28774
Reported-by: Michael Neuhauser

Change-Id: I8b62012be7685849e8fb2b1c5dd39d35313ca2d1
---
M channels/chan_pjsip.c
M res/res_pjsip_sdp_rtp.c
2 files changed, 41 insertions(+), 10 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 0466fd3..33c023d 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -295,6 +295,14 @@
 		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);
 			ast_channel_set_fd(chan, rtcp_fd, ast_rtp_instance_fd(media->rtp, 1));
 		}
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index b0a9188..018074c 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -144,30 +144,53 @@
 	struct ast_sip_session_media *session_media = (struct ast_sip_session_media *)data;
 	struct ast_rtp_instance *rtp = session_media->rtp;
 	int elapsed;
+	int timeout;
 	struct ast_channel *chan;
 
 	if (!rtp) {
 		return 0;
 	}
 
-	elapsed = time(NULL) - ast_rtp_instance_get_last_rx(rtp);
-	if (elapsed < ast_rtp_instance_get_timeout(rtp)) {
-		return (ast_rtp_instance_get_timeout(rtp) - elapsed) * 1000;
-	}
-
 	chan = ast_channel_get_by_name(ast_rtp_instance_get_channel_id(rtp));
 	if (!chan) {
 		return 0;
 	}
 
-	ast_log(LOG_NOTICE, "Disconnecting channel '%s' for lack of RTP activity in %d seconds\n",
-		ast_channel_name(chan), elapsed);
-
+	/* 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);
-	ast_channel_hangupcause_set(chan, AST_CAUSE_REQUESTED_CHAN_UNAVAIL);
-	ast_channel_unlock(chan);
 
+	elapsed = time(NULL) - ast_rtp_instance_get_last_rx(rtp);
+	timeout = ast_rtp_instance_get_timeout(rtp);
+	if (elapsed < timeout) {
+		ast_channel_unlock(chan);
+		ast_channel_unref(chan);
+		return (timeout - elapsed) * 1000;
+	}
+
+	/* 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(3, "Not disconnecting channel '%s' for lack of %s RTP activity in %d seconds "
+			"since direct media is in use\n", ast_channel_name(chan),
+			session_media->stream_type, elapsed);
+		ast_channel_unlock(chan);
+		ast_channel_unref(chan);
+		return timeout * 1000; /* recheck later, direct media may have ended then */
+	}
+
+	ast_log(LOG_NOTICE, "Disconnecting channel '%s' for lack of %s RTP activity in %d seconds\n",
+		ast_channel_name(chan), session_media->stream_type, elapsed);
+
+	ast_channel_hangupcause_set(chan, AST_CAUSE_REQUESTED_CHAN_UNAVAIL);
 	ast_softhangup(chan, AST_SOFTHANGUP_DEV);
+
+	ast_channel_unlock(chan);
 	ast_channel_unref(chan);
 
 	return 0;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13976
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I8b62012be7685849e8fb2b1c5dd39d35313ca2d1
Gerrit-Change-Number: 13976
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Neuhauser <mike at firmix.at>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200320/b8735bf1/attachment.html>