[Asterisk-code-review] res rtp asterisk: Pausing RTP and clearing remote address ca... (asterisk[master])

[Kevin Harwell]

Kevin Harwell has uploaded a new change for review. ( https://gerrit.asterisk.org/5566 )

Change subject: res_rtp_asterisk: Pausing RTP and clearing remote address causes RTCP failures
......................................................................

res_rtp_asterisk: Pausing RTP and clearing remote address causes RTCP failures

When a call gets put on hold RTP is temporarily stopped and Asterisk was setting
the remote's address to NULL. Then when RTCP data was received from the remote
endpoint, Asterisk would be missing this information when publishing the
rtcp_message stasis event. Consequently, message subscribers (in this case
res_hep_rtcp) trying to parse the "from" field output the following error:

"ast_sockaddr_split_hostport: Port missing in (null)"

After reviewing the code and making sure it was okay to do so, this patch makes
it so the remote address is no longer set to NULL when stopping RTP. There was
only one place that appeared to check if the remote address was NULL as a way
to tell if RTCP was running. This patch added an additional check on the schedid
for that case to make sure RTCP was truly not running.

ASTERISK-26860 #close

Change-Id: I6be200fb20db647e48b5138ea4b81dfa7962974b
---
M res/res_rtp_asterisk.c
1 file changed, 6 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/66/5566/1

diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index d0d7959..87184a0 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -3904,7 +3904,7 @@
 			return 0;
 		}
 
-		if (ast_sockaddr_isnull(&rtp->rtcp->them)) {
+		if (ast_sockaddr_isnull(&rtp->rtcp->them) || rtp->rtcp->schedid < 0) {
 			/*
 			 * RTCP was stopped.
 			 */
@@ -5715,7 +5715,6 @@
 static void ast_rtp_stop(struct ast_rtp_instance *instance)
 {
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
-	struct ast_sockaddr addr = { {0,} };
 
 #ifdef HAVE_OPENSSL_SRTP
 	ao2_unlock(instance);
@@ -5738,17 +5737,17 @@
 		rtp->rtcp->schedid = -1;
 	}
 
+	/*
+	 * Do not clear the remote address (rtcp->them) here. We may
+	 * continue to receive rtcp from the remote.
+	 */
+
 	if (rtp->red) {
 		ao2_unlock(instance);
 		AST_SCHED_DEL(rtp->sched, rtp->red->schedid);
 		ao2_lock(instance);
 		ast_free(rtp->red);
 		rtp->red = NULL;
-	}
-
-	ast_rtp_instance_set_remote_address(instance, &addr);
-	if (rtp->rtcp) {
-		ast_sockaddr_setnull(&rtp->rtcp->them);
 	}
 
 	ast_set_flag(rtp, FLAG_NEED_MARKER_BIT);

-- 
To view, visit https://gerrit.asterisk.org/5566
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6be200fb20db647e48b5138ea4b81dfa7962974b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>