Summary:	ASTERISK-05601: randomly interrupted playback (ast_waitstream)
Reporter:	Lars Sundqvist (lasse)	Labels:
Date Opened:	2005-11-14 16:05:46.000-0600	Date Closed:	2011-06-07 14:10:24
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) mydiff.txt
Description:	file.c:ast_waitstream streams a file until a dtmf tone that matches a character in char* parameter breakon is received. To determine if the DTMF tone matches breakon, strchr is used and thats where the problem lies, strchr matches any character in the string INCLUDING the \0 char. In the system i noticed the problem there is some random noise coming in from PSTN (via a iax trunk) which is detected as DTMF characters with value 0 (0, not '0') and with the current code in ast_waitstream that stops the playback. As far as I can tell (new to asterisk code) either invalid dtmf tones must be stopped erlier or a check for 0 values must be done before the call to strchr.
Comments:	By: BJ Weschke (bweschke) 2005-11-14 16:45:49.000-0600 I just talked with kpfleming about this. He's of the belief that we should never receive a frame of type DTMF that has a value of 0 in it. That being the case, we probably need to determine which channel is giving us a frame that looks like that and fix that. I will also try and check with markster once I see him. In any case, I agree with you that it probably wouldn't hurt to make sure that the value getting compared is never a NULL value. Try the patch I've put up and let me know if you have any success with that. By: Lars Sundqvist (lasse) 2005-11-14 17:08:39.000-0600 I didn't try the patch but I am pretty sure it should be ... if(res > 0 && strchr(breakon,res)) ... as res holds the invalid code and breakon is the pointer to the string (ie "1234567890*#ABCD" in the case of AST_DIGIT_ANY). I did try the above code and it works fine. I will try to determine where the invalid DTMF frames get generated. I belive our provider is using Auidocode servers: "Audiocode" -> SIP -> "Our Asterisk" -> IAX2 -> "Our Asterisk with a problem" By: BJ Weschke (bweschke) 2005-11-14 17:20:34.000-0600 I was going more along the line of thinking that a DTMF frame shouldn't contain a 0 value, and even if we received one, we shouldn't be trying to compare it to a 0 value. By: Lars Sundqvist (lasse) 2005-11-14 17:55:11.000-0600 Sorry, I wasn't being very clear. res holds the dtmf code, invalid or valid. breakon is the string with codes ast_waitstream should break on, could be AST_DIGIT_ANY or could be "" in case you do not want to break on anything. So res has the ascii value of the digit you pressed and strchr checks if that value is in the breakon string, if it is, it stops the playback (as the user has started to input dtmf values). Checking breakon[0] will always return true unless ast_waitstream is called with breakon = "", problem is strchr(<any null terminated char*>, 0) always returns true since strchr includes the \0 in the search so you have to make sure that res != 0 By: Kevin P. Fleming (kpfleming) 2005-11-15 15:13:48.000-0600 There are _no_ circumstances under which AST_FRAME_DTMF should have a 'subclass' of zero. If your calls are coming in via IAX2, then they are being received that way from the remote IAX2 server, since DTMF events are always carried out of band in IAX2 (the inband DTMF detector is not involved here). You will need to find out whether your in-the-middle Asterisk server is somehow generating these frames, or if they are being caused by something incorrect in the SIP signaling that is coming from the media gateway. By: Lars Sundqvist (lasse) 2005-11-17 04:28:12.000-0600 Seems the problem is on our providers side, rtp debug yields: Got rfc2833 RTP packet from xxx.xxx.xxx.xxx:6200 (type 101, seq 63988, ts 1655702323, len 4, mark 0, event 00000091, end 0, duration 14564) (the call was outbound to PSTN via SIP to (what i belive is) an audiocode server) However, I still think you should add a check for invalid dtmf codes as this problem is very confusing (messages just gets interupted). By: BJ Weschke (bweschke) 2005-11-17 09:14:49.000-0600 I'm going to have to disagree here. The provider, as you've now shown, is clearly in the wrong here. What happens when they start sending DTMF packets that have valid DTMF values but the user didn't actually press that key? You won't be able to compensate for that. Asterisk would take a serious performance hit if we pre-screened everything we got for "wrong" values that we shouldn't be receiving in the first place and a patch, whatever way it would have been implemented, sets a bad precedent in that direction. I shouldn't have provided one in the first place. My bad. By: damin (damin) 2005-11-22 22:17:19.000-0600 [housekeeping] - Since we've determined that the SIP provider is sending incorrect data, and bweschke has pointed out that we shouldn't be trying to interpret bad data in the first place, are we ready to close this bug out? Or is there still an issue that needs to be addressed here? By: Lars Sundqvist (lasse) 2005-11-23 02:19:07.000-0600 I noticed ethereal displays the problematic rfc2833 packets as MF S3, so perhaps they are not so corrupted after all and there is a problem elsewhere in asterisk. Anyway I asked our provider to disable them if possible, and until then ill patch asterisk myself. So feel free to close the bug. By: Mark Spencer (markster) 2005-11-25 15:23:25.000-0600 Actually it's an asterisk bug in that we allow an RTP message with 0 type to get through, so that's fixed in Asterisk CVS head. By: Russell Bryant (russell) 2005-11-30 08:28:41.000-0600 This never actually made it into cvs head.  I have committed the fix to both the svn trunk and 1.2 branch.  It will be in the 1.2.1 release. By: Digium Subversion (svnbot) 2008-01-15 16:02:30.000-0600 Repository: asterisk Revision: 7232 U   trunk/rtp.c ------------------------------------------------------------------------ r7232 | russell | 2008-01-15 16:02:30 -0600 (Tue, 15 Jan 2008) | 2 lines do not allow an rtp message with zero type (issue ASTERISK-5601) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=7232 By: Digium Subversion (svnbot) 2008-01-15 16:02:32.000-0600 Repository: asterisk Revision: 7233 U   branches/1.2/rtp.c ------------------------------------------------------------------------ r7233 | russell | 2008-01-15 16:02:32 -0600 (Tue, 15 Jan 2008) | 2 lines do not allow an rtp message with zero type (issue ASTERISK-5601) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=7233