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

Joshua Colp jcolp at digium.com
Mon Nov 2 16:16:10 CST 2009


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

Ship it!


Besides the small verbose message problem below this looks good and also performs as expected under my testing.


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/414/#comment2867>

    These log messages are referencing video when they should be referencing text (including the address information).


- Joshua


On 2009-11-02 15:24:49, frawd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/414/
> -----------------------------------------------------------
> 
> (Updated 2009-11-02 15:24:49)
> 
> 
> Review request for Asterisk Developers, Olle E Johansson and Matthew Nicholson.
> 
> 
> Summary
> -------
> 
> See https://reviewboard.asterisk.org/r/385/ for the initial 1.4 patch.
> 
> Changes from 1.4 patch:
> - Port to trunk API and its new things (SIREN, T.38, ...)
> - Added function "process_sdp_o" for version checking
> - Added function "process_sdp_a_text" for T.140 and RED stuff
> 
> 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).
> 
> 
> This addresses bug 0014994.
>     https://issues.asterisk.org/view.php?id=0014994
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 226973 
> 
> Diff: https://reviewboard.asterisk.org/r/414/diff
> 
> 
> Testing
> -------
> 
> It compiles OK.
> 
> When I can figure out how to load modules in trunk, I can do some basic testing. But I cannot test that in production (trunk...).
> For now I got dozens of:
> "Module '*****' could not be loaded."
> 
> 
> Thanks,
> 
> frawd
> 
>




More information about the asterisk-dev mailing list