Merged revisions 254450 via svnmerge from

https://origsvn.digium.com/svn/asterisk/trunk

........
  r254450 | kpfleming | 2010-03-25 10:27:31 -0500 (Thu, 25 Mar 2010) | 49 lines
  
  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: http://svn.digium.com/svn/asterisk/branches/1.6.2@260884 f38db490-d61c-443f-a65b-d21fe96a405b

4 files changed, 107 insertions, 26 deletions

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index a37cca565..9ae200eb8 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -1397,7 +1397,10 @@ struct sip_auth {
 #define SIP_PAGE2_RFC2833_COMPENSATE    (1 << 25)	/*!< DP: Compensate for buggy RFC2833 implementations */
 #define SIP_PAGE2_BUGGY_MWI		(1 << 26)	/*!< DP: Buggy CISCO MWI fix */
 #define SIP_PAGE2_DIALOG_ESTABLISHED    (1 << 27)       /*!< 29: Has a dialog been established? */
-#define SIP_PAGE2_FAX_DETECT		(1 << 28)		/*!< DP: Fax Detection support */
+#define SIP_PAGE2_FAX_DETECT		(3 << 28)	/*!< DP: Fax Detection support */
+#define SIP_PAGE2_FAX_DETECT_CNG	(1 << 28)	/*!< DP: Fax Detection support - detect CNG in audio */
+#define SIP_PAGE2_FAX_DETECT_T38	(2 << 28)	/*!< DP: Fax Detection support - detect T.38 reinvite from peer */
+#define SIP_PAGE2_FAX_DETECT_BOTH	(3 << 28)	/*!< DP: Fax Detection support - detect both */
 #define SIP_PAGE2_REGISTERTRYING        (1 << 29)       /*!< DP: Send 100 Trying on REGISTER attempts */
 #define SIP_PAGE2_UDPTL_DESTINATION     (1 << 30)       /*!< DP: Use source IP of RTP as destination if NAT is enabled */
 #define SIP_PAGE2_VIDEOSUPPORT_ALWAYS	(1 << 31)       /*!< DP: Always set up video, even if endpoints don't support it */
@@ -5406,6 +5409,11 @@ static int sip_call(struct ast_channel *ast, char *dest, int timeout)
 	res = 0;
 	ast_set_flag(&p->flags[0], SIP_OUTGOING);
 
+	/* T.38 re-INVITE FAX detection should never be done for outgoing calls,
+	 * so ensure it is disabled.
+	 */
+	ast_clear_flag(&p->flags[1], SIP_PAGE2_FAX_DETECT_T38);
+
 	if (p->options->transfer) {
 		char buf[SIPBUFSIZE/2];
 
@@ -6416,22 +6424,24 @@ static int sip_transfer(struct ast_channel *ast, const char *dest)
 }
 
 /*! \brief Helper function which updates T.38 capability information and triggers a reinvite */
-static void interpret_t38_parameters(struct sip_pvt *p, const struct ast_control_t38_parameters *parameters)
+static int interpret_t38_parameters(struct sip_pvt *p, const struct ast_control_t38_parameters *parameters)
 {
+	int res = 0;
+
 	if (!ast_test_flag(&p->flags[1], SIP_PAGE2_T38SUPPORT)) {
-		return;
+		return -1;
 	}
 	switch (parameters->request_response) {
 	case AST_T38_NEGOTIATED:
 	case AST_T38_REQUEST_NEGOTIATE:         /* Request T38 */
 		/* Negotiation can not take place without a valid max_ifp value. */
 		if (!parameters->max_ifp) {
-				change_t38_state(p, T38_DISABLED);
-				if (p->t38.state == T38_PEER_REINVITE) {
-					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"));
-					transmit_response_reliable(p, "488 Not acceptable here", &p->initreq);
-				}
-				break;
+			change_t38_state(p, T38_DISABLED);
+			if (p->t38.state == T38_PEER_REINVITE) {
+				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"));
+				transmit_response_reliable(p, "488 Not acceptable here", &p->initreq);
+			}
+			break;
 		} else if (p->t38.state == T38_PEER_REINVITE) {
 			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"));
 			p->t38.our_parms = *parameters;
@@ -6473,9 +6483,28 @@ static void interpret_t38_parameters(struct sip_pvt *p, const struct ast_control
 		} else if (p->t38.state == T38_ENABLED)
 			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;
+
+		if (p->t38.state == T38_PEER_REINVITE) {
+			AST_SCHED_DEL(sched, p->t38id);
+			parameters.max_ifp = ast_udptl_get_far_max_ifp(p->udptl);
+			parameters.request_response = AST_T38_REQUEST_NEGOTIATE;
+			ast_queue_control_data(p->owner, AST_CONTROL_T38_PARAMETERS, &parameters, sizeof(parameters));
+			/* we need to return a positive value here, so that applications that
+			 * send this request can determine conclusively whether it was accepted or not...
+			 * older versions of chan_sip would just silently accept it and return zero.
+			 */
+			res = AST_T38_REQUEST_PARMS;
+		}
+		break;
+	}
 	default:
+		res = -1;
 		break;
 	}
+
+	return res;
 }
 
 /*! \brief Play indication to user 
@@ -6565,9 +6594,10 @@ static int sip_indicate(struct ast_channel *ast, int condition, const void *data
 	case AST_CONTROL_T38_PARAMETERS:
 		if (datalen != sizeof(struct ast_control_t38_parameters)) {
 			ast_log(LOG_ERROR, "Invalid datalen for AST_CONTROL_T38_PARAMETERS. Expected %d, got %d\n", (int) sizeof(struct ast_control_t38_parameters), (int) datalen);
+			res = -1;
 		} else {
 			const struct ast_control_t38_parameters *parameters = data;
-			interpret_t38_parameters(p, parameters);
+			res = interpret_t38_parameters(p, parameters);
 		}
 		break;
 	case AST_CONTROL_SRCUPDATE:
@@ -7051,20 +7081,20 @@ static struct ast_frame *sip_read(struct ast_channel *ast)
 	p->lastrtprx = time(NULL);
 
 	/* If we detect a CNG tone and fax detection is enabled then send us off to the fax extension */
-	if (faxdetected && ast_test_flag(&p->flags[1], SIP_PAGE2_FAX_DETECT)) {
+	if (faxdetected && ast_test_flag(&p->flags[1], SIP_PAGE2_FAX_DETECT_CNG)) {
 		ast_channel_lock(ast);
 		if (strcmp(ast->exten, "fax")) {
 			const char *target_context = S_OR(ast->macrocontext, ast->context);
 			ast_channel_unlock(ast);
 			if (ast_exists_extension(ast, target_context, "fax", 1, ast->cid.cid_num)) {
-				ast_verbose(VERBOSE_PREFIX_2 "Redirecting '%s' to fax extension\n", ast->name);
+				ast_verbose(VERBOSE_PREFIX_2 "Redirecting '%s' to fax extension due to CNG detection\n", ast->name);
 				pbx_builtin_setvar_helper(ast, "FAXEXTEN", ast->exten);
 				if (ast_async_goto(ast, target_context, "fax", 1)) {
 					ast_log(LOG_NOTICE, "Failed to async goto '%s' into fax of '%s'\n", ast->name, target_context);
 				}
 				fr = &ast_null_frame;
 			} else {
-				ast_log(LOG_NOTICE, "Fax detected but no fax extension\n");
+				ast_log(LOG_NOTICE, "FAX CNG detected but no fax extension\n");
                         }
 		} else {
 			ast_channel_unlock(ast);
@@ -8525,6 +8555,25 @@ static int process_sdp(struct sip_pvt *p, struct sip_request *req, int t38action
 			} else if ((t38action == SDP_T38_INITIATE) &&
 				   p->owner && p->lastinvite) {
 				change_t38_state(p, T38_PEER_REINVITE); /* T38 Offered in re-invite from remote party */
+				/* If fax detection is enabled then send us off to the fax extension */
+				if (ast_test_flag(&p->flags[1], SIP_PAGE2_FAX_DETECT_T38)) {
+					ast_channel_lock(p->owner);
+					if (strcmp(p->owner->exten, "fax")) {
+						const char *target_context = S_OR(p->owner->macrocontext, p->owner->context);
+						ast_channel_unlock(p->owner);
+						if (ast_exists_extension(p->owner, target_context, "fax", 1, p->owner->cid.cid_num)) {
+							ast_verbose(VERBOSE_PREFIX_2 "Redirecting '%s' to fax extension due to peer T.38 re-INVITE\n", p->owner->name);
+							pbx_builtin_setvar_helper(p->owner, "FAXEXTEN", p->owner->exten);
+							if (ast_async_goto(p->owner, target_context, "fax", 1)) {
+								ast_log(LOG_NOTICE, "Failed to async goto '%s' into fax of '%s'\n", p->owner->name, target_context);
+							}
+						} else {
+							ast_log(LOG_NOTICE, "T.38 re-INVITE detected but no fax extension\n");
+						}
+					} else {
+						ast_channel_unlock(p->owner);
+					}
+				}
 			}
 		} else {
 			ast_udptl_stop(p->udptl);
@@ -19650,15 +19699,24 @@ static int do_magic_pickup(struct ast_channel *channel, const char *extension, c
 	return 0;
 }
 
+/*! \brief Called to deny a T38 reinvite if the core does not respond to our request */
 static int sip_t38_abort(const void *data)
 {
 	struct sip_pvt *p = (struct sip_pvt *) data;
 
-	change_t38_state(p, T38_DISABLED);
-	transmit_response_reliable(p, "488 Not acceptable here", &p->initreq);
-	p->t38id = -1;
-	dialog_unref(p, "unref the dialog ptr from sip_t38_abort, because it held a dialog ptr");
-
+	sip_pvt_lock(p);
+	/* an application may have taken ownership of the T.38 negotiation on this
+	 * channel while we were waiting to grab the lock... if it did, the scheduler
+	 * id will have been reset to -1, which is our indication that we do *not*
+	 * want to abort the negotiation process
+	 */
+	if (p->t38id != -1) {
+		change_t38_state(p, T38_DISABLED);
+		transmit_response_reliable(p, "488 Not acceptable here", &p->initreq);
+		p->t38id = -1;
+		dialog_unref(p, "unref the dialog ptr from sip_t38_abort, because it held a dialog ptr");
+	}
+	sip_pvt_unlock(p);
 	return 0;
 }
 
@@ -23428,7 +23486,24 @@ static int handle_common_options(struct ast_flags *flags, struct ast_flags *mask
 		ast_set2_flag(&flags[1], ast_true(v->value), SIP_PAGE2_IGNORESDPVERSION);
 	} else if (!strcasecmp(v->name, "faxdetect")) {
 		ast_set_flag(&mask[1], SIP_PAGE2_FAX_DETECT);
-		ast_set2_flag(&flags[1], ast_true(v->value), SIP_PAGE2_FAX_DETECT);
+		if (ast_true(v->value)) {
+			ast_set_flag(&flags[1], SIP_PAGE2_FAX_DETECT_BOTH);
+		} else if (ast_false(v->value)) {
+			ast_clear_flag(&flags[1], SIP_PAGE2_FAX_DETECT_BOTH);
+		} else {
+			char *buf = ast_strdupa(v->value);
+			char *word, *next = buf;
+
+			while ((word = strsep(&next, ","))) {
+				if (!strcasecmp(word, "cng")) {
+					ast_set_flag(&flags[1], SIP_PAGE2_FAX_DETECT_CNG);
+				} else if (!strcasecmp(word, "t38")) {
+					ast_set_flag(&flags[1], SIP_PAGE2_FAX_DETECT_T38);
+				} else {
+					ast_log(LOG_WARNING, "Unknown faxdetect mode '%s' on line %d.\n", word, v->lineno);
+				}
+			}
+		}
 	} else if (!strcasecmp(v->name, "rfc2833compensate")) {
 		ast_set_flag(&mask[1], SIP_PAGE2_RFC2833_COMPENSATE);
 		ast_set2_flag(&flags[1], ast_true(v->value), SIP_PAGE2_RFC2833_COMPENSATE);
diff --git a/configs/sip.conf.sample b/configs/sip.conf.sample
index 760b1fa53..291f86e8d 100644
--- a/configs/sip.conf.sample
+++ b/configs/sip.conf.sample
@@ -471,9 +471,13 @@ srvlookup=yes                   ; Enable DNS SRV lookups on outbound calls
 ;                                       ; send 400 byte T.38 FAX packets to it.
 ;
 ; FAX detection will cause the SIP channel to jump to the 'fax' extension (if it exists)
-; when a CNG tone is detected on an incoming call.
-; 
-; faxdetect = yes              ; Default false	
+; based one or more events being detected. The events that can be detected are an incoming
+; CNG tone or an incoming T.38 re-INVITE request.
+;
+; faxdetect = yes		; Default 'no', 'yes' enables both CNG and T.38 detection
+; faxdetect = cng		; Enables only CNG detection
+; faxdetect = t38		; Enables only T.38 detection
+; faxdetect = both		; Enables both CNG and T.38 detection (same as 'yes')
 ;
 ;----------------------------------------- OUTBOUND SIP REGISTRATIONS  ------------------------
 ; Asterisk can register as a SIP user agent to a SIP proxy (provider)
diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index 57760f162..8d38ffefb 100644
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -325,7 +325,8 @@ enum ast_control_t38 {
 	AST_T38_REQUEST_TERMINATE,	/*!< Terminate T38 on a channel (fax to voice) */
 	AST_T38_NEGOTIATED,		/*!< T38 negotiated (fax mode) */
 	AST_T38_TERMINATED,		/*!< T38 terminated (back to voice) */
-	AST_T38_REFUSED			/*!< T38 refused for some reason (usually rejected by remote end) */
+	AST_T38_REFUSED,		/*!< T38 refused for some reason (usually rejected by remote end) */
+	AST_T38_REQUEST_PARMS,		/*!< request far end T.38 parameters for a channel in 'negotiating' state */
 };
 
 enum ast_control_t38_rate {
diff --git a/main/channel.c b/main/channel.c
index 974420a79..4d1efab6e 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -3204,9 +3204,10 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
 		 * control frames, so we need to return failure, but there
 		 * is also no value in the log message below being emitted
 		 * since failure to handle these frames is not an 'error'
-		 * so just return right now.
-		 */
-		return -1;
+		 * so just return right now. in addition, we want to return
+		 * whatever value the channel driver returned, in case it
+		 * has some meaning.*/
+		return res;
 	case AST_CONTROL_RINGING:
 		ts = ast_get_indication_tone(chan->zone, "ring");
 		/* It is common practice for channel drivers to return -1 if trying