From 717fa3ae8a1cf913238e520d94cfa9f8485cf143 Mon Sep 17 00:00:00 2001 From: mnicholson Date: Tue, 4 May 2010 16:58:56 +0000 Subject: 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 --- channels/chan_sip.c | 113 +++++++++++++++++++++++++++++++++++++++-------- configs/sip.conf.sample | 10 +++-- include/asterisk/frame.h | 3 +- main/channel.c | 7 +-- 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, ¶meters, 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 -- cgit v1.2.3