[asterisk-bugs] [JIRA] (ASTERISK-27816) func_talkdetect's logic is completely broken

[Richard Mudgett (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-27816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=243133#comment-243133 ] 

Richard Mudgett commented on ASTERISK-27816:
--------------------------------------------

{quote}
commit 53968c00b34a913e0467200b95add483f79dd3b6
Author:     Matthew Jordan <mjordan at digium.com>
AuthorDate: Fri May 30 12:42:57 2014 +0000
Commit:     Matthew Jordan <mjordan at digium.com>
CommitDate: Fri May 30 12:42:57 2014 +0000

    TALK_DETECT: A channel function that raises events when talking is detected

    This patch adds a new channel function TALK_DETECT that, when set on a
    channel, causes events indicating the start/stop of talking on a channel to be
    emitted to both AMI and ARI clients.

    The function allows setting both the silence threshold (the length of silence
    after which we decide no one is talking) as well as the talking threshold (the
    amount of energy that counts as talking). Parameters can be updated on a channel
    after talk detection has been enabled, and talk detection can be removed at
    any time.

    The events raised by the function use a nomenclature similar to existing AMI/ARI
    events.
    For AMI: ChannelTalkingStart/ChannelTalkingStop
    For ARI: ChannelTalkingStarted/ChannelTalkingFinished

    Review: https://reviewboard.asterisk.org/r/3563/

    #ASTERISK-23786 #close
    Reported by: Matt Jordan
    ........

    Merged revisions 414934 from http://svn.asterisk.org/svn/asterisk/branches/12


    git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@414935 65c4cc65-6c06-0410-ace0-fbb531ad65f3
{quote}

The initial commit of the function shows the intent of TALK_DETECT to be just like the other talk detect options and behavior in the system.  There is a talking level threshold to specify the energy level boundary between silence and talking.  There is a silence time threshold to determine when to declare talking ended.  The user documentation is just wrong.  It does not match the commit message description which is what the code does.

The code does use information from the DSP to look for edges where talking/noise starts and stops.  The total_silence value comes from the DSP code's accumulated silence history.  What you point out is a problem in the code when determining the *initial* talking state.

What you are proposing to do is alter the definition of the talk detection event slightly to add the talking time threshold option.  I was initially concerned that changing the definition would cause inconsistencies in behavior between other modules generating talk detection events.  After some research I see that no other code generates the *same* talking events as generated by the TALK_DETECT function.  e.g., ConfBridge generates its own version of the talking events with just the two detection options.

To remain backward compatible with existing implementations the new option needs to be last.  Thus the {{Set(TALK_DETECT(set)=a,b,c)}} parameters should be:
# silence_time_threshold (in milliseconds, default 2500 ms)
# dsp_silence_threshold (average magnitude, default from dsp.conf silencethreshold option)
# talking_time_threshold (in milliseconds, default 0 ms)

These changes need to be broken into two patches.  The first patch needs to fix the documentation and the two bugs identified: a) The cut and paste error in talk_detect_fn_write() where dsp_silence_threshold is used when it should be dsp_talking_threshold.  b) The initial talking state.  The second patch to alter the talking event definition to add the talking time threshold option.

The first patch can go into Asterisk v13, v15, and master as it only fixes the documentation and bugs.  The second patch can only go into master unless testsuite tests are created to test the new feature.


> func_talkdetect's logic is completely broken
> --------------------------------------------
>
>                 Key: ASTERISK-27816
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-27816
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Functions/func_talkdetect
>    Affects Versions: 15.3.0
>            Reporter: Moritz Fain
>
> Logic in func_talkdetect.c is completely wrong regarding the thresholds. It is mixing millisecond thresholds (the one's that are passed to TALK_DETECT via set) with the DSP's avg. amplitude thresholds.



--
This message was sent by Atlassian JIRA
(v6.2#6252)