[asterisk-dev] [Code Review]: Fix double DTMF digits when 'dtmfmode=inband' and client sends both 'inband' and 'SIP INFO' packets
Alec Davis
reviewboard at asterisk.org
Fri Oct 19 03:26:48 CDT 2012
> On Oct. 18, 2012, 3:24 p.m., Joshua Colp wrote:
> > I think we should have *some* kind of message. If someone doesn't configure things properly they'll be confused very very fast when DTMF comes in via INFO and nothing happens.
>
> Alec Davis wrote:
> The reason why I agreed with opticron, if you're operating asterisk is a hosted PBX environment, where the clients have the INFO+INBAND or INFO+RFC2833 setting wrong already. A new release would spam the CLI, each time they press a digit if the warning was still there.
>
> I'm also wondering now about other permutations;
> dtmfmode=auto
>
> What if the client sends RFC2833+INBAND+INFO.
> There are 7 senarios to consider from a client point of view.
> And a few less from Asterisk's dtmfmode point of view.
>
> Joshua Colp wrote:
> Yes, and I'm thinking from a simple user perspective. If their configuration was/is incorrect we would now silently drop the DTMF and give no warning. That's not good. As for auto stuff can certainly fall apart with that, I personally think you should be explicit with DTMF.
>
> opticron wrote:
> Actually, I was suggesting more that it just not be a warning. I would definitely prefer to have the message in a 3+ verbose or debug level.
>
> rmudgett wrote:
> How about as a LOG_DTMF message instead?
>
> jcolp wrote:
> LOG_DTMF isn't on by default, verbose has more of a chance of being seen for normal users
@Joshua: looking at sip_rtp_read(), the test as to whether to forward an RF2833 frame isn't explicit. Maybe it should be.
/* Don't forward RFC2833 if we're not supposed to */
if (f && (f->frametype == AST_FRAME_DTMF_BEGIN || f->frametype == AST_FRAME_DTMF_END) &&
(ast_test_flag(&p->flags[0], SIP_DTMF) != SIP_DTMF_RFC2833)) {
ast_debug(1, "Ignoring DTMF (%c) RTP frame because dtmfmode is not RFC2833\n", f->subclass.integer);
ast_frfree(f);
return &ast_null_frame;
}
- Alec
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2165/#review7308
-----------------------------------------------------------
On Oct. 19, 2012, 3:25 a.m., Alec Davis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2165/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2012, 3:25 a.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Summary
> -------
>
> Asterisk 1.8.16.0
>
> file:/var/log/asterisk/dtmf when only '2' was hit once
> [2012-10-17 19:46:17.879406] DTMF[17084] channel.c: DTMF begin '2' received on SIP/822-00000000
> [2012-10-17 19:46:17.879467] DTMF[17084] channel.c: DTMF begin ignored '2' on SIP/822-00000000
> [2012-10-17 19:46:17.950953] DTMF[17084] channel.c: DTMF end '2' received on SIP/822-00000000, duration 800 ms
> [2012-10-17 19:46:17.951004] DTMF[17084] channel.c: DTMF end passthrough '2' on SIP/822-00000000
> [2012-10-17 19:46:18.019135] DTMF[17084] channel.c: DTMF end '2' received on SIP/822-00000000, duration 51 ms
> [2012-10-17 19:46:18.019228] DTMF[17084] channel.c: DTMF end passthrough '2' on SIP/822-00000000
>
>
> In ASTERISK-20218 the attached file it can be seen that both PA2P.rtf has both 'inband' and 'SIP INFO' set.
>
>
> This addresses bug ASTERISK-20218.
> https://issues.asterisk.org/jira/browse/ASTERISK-20218
>
>
> Diffs
> -----
>
> branches/1.8/channels/chan_sip.c 375136
>
> Diff: https://reviewboard.asterisk.org/r/2165/diff
>
>
> Testing
> -------
>
> Asterisk SVN-branch-1.8-r375111M
>
> I was able to verify the same conditions on a Grandstream GXP2000.
>
> Below is after proposed patch:
>
> file:/var/log/asterisk/dtmf when '820' was entered
>
> [2012-10-17 20:16:18.883821] DTMF[23460] channel.c: DTMF begin '8' received on SIP/822-00000004
> [2012-10-17 20:16:18.883868] DTMF[23460] channel.c: DTMF begin ignored '8' on SIP/822-00000004
> [2012-10-17 20:16:18.963537] DTMF[23460] channel.c: DTMF end '8' received on SIP/822-00000004, duration 89 ms
> [2012-10-17 20:16:18.963559] DTMF[23460] channel.c: DTMF end passthrough '8' on SIP/822-00000004
> [2012-10-17 20:16:19.263805] DTMF[23460] channel.c: DTMF begin '2' received on SIP/822-00000004
> [2012-10-17 20:16:19.263851] DTMF[23460] channel.c: DTMF begin ignored '2' on SIP/822-00000004
> [2012-10-17 20:16:19.363423] DTMF[23460] channel.c: DTMF end '2' received on SIP/822-00000004, duration 89 ms
> [2012-10-17 20:16:19.363444] DTMF[23460] channel.c: DTMF end passthrough '2' on SIP/822-00000004
> [2012-10-17 20:16:19.643830] DTMF[23460] channel.c: DTMF begin '0' received on SIP/822-00000004
> [2012-10-17 20:16:19.643876] DTMF[23460] channel.c: DTMF begin ignored '0' on SIP/822-00000004
> [2012-10-17 20:16:19.783561] DTMF[23460] channel.c: DTMF end '0' received on SIP/822-00000004, duration 140 ms
> [2012-10-17 20:16:19.783582] DTMF[23460] channel.c: DTMF end passthrough '0' on SIP/822-00000004
>
>
> Console:
>
> -- Executing [s at voicemail-main:2] VoiceMailMain("SIP/822-00000004", "") in new stack
> -- <SIP/822-00000004> Playing 'vm-login.gsm' (language 'en')
> [2012-10-17 20:16:18.935436] WARNING[23399]: chan_sip.c:19239 handle_request_info: Ignoring DTMF_INFO message as DTMF_INBAND is set on channel SIP/822-00000004
> [2012-10-17 20:16:19.325618] WARNING[23399]: chan_sip.c:19239 handle_request_info: Ignoring DTMF_INFO message as DTMF_INBAND is set on channel SIP/822-00000004
> [2012-10-17 20:16:19.734910] WARNING[23399]: chan_sip.c:19239 handle_request_info: Ignoring DTMF_INFO message as DTMF_INBAND is set on channel SIP/822-00000004
> -- <SIP/822-00000004> Playing 'vm-password.gsm' (language 'en')
> asterix*CLI>
>
>
> Thanks,
>
> Alec
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121019/067d6dc4/attachment.htm>
More information about the asterisk-dev
mailing list