Summary:	ASTERISK-00682: [patch] Video negotiation error, with fix
Reporter:	matt (matt)	Labels:
Date Opened:	2003-12-18 20:34:27.000-0600	Date Closed:	2008-01-15 14:49:46.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_sip.c.patch ( 1) sip_conf_example.txt ( 2) video_sdp_bad.txt
Description:	There are two bugs I noticed with regard to video negotiation.  The provided patch fixed them both.  Section 3 is a discussion of quirky behavior with regard to the allow/disallow statements. Of course none of this comes into play until you turn videosupport=yes on. The provided patch to chan_sip.c is relative to the CVS at of Thurs. night, 12/18/03, approx. 8:40pm EST (Does 1.263 sound right?) 1.  Non-initialization of video RTP types results in "garbage" selections for a non-video call There's a line in chan_sip.c that looks like this: ast_rtp_pt_clear(p->vrtp); and there is a similar one for audio.  Problem is, it doesn't get called until the first codec is found.  The assumption is that there will always be at least one codec, so it works for audio.  The problem is, Asterisk may receive an invitation with no video codecs, even though video is enabled.  As a result, this structure is left with garbage data and does whacky things to the negotiation.  See the example video_sdp_bad.txt file.  On line 36 is a line that looks like this: Capabilities: us - 524302, them - 4/854015, combined - 524302 Which is WAY wrong.  This is a Snom 200 audio-only phone calling.  The "4" format (for audio) is correct but it's showing video options of "854015" which is a bunch of stuff that doesn't exist.  The patch correctly initializes the structure so that the video capabilities will be "0" if no "m=video" line was present in the INVITE.  In this case, the message ends cleanly after Asterisk's erroneous H263 response, but I have seen many times where there will be additional lines of garbage in the SIP message following the H263 line, presumably from Asterisk trying to add lines about the non-existent codecs to the OK message. 2.  Asterisk was always adding the video responses to the "200 OK" message anytime video support was enabled, regardless of whether video was invited or not. This is probably a result of the video implementation being done expeditiously, but it doesn't seem right that Asterisk would reply with video if the invitation doesn't include video.  So I added a check to see if the joint capabilities include any video codec, then send the video portion of the reply, otherwise don't.  Besides what was in the invitation, though, this will be influenced/overridden by what is specified in sip.conf.  For example, if I have a video phone that normally invites video but doesn't this time, because it knows its camera is broken (this actually happened today), Asterisk will still send the video reply because it was told in sip.conf that this device "should" have video ability.  I'm presuming that this is the desired behavior. 3.  Quirky behavior of allow/disallow statements. I didn't fix this, nor did I introduce it.  The allow/disallow settings in sip.conf are a bit picky.  Different combinations of allows in the general section versus the device-sepecific section can give you a combination where it will reply with the "m=video" line and then not list any codecs below.  One case of this is when you don't specify any video codec in the general section, but then try to specify one for a particular device.  Another example is trying to use allow=all in the general section instead of specifying a video codec before the videosupport=yes line. This can be worked around with config files.  If you have a combination of video and non-video phones, this is the order I found that works: [general] (other desired general options) disallow=all (optional?) allow= (for each audio codec) allow= (your video codec) videosupport=yes [phone1] disallow=h263 ; switch off video support for audio-only phone (other phone options) [videophone1] ; by default video is enabled (other phone options) Files included are the sip.conf patch, also a text file showing the previously erroneous behavior, and an example sip.conf section.
Comments:	By: Olle Johansson (oej) 2004-03-19 04:23:56.000-0600 Is this tested by anyone else? Matt: do you still think this is a good solution? By: matt (matt) 2004-03-19 10:27:11.000-0600 Yes, I would expect this to be an uncontested solution.  I believe that the code has been in CVS for a while now.  I don't know if anyone else has tested it. By: Olle Johansson (oej) 2004-03-19 10:32:23.000-0600 If it's in CVS, this bug shold be closed. By: Mark Spencer (markster) 2004-04-04 17:58:12 Added to CVS By: Digium Subversion (svnbot) 2008-01-15 14:49:46.000-0600 Repository: asterisk Revision: 2616 U   trunk/channels/chan_sip.c ------------------------------------------------------------------------ r2616 | markster | 2008-01-15 14:49:45 -0600 (Tue, 15 Jan 2008) | 2 lines Various video fixes (bug ASTERISK-682) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=2616