Improve handling of T.38 re-INVITEs that arrive before a T.38-capable

application is executing on a channel.

This patch addresses an issue found during working with end-users
using res_fax. If an incoming call is answered in the dialplan, or
jumps to the 'fax' extension due to reception of a CNG tone (with
faxdetect enabled), and then the remote endpoint sends a T.38
re-INVITE, it is possible for the channel's T.38 state to be
'T38_STATE_NEGOTIATING' when the application starts up. Unfortunately,
even if the application wants to use T.38, it can't respond to the
peer's negotiation request, because the AST_CONTROL_T38_PARAMETERS
control frame that chan_sip sent originally has been lost, and the
application needs the content of that frame to be able to formulate a
reply.

This patch adds a new 'request' type to AST_CONTROL_T38_PARAMETERS,
AST_T38_REQUEST_PARMS. If the application sends this request, chan_sip
will re-send the original control frame (with
AST_T38_REQUEST_NEGOTIATE as the request type), and the application
can respond as normal. If this occurs within the five second timeout
in chan_sip, the automatic cancellation of the peer reinvite will be
stopped, and the application will 'own' the negotiation process from
that point onwards.

This also improves the code path in chan_sip to allow sip_indicate(),
when called for AST_CONTROL_T38_PARAMETERS, to be able to return a
non-zero response, which should have been in place before since the
control frame *can* fail to be processed properly. It also modifies
ast_indicate() to return whatever result the channel driver returned
for this control frame, rather than converting all non-zero results
into '-1'. Finally, the new request type intentionally returns a
positive value, so that an application that sends
AST_T38_REQUEST_PARMS can know for certain whether the channel driver
accepted it and will be replying with a control frame of its own, or
whether it was ignored (if the sip_indicate()/ast_indicate() path had
properly supported failure responses before, this would not be
necessary).

This patch also modifies res_fax to take advantage of the new request.

In addition, this patch makes sip_t38_abort() actually lock the
private structure before doing its work... bad programmer, no donut.

This patch also enhances chan_sip's 'faxdetect' support to allow
triggering on T.38 re-INVITEs received as well as CNG tone detection.

Review: https://reviewboard.asterisk.org/r/556/



git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@254450 65c4cc65-6c06-0410-ace0-fbb531ad65f3

1 files changed, 63 insertions, 38 deletions

diff --git a/res/res_fax.c b/res/res_fax.c
index 55e30b0c2..0b04b1b31 100644
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -654,6 +654,7 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
 	struct ast_channel *c = chan;
 	unsigned int orig_write_format = 0, orig_read_format = 0;
 	unsigned int request_t38 = 0;
+	unsigned int send_audio = 1;
 
 	details->our_t38_parameters.version = 0;
 	details->our_t38_parameters.max_ifp = 400;
@@ -662,8 +663,57 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
 
 	chancount = 1;
 
-	/* generate 3 seconds of CED if we are in receive mode */
-	if (details->caps & AST_FAX_TECH_RECEIVE) {
+	switch ((t38_state = ast_channel_get_t38_state(chan))) {
+	case T38_STATE_UNKNOWN:
+		if (details->caps & AST_FAX_TECH_SEND) {
+			if (details->option.allow_audio) {
+				details->caps |= AST_FAX_TECH_AUDIO;
+			} else {
+				/* we are going to send CNG to attempt to stimulate the receiver
+				 * into switching to T.38, since audio mode is not allowed
+				 */
+				send_cng = 0;
+			}
+		} else {
+			/* we *always* request a switch to T.38 if allowed; if audio is also
+			 * allowed, then we will allow the switch to happen later if needed
+			 */
+			if (details->option.allow_audio) {
+				details->caps |= AST_FAX_TECH_AUDIO;
+			}
+			request_t38 = 1;
+		}
+		details->caps |= AST_FAX_TECH_T38;
+		break;
+	case T38_STATE_UNAVAILABLE:
+		details->caps |= AST_FAX_TECH_AUDIO;
+		break;
+	case T38_STATE_NEGOTIATING: {
+		/* the other end already sent us a T.38 reinvite, so we need to prod the channel
+		 * driver into resending their parameters to us if it supports doing so... if
+		 * not, we can't proceed, because we can't create a proper reply without them.
+		 * if it does work, the channel driver will send an AST_CONTROL_T38_PARAMETERS
+		 * with a request of AST_T38_REQUEST_NEGOTIATE, which will be read by the function
+		 * that gets called after this one completes
+		 */
+		struct ast_control_t38_parameters parameters = { .request_response = AST_T38_REQUEST_PARMS, };
+		ast_log(LOG_NOTICE, "Channel is already in T.38 negotiation state; retrieving remote parameters.\n");
+		if (ast_indicate_data(chan, AST_CONTROL_T38_PARAMETERS, &parameters, sizeof(parameters)) != AST_T38_REQUEST_PARMS) {
+			ast_log(LOG_ERROR, "channel '%s' is in an unsupported T.38 negotiation state, cannot continue.\n", chan->name);
+			return -1;
+		}
+		details->caps |= AST_FAX_TECH_T38;
+		details->option.allow_audio = 0;
+		send_audio = 0;
+		break;
+	}
+	default:
+		ast_log(LOG_ERROR, "channel '%s' is in an unsupported T.38 negotiation state, cannot continue.\n", chan->name);
+		return -1;
+	}
+
+	/* generate 3 seconds of CED if we are in receive mode and not already negotiating T.38 */
+	if (send_audio && (details->caps & AST_FAX_TECH_RECEIVE)) {
 		ms = 3000;
 		if (ast_tonepair_start(chan, 2100, 0, ms, 0)) {
 			ast_log(LOG_ERROR, "error generating CED tone on %s\n", chan->name);
@@ -719,36 +769,6 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
 		ast_tonepair_stop(chan);
 	}
 
-	switch ((t38_state = ast_channel_get_t38_state(chan))) {
-	case T38_STATE_UNKNOWN:
-		if (details->caps & AST_FAX_TECH_SEND) {
-			if (details->option.allow_audio) {
-				details->caps |= AST_FAX_TECH_AUDIO;
-			} else {
-				/* we are going to send CNG to attempt to stimulate the receiver
-				 * into switching to T.38, since audio mode is not allowed
-				 */
-				send_cng = 0;
-			}
-		} else {
-			/* we *always* request a switch to T.38 if allowed; if audio is also
-			 * allowed, then we will allow the switch to happen later if needed
-			 */
-			if (details->option.allow_audio) {
-				details->caps |= AST_FAX_TECH_AUDIO;
-			}
-			request_t38 = 1;
-		}
-		details->caps |= AST_FAX_TECH_T38;
-		break;
-	case T38_STATE_UNAVAILABLE:
-		details->caps |= AST_FAX_TECH_AUDIO;
-		break;
-	default:
-		ast_log(LOG_ERROR, "channel '%s' is in an unsupported T.38 negotiation state, cannot continue.\n", chan->name);
-		return -1;
-	}
-
 	if (request_t38) {
 		/* wait up to five seconds for negotiation to complete */
 		timeout = 5000;
@@ -772,19 +792,23 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
 	if (request_t38 || !details->option.allow_audio) {
 		struct ast_silence_generator *silence_gen = NULL;
 
-		if (send_cng != -1) {
+		if (send_audio && (send_cng != -1)) {
 			silence_gen = ast_channel_start_silence_generator(chan);
 		}
 
 		while (timeout > 0) {
 			if (send_cng > 3000) {
-				ast_channel_stop_silence_generator(chan, silence_gen);
-				silence_gen = NULL;
-				ast_tonepair_start(chan, 1100, 0, 500, 0);
+				if (send_audio) {
+					ast_channel_stop_silence_generator(chan, silence_gen);
+					silence_gen = NULL;
+					ast_tonepair_start(chan, 1100, 0, 500, 0);
+				}
 				send_cng = 0;
 			} else if (!chan->generator && (send_cng != -1)) {
-				/* The CNG tone is done so restart silence generation. */
-				silence_gen = ast_channel_start_silence_generator(chan);
+				if (send_audio) {
+					/* The CNG tone is done so restart silence generation. */
+					silence_gen = ast_channel_start_silence_generator(chan);
+				}
 			}
 			/* this timeout *MUST* be 500ms, in order to keep the spacing
 			 * of CNG tones correct when this loop is sending them
@@ -828,6 +852,7 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
 					t38_parameters.request_response = AST_T38_NEGOTIATED;
 					ast_indicate_data(chan, AST_CONTROL_T38_PARAMETERS, &t38_parameters, sizeof(t38_parameters));
 					stop = 0;
+					send_audio = 0;
 					break;
 				case AST_T38_NEGOTIATED:
 					ast_log(LOG_NOTICE, "Negotiated T.38 for %s on %s\n", (details->caps & AST_FAX_TECH_SEND) ? "send" : "receive", chan->name);