[Asterisk-code-review] chan sip/res pjsip t38: Handle a request to negotiate T.38 a... (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: chan_sip/res_pjsip_t38: Handle a request to negotiate T.38 after it is enabled.
......................................................................


chan_sip/res_pjsip_t38: Handle a request to negotiate T.38 after it is enabled.

Some T.38 implementations may send another re-invite after the initial
one which adds additional negotiation details (such as the max bitrate).
Currently this will fail when passthrough is being done in chan_sip as we
do nothing if T.38 is already active.

Other handlers of T.38 inside of Asterisk (such as res_fax) handle this
scenario so this change adds support for it to chan_sip and res_pjsip_t38.
If a request to negotiate is received while T.38 is already enabled a
new re-INVITE is sent and negotiation is done again.

ASTERISK-26179 #close

Change-Id: I0298494d3da6df3219bbfa4be9aa04015043145c
---
M channels/chan_sip.c
M res/res_pjsip_t38.c
2 files changed, 5 insertions(+), 2 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Matt Jordan: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index e086711..28379ff 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -7669,7 +7669,8 @@
 			ast_udptl_set_local_max_ifp(p->udptl, p->t38.our_parms.max_ifp);
 			change_t38_state(p, T38_ENABLED);
 			transmit_response_with_t38_sdp(p, "200 OK", &p->initreq, XMIT_CRITICAL);
-		} else if (p->t38.state != T38_ENABLED) {
+		} else if ((p->t38.state != T38_ENABLED) || ((p->t38.state == T38_ENABLED) &&
+				(parameters->request_response == AST_T38_REQUEST_NEGOTIATE))) {
 			p->t38.our_parms = *parameters;
 			ast_udptl_set_local_max_ifp(p->udptl, p->t38.our_parms.max_ifp);
 			change_t38_state(p, T38_LOCAL_REINVITE);
diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c
index c025171..76720ac 100644
--- a/res/res_pjsip_t38.c
+++ b/res/res_pjsip_t38.c
@@ -361,7 +361,9 @@
 			ast_udptl_set_local_max_ifp(session_media->udptl, state->our_parms.max_ifp);
 			t38_change_state(data->session, session_media, state, T38_ENABLED);
 			ast_sip_session_resume_reinvite(data->session);
-		} else if (data->session->t38state != T38_ENABLED) {
+		} else if ((data->session->t38state != T38_ENABLED) ||
+				((data->session->t38state == T38_ENABLED) &&
+                                (parameters->request_response == AST_T38_REQUEST_NEGOTIATE))) {
 			if (t38_initialize_session(data->session, session_media)) {
 				break;
 			}

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0298494d3da6df3219bbfa4be9aa04015043145c
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>