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

[Joshua Colp]

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

Ship it!


I've reviewed the code and besides the issue below everything seems great. I also threw various SDPs that I could come up with at it to see how it faired and it behaved exactly as it should. Great job! Once the issue is taken care of then I can see no reason why this can't go in.


branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/385/#comment2859>

    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.


- Joshua


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
> 
>