[Asterisk-code-review] chan sip.c: Fix T.38 issues caused by leaving a bridge. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has uploaded a new change for review.

  https://gerrit.asterisk.org/2309

Change subject: chan_sip.c: Fix T.38 issues caused by leaving a bridge.
......................................................................

chan_sip.c: Fix T.38 issues caused by leaving a bridge.

chan_sip could not handle AST_T38_TERMINATED frames being sent to it when
the channel left the bridge.  The action resulted in overlapping outgoing
reINVITEs.  The testsuite tests/fax/sip/directmedia_reinvite_t38 was not
happy.

* Force T.38 to be remembered as locally bridged.  Now when the channel
leaves the native RTP bridge after T.38, the channel remembers that it has
already reINVITEed the media back to Asterisk.  It just needs to terminate
T.38 when the AST_T38_TERMINATED arrives.

* Prevent redundant AST_T38_TERMINATED from causing problems.  Redundant
AST_T38_TERMINATED frames could cause overlapping outgoing reINVITEs if
they happen before the T.38 state changes to disabled.  Now the T.38 state
is set to disabled before the reINVITE is sent.

ASTERISK-25582 #close

Change-Id: I53f5c6ce7d90b3f322a942af1a9bcab6d967b7ce
---
M channels/chan_sip.c
1 file changed, 30 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/09/2309/1

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index e1c391e..f036692 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -7522,8 +7522,10 @@
 			AST_SCHED_DEL_UNREF(sched, p->t38id, dialog_unref(p, "when you delete the t38id sched, you should dec the refcount for the stored dialog ptr"));
 			change_t38_state(p, T38_REJECTED);
 			transmit_response_reliable(p, "488 Not acceptable here", &p->initreq);
-		} else if (p->t38.state == T38_ENABLED)
+		} else if (p->t38.state == T38_ENABLED) {
+			change_t38_state(p, T38_DISABLED);
 			transmit_reinvite_with_sdp(p, FALSE, FALSE);
+		}
 		break;
 	case AST_T38_REQUEST_PARMS: {		/* Application wants remote's parameters re-sent */
 		struct ast_control_t38_parameters parameters = p->t38.their_parms;
@@ -10677,6 +10679,10 @@
 		} else if (udptlportno > 0) {
 			if (debug)
 				ast_verbose("Got T.38 Re-invite without audio. Keeping RTP active during T.38 session.\n");
+
+			/* Force media to go through us for T.38. */
+			memset(&p->redirip, 0, sizeof(p->redirip));
+
 			/* Prevent audio RTCP reads */
 			if (p->owner) {
 				ast_channel_set_fd(p->owner, 1, -1);
@@ -13787,6 +13793,29 @@
 static int transmit_reinvite_with_sdp(struct sip_pvt *p, int t38version, int oldsdp)
 {
 	struct sip_request req;
+
+	if (t38version) {
+		/* Force media to go through us for T.38. */
+		memset(&p->redirip, 0, sizeof(p->redirip));
+	}
+	if (p->rtp) {
+		if (t38version) {
+			/* Silence RTCP while audio RTP is inactive */
+			ast_rtp_instance_set_prop(p->rtp, AST_RTP_PROPERTY_RTCP, 0);
+			if (p->owner) {
+				/* Prevent audio RTCP reads */
+				ast_channel_set_fd(p->owner, 1, -1); 
+			}
+		} else if (ast_sockaddr_isnull(&p->redirip)) {
+			/* Enable RTCP since it will be inactive if we're coming back
+			 * with this reinvite */
+			ast_rtp_instance_set_prop(p->rtp, AST_RTP_PROPERTY_RTCP, 1);
+			if (p->owner) {
+				/* Enable audio RTCP reads */
+				ast_channel_set_fd(p->owner, 1, ast_rtp_instance_fd(p->rtp, 1));
+			}
+		}
+	}
 
 	reqprep(&req, p, ast_test_flag(&p->flags[0], SIP_REINVITE_UPDATE) ?  SIP_UPDATE : SIP_INVITE, 0, 1);
 
@@ -32741,14 +32770,6 @@
 	} else if (!ast_sockaddr_isnull(&p->redirip)) {
 		memset(&p->redirip, 0, sizeof(p->redirip));
 		changed = 1;
-
-		if (p->rtp) {
-			/* Enable RTCP since it will be inactive if we're coming back
-			 * from a reinvite */
-			ast_rtp_instance_set_prop(p->rtp, AST_RTP_PROPERTY_RTCP, 1);
-			/* Enable audio RTCP reads */
-			ast_channel_set_fd(chan, 1, ast_rtp_instance_fd(p->rtp, 1));
-		}
 	}
 
 	if (vinstance) {

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I53f5c6ce7d90b3f322a942af1a9bcab6d967b7ce
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>