[asterisk-dev] [Code Review] 2723: Add pass through support for both VP8 and Opus

[Joshua Colp]

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


With what is available within Asterisk and how things are done, this code is fine besides the maxptime implementation.


/trunk/res/res_format_attr_opus.c
<https://reviewboard.asterisk.org/r/2723/#comment18496>

    maxptime should be done as part of chan_sip / chan_pjsip, not in format specific code


- Joshua Colp


On Aug. 19, 2013, 2:55 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2723/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2013, 2:55 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp and Mark Michelson.
> 
> 
> Bugs: ASTERISK-21981
>     https://issues.asterisk.org/jira/browse/ASTERISK-21981
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Note: This patch was written by Lorenzo Miniero. I know he's at the IETF this week, but I figured we could get the formal code review going for him :-)
> 
> This patch adds pass through support for Opus and VP8. That includes:
> * Format attribute negotiation for Opus. Note that unlike some other codecs, the draft RFC specifies having spaces delimiting the attributes in addition to ';', so you have "attra=X; attrb=Y". This broke the attribute parsing in chan_sip, so a small tweak was also included in this patch for that.
> * A format attribute negotiation module for Opus
> * Fast picture update for VP8. Since VP8 uses a different RTCP packet number than FIR, this really is specific to VP8 at this time. Ideally this would be more generic and flexible for user preferences and other video codecs, but that could be done at a latter date.
> 
> The only part of this work that I did was port over the fast picture update code to chan_pjsip. I *think* that chan_pjsip will still suck out the attributes in res_pjsip_sdp_rtp, but I could be mistaken (Josh?)
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_pjsip.c 396921 
>   /trunk/channels/chan_sip.c 396921 
>   /trunk/include/asterisk/format.h 396921 
>   /trunk/include/asterisk/opus.h PRE-CREATION 
>   /trunk/main/channel.c 396921 
>   /trunk/main/format.c 396921 
>   /trunk/main/frame.c 396921 
>   /trunk/main/rtp_engine.c 396921 
>   /trunk/res/res_format_attr_opus.c PRE-CREATION 
>   /trunk/res/res_rtp_asterisk.c 396921 
> 
> Diff: https://reviewboard.asterisk.org/r/2723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130819/c8944066/attachment-0001.htm>