[asterisk-bugs] [Asterisk 0011796]: [patch] refactoring of fax tone detection in DSP

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=11796 
====================================================================== 
Reported By:                dimas
Assigned To:                
====================================================================== 
Project:                    Asterisk
Issue ID:                   11796
Category:                   PBX/General
Reproducibility:            always
Severity:                   feature
Priority:                   normal
Status:                     new
Asterisk Version:           SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  trunk 
SVN Revision (number only!): 99085 
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             01-18-2008 20:36 CST
Last Modified:              01-21-2008 16:10 CST
====================================================================== 
Summary:                    [patch] refactoring of fax tone detection in DSP
Description: 
Refactored DTMF digit detection, fax tone detection & added CED fax tone
detection. See Additional Infrmation for details.
====================================================================== 

---------------------------------------------------------------------- 
 qwell - 01-21-08 16:10  
---------------------------------------------------------------------- 
I looked at this, and it looks pretty good.  I only had a few minor
comments.

mhit in dtmf_detect_state_t was renamed to current_hit, but it remains
mhit in mf_detect_state_t.  I don't see any reason not to change it in both
places.

Could you explain where you got 10% from?  It seems somewhat arbitrary,
but perhaps there is a reason.

You say that the fax detection shouldn't change at all, but previously,
fax_hits needed to be 5.  Can you explain a little more on how this has
changed?


Obviously, this patch is going to require a bit of testing before we would
feel comfortable putting it in.  I think answering the above questions
would help with that, but I'd certainly like to see "outside" users test
this (perhaps you could send a list message requesting that a few people
test?)


This is unrelated to this patch, but I happened to notice it while I was
reviewing.  I was curious if you were able to explain the MF_GSIZE define,
and what is meant by "80 is optimised to meet the MF spec", and more
importantly - why it's set to 120, when the allegedly optimized value is
80? 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
01-21-08 16:10  qwell          Note Added: 0080972                          
======================================================================