[asterisk-dev] [Code Review]: Allow dialplan control of faxdetection via faxdetect framehook / options in func CHANNEL

[Matthew Nicholson]

> On Oct. 3, 2011, 8:44 a.m., Kevin Fleming wrote:
> > /trunk/res/res_fax.c, line 3331
> > <https://reviewboard.asterisk.org/r/1116/diff/29/?file=20971#file20971line3331>
> >
> >     There is already a reference to 'faxdetect' in use here (the one created by fax_detect_new()) which hasn't been released. Why is another reference necessary?
> >     
> >     On the line 'fr_hook.data = faxdetect;' that reference is *transferred* to the fr_hook structure.
> 
> irroot wrote:
>     this ref is droped by the caller the reason is to allow a program to set up a framehook and then 
>     hold the ref checking returned results after the framehook is gone this may not be required latter.
>     
>     it also prevents a race condition as im returning the pointer i only unref when ive had my way with it.
>     
>     the other ref will be held in the framehook while its active it may die at any time.

The intention here would be more obvious if the ref count was incremented when you set fr_hook.data = faxdetect. It may also be better to increase the ref count in the ATTACHED event handler inside of the framehook it self.


- Matthew


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


On Oct. 2, 2011, 2:13 p.m., irroot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1116/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2011, 2:13 p.m.)
> 
> 
> Review request for Asterisk Developers, Russell Bryant, Matthew Nicholson, dimas, may213, Leif Madsen, and dafe_von_cetin.
> 
> 
> Summary
> -------
> 
> Hi there the patch that was going around circa 2008 to implement this in 1.4/1.6 app_fax has been moved to trunk [1.10]
> ive made some cleanups and moved it into res_fax res_fax_spandsp this is the framework and not production code
> unfortunately i have no means of testing it at the moment and require help.
> 
> i have cleaned the code up substantially it is related to R459
> 
> hope this is found useful and aids in the goal to get it in 1.10.
> 
> Adds application FaxGateway / FaxDetect
> Adds alternate bridge to Dial with new option.
> 
> 
> This addresses bug ASTERISK-18569.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18569
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/chan_ooh323.c 338905 
>   /trunk/funcs/func_channel.c 338905 
>   /trunk/include/asterisk/res_fax.h 338905 
>   /trunk/res/res_fax.c 338905 
> 
> Diff: https://reviewboard.asterisk.org/r/1116/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> irroot
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111003/bcbe14c8/attachment-0001.htm>