[asterisk-dev] [Code Review] better SDP parsing algorithm for 1.4

[Matthew Nicholson]

> On 2009-10-02 12:20:19, Matthew Nicholson wrote:
> > At first glance this looks good.  Please describe the testing you have done in a little more detail.  Before committing, I would like to take a deeper look at this and get a few more eyes on it.  Good work.
> 
> frawd wrote:
>     Currently testing in lab and a couple of production servers, a few thousand calls between multiple SIP devices and providers in audio and video without apparent issues. Hooked with the sometimes troublesome Nortel CS2K gives good results when different connection information (c=) lines for audio and video streams (resolves #14994).
>     
>     It also resolves another bug that happened when some device was trying to pause only video stream (video specific sendonly), and Asterisk was stopping the audio RTP. This is why I added the vsendonly variable that should later be used to stop the video RTP, but I didn't want to add too much in that patch (new feature or bug fix?).
>     
>     Still I would appreciate a deeper review and wish for a broader test if it were possible, especially in T.38 cases that I haven't been able to test so far.
>     
>     Thanks.

Please upload some sample SDP packets that pass with the new parsing, and perhaps some that work with the new one but fail with the current one.  I would like to use them in my testing here.

I am treating this as a bugfix.  The vsendonly modification is fine.


- Matthew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/385/#review1138
-----------------------------------------------------------


On 2009-10-02 07:44:36, frawd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/385/
> -----------------------------------------------------------
> 
> (Updated 2009-10-02 07:44:36)
> 
> 
> Review request for Asterisk Developers, Olle E Johansson and Matthew Nicholson.
> 
> 
> Summary
> -------
> 
> This patch cleans up asterisk's SDP parsing algorithm, resolving a few bugs including #14994. It does the parsing line by line, making a distinction between session-level and media-specific parameters. It also optimizes the parsing adding functions for audio/video/image specific scanning.
> I added debug information for a better understanding on how the parsing is actually done (shows each SDP line parsed with OK or UNSUPPORTED).
> 
> If ported to 1.6 (sorry but I have no idea how the SIP code changed between 1.4 and 1.6 so I don't know how to do it), it will allow to easily add SDP functionality.
> 
> 
> This addresses bug 0014994.
>     https://issues.asterisk.org/view.php?id=0014994
> 
> 
> Diffs
> -----
> 
>   branches/1.4/channels/chan_sip.c 221961 
> 
> Diff: https://reviewboard.asterisk.org/r/385/diff
> 
> 
> Testing
> -------
> 
> Production tested for multiple audio and video devices in a version of the patch for 1.4.26.2 (see bug #14994).
> 
> Not tested with T.38, but it should work well.
> 
> 
> Thanks,
> 
> frawd
> 
>