[asterisk-dev] [Code Review] Enabling Inband DTMF Passthrough for Appropriate Channel Types

[paul.belanger at polybeacon.com]

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


I've include a few comments, mostly trivial.  But, there must be a better way to handle you IF Statement logic.  I'm not sure if my example will work properly, I have not tested it.

First code review, so take it with a grain of salt.


/branches/1.6.1/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/625/#comment4117>

    Evil red, err white-space.



/branches/1.6.1/include/asterisk/channel.h
<https://reviewboard.asterisk.org/r/625/#comment4118>

    Evil white-space.



/branches/1.6.1/main/channel.c
<https://reviewboard.asterisk.org/r/625/#comment4119>

    Convert to doxygen.



/branches/1.6.1/main/channel.c
<https://reviewboard.asterisk.org/r/625/#comment4122>

    int res = 0 (see below)



/branches/1.6.1/main/channel.c
<https://reviewboard.asterisk.org/r/625/#comment4120>

    There must be a better way to handle this.
    
    if (strcasecmp(chan->type->type, "DAHDI") || (strcasecmp(chan->type->type, "SIP") && (chan->tech->dtmfmode_inband(chan)) {
        res = 1;
    }
    
    I believe that should be the same?



/branches/1.6.1/main/channel.c
<https://reviewboard.asterisk.org/r/625/#comment4123>

    Same as above.


- pabelanger


On 2010-04-27 10:04:31, kenlee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/625/
> -----------------------------------------------------------
> 
> (Updated 2010-04-27 10:04:31)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I need to conditionally allow inband DTMF audio to pass straight through Asterisk unscathed.  Currently, Asterisk always silences DTMF and then regenerates it on the bridged channel.  The silencing part is not 100% accurate and small portions of the original inband DTMF sneak through on the head end.  The act of detecting and silencing is by nature imperfect.  Until you detect DTMF you must pass the audio through.  Detection doesn't occur instantly and you cannot silence what you have already passed through.  This could only be solved with a buffer, introducing delay and other problems.  Inband DTMF Passthrough is a simple and time tested solution to this problem. 
> 
> Sensitive DTMF receiving devices detect the brief end user generated DTMF as a digit, then the Asterisk generated DTMF as a digit, resulting in a double digit error.  My tests have placed this at an error rate of about 1.5% using the Metro-Tel Corp Digit Grabber, Model TPM32.  This is an improvement over asterisk 1.6.0 which had a 5% error rate*, however by passing the inband DTMF straight through you can achieve 0% error.  This is absolutely necessary if you want to put asterisk servers in tandem, otherwise the error rates compound.**
> 
> The solution in the attached patch is to conditionally opt-out of silence and regeneration.  The conditions upon which this patch takes affect are:
> 1) DTMF is detected
> 2) The channel on which DTMF is detected is bridged
> 3) Both the channel on which DTMF is detected *and* the bridged channel are already doing inband DTMF.
> When those conditions are met silencing does not occur, nor does the regeneration of the digit.  Instead the inband DTMF audio is allowed to pass through the system.
> The only channel types i've supported in this patch are dahdi and sip with dtfmmode=inband.
> 
> If any of those conditions aren't met asterisk behaves "normally."
> 
> * When I reported that the issue (0016243) was fixed I was mistaken.  I thought that this issue was fixed with the dsp.c reorganization that qwell did in r111022 (related to issue 0011968).  However qwell's changes made the silencing better (5% to 1.5% error rate improvement) but still not perfect.
> 
> **The actual scenario that is exercising this problem for me is two Asterisk Gateways in the same call, which terminates to an dictation service that requires you to enter a 10 digit pin.  The call has sip with inband dtmf and pri's with inband dtmf, ie: its all inband dtmf.  With two asterisk's in tandem the error rate jumps from 1.5% to 3% and 1 out of 10 calls results with an incorrect pin, resulting in many failed calls per day.
> 
> 
> This addresses bugs 0009382, 0011968 and 0016243.
>     https://issues.asterisk.org/view.php?id=0009382
>     https://issues.asterisk.org/view.php?id=0011968
>     https://issues.asterisk.org/view.php?id=0016243
> 
> 
> Diffs
> -----
> 
>   /branches/1.6.1/channels/chan_sip.c 230383 
>   /branches/1.6.1/include/asterisk/channel.h 230383 
>   /branches/1.6.1/main/channel.c 230383 
>   /branches/1.6.1/main/dsp.c 230383 
> 
> Diff: https://reviewboard.asterisk.org/r/625/diff
> 
> 
> Testing
> -------
> 
> This patch has been tested to work with inband sip and with pri's using dahdi in which case i was unable to reproduce a single double digit out of 1024 tests.
> This patch has been tested to be unobtrusive with sip channels using out of band signalling (rfc2833).
> We are in the process of phasing this patch into production and has been live for 1 day with 10 calls at peak.
> Monday if all goes well we will move it to a gateway system with 184 calls at peak doing sip inband, sip rfc2833, and dahdi pri channels.
> Note that the systems we tested on do *not* terminate channels.  All channels are bridged.
> 
> 
> Thanks,
> 
> kenlee
> 
>