[Home]

Summary:ASTERISK-10148: [patch] DTMF detect debouncing logic can miss whole digits (dsp.c)
Reporter:Tony Mountifield (softins)Labels:
Date Opened:2007-08-23 08:51:17Date Closed:2007-08-24 16:59:54
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) dsp.c-1.2.diff
( 1) dsp.c-1.4.diff
( 2) dsp.c-trunk.diff
Description:As reported on asterisk-dev at http://lists.digium.com/pipermail/asterisk-dev/2007-August/029149.html, inband DTMF detection is unreliable when the tones are not perfect.

Having investigated the issue using a test harness around dsp.c, using audio files of varying quality captured directly from the Zaptel data before the dsp processing, I believe I have found the issue.

It turns out that the problem with missing digits is in the debouncing,
for which I think the logic is wrong.

The debouncing of hits within dtmf_detect() is currently as follows:

if (hit) {
       if (hit == s->hits[2]  &&  hit != s->hits[1]  &&  hit != s->hits[0]) {
               // we register that we really have a digit
       }
}
s->hits[0] = s->hits[1];
s->hits[1] = s->hits[2];
s->hits[2] = hit;

So hits[] is a shift register of old hits/non-hits.

If we get a clean start of digit (say, 0), the sequence is as follows:

(- - -) 0
(- - 0) 0 : register that we have a digit (leading edge)
(- 0 0) 0
(0 0 0) 0

However, if we get a bounce at the start, the whole digit can get missed:

(- - -) 0
(- - 0) -
(- 0 -) 0
(0 - 0) 0
(- 0 0) 0
(0 0 0) 0

None of the above sequences triggers the leading-edge detector, which is looking for a match in the previous hit, preceded by two non-matches. Consequently the whole digit is missed.

What should really happen is that the previous hit should match the current hit, and they should be different from the PREVIOUS DEBOUNCED HIT. It also appears that there is currently no debouncing of the trailing edge; the first non-hit terminates the digit.

I have reworked the debouncing logic as follows:

a) Debouncing is now done on both leading and trailing edges.

b) The dtmf_detect() function now returns the debounced value (mhit).

c) The behaviour if OLD_DSP_ROUTINES is defined has been left unchanged.

With the change, all my test files had their DTMF detected reliably, where some of them previously had several digits missed. I will shortly be applying it to a live 1.2 system that has been having chronic DTMF issues in some calls. I will post the results here.

The same technique should probably be applied to MF detection too, but I have not done so.

I have attached patches for 1.2, 1.4 and trunk. This really ought to be fixed in 1.2, but if policy dictates otherwise, then at least the patch here can be fetched and applied by people who need it.
Comments:By: Curt Moore (jcmoore) 2007-08-23 11:07:28

I've noticed some weirdness with DTMF detection when entering a voicemail PIN when dialing in to check voicemail; hopefully these changes will help.  I'll be happy to test once the license is accepted and I can pull the patches.  Great work! :-)

By: Tony Mountifield (softins) 2007-08-24 11:20:01

I can confirm that after a day or two operation with the new code, the live system has been working fine, and does not appear to randomly drop incoming digits any more.

By: Digium Subversion (svnbot) 2007-08-24 15:06:01

Repository: asterisk
Revision: 80820

------------------------------------------------------------------------
r80820 | russell | 2007-08-24 15:06:00 -0500 (Fri, 24 Aug 2007) | 7 lines

Improve the debouncing logic in the DTMF detector to fix some reliability
issues.  Previously, this code used a shift register of hits and non-hits.
However, if the start of the digit isn't clean, it is possible for the
leading edge detector to miss the digit.  These changes replace the flawed
shift register logic and also does the debouncing on the trailing edge as well.
(closes issue ASTERISK-10148, many thanks to softins for the patch)

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

By: Digium Subversion (svnbot) 2007-08-24 15:07:34

Repository: asterisk
Revision: 80821

------------------------------------------------------------------------
r80821 | russell | 2007-08-24 15:07:34 -0500 (Fri, 24 Aug 2007) | 15 lines

Merged revisions 80820 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r80820 | russell | 2007-08-24 15:24:05 -0500 (Fri, 24 Aug 2007) | 7 lines

Improve the debouncing logic in the DTMF detector to fix some reliability
issues.  Previously, this code used a shift register of hits and non-hits.
However, if the start of the digit isn't clean, it is possible for the
leading edge detector to miss the digit.  These changes replace the flawed
shift register logic and also does the debouncing on the trailing edge as well.
(closes issue ASTERISK-10148, many thanks to softins for the patch)

........

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

By: Digium Subversion (svnbot) 2007-08-24 16:59:54

Repository: asterisk
Revision: 80877

------------------------------------------------------------------------
r80877 | murf | 2007-08-24 16:59:52 -0500 (Fri, 24 Aug 2007) | 70 lines

Merged revisions 80790,80817,80819,80821,80850 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r80790 | murf | 2007-08-24 13:03:39 -0600 (Fri, 24 Aug 2007) | 9 lines

Merged revisions 80789 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r80789 | murf | 2007-08-24 12:52:15 -0600 (Fri, 24 Aug 2007) | 1 line

From a complaint by jmls, I realize that the message in cdr_disposition is unnecessary. To get failure disposition, just return -1; no use having more than one case do that.
........

................
r80817 | tilghman | 2007-08-24 13:50:16 -0600 (Fri, 24 Aug 2007) | 2 lines

Fix documentation for Set (closes issue ASTERISK-10160)

................
r80819 | bweschke | 2007-08-24 14:21:17 -0600 (Fri, 24 Aug 2007) | 11 lines

Merged revisions 80818 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r80818 | bweschke | 2007-08-24 15:52:06 -0400 (Fri, 24 Aug 2007) | 3 lines

A minor correction to the available logic of autofill. If a queue member is paused, they're not really "available" so don't count them as such. Somewhat related to issue ASTERISK-9835


........

................
r80821 | russell | 2007-08-24 14:25:39 -0600 (Fri, 24 Aug 2007) | 15 lines

Merged revisions 80820 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r80820 | russell | 2007-08-24 15:24:05 -0500 (Fri, 24 Aug 2007) | 7 lines

Improve the debouncing logic in the DTMF detector to fix some reliability
issues.  Previously, this code used a shift register of hits and non-hits.
However, if the start of the digit isn't clean, it is possible for the
leading edge detector to miss the digit.  These changes replace the flawed
shift register logic and also does the debouncing on the trailing edge as well.
(closes issue ASTERISK-10148, many thanks to softins for the patch)

........

................
r80850 | russell | 2007-08-24 15:23:14 -0600 (Fri, 24 Aug 2007) | 13 lines

Merged revisions 80849 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r80849 | russell | 2007-08-24 16:22:50 -0500 (Fri, 24 Aug 2007) | 5 lines

If dnsmgr is in use, and no DNS servers are available when Asterisk first
starts, then don't give up on poking peers.  Allow the poke to get rescheduled
so that it will work once the dnsmgr is able to resolve the host.
(closes issue ASTERISK-10138, patch by jamesgolovich)

........

................

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