[Asterisk-code-review] res rtp asterisk: Clearing the remote RTCP address causes RT... (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/5564 )

Change subject: res_rtp_asterisk: Clearing the remote RTCP address causes RTCP failures
......................................................................


res_rtp_asterisk: Clearing the remote RTCP address causes RTCP failures

When a call gets put on hold RTP is temporarily stopped and Asterisk was
setting the remote RTCP 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)"

This patch makes it so the remote RTCP address is no longer set to NULL when
stopping RTP. There was only one place that appeared to check if the remote
RTCP address was NULL as a way to tell if RTCP was running. This patch added
an additional check on the RTCP 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, 10 insertions(+), 14 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  George Joseph: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index f407177..aa0e9f1 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -3902,7 +3902,7 @@
 			return 0;
 		}
 
-		if (ast_sockaddr_isnull(&rtp->rtcp->them)) {
+		if (ast_sockaddr_isnull(&rtp->rtcp->them) || rtp->rtcp->schedid < 0) {
 			/*
 			 * RTCP was stopped.
 			 */
@@ -5499,21 +5499,20 @@
 		}
 	}
 
-	if (rtp->rtcp) {
+	if (rtp->rtcp && !ast_sockaddr_isnull(addr)) {
 		ast_debug(1, "Setting RTCP address on RTP instance '%p'\n", instance);
 		ast_sockaddr_copy(&rtp->rtcp->them, addr);
-		if (!ast_sockaddr_isnull(addr)) {
-			if (rtp->rtcp->type == AST_RTP_INSTANCE_RTCP_STANDARD) {
-				ast_sockaddr_set_port(&rtp->rtcp->them, ast_sockaddr_port(addr) + 1);
 
-				/* Update the local RTCP address with what is being used */
-				ast_sockaddr_set_port(&local, ast_sockaddr_port(&local) + 1);
-			}
-			ast_sockaddr_copy(&rtp->rtcp->us, &local);
+		if (rtp->rtcp->type == AST_RTP_INSTANCE_RTCP_STANDARD) {
+			ast_sockaddr_set_port(&rtp->rtcp->them, ast_sockaddr_port(addr) + 1);
 
-			ast_free(rtp->rtcp->local_addr_str);
-			rtp->rtcp->local_addr_str = ast_strdup(ast_sockaddr_stringify(&local));
+			/* Update the local RTCP address with what is being used */
+			ast_sockaddr_set_port(&local, ast_sockaddr_port(&local) + 1);
 		}
+		ast_sockaddr_copy(&rtp->rtcp->us, &local);
+
+		ast_free(rtp->rtcp->local_addr_str);
+		rtp->rtcp->local_addr_str = ast_strdup(ast_sockaddr_stringify(&local));
 	}
 
 	rtp->rxseqno = 0;
@@ -5723,9 +5722,6 @@
 	}
 
 	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/5564
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6be200fb20db647e48b5138ea4b81dfa7962974b
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>