[Home]

Summary:ASTERISK-10380: [patch] SendFAX/ReceiveFAX
Reporter:Dmitry Andrianov (dimas)Labels:
Date Opened:2007-09-24 04:57:05Date Closed:2007-12-14 15:53:06.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 10815.patch
( 1) 10815-2.patch
( 2) addons-trunk.patch
( 3) app_fax.c
( 4) app_fax-451-1.patch
( 5) conf-10815.patch
( 6) fax_configs_patch.patch
( 7) fax_makefiles_fix.patch
( 8) v2-app_fax.c
( 9) v2-app_fax.c.patch
(10) v2-full.patch
Description:sendfax/receivefax applications are replacement for well known txfax/rxfax. These are based on the same idea and also use SpanDSP GPL library by Steve Underwood.

I hope these can be included in basic Asterisk distribution (or to addons) just to make it easier for people obtaining fax support for Asterisk. Also, this implementation fixes couple of bugs in applications which prevented me using txfax/rxfax out of the box.

Notes:
* receivefax does not allow specifying %d in the file name. To me, it is up to dialplan to form really unique file name.
* debug and answering/calling mode options are specified in a way different from txfax/rxfax

****** ADDITIONAL INFORMATION ******

Items 1 and 2 from the list below were critical for me because I weren't able using rxfax/txfax without crashing and locking Zap channels. I guess if busy detection works well on your zap channels, you are not affected. Unfortunately busy detection does not work very well on my lines and may take time and since number of Zap lines in my case is very limited, I was affected by every single fax application hung.

1. In txfax/rxfax all conditions allowing application to leave main loop were related status of the Asterisk channel. That is ast_waitfor/ast_write returning -1 or ast_read returning NULL. That usually means channel is closed. Loop does not terminate when fax sent/received successfully. This means application depends on good busy detection on Zap channels and if that detection does not work properly (as it was in my case), loop can run forever and you won't ever know. sendfax/receivefax have three conditions which terminate main loop:
* same as in txfax/rxfax (channel is closed, hangup)
* completion of transmission (either successful or not)
* watchdog timer - if SpanDSP T30 state does not change for 5 minutes or if the application runs more than 30 minutes in total

2. sndfax/receivefax verify format of frames before processing. This fixes crash when frame in ulaw/alaw format instead of expected slinear was processed. (This happend because if alaw frame was queued before ast_set_read_format(SLINEAR), the queued frame won't be converted and ast_read will return it as is).

3. Because for txfax/rxfax the only way when loop is terminated was related to errors with the channel, there was no sense in continuing dialplan execution ever. sendfax/receivefax do return zero if operation was successful while channel is still open so dialplan may continue.

4. Option parsing in sendfax/receivefax done "in the Asterisk way" using corresponding API so second parameter is a set of single-letter options and parameter delimiter is not hardcoded into application.

5. In addition to FaxReceived event rxfax used to fire, sendfax also fires FaxSent with exactly the same parameters. Not sure if it is needed feature, but it was easier to implement that not.

6. FAXPAGES, FAXRESOLUTION and FAXBITRATE variables are set not only by receivefax but also by sendfax. Again, it was just easier to have this feature than to suppress it.
Comments:By: Dmitry Andrianov (dimas) 2007-09-24 05:04:09

In addition to the app_fax.c there is a need to patch makefile/configure/menuselect in the same way txfax/rxfax used to do. I'm attaching the patch for asterisk 1.4 rev 81923 but it needs to be changed appropriately if the application goes to asterisk-addons.

By: Igor Goncharovsky (igorg) 2007-09-24 09:09:03

I think, because this use spandsp it can be only in addons. Also you need prepare pathches for trunk, not for 1.4

By: Eliel Sardanons (eliel) 2007-09-24 11:18:20

This is based on app_rxfax and app_txfax from Steve Underwood, and is GPL, you can't give a disclaimer on file cause the dual license needed by digium to commit to asterisk won't accept it...

By: Dmitry Andrianov (dimas) 2007-09-24 11:34:14

I definitely can disclaim my changes.
Steve will also need to give Digium rights on original app_txfax/app_rxfax. It is clear. In email conversation Steve wrote that he consider these two files "trivial piece of code" so I hope he will be able to give them to Digium.

By: Dmitry Andrianov (dimas) 2007-09-24 11:50:19

addons-trunk.patch patches addons to include app_fax into makefiles.

I haven't tested the application under trunk because I only use 1.4 branch.

By: Igor Goncharovsky (igorg) 2007-10-01 05:11:22

I compile it with trunk. Full patch for asterisk-addons attached. Note, that you need to create new configure with autoconf.

By: Dmitry Andrianov (dimas) 2007-10-01 05:17:52

note that your patch also changes something related to app_saycountpl in menuselect-tree...

By: Igor Goncharovsky (igorg) 2007-10-01 23:17:58

Yes, I know it. I'll make another patch may be fo fix issues with app_saycountpl. Can you test also this applications with trunk and directly ask Steve about disclaim this code and put it into addons?

With patch I've posted asterisk crash only on try of execute both of apps.

By: Dmitry Andrianov (dimas) 2007-10-02 01:00:44

Igor, first of all I'm not sure if Steve actually needs to disclaim anything anymore because asterisk-addons allow using GPL licensed stuff and original txfax/rxfax are in fact GPL. So I need some clarification from Digium guys.

And could you explain what you mean by "With patch I've posted asterisk crash only on try of execute both of apps"? Do the applications crash when you applied them to trunk? Could you show me a stacktrace please.

By: Igor Goncharovsky (igorg) 2007-10-02 02:02:19

Addons can use GPL libraries, but it is also under dual-licensing, I think. Also we can talk about bugs on some form (asteriskforum.ru), over IRC (IgorG on #asterisk-dev) or email (igi-go(doggy)ya.ru).

Asterisk crash and do not produce any core file. I think this caused by some changes in trunk. I have no compilation warnings, but execution of dialplan with SendFAX no ever begin when I've call fax number.

By: Dmitry Andrianov (dimas) 2007-10-02 04:28:45

I've put everything into single patch (10815.patch) including rebuilt configure script so autoconf is not required to be run.

By: Igor Goncharovsky (igorg) 2007-10-02 04:43:17

Looks good, you need to add following in Makefile:

@@ -143,6 +145,9 @@
app_addon_sql_mysql.so: app_addon_sql_mysql.o
       $(CC) $(SOLINK) -o $@ $< $(MYSQLCLIENT_LIB)

+app_fax.so: app_fax.o
+       $(CC) $(SOLINK) -o $@ $< $(SPANDSP_LIB)
+
chan_mobile.so: chan_mobile.o
       $(CC) $(SOLINK) -o $@ $< $(BLUETOOTH_LIB)

This make link necessary  libraries linked to app_fax.so

By: Dmitry Andrianov (dimas) 2007-10-02 04:59:33

Uploaded 10815-2.patch with your changes

By: Andrew Lindh (andrew) 2007-10-02 09:07:28

I have been using RXfax for a while. The newer SPANDSP code changed how the exit was done when the fax worked/failed but not due to a channel issue. I changed the code so it exists in a standard manner and then do post fax processing in the dialplan. It works great for me every time. It does not support T.38 and will fail if T.38 is enabled and used with an external gateway (ie. cisco). This would need to be fixed by supporting T.38 or a workaround to disable T.38 when called.

By: Eliel Sardanons (eliel) 2007-10-02 10:41:14

I thought asterisk-addons was just GPL and not need for dual-licensing, if not, why you are putting this application on addons and not on the core if you have the disclaimer from Steve?

By: Dmitry Andrianov (dimas) 2007-10-02 10:59:21

No, I do not have disclaimer from Steve at the moment. I only have his words that "that I couldn't give a damn about app_rxfax.c and app_txfax.c, as they are trivial pieces of code".

Also, it is unclear at the moment if the disclaimer is needed at all to add applications to addons.

But even if we had this disclaimer, sendfax/receivefax still could not be added to core asterisk because these applications depends on GPL library (SpanDSP) which is not disclaimed. And without the library they are useless.

By: Jason Parker (jparker) 2007-10-02 10:59:38

We would need a disclaimer from Steve for rxfax/txfax, and they would also only be able to go into addons, since spandsp is GPL.

By: Andrew Lindh (andrew) 2007-10-02 14:06:13

There are other parts of Asterisk that depend on external support that are not disclamed.

By: Jason Parker (jparker) 2007-10-02 14:12:11

They don't have to be disclaimed, but they do have to allow for linking from non-GPL applications.

Most of the things we link against are LGPL or BSD-style.

By: Dmitry Andrianov (dimas) 2007-10-02 14:14:35

andrew, regardin your note 0071308. I'm interested to know what kind of modifications you applied to txfax/rxfax. Also, I do not quite understand what you mean by "newer SpanDSP code" since I started looking at _latest_ version of rxfax/txfax and it had all the problem I mentioned.
Could you please contact me at asterisk(atsign) dima.spb.ru

By: Dmitry Andrianov (dimas) 2007-10-02 14:16:48

qwell, what is the difference between licensing policy for asterisk-core and asterisk-addons then?

By: Jason Parker (jparker) 2007-10-02 14:23:25

Take a look at the file doc/mysql.txt in asterisk/branches/1.4/

By: steveu (steveu) 2007-10-02 19:08:27

You can do as you please with rxfax and txfax. They are just trivial pieces of code to interface Asterisk with spandsp.

By: Igor Goncharovsky (igorg) 2007-10-03 21:35:33

Here is one response about testing from Russian community:

----
installed good on 1.4.12 with spandsp4pre9

--------
SendFAX
--------
1. from 56 faxes send 46 using SIP <-> SIP (hardware fax)
2. using SIP <-> ZAP sent 10 from 56 with some tuning of hardware fax on ZAP
3. using ZAP <-> ZAP setn 52 from 56
(codec is always g711)

----------
ReceiveFAX
----------
codec is g711
1. 26 from 56 received
2. 0 from 56 received
3. 47 from 56 received

While receiving/sending faxes with G.711 there is no crash. :)

By: Badalian Vyacheslav (slavon) 2007-10-04 06:03:13

Do you add T38 support?

By: Igor Goncharovsky (igorg) 2007-10-04 06:41:12

No, this patch without T.38 support.

By: steveu (steveu) 2007-10-05 02:21:13

The current offering is only applications code for fax origination/termination. T.38 termination support requires further changes to the Asterisk core, beyond those made for T.38 passthrough (those additions, in 1.4, were intended to put al the infrastructure in place, but some bits were missed). T.38 gateway really requires a new, though not very complex, application. The basic gateway stuff is in spandsp.

By: Dmitry Andrianov (dimas) 2007-10-05 02:37:33

Steve, why new application? I balieve if "basic T38 stuff" is in SpanDSP, we will be able to extend SendFAX/ReceiveFAX in a way that it understands current situation (whether T38 is present on the channel or not) and act appropriately?

I would do it already but I just do not have T38 peers and haven't read the T38 specs yet. For "regular" faxes reading of specs was not required because your SpanDSP library does everything :) hiding complexity from us but for T38 it looks like a lot of things involved on the SIP side, so I need to understand how the protocol works, how each peer signesl capabilities to each other etc...

By: steveu (steveu) 2007-10-05 04:22:10

The functionality for a T.38 gateway is completely different from the termination applications, so gateway operation seems a poor fit in the current code.

What we did in Callweaver is to provide a new application - app_t83gateway.c - that is a bit like app_dial.c, but monitors for a T.38 call. When one is seen, it switches from simply passing audio/video/whatever through, to acting as a T.38 gateway. In this mode it receives and transmits audio on the PSTN side, and receives and transmits T.38 packets on the IP side. Of course, like rxfax and txfax, spandsp is doing 99% of the work, and app_t38gateway.c is very simple interface code. You need something in the system to actually figure out when a call requires T.38 facilities, and tries to negotiate it with the far end, through SIP, H.323 or MGCP. Remember that the called end is the one which is supposed to initiate this negotiation. Exactly how to add it to the system is left as an exercise for the reader. :-)

By: Dmitry Andrianov (dimas) 2007-10-11 18:25:57

slavon,
just curious how do you use FAX_DONE variable? SendFAX/ReceiveFAX allow you to check if transmission was successful or not. In case of successful transmission, sndfax_exec returns zero in any other case it returns -1. However you can not distinguish between some kind of fatal error which prevented transmission from even starting (like wrong channel status) and error ocurred during transmission. I'm not sure if information is really important and that is why I would like to know how you really use FAX_DONE and FAX_ERROR vars. Maybe you could show piece of dialplan?

By: Badalian Vyacheslav (slavon) 2007-10-12 00:07:35

i use it to send report status to email (send or not and why)

By: Jason Parker (jparker) 2007-10-15 17:05:03

slavon, you'll need to upload that as a patch, and have a license agreement submitted.

By: Dmitry Andrianov (dimas) 2007-10-15 17:11:31

I will add similar functionality and create new "full" patch.

By: David Brillert (aragon) 2007-10-18 07:55:24

Hi Dimas

Any idea when you might be posting a new patch?

"(0072053)
dimas - reporter
10-15-07 17:11

I will add similar functionality and create new "full" patch."

By: Dmitry Andrianov (dimas) 2007-10-18 07:57:15

aragon,
weekend probably. But please note that new patch will just add couple of new channel variables to let dialplan read transmission status/error. The patch is not going to fix any bugs in the in the current implementation because Ii'm not aware of any :)

By: David Brillert (aragon) 2007-10-18 08:02:53

Dimas

Thanks I hope to test the current patch today.

By: Russell Bryant (russell) 2007-10-22 16:20:23

Please do not post patches as bug notes.  They must be uploaded so they can be checked against our license agreement database.

By: David Brillert (aragon) 2007-10-24 08:00:32

Hi Dimas your patch seems to be working for me

By: Dmitry Andrianov (dimas) 2007-11-02 17:42:29

Ok, it took a bit longer but here is the new version.

New version available in 3 different alternatives:
* v2-app_fax.c - just the new version of app_fax.c for these who already have makefile/configure/etc patched
* v2-app_fax.c.patch - is just a unified diff of app_fax.c for those who want to see changes (or to apply them)
* v2-full.patch - the full patch to be applied to freshly checked out asterisk-addons trunk (updated for latest trunk revision)

The only significant change is adition of requested FAXSTATUS+FAXERROR variables which are set by the application. Dialplan may check these in order to do something in case of success/failure. After some thought and advices from other people on asterisk-dev list I made the application to work the following way:

* If fax is sent ok, application sets FAXSTATUS=SUCCESS and returns successfully (dialplan execution continues)
* If there were some problems with the fax data (no fax for example) but the channel still up and ok, application sets FAXSTATUS=FAILED and returns successfully (dialplan execution continues)
* If there are problems with channel (like user hangup), application sets FAXSTATUS=FAILED and returns -1 which makes Asterisk to immediately abort dialplan execution.

Note that in the last case, dialplan code immediately following SendFAX/ReceiveFAX won't be executed. If you wamt to perform some action in any failure including channel hangup, you better not placing the code after the SendFAX/ReceiveFAX but place it in the 'h' extension instead. This extension gets executed on the hangup. You will have access to all FAX* variables there.

Something like:

[faxcontext]
exten = 100,1,ReceiveFAX(.......)
exten = 100,n,Hangup

exten = h,1,Noop(This will be executed regardles fax received Ok or user hung up)
exten = h,n,Noop(FAXSTATUS = ${FAXSTATUS})

(Note that you may need to update Asterisk trunk to 88209 in order to make 'h' extension working - see issue 11146)

I encourage everrybody interested to test new version.

By: Badalian Vyacheslav (slavon) 2007-11-05 16:28:02.000-0600

looks great for me =) i remeber to t38 feature request =)

By: Dmitry Andrianov (dimas) 2007-11-21 17:51:34.000-0600

Digium guys, what is missing in order to have this stuff committed?

By: Badalian Vyacheslav (slavon) 2007-11-22 01:41:46.000-0600

I upload patch that patch makefile/configure/etc in current Brunch (Revision 487)

fax_configs_patch.patch

Thanks for you work.

By: Badalian Vyacheslav (slavon) 2007-11-22 01:45:51.000-0600

sorry. have small compile error
simple delete cham_mobile from Makefile.
new file uploaded

By: Dmitry Andrianov (dimas) 2007-11-22 07:04:35.000-0600

Slavon,
I'm not quite sure what your patch does because my v2-full.patch already has these modifications so you can apply it to "clean" addons trunk without need to do anything else...

By: Badalian Vyacheslav (slavon) 2007-11-22 09:09:38.000-0600

dimas i use Brunch. Not trunk. In brunch not have chan_mobile module and other things =)

By: Dmitry Andrianov (dimas) 2007-11-22 12:33:52.000-0600

I see no reason in posting 1.4 branch patches here because they won't be accepted by Digium anyway. Also, latest version of app_fax.c depends on ast_debug which is only available in trunk.

By: Digium Subversion (svnbot) 2007-12-14 15:53:06.000-0600

Repository: asterisk-addons
Revision: 497

U   trunk/Makefile
A   trunk/app_fax.c
U   trunk/build_tools/menuselect-deps.in
U   trunk/configure
U   trunk/configure.ac
U   trunk/makeopts.in
U   trunk/menuselect-tree

------------------------------------------------------------------------
r497 | tilghman | 2007-12-14 15:53:06 -0600 (Fri, 14 Dec 2007) | 4 lines

Add faxing application
Patch by: dimas
(Closes issue ASTERISK-10380)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk-addons?view=rev&revision=497