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

Kevin Fleming kpfleming at digium.com
Thu Mar 18 13:21:09 CDT 2010


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

(Updated 2010-03-18 13:21:09.278234)


Review request for Asterisk Developers.


Changes
-------

Minor whitespace fix and small optimization.

Note that this patch needed another flag in SIP_PAGE2, so I renumbered all the existing SIP_PAGE2 flags to eliminate gaps and ensure that all available flag bits would be at the end of the flag word.


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.

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


Diffs (updated)
-----

  /trunk/channels/chan_sip.c 253312 
  /trunk/channels/sip/include/sip.h 253312 
  /trunk/configs/sip.conf.sample 253312 
  /trunk/include/asterisk/frame.h 253312 
  /trunk/main/channel.c 253312 
  /trunk/res/res_fax.c 253312 

Diff: https://reviewboard.asterisk.org/r/556/diff


Testing
-------

Tested with Zoiper Free; verified that without this patch Asterisk cannot receive a FAX from Zoiper because the T.38 invite arrives too early, but with this patch the FAX is received as expected. 'faxdetect' tested with Zoiper Free as well.


Thanks,

Kevin




More information about the asterisk-dev mailing list