Summary:	ASTERISK-10588: implementation of application/dtmf for SIP INFO
Reporter:	Sergey Tamkovich (sergee)	Labels:
Date Opened:	2007-10-22 08:27:43	Date Closed:	2008-01-24 09:59:59.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/Interoperability
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) dtmf-shortinfo-v2-r89267.diff ( 1) sip-shortinfo-r86631.diff
Description:	Currently Asterisk treats only application/dtmf-relay payload of SIP info messages. There are some devices software which works only with "application/dtmf" payload and doesn't support "application/dtmf-relay". This patch implements support for "application/dtmf". To engage it use dtmfmode=shortinfo in your sip.conf ****** ADDITIONAL INFORMATION ****** http://www.voip-info.org/wiki/view/SIP+Info+DTMF
Comments:	By: Sergey Tamkovich (sergee) 2007-10-22 08:54:58 Tested with NaumenCC (software callcenter) - works good. By: Olle Johansson (oej) 2007-11-14 02:26:37.000-0600 As said on IRC, don't duplicate all this code. It makes chan_sip too complicated and hard to manage. You should propably just check the value before transmitting, to decide which of the two application/ headers to use and accept both on incoming. This is buggy, but neither application/dtmf-relay or application/dtmf is a standard. At least, if they implement a non-standard implementation, they should follow the non-standard reference document on Cisco's web site... :-) Please try again! /O PS. Thanks for the comment on the new function though :-) By: Sergey Tamkovich (sergee) 2007-11-14 04:16:19.000-0600 second try :) Duplicated code removed in dtmf-shortinfo-v2-r89267.diff By: Olle Johansson (oej) 2007-11-14 04:33:11.000-0600 Ok, looks much better. Thank you for quick turnaround, let's see if I can do the same. By: Olle Johansson (oej) 2007-11-15 04:14:04.000-0600 Ouch, doesn't compile cleanly. Did you really test this code? By: Digium Subversion (svnbot) 2007-11-15 04:19:34.000-0600 Repository: asterisk Revision: 89278 U   trunk/channels/chan_sip.c U   trunk/configs/sip.conf.sample ------------------------------------------------------------------------ r89278 | oej | 2007-11-15 04:19:32 -0600 (Thu, 15 Nov 2007) | 8 lines Add support for application/dtmf SIP INFO dtmf handling. Yep, another way of handling DTMF in SIP. Totally undocumented, but implemented in enough devices so we have to support it. Code by sergee, small changes by oej. Closes issue ASTERISK-10588 ------------------------------------------------------------------------ By: Kevin P. Fleming (kpfleming) 2008-01-23 17:52:46.000-0600 I am reopening this because the patch as uploaded and committed cannot possibly work as intended, and could easily break NAT functionality in chan_sip. SIP_DTMF_SHORTINFO is defined as (4 << 16), which puts it as bit 18 in the flags word. However, the SIP_DTMF bits are only bits 16 and 17, SIP_NAT uses bits 18 and 19. This means that every single test to see if the value of SIP_DTMF is SIP_DTMF_SHORTINFO will *always* return false, because it never looks at bit 18. This sort of problem is why the comment for SIP_DTMF explicitly says 'four settings, uses two bits'... adding a fifth value as this patch does would make it obvious that two bits are not enough to hold five values. The only reason this has not broken NAT support for most people is that bit 18 is SIP_NAT_RFC3581, which is the default when people use 'nat=yes'. Anyone trying to use SIP_NAT_ROUTE along with SIP_DTMF_SHORTINFO would see that their system does not work as expected. I would like to see a SIP call trace of a system using SIP_DTMF_SHORTINFO that supposedly is working properly; I suspect that if anyone tries to generate one, they'll find that they cannot do so. Fixing this will take some work; SIP_DTMF will need to become three bits, and there aren't any spare bits on either side of SIP_DTMF. Since there some free bits elsewhere in the flags word, I'll have to adjust the other flag bits to make room. By: Digium Subversion (svnbot) 2008-01-23 18:02:22.000-0600 Repository: asterisk Revision: 100057 U   trunk/channels/chan_sip.c ------------------------------------------------------------------------ r100057 | kpfleming | 2008-01-23 18:02:22 -0600 (Wed, 23 Jan 2008) | 4 lines fix flag bit definitions to make code from issue ASTERISK-10588 actually work; along the way, clarify comments and add some dummy flag definitions for other multi-bit flags to hopefully stop this from happening in the future (closes issue ASTERISK-10588) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=100057 By: Digium Subversion (svnbot) 2008-01-24 09:59:59.000-0600 Repository: asterisk Revision: 100113 _U  team/file/bridging/ U   team/file/bridging/.cleancount U   team/file/bridging/CHANGES U   team/file/bridging/channels/chan_iax2.c U   team/file/bridging/channels/chan_mgcp.c U   team/file/bridging/channels/chan_sip.c U   team/file/bridging/channels/chan_zap.c U   team/file/bridging/include/asterisk/_private.h U   team/file/bridging/include/asterisk/features.h U   team/file/bridging/main/Makefile U   team/file/bridging/main/asterisk.c U   team/file/bridging/main/channel.c U   team/file/bridging/main/loader.c D   team/file/bridging/res/res_features.c ------------------------------------------------------------------------ r100113 | file | 2008-01-24 09:59:56 -0600 (Thu, 24 Jan 2008) | 41 lines Merged revisions 100039,100057,100076,100093-100095,100112 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r100039 | qwell | 2008-01-23 19:09:11 -0400 (Wed, 23 Jan 2008) | 1 line Move code from res_features into (new file) main/features.c ........ r100057 | kpfleming | 2008-01-23 20:04:35 -0400 (Wed, 23 Jan 2008) | 4 lines fix flag bit definitions to make code from issue ASTERISK-10588 actually work; along the way, clarify comments and add some dummy flag definitions for other multi-bit flags to hopefully stop this from happening in the future (closes issue ASTERISK-10588) ........ r100076 | file | 2008-01-23 23:07:34 -0400 (Wed, 23 Jan 2008) | 2 lines Testing something... ........ r100093 | file | 2008-01-23 23:25:52 -0400 (Wed, 23 Jan 2008) | 2 lines Test hopefully over. ........ r100094 | file | 2008-01-23 23:30:56 -0400 (Wed, 23 Jan 2008) | 2 lines Add some spacing. ........ r100095 | file | 2008-01-23 23:34:57 -0400 (Wed, 23 Jan 2008) | 2 lines Some more cosmetic changes. ........ r100112 | file | 2008-01-24 11:54:32 -0400 (Thu, 24 Jan 2008) | 2 lines Remove dependency on res_features from some channel drivers. It is now part of the core and no longer exists as a module. ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=100113