[Asterisk-code-review] res_fax: gateway sends T.38 request to both endpoints if V.21 detected (...asterisk[13])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/11485 )

Change subject: res_fax: gateway sends T.38 request to both endpoints if V.21 detected
......................................................................

res_fax: gateway sends T.38 request to both endpoints if V.21 detected

According T.38 Gateway 'Use case 3'
https://wiki.asterisk.org/wiki/display/AST/T.38+Gateway
T.38 Gateway should send T.38 negotiation request to called endpoint
if FAX preamble (using V.21 detector) generated by called endpoint.
But it does not, because fax_gateway_detect_v21 constructs T.38
negotiation request, but forwards it only to other channel,
not to the channel on which FAX preamble is detected.

Some SIP endpoints could be improperly configured to rely on the other side
to initiate T.38 re-INVITEs.

With this patch the T.38 Gateway tries to negotiate with both sides
by sending T.38 negotiation request to both endpoints supported T.38.

Change-Id: I73bb24799bfe1a48adae9c034a2edbae54cc2a39
---
M res/res_fax.c
1 file changed, 34 insertions(+), 8 deletions(-)

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



diff --git a/res/res_fax.c b/res/res_fax.c
index d3866ad..8650945 100644
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -2921,6 +2921,9 @@
 		return 0;
 	}
 
+	/* if we start gateway we don't need v21 detection sessions any more */
+	destroy_v21_sessions(gateway);
+
 	/* create the FAX session */
 	if (!(s = fax_session_new(details, chan, gateway->s, gateway->token))) {
 		gateway->token = NULL;
@@ -2962,7 +2965,7 @@
 }
 
 /*! \pre chan is locked on entry */
-static struct ast_frame *fax_gateway_request_t38(struct fax_gateway *gateway, struct ast_channel *chan, struct ast_frame *f)
+static struct ast_frame *fax_gateway_request_t38(struct fax_gateway *gateway, struct ast_channel *chan)
 {
 	struct ast_frame *fp;
 	struct ast_control_t38_parameters t38_parameters = {
@@ -2981,7 +2984,7 @@
 	if (!details) {
 		ast_log(LOG_ERROR, "no FAX session details found on chan %s for T.38 gateway session, odd\n", ast_channel_name(chan));
 		ast_framehook_detach(chan, gateway->framehook);
-		return f;
+		return NULL;
 	}
 
 	t38_parameters_fax_to_ast(&t38_parameters, &details->our_t38_parameters);
@@ -2989,7 +2992,7 @@
 
 	if (!(fp = ast_frisolate(&control_frame))) {
 		ast_log(LOG_ERROR, "error generating T.38 request control frame on chan %s for T.38 gateway session\n", ast_channel_name(chan));
-		return f;
+		return NULL;
 	}
 
 	gateway->t38_state = T38_STATE_NEGOTIATING;
@@ -3018,17 +3021,40 @@
 
 	if (gateway->detected_v21) {
 		enum ast_t38_state state_other;
+		enum ast_t38_state state_active;
+		struct ast_frame *fp;
 
 		destroy_v21_sessions(gateway);
 
 		ast_channel_unlock(chan);
+		state_active = ast_channel_get_t38_state(active);
 		state_other = ast_channel_get_t38_state(other);
 		ast_channel_lock(chan);
-		if (state_other == T38_STATE_UNKNOWN) {
-			ast_debug(1, "detected v21 preamble from %s\n", ast_channel_name(active));
-			return fax_gateway_request_t38(gateway, chan, f);
+
+		ast_debug(1, "detected v21 preamble from %s\n", ast_channel_name(active));
+
+		if (state_active == T38_STATE_UNKNOWN || state_other == T38_STATE_UNKNOWN) {
+			if (!(fp = fax_gateway_request_t38(gateway, chan))) {
+				return f;
+			}
+			/* May be called endpoint is improperly configured to rely on the calling endpoint
+			 * to initiate T.38 re-INVITEs, send T.38 negotiation request to called endpoint */
+			if (state_active == T38_STATE_UNKNOWN) {
+				ast_debug(1, "sending T.38 negotiation request to %s\n", ast_channel_name(active));
+				if (active == chan) {
+					ast_channel_unlock(chan);
+				}
+				ast_write(active, fp);
+				if (active == chan) {
+					ast_channel_lock(chan);
+				}
+			}
+			if (state_other == T38_STATE_UNKNOWN) {
+				ast_debug(1, "sending T.38 negotiation request to %s\n", ast_channel_name(other));
+				return fp;
+			}
 		} else {
-			ast_debug(1, "detected v21 preamble on %s, but %s does not support T.38 for T.38 gateway session\n", ast_channel_name(active), ast_channel_name(other));
+			ast_debug(1, "neither %s nor %s support T.38 for T.38 gateway session\n", ast_channel_name(active), ast_channel_name(other));
 		}
 	}
 
@@ -3190,7 +3216,7 @@
 		ast_channel_lock(chan);
 		if (state_other == T38_STATE_UNKNOWN) {
 			gateway->t38_state = T38_STATE_UNAVAILABLE;
-		} else {
+		} else if (state_other != T38_STATE_NEGOTIATING) {
 			ast_framehook_detach(chan, details->gateway_id);
 			details->gateway_id = -1;
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I73bb24799bfe1a48adae9c034a2edbae54cc2a39
Gerrit-Change-Number: 11485
Gerrit-PatchSet: 1
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190624/68569315/attachment.html>