[asterisk-bugs] [JIRA] (ASTERISK-21130) sip_pvt.dsp incorrect manipulation related to inband dtmfmode and faxdetect in SIPDtmfMode() app. and enable_dsp_detect()

[Roman S. (JIRA)]

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

Roman S. edited comment on ASTERISK-21130 at 2/20/13 7:10 AM:
--------------------------------------------------------------

Description for asterisk-1.8.20.0-dsp-fix.diff:

It shouldn't be an issue because {{SIPDtmfMode()}} app. can't call {{enable_dsp_detect()}} while sending/receiving fax. Other calls of {{enable_dsp_detect()}} are safe too:

{noformat}
  9 + *  \note Can (re)enable temporarily disabled with update_dsp_detect()
 10 + *  function features, e.g. for sndfax_exec() and rcvfax_exec() */
{noformat}

{{enable_dsp_detect()}} still should be called because of {{ast_rtp_instance_dtmf_mode_set()}} and {{ast_dsp_set_digitmode()}} -- this can be full DSP initialization:

{noformat}
107 +           ast_test_flag(&p->flags[1], SIP_PAGE2_FAX_DETECT_CNG)) {
108                 enable_dsp_detect(p);
{noformat}

Should be used only to temp. update DSP features and keep related SIP flags -- e.g. for fax related tasks:

{noformat}
 35 +/*! \brief Update detect features for already enabled DSP */
 36 +static void update_dsp_detect(struct sip_pvt *p, int onoff, int digit, int fax)
{noformat}

AST_OPTION_FAX_DETECT operation should be separated from AST_OPTION_DIGIT_DETECT:

{noformat}
 81 +       case AST_OPTION_FAX_DETECT:
 82 +               if (ast_test_flag(&p->flags[1], SIP_PAGE2_FAX_DETECT_CNG)) {
{noformat}

Almost the same as above:

{noformat}
 94 -               *cp = p->dsp ? 1 : 0;
 95 +               *cp = (ast_dsp_get_features(p->dsp) & DSP_FEATURE_DIGIT_DETECT) ? 1 : 0;
{noformat}

DSP shouldn't be destroyed, but features should be disabled/reenabled only:

{noformat}
 72 -                       if (*cp) {
 73 -                               enable_dsp_detect(p);
 74 -                       } else {
 75 -                               disable_dsp_detect(p);
 76 -                       }
 77 +                       update_dsp_detect(p, *cp, 1, 0);
{noformat}
                
      was (Author: rolesok):
    Fix
                  
> sip_pvt.dsp incorrect manipulation related to inband dtmfmode and faxdetect in SIPDtmfMode() app. and enable_dsp_detect()
> -------------------------------------------------------------------------------------------------------------------------
>
>                 Key: ASTERISK-21130
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-21130
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Channels/chan_sip/General
>    Affects Versions: 1.8.20.0
>            Reporter: Roman S.
>            Assignee: Roman S.
>            Severity: Minor
>         Attachments: asterisk-1.8.20.0-dsp-fix.diff
>
>
> 1. If {{sip_pvt.dsp}} is already inited (e.g. with {{faxdetect=yes}}) then it can't be changed with {{SIPDtmfMode()}} app.
> {noformat}
> 30013 static int sip_dtmfmode(struct ast_channel *chan, const char *data)
> 30055         if ((ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_INBAND) ||
> 30056             (ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_AUTO)) {
> 30057                 enable_dsp_detect(p);
>  4598 static void enable_dsp_detect(struct sip_pvt *p)
>  4602         if (p->dsp) {
>  4603                 return;
> {noformat}
> Example:
> {noformat}
> sip.conf:
>  faxdetect = yes ; this enables dsp with DSP_FEATURE_FAX_DETECT flag
>  dtmfmode = rfc2833 ; no DSP_FEATURE_DIGIT_DETECT flag for dsp
> extensions.conf:
>  ...,SIPDtmfMode(inband)
> {noformat}
> {{SIPDtmfMode()}} doesn't set {{DSP_FEATURE_DIGIT_DETECT}} flag for dsp in {{enable_dsp_detect()}}.
> 2. If {{SIPDtmfMode()}} app. disables inband mode (e.g. switch to rfc2833) it also disables faxdetect (indeed any dsp feature).
> {noformat}
> 30013 static int sip_dtmfmode(struct ast_channel *chan, const char *data)
> 30055         if ((ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_INBAND) ||
> 30056             (ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_AUTO)) {
> 30057                 enable_dsp_detect(p);
> 30058         } else {
> 30059                 disable_dsp_detect(p);
> 30060         }
> {noformat}
> This check (line 30055) should also contain {{SIP_PAGE2_FAX_DETECT_CNG}} in condition and keep dsp also for faxdetect.
> Example:
> {noformat}
> sip.conf:
>  faxdetect = yes ; this enables dsp with DSP_FEATURE_FAX_DETECT flag
>  dtmfmode = inband ; dsp with DSP_FEATURE_DIGIT_DETECT flag
> extensions.conf:
>  ...,SIPDtmfMode(rfc2833)
> {noformat}
> {{SIPDtmfMode()}} disables any dsp feature including faxdetect (which indeed should be kept).
> Thanks

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira