Summary:	ASTERISK-05891: Fix for bug 5427 breaks video in SIP (I think)
Reporter:	John Martin (jfp_martin)	Labels:
Date Opened:	2005-12-22 12:41:37.000-0600	Date Closed:	2005-12-22 13:40:45.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_sip.c.2005-12-22.patch
Description:	I've downloaded the latest chan_sip.c produced by Twisted and I think it no longer allows video to work in SIP. ****** ADDITIONAL INFORMATION ****** I've been working on chan_sip for a few weeks now and I've been trying to keep my work in step with other work on it. However, updating to 7594 seems to have broken videosupport in chan_sip. The problem comes from the new peer/user videosupport. The ast_copy_flags in sip_alloc happens too late. It needs to be done before the video ast_rtp_new_with_bindaddr is called. This means that the video rtp stream is never alloc'ed and so you never get video. There are two possible fixes (I think): 1. Move the ast_copy_flags to above the video rtp creation. The downside of this is that you must have videosupport=yes in your general section. 2. Create the rtp session for video only when the peer/user has been found. I've submitted a patch done the first way because that seemed to be more in keeping with the original and less likely to break anything. I've also noticed that the creation of vrtp based on an overall videosupport has the side effect that ast_streamfile will try and send a video file even if the called party has no video support. Therefore, I've added support for destroying the vrtp when we know if the peer/user can support video. Thirdly, this patch includes support for b=C:bitrate, because that's what I've been working on. I'd like to get this into the mix if I can - I've tested it against our videophone and a number of other video sip UAs so I'm reasonably confident it works and is benign for non-video.
Comments:	By: twisted (twisted) 2005-12-22 13:16:46.000-0600 seeing as I did not move the ast_copy_flags() i do not see how my patch broke anything.  If it happens too late, then wouldn't that suggest that other sip options would be broken as well?  The only changes that were made were allowing us to copy in the global video setting, then allowing us to parse in a per user/peer setting that will override that.   it should work the same way it used to, just allowing us to enable/disable it on a per peer basis.  I tested this in my environment, and the flags were getting copied to the peer/user upon user/peer creation, and copied correctly. By: Russell Bryant (russell) 2005-12-22 13:30:23.000-0600 I'm just going to revert the patch that went in since it isn't quite correct, and there is not an obvious easy solution to make the per-peer setting work correctly.  Once that is done, please update your patch to include your proposed changes. Thanks! By: Russell Bryant (russell) 2005-12-22 13:40:31.000-0600 I'm just going to mark this as resolved.  For your new additions to chan_sip, please open a new issue that is specific to those changes. Thank you!