[Home]

Summary:ASTERISK-05281: [patch] videosupport option in sip.conf should not be global
Reporter:hypherion (hypherion)Labels:
Date Opened:2005-10-11 20:05:30Date Closed:2006-03-26 21:39:25.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Video
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip_8932_videosupport.patch
( 1) chan_sip.c_2006-01-04.patch
Description:Currently, videosupport option can only be set in the [general] section.
There are many broken gateways out there that do not handle SDP's which contain video.  Some Cisco implementation will simply send back a 183 Session Progress with a Content-Type: application/sdp\r\n and a Content-Length: 0\r\n when they see a m=video in the INVITE (see issue 0004002).
As a result, Asterisk outputs:
process_sdp: Insufficient information for SDP (m = '', c = ''), the call goes through but there is no audio. Audio works fine if videosupport is off and the INVITE is sent without video.
Setting videosupport=no only for those peers that are known to exhibit such bad behaviour would be helpful.
Comments:By: twisted (twisted) 2005-10-11 21:34:59

Added patch to (hopefully) solve this issue.  Please verify operation, and make sure it doesn't break your normal calls (have no means of testing attm).

By: twisted (twisted) 2005-10-11 21:36:10

This patch also makes small formatting changes in the blocks I was working in to preserve uniformity.

By: twisted (twisted) 2005-10-11 21:52:23

should be fixed in CVS HEAD.  Thanks!

By: hypherion (hypherion) 2005-11-29 15:19:43.000-0600

I have tried this with 1.2, it does not seem to work as expected.
I set videosupport=yes in [general], then videosupport=no for my provider peer.  The outgoing INVITE still contains m=video in SDP.

By: twisted (twisted) 2005-11-29 15:36:04.000-0600

Looking back, only the config sample snippit seems to have merged into cvs... I'm not sure what happened, because I explicitly remember commiting the fix... sitting right next to kpfleming and markster at astricon.. hmm...   I do not remember deleting the patch i pasted online either, but that's neither here nor there..  I'm not sure if i'm going to have the time to recreate and recommit the fix, so ask kpfleming or brookshire on IRC if they can recover the patch that was attached to this bug (should be archived), and if so, i'll make sure it gets commited correctly.

By: hypherion (hypherion) 2005-12-06 16:54:41.000-0600

I was not able to find either on IRC.  Does anyone else have access to the database?  I think this feature makes sense to be included in Asterisk.

By: Matt Brooks (mbrooks) 2005-12-07 09:39:38.000-0600

Our backups only go back to 11-01-2005, sorry we do not have this anymore.

By: John Martin (jfp_martin) 2005-12-13 02:05:02.000-0600

twisted and hypherion,
 I have a chan_sip patch that adds videosupport to each peer or user entry. It also supports the "b=CT:%d\r\n" SIP header with setup for users and peers via a new entry maxcallbitrate=.
Let me know if you'd like me to upload it.

By: twisted (twisted) 2005-12-21 15:24:46.000-0600

Fixed in SVN 1.2 (automerge to trunk)

By: Russell Bryant (russell) 2005-12-22 16:19:51.000-0600

The committed patch has been reverted for now until we can come up with a complete solution for this.

By: Olle Johansson (oej) 2006-01-04 08:51:01.000-0600

What is "a complete" solution? What was the problem?

By: John Martin (jfp_martin) 2006-01-04 09:23:43.000-0600

OEJ, etc.
 so going back a while there was a bug filed that videosupport needed to be done on a per peer basis. The reason being that some network infrastructure doesn't like the SDP for video and being able to define it on a peer basis would allow an admin to work around it. Twisted submitted a patch but there was subtle problem in the timing of the vrtp creation.
 I had a look and submitted a bug (this bug) and a patch. I was a bit hasty with the patch because there were a couple of other patches mingled in and drumkilla (rightly) didn't want it all confused. I promised drumkilla that I'd have a look at this issue over the Christmas break... which I duely have done.
 The simple fix is as I suggested: use twisted's patch but move the point where the global_flags are copied in sip_alloc so that if videosupport is defined at the global level then the vrtp structures are created and everything progresses as normal.
 However, there are some more subtleties with this approach:
1. Videosupport at the global level must be on for any peer to have videosupport enabled (or else the vrtp is not created in sip_alloc and there will be no video). This may come down to documentation though I appreciate it's not like other options are set (perhaps calling the global option videosupport=on and the peer/user option as video=yes/no would do the trick).
2. The parsing of the sip.conf peer elements should check that video support is defined at the global level or else video could be declared in the SDP and there could be all sorts of problems. (solution: check the global flag in parsing the peer element and only allow video if supported globally).
3. If vrtp is opened whenever videosupport is defined at the global level then there are side effects: more memory used for all calls on that server, a valid p->vrtp (even with a peer with no video capabilities) can cause streamfile to see a video file as well as audio and try to stream video to a partially configured vrtp. (solution: destroy the vrtp session as soon as we know that the peer doesn't support video and set p->vrtp=NULL - this solves the first issue by only having vrtp memory allocated while a call is being set up and satisfies the second issue as well)

I have a patch (or can create one) with all these features in.

However, it would be nice to not have to declare videosupport in global but only in the peer declarations. So I went away to look at some sort of late creation of vrtp when we know the caps of the user/peer. Having looked at this I think it would require too much of a change. Someone else may be able to see a better way forward but I couldn't with the limited time I have.

My recommendation would be to go with the patch I have and to think about these issues for any future work in this area.

John

By: Olle Johansson (oej) 2006-01-04 09:27:01.000-0600

So where's the patch? In another bug or being uploaded soon?

By: John Martin (jfp_martin) 2006-01-04 09:41:35.000-0600

OEJ,
 My initial attempt at the fix was added to bug 6049. It'll take a couple of hours to create a new patch against trunk. I'll add it to this bug later today.

John

By: Olle Johansson (oej) 2006-01-04 09:46:12.000-0600

Thank you very much, I'll look into it as soon as you are ready. No worries.

By: John Martin (jfp_martin) 2006-01-04 11:31:18.000-0600

oej,
 I've added a patch file as discussed (chan_sip.c_2006-01-04.patch). I wasn't sure if videosupport should be added to build_user as well as build_peer??? Can you advise?

John

By: John Martin (jfp_martin) 2006-01-09 05:58:17.000-0600

Any news on testing this one? I've some other additions I'd like to get into chan_sip after this one's been ironed out...

John

By: Jean-Yves Avenard (jyavenard) 2006-01-23 03:10:15.000-0600

Hi,

I'm not sure this is the same problem.
Running 1.2.2.

When we are receiving a call from our VoIP providers, no calls go through, however we see on the console:
Jan 23 21:07:04 WARNING[16007]: chan_sip.c:3420 process_sdp: Insufficient information for SDP (m = '', c = '')


I've applied this patch (just in case), but it doesn't fix the problem.

Any ideas?
Thanks

By: Olle Johansson (oej) 2006-01-25 01:40:10.000-0600

jyavenard: If it's not related to video, please open another report.

jfp_martin: Need confirmation on a disclaimer from you before I can look at your patch. Sorry for keeping this patch on hold, but there's a few patches to work on as well as other stuff going on :-)

By: John Martin (jfp_martin) 2006-01-25 01:50:37.000-0600

oej,
 I'll get the disclaimer going today. There have been a lot of changes to chan_sip in the last few weeks that I'll have to merge, I should be able to get an updated version by the end of the week though. If you let me know when you'll look at it then I can make sure I've merged for then - also saves me trying to keep up with changes every few hours.

John

By: John Martin (jfp_martin) 2006-01-25 03:45:49.000-0600

oej,
 I have filed a disclaimer now. It's under my AuPix login. I'll use that account from now on.

John, AuPix (aka jfp_martin)

By: Olle Johansson (oej) 2006-01-30 12:25:59.000-0600

Looking forward to your patch :-)

By: John Martin (jfp_martin) 2006-01-31 04:22:21.000-0600

oej,
 I'm working on updating chan_sip today. Someone's gone and used bit 31 or the flags :-) , so I'm having to make some changes to get the videosupport flag into a page2 mechanism...

AuPix

By: John Martin (jfp_martin) 2006-01-31 10:37:17.000-0600

OEJ,
 I've uploaded a patch for by-peer video support. I see you've moved some flags around today so maybe this patch can be done with the pvt/peer/user flags and not page2 flags.
 To make use of this patch videosupport=yes/no is set in sip.conf->general and then video support can be enabled on a per peer/user.
 This patch also has code in it to do call bandwidth signalling - I'm sorry but without my own team I can't seem to keep up with all the changes everyone else is making and with what I'm working on at the moment... I've been running with this general setup for a couple of months now with H.323, SIP and ISDN, so I hope it's pretty solid.
 In case anyone reading about video is interested: I have fixes/reworks for video on H.323 (limited support for H.263), video in chan_agent, video in chan_local, video fastupdate support for bridged calls and I think I've got most of the RTCP stuff working now without seg faults (though that's another long time thread)...

John

By: Olle Johansson (oej) 2006-01-31 10:52:16.000-0600

John, I am very interested in the video stuff. I can set up a branch for testing that if you want.

I'll look at this a.s.a.p.

By: Olle Johansson (oej) 2006-01-31 11:44:31.000-0600

Created a branch for testing

http://svn.digium.com/svn/asterisk/team/oej/videosupport

Updated code to latest svn trunk automagically. Resolved conflicts :-)

Now you all : TEST, TEST, TEST and confirm your findings to this bug report.

By: Olle Johansson (oej) 2006-02-02 00:28:40.000-0600

Please test this code so we can move forward on this issue, thank you!

By: Neil Stratford (Vipadia Limited) (fenlander) 2006-02-03 04:48:59.000-0600

I have tested the svn test branch and have had no problems. Both enabling video per client and maxcallbitrate work as expected. (I had a similar patch in my own development tree for maxcallbitrate which I can remove now.) I'll continue to test and will report back if I do encounter any problems.

I would be interested in any other video patches that you have - I'm currently working on improving the RTCP code and would like to avoid duplicating effort :-).

Neil

By: Olle Johansson (oej) 2006-02-03 05:16:57.000-0600

For RTCP, please vist bug ASTERISK-2815 where work is going on. The patch in there was updated today.

For other video functions, we will open another report/branch. Feel free to submit your own additions to a bug report, and we'll be on our way!

By: Olle Johansson (oej) 2006-02-09 08:20:34.000-0600

Updated branch to trunk. Please test!

By: Olle Johansson (oej) 2006-03-02 11:47:15.000-0600

Seems like this code works and is ready to be implemented into trunk.

By: Olle Johansson (oej) 2006-03-02 11:48:03.000-0600

Ready for svn, disclaimed, tested, confirmed.

By: Olle Johansson (oej) 2006-03-26 21:36:30.000-0600

Committed to svn trunk rev 15148.

Thanks to everyone involved in this work!