[Home]

Summary:ASTERISK-05603: Flash events is not sent correctly as sip event
Reporter:Leif Neland (lenne_dk)Labels:
Date Opened:2005-11-14 19:26:07.000-0600Date Closed:2008-01-15 15:56:10.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Interoperability
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) flashevent.diff
( 1) flashevent2.diff
( 2) flashevent3.diff
Description:If the phone is sending flash as sip event, asterisk doesn't relay it properly.



****** ADDITIONAL INFORMATION ******

chan_sip.c, line 8579

     if (buf[0] == '*')
           event = 10;

thinks buf contains DTMF as single characters, while it is actually the event number in ascii, eg * is "10" and flash is "16"

So a simple  event = atoi(buf) is enough to decode the event.

A few lines later, event is encoded in ascii to resp, but there is no encoding done for flash, 16. I used "!" for flash.
Also if event is > 16, resp was undefined, I encode it to '?', but '' might be more appropriate.

In rtp.c the ascii in resp is decoded back to numeric, I added the decoding of '!' there.
Comments:By: Leif Neland (lenne_dk) 2005-11-14 19:28:15.000-0600

Oops, wrong category, not "Code formatting/comments"
By: Leif Neland (lenne_dk) 2005-11-14 19:51:42.000-0600

Appearently Fritz!Box fon sends a "*" as "*" while Grandstream sends it as "10"
And Fritz!Box doesn't send a flash, it acts on it itself.

I'm making a new patch which leaves the original coding in.
(My first patch failed to upload anyway)
By: Olle Johansson (oej) 2005-11-15 01:34:00.000-0600

Usually the flash is handled locally by the phone, it's not an event sent across the network. In Asterisk, flash is sent as a separate event, not as a DTMF frame. It's actually not mandatory at all to handle incoming flashes in RFC2833.  Sending DTMF over INFO is a Cisco standard at it doesn't document how to handle "*" really...

In rtp.c, flash is decoded from 'X' in the send_dtmf function - why change to "!" ? Also, I do not think implementing the "?" is a good idea - if we send an unknown DTMF code across Asterisk we need to implement it in all channels at the same time. Are there anything else in the standards or devices that send other codes, outside of the standards?

I would propose that you add handling of the incoming flash and duplicate the code sending an AST_CONTROL frame with a FLASH from rtp.c instead of this patch.

You've also removed a few lines used for debugging...
By: Leif Neland (lenne_dk) 2005-11-15 02:57:50.000-0600

New patch uses 'X' instead of '!'.

Unknown (>16) are ignored. (I think...)

The debugging code was moved a few lines down, to show both event received and decoded.
By: Olle Johansson (oej) 2005-11-15 03:12:13.000-0600

You're still sending a FLASH as a DTMF frame into Asterisk. Again, check rtp.c and see how to send it in a CONTROL frame. Thank you for your patience :-)
By: Leif Neland (lenne_dk) 2005-11-15 04:46:03.000-0600

Now sending an AST_CONTROL frame with a FLASH

Then perhaps the patch to rtp.c is not needed.
By: Olle Johansson (oej) 2005-11-15 08:18:55.000-0600

Yes, the rtp patch seems redundant now. Have you ever seen any device send illegal DTMF INFO packets with bad content? Is there really a need for the '?' ?
By: Leif Neland (lenne_dk) 2005-11-15 09:35:49.000-0600

Defensive programming :-)

Catch it early, to avoid possible problems later.
By: Kevin P. Fleming (kpfleming) 2005-11-15 15:03:17.000-0600

I have committed a different fix to CVS HEAD. There was no need to modify rtp.c, since the 'X' values are never sent as queued frames. I also simplified the code a great deal, since the patch made already-convoluted code even worse :-)
By: Digium Subversion (svnbot) 2008-01-15 15:56:10.000-0600

Repository: asterisk
Revision: 7104

U   trunk/ChangeLog
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r7104 | kpfleming | 2008-01-15 15:56:10 -0600 (Tue, 15 Jan 2008) | 2 lines

issue ASTERISK-5603

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=7104