[asterisk-dev] [Code Review] Improve handling of T.38 re-INVITEs that arrive before a T.38-capable application is executing on a channel.

[Matthew Nicholson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/556/#review1674
-----------------------------------------------------------



/trunk/include/asterisk/frame.h
<https://reviewboard.asterisk.org/r/556/#comment3749>

    This line is indented with tabs, where as the other lines in this enum are indented with spaces.  The rest of the file seems to be indented with tabs.


- Matthew


On 2010-03-15 18:01:38, Kevin Fleming wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/556/
> -----------------------------------------------------------
> 
> (Updated 2010-03-15 18:01:38)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 252670 
>   /trunk/include/asterisk/frame.h 252670 
>   /trunk/main/channel.c 252670 
>   /trunk/res/res_fax.c 252709 
> 
> Diff: https://reviewboard.asterisk.org/r/556/diff
> 
> 
> Testing
> -------
> 
> Compile testing only so far.
> 
> 
> Thanks,
> 
> Kevin
> 
>