[asterisk-dev] [Code Review]: dsp.c: dtmf_detect, Fix multiple issues when no-interdigit delay is present.

[Alec Davis]

> On Aug. 31, 2012, 6:24 p.m., rmudgett wrote:
> > After comparing this algorithm with the original algorithm (trunk -r349249) replaced by ASTERISK-17493, the original algorithm handles the same cases and is simpler.
> > 
> > Reverting to the original algorithm, keeping your "Character recognition: When not DRC" comments, and removing the unnecessary clearing of the hit count would be all that is necessary.  Of course this just brings back the need for a fix to ASTERISK-17493.

The original -r349249 does indeed work
I agree regarding unnecessary clearing the hits every no signal.

I'd recommend the following changes to the -r349249 hit code;
This removes unnesesary;
  1. reseting of hits=0, when no signal, only need to set it once.
  2. incrementing of hits, when the hit is the same as the current hit.
  3. setting of lasthit, when it's the same as before. 

                if (hit != s->td.dtmf.lasthit) {
                        s->td.dtmf.lasthit = hit;
                        s->td.dtmf.hits = hit ? 1 : 0;
                }
                if (hit && hit != s->td.dtmf.current_hit) {
                        s->td.dtmf.hits++;
                        if (s->td.dtmf.hits == s->td.dtmf.hits_to_begin) {
                                store_digit(s, hit);
                                s->td.dtmf.current_hit = hit;
                                s->td.dtmf.misses = 0;
                        }
                }

But does -r349249 need to support hits_to_begin=1 ?
If it doesn't then the 'hit' code could look like this.

                if (hit != s->td.dtmf.lasthit) {
                        s->td.dtmf.lasthit = hit;
                        s->td.dtmf.hits = hit ? 1 : 0;
                } else if (hit && hit != s->td.dtmf.current_hit) {
                        s->td.dtmf.hits++;
                        if (s->td.dtmf.hits == s->td.dtmf.hits_to_begin) {
                                store_digit(s, hit);
                                s->td.dtmf.current_hit = hit;
                                s->td.dtmf.misses = 0;
                        }
                }


Documenting -r349249 before recommended changes 
/*
 * Example: hits_to_begin=2 misses_to_end=3
 * -------A hits=1
 * ------AA hits=2 current_hit=A misses=0       BEGIN A
 * -----AA- misses=1 hits=0
 * ----AA-- misses=2 hits=0
 * ---AA--- misses=3 current_hit=' ' hits=0     END A
 * --AA---B hits=1
 * -AA---BC hits=1
 * AA---BCC hits=2 current_hit=C misses=0       BEGIN C
 * A---BCC- misses=1 hits=0
 * ---BCC-C misses=0 hits=1
 * --BCC-CC misses=0 hits=2
 * -BCC-CCC misses=0 hits=3       ??? hits=3 ???? but doesn't affect operation.

 * Example: hits_to_begin=3 misses_to_end=2
 * -------A hits=1
 * ------AA hits=2
 * -----AAA hits=3 current_hit=A misses=0       BEGIN A
 * ----AAAB misses=1 hits=1
 * ---AAABB misses=2 current_hit=' ' hits=2     END A
 * --AAABBB hits=3                              BEGIN B

 * Example: hits_to_begin=2 misses_to_end=2
 * -------A hits=1
 * ------AA hits=2 current_hit=A misses=0       BEGIN A
 * -----AAB misses=1 hits=1
 * ----AABB misses=2 current_hit=' ' hits=2 current_hit=B misses=0 BEGIN B   ??? double setting of current_hit and misses ???? but doesn't affect operation.
 */


- Alec


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


On Aug. 31, 2012, 6:11 p.m., Alec Davis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2085/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2012, 6:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Document DTMF events for different sequences.
> 
> * Fix extra hit required when new DIGIT is sent without any interdigit delay.
> * Fix when DTMF_HITS_TO_BEGIN = 2, and no interdigit gap, that BEGIN is posted. 
> * Fix situation where a flakey detect, would clear the current_hit (indicating an END), which may not have been the case.
> 
> 
> Diffs
> -----
> 
>   trunk/main/dsp.c 371689 
> 
> Diff: https://reviewboard.asterisk.org/r/2085/diff
> 
> 
> Testing
> -------
> 
> On production box for 24 hours. 888 digit's now received.
> 
> The change here doesn't affect normal tone on/tone off, it changes the case when no interdigit pause is received between 2 digits, now the 2 digit has the same detect time as if there had been an interdigit pause before hand.
> 
> 
> Thanks,
> 
> Alec
> 
>

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