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

[Joshua Colp]

> On 2009-11-02 09:43:07, Joshua Colp wrote:
> > branches/1.4/channels/chan_sip.c, lines 5417-5421
> > <https://reviewboard.asterisk.org/r/385/diff/6/?file=6835#file6835line5417>
> >
> >     There's a bug with this code right here. If the video stream does not have a 'c' line specified but the audio stream does then the video stream will also use that same IP address, even though it should be using session level.
> >     
> >     I think having another ast_hostent for session level is reasonable and just setting hp and vhp to use that at the start.
> 
>  wrote:
>     I am not sure you analysis here is correct.  The line you referenced here is the session level 'c' processing.

Correct. In that session level processing the video address is set to point to audiohp. When doing media stream level processing audiohp can be changed, and as a result the video address will also be incorrect since it points to the same thing.


- Joshua


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


On 2009-10-20 06:56:31, frawd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/385/
> -----------------------------------------------------------
> 
> (Updated 2009-10-20 06:56:31)
> 
> 
> 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 224737 
> 
> 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
> 
>