[asterisk-bugs] [Asterisk 0012658]: [patch] DTMF issues on Zap

Fri Dec 12 07:44:05 CST 2008

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=12658 
====================================================================== 
Reported By:                dimas
Assigned To:                russell
====================================================================== 
Project:                    Asterisk
Issue ID:                   12658
Category:                   Core/PBX
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     assigned
Asterisk Version:           SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!): 116466 
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             2008-05-15 08:58 CDT
Last Modified:              2008-12-12 07:44 CST
====================================================================== 
Summary:                    [patch] DTMF issues on Zap
Description: 
ohh. The long one.
replaces 11635 and 12592

I often have unreliable DTMF recognition on Zap channels. Research shown
that DSP works fine, and digits are read from the Zap driver correctly
almost 100% of time and corruption occurs later in the VLDTMF emulation
code.

So the problems I encountered:
1. There is a situation when DTMF emulation swaps digits. Scenario (user
presses 12 with short durations and short pause):

  A. ast_read is called by some application
  B. ast_read calls channel's read and gets DTMF_END(1)
  C. DTMF emulation is started and ast_read sets AST_FLAG_EMULATE_DTMF
flag and returns AST_FRAME_DTMF_BEGIN(1)
  D. application calls autoservice_start and AST_FLAG_END_DTMF_ONLY is set
on the channel
  E. autoservice thread calls ast_read
  F. eventually channel's technology diver returns DTMF_END(2).
  G. since AST_FLAG_END_DTMF_ONLY is set, ast_read passes DTMF_END(2) thru
ast_read returns DTMF_END(2). However it is still in the emulation mode!
  H. Eventually, emulation finishes and ast_read returns DTMF_END(1)

bottom line: 1 and 2 got swapped

Solution (one of, which I choosed) is to always queue DTMFs when we are
already in emulation mode. This fix immediately led to next issue:

2. when user quickly rolls over digits (1234) there can be a situation
when autoservice thread already read 12 and 34 are already queued into
channel's dtmfq but not yet read. Then when autoservice_stop "returns"
digits into channels readq, and they get processed by the VLDTMF emulation
code AGAIN - they will be queued to dtmfq. 

Result - 3412 will be read from the channel instead of 1234.

Solution: mark frames "returned" by autoservice and do NOT process them
again.

Major drawback: DTMFs which "passed thru" autoservice won't be re-emulated
so only DTMF_ENDs remain and no proper interval between them. In my case
when I mess with autoservice it is always an application terminates the
call so no worries - to my knowledge applications only need DTMF_END and
never BEGINs

3. The next problem is that the code at the beginning of ast_read which is
responsible for "injecting" deferred DTMF can inject content of dtmfq even
if there is non-empty readq (possible filled by just finished autoservice
which means they ae higher prioity).

Resul - again, swapped digits.

Solution: add additional check - do not inject digits from the dtmfq while
there is somethign in the channel's readq.

4. Finally AST_FLAG_EMULATE_DTMF is not always set when digit is added to
dtmfq. To make sure we ALWAYS queue when somethign is queued already, I
added "|| !ast_strlen_zero(chan->dtmfq)" to the conditions used to queue
DTMFs.
======================================================================
Relationships       ID      Summary
----------------------------------------------------------------------
related to          0011740 DTMF problem on 1.4.17
related to          0011911 [patch] DTMF digits duplicated
related to          0012656 [patch] Autoservice loses DTMF digits
related to          0012874 [patch] small bug introduced with fix f...
related to          0011635 [patch] multiple issues with autoservice
related to          0009096 [patch] DTMF CID without polarity rever...
====================================================================== 

---------------------------------------------------------------------- 
 (0096308) svnbot (reporter) - 2008-12-12 07:44
 http://bugs.digium.com/view.php?id=12658#c96308 
---------------------------------------------------------------------- 
Repository: asterisk
Revision: 163448

U   branches/1.4/include/asterisk/channel.h
U   branches/1.4/main/autoservice.c
U   branches/1.4/main/channel.c

------------------------------------------------------------------------
r163448 | russell | 2008-12-12 07:44:03 -0600 (Fri, 12 Dec 2008) | 26
lines

Resolve issues that could cause DTMF to be processed out of order.

These changes come from team/russell/issue_12658

1) Change autoservice to put digits on the head of the channel's frame
readq 
   instead of the tail.  If there were frames on the readq that
autoservice 
   had not yet read, the previous code would have resulted in out of order

   processing.  This required a new API call to queue a frame to the head 
   of the queue instead of the tail.

2) Change up the processing of DTMF in ast_read().  Some of the problems 
   were the result of having two sources of pending DTMF frames.  There 
   was the dtmfq and the more generic readq.  Both were used for pending 
   DTMF in various scenarios.  Simplifying things to only use the frame 
   readq avoids some of the problems.

3) Fix a bug where a DTMF END frame could get passed through when it 
   shouldn't have.  If code set END_DTMF_ONLY in the middle of digit
emulation,
   and a digit arrived before emulation was complete, digits would get 
   processed out of order.

(closes issue http://bugs.digium.com/view.php?id=12658)
Reported by: dimas
Tested by: russell, file
Review: http://reviewboard.digium.com/r/85/

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

http://svn.digium.com/view/asterisk?view=rev&revision=163448 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2008-12-12 07:44 svnbot         Note Added: 0096308                          
======================================================================