Summary:	ASTERISK-11795: [patch] T.38 support for chan_h323 with t.38 - t30 conversion
Reporter:	Alexander Anikin (may213)	Labels:
Date Opened:	2008-04-07 19:33:21	Date Closed:	2011-06-07 14:02:47
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) makefile.patch ( 1) t38-patch-20080407.patch
Description:	Hello! There is patch for h323 channel code which enable t.38 proto support, translate t.38 packets to alaw sound packets. ****** ADDITIONAL INFORMATION ****** Also it send request to switch to t.38 mode on receiving 'f' digit (what is fax cng detect) and accept request to switch to t.38 from peer. It can to switch back to audio mode from t.38 by request from peer but can't initiate such request. I've tested this between two asterisk systems. Both have PRI connection to PSTN and interconnect by H.323. Faxes are sent sucessfully on both direction up to 14400KBps. Also tested between asterisk and AudioCodes gate of my external VOIP provider which connect to PSTN by PRI. Faxes is sent to PSTN via AudioCodes at 4800Kbps. This code is based on chan_h323 version from PCadach named chan_h323_exts and spandsp library from Steve Underwood. Thanks Paul and Steve!
Comments:	By: ssfarrel (ssfarrel) 2008-04-08 09:51:45 Fails to build for me On revision 112599  [CC] chan_h323.c -> chan_h323.o chan_h323.c:100:1: warning: "T38_SUPPORT" redefined <command line>:3:1: warning: this is the location of the previous definition chan_h323.c: In function `changemode': chan_h323.c:2637: warning: passing arg 2 of `t38_gateway_init' from incompatible pointer type chan_h323.c:2616: warning: unused variable `f' chan_h323.c: At top level: chan_h323.c:3376: warning: no previous prototype for 'external_udptl_create' chan_h323.c:3407: warning: no previous prototype for 'setup_udptl_connection' chan_h323.c:3446: warning: no previous prototype for 'shutdown_udptl_connection' chan_h323.c: In function `oh323_write': chan_h323.c:902: warning: 't38data' might be used uninitialized in this function g++ -DNDEBUG -DT38_SUPPORT -I../../include -include ../../include/asterisk/autoconfig.h -fPIC  -DP_USE_PRAGMA -D_REENTRANT -Wall  -fPIC -DPIC -I/usr/local/share/pwlib//include -DPTRACING -I/usr/src/openh323_v1_18_0/include -Os  -pipe -felide-constructors -c compat_h323.cxx -o compat_h323.o In file included from compat_h323.cxx:35: ast_h323.h:132: error: ISO C++ forbids declaration of `AST_T38Capability' with no type ast_h323.h:132: error: expected `;' before '*' token make[2]: *** [compat_h323.o] Error 1 make[1]: *** [h323/libchanh323.a] Error 2 make: *** [channels] Error 2 By: Jason Parker (jparker) 2008-04-08 10:21:58 We cannot accept these patches, if they contain any work from people that do not have license agreements with Digium. By: Alexander Anikin (may213) 2008-04-08 11:18:54 Sorry, i forgot to include difference for compat_h323.cxx. It is minimal: --- ../asterisk/channels/h323/compat_h323.cxx +++ channels/h323/compat_h323.cxx @@ -32,6 +32,11 @@ #include <h323.h> #include <transports.h> +#ifdef T38_SUPPORT +#include <h323t38.h> +#include <t38proto.h> +#include "ast_t38.h" +#endif #include "ast_h323.h" #if VERSION(OPENH323_MAJOR,OPENH323_MINOR,OPENH323_BUILD) < VERSION(1,17,3) By: Sergey Tamkovich (sergee) 2008-04-08 15:10:39 may213, First of all new features should be adjusted to trunk, and not to 1.4 branch. Second, i believe that most part of Asterisk developers are using uLaw - not aLaw as we do :) so i think new feature should support both modes natively. Next thing - this patch depends on SpanDSP, so it can't be accepted into asterisk, due to dual-licensing issues. BUT! It can be accepted into asterisk-addons. Here is a roadmap to get this feature in asterisk-addons (as i see it, maybe someone will correct me): 1. Put hooks for t38 processing (converting to *Law) inside of h323 2. Add support for ulaw 3. Put all code related to spanDSP inside asterisk-addons (e.g. inside app_fax, or may be res_fax etc..) Also, i'm not sure that this feature is good from architectural point of view. I believe that we need some general facility for relaying fax between different channels, not some local hacks. But that's a long story, and your code is already works. By: Vlasis Hatzistavrou (vhatz) 2008-04-08 15:52:27 I assume that this patch cannot do H323->SIP or SIP->H323 fax, but can it do H323 to H323 T38 fax? By: Sergey Tamkovich (sergee) 2008-04-08 16:26:54 vhatz, as far as i understand, it can  h323 (t38) -> whatever (alaw) By: Alexander Anikin (may213) 2008-04-08 16:31:11 vhatz, this patch can do H323-SIP, SIP-H323 fax if SIP is in audio mode not t.38. Also it can do Zap-H323 and other-H323 fax if other is in audio mode. this patch cannot t38 passhtrough, but it convert fax audio to t.38 if H323 connection switched to t38 mode. I will try to add same functionality to chan_sip. By: Alexander Anikin (may213) 2008-04-08 16:51:13 sergee, about alaw - h323 channel setup own nativeformat to alaw, but it is not meaning what * can send to this channel only alaw. * setup codec converter what may be ulaw->alaw, g726->alaw, g729->alaw or any others. But fax will work only on fax-ready audio codecs that are *law and g726-32. I think that ulaw is not problem here. about module for addons - i will try to make patch in this way but later. I know and agree, that this model is not the best, but others it is even more complex, as i think. I think we can talk about it in irc channel. By: ssfarrel (ssfarrel) 2008-04-18 08:23:03 Any chance of a sample config for the current pactg. im not seeing it switch to t38 with faxes. By: Dmitry Andrianov (dimas) 2008-04-21 20:06:04 Hmm... I did not look at the patch itsel because I have no idea how H323 works anyway but from your comments here it looks like what patch does is T38 gateway. And I believe that these things should be implemented differently. Namely: Step 1. trunk/1.6 should be taken as a base - there is new AST_CONTROL_T38 frame which allows channel driver to notify the core about T38 switchover as well as core to request a switchover from the channel driver. H323 channel should be modified to generate these control messages as well as react on them. For ideas - grep chan_sip.c for "sip_queryoption" and "AST_CONTROL_T38". When done, it will allow app_fax from addons to send/receive faxes on H323 channels in addition to Zap and SIP ones. That is T38 termination will be done. Step 2. Bridging code (or not?) needs to react on AST_CONTROL_T38 messages from channels relaying them to other leg. This way when H323 switches to T38, it will cause the same switchover to be triggered on a bridged SIP channel. This is something not done currently and there is something to think about because lots of T38 options needs to be passed (which are not passed in a control frame yet). Step 3. Gateway (T38<=>audio) code should not be part of any channel driver. I'm not really sure part of what should it be but probably a part of core/bringing code. This way, again, the same code will be able to serve any technology type - SIP/H323. I understand that a already existin something is better that some theory, however I see no point in making common things specific to different technologies. This way we will end up in partially implemented T38 for H323, partially (the other part!) implemented for SIP etc. By: Sergey Tamkovich (sergee) 2008-04-22 01:38:21 dimas, agreed By: Dmitry Andrianov (dimas) 2008-04-22 03:59:53 Just wanted to add that I would suggest making each step a separate issue - in my experience it lets patches to be commited more quickly. For example Step 1 should be (in theory!) easy to implement and it may be immediately used/tested by people - there are a lot who wants T38 termination for H323 channels. By: Dmitry Andrianov (dimas) 2008-05-12 04:56:23 may213, have you lost any interest to your patch ? We can chat on IRC/email to discuss things... By: Joshua C. Colp (jcolp) 2008-05-13 15:36:15 Suspending due to lack of response to dimas' comments. If you wish to follow them feel free to reopen.