[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]

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

(Updated 2010-03-18 15:20:02.573406)


Review request for Asterisk Developers.


Changes
-------

Jean pointed out (off line) that the T.38 re-INVITE faxdetect functionality should only be available on incoming calls; for outgoing calls, we're the FAX sender, and very likely already running (or preparing to run) a sending FAX application, so there's no value in faxdetect being enabled.


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 253459 
  /trunk/channels/sip/include/sip.h 253459 
  /trunk/configs/sip.conf.sample 253459 
  /trunk/include/asterisk/frame.h 253459 
  /trunk/main/channel.c 253459 
  /trunk/res/res_fax.c 253459 

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