Summary:	ASTERISK-24680: [patch] - Added FAXOPT(gateway)=t38 option
Reporter:	adomjan (adomjan)	Labels:	patch
Date Opened:	2015-01-13 08:45:58.000-0600	Date Closed:
Priority:	Minor	Regression?
Status:	Open/New	Components:	Resources/res_fax
Versions:	11.7.0 13.18.4	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) res_fax.c-disable_v21_detection_on_gateway_fix_formatting.patch ( 1) res_fax.c-disable_v21_detection_on_gateway.patch
Description:	Added faxgateway function without v21 preamble detection. The faxgateway starts only on T38 ReINVITE.
Comments:	By: Matt Jordan (mjordan) 2015-01-13 14:03:45.041-0600 A few comments: # What's the reason for needing this option? # Your patch has some coding guideline violations. Please read the coding guidelines on the Asterisk wiki: [https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines] # A new option such as this really needs a test that verifies that the existing Fax options are not impacted. In particular, I'm also curious what happens if you force the framehook into gateway mode with no V.21 preamble detection, and the V.21 preamble is never received. By: adomjan (adomjan) 2015-01-14 03:38:45.107-0600 1. - the most gatways have configurable fax detection mode, cng and|or t38 | Telco | --- SIP trunk inband fax ---| asterisk | ----  | user gateways |  - many user gateways have not realible IP connection  - sume users have POS terminals it's better not trying T38, these GWs T38 mode off  - spare CPU cycles by avoiding the tone detection  - I can reach a similar funkcionality whithout this patch. When I enable T38 on Telco SIP trunk, the gateway won't detect V.21. The user GW send T38 REINVITE, asterisk send T38 REINVITE to telco, refuse the TELCO and fall back inband, asterisk starts T38 fax gateway. Some case the fallback is not successful  due to the bug in system of the Telco. 2. I will check 3. I don't now... All these kind of case the GWs sent REINVITE to switch back from T38. You can reach it without this patch see last element of 1. By: Rusty Newton (rnewton) 2015-01-29 18:46:15.421-0600 {quote} 2. I will check {quote} Setting back into Waiting on Feedback By: adomjan (adomjan) 2015-01-30 07:15:32.579-0600 Just a little code formatting fix. By: Matt Jordan (mjordan) 2015-02-21 11:39:39.085-0600 Be sure you hit "Send Back" when you've provided feedback. Otherwise, a bug marshal might miss the issue. As this is a New Feature, this should be put up for a detailed peer review. Note that without tests, this patch can only go into trunk, and you may still be asked to provide tests that help to (a) verify its correctness and (b) prevent future enhancements and bug fixes from breaking the new feature. Other developers can help point you in the right direction for doing that. I'll comment here with some additional helpful links in putting the patch up for review. Thanks for the contribution! By: Matt Jordan (mjordan) 2015-02-21 11:40:11.708-0600 Thanks for the contribution! If you'd like your contribution to be included faster, you should submit your patch for code review by the Asterisk Developer Community. To do so, please follow the Code Review [1] instructions on the wiki. Be sure to: * Verify that your patch conforms to the Coding Guidelines [2] * Review the Code Review Checklist [3] for common items reviewers will look for * If necessary, provide tests for the Asterisk Test Suite that verify the correctness of your patch [4] * As this is a new feature, please read the New Feature Guidelines [5] * Make sure your new feature applies cleanly to Asterisk trunk When ready, submit your patch and any tests to Review Board [6] for code review. Thanks! [1] https://wiki.asterisk.org/wiki/display/AST/Code+Review [2] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines [3] https://wiki.asterisk.org/wiki/display/AST/Code+Review+Checklist [4] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Test+Suite+Documentation [5] https://wiki.asterisk.org/wiki/display/AST/New+Feature+Guidelines [6] https://wiki.asterisk.org/wiki/display/AST/Review+Board+Usage