[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: [asterisk-dev] [Code Review] 2723: Add pass through support for both VP8 and Opus
From: "Matt Jordan" <reviewboard () asterisk ! org>
Date: 2013-07-31 22:32:20
Message-ID: 20130731223220.21405.14056 () hotblack ! digium ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2723/
-----------------------------------------------------------
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 395904
/trunk/channels/chan_sip.c 395904
/trunk/channels/sip/include/sip.h 395904
/trunk/include/asterisk/format.h 395904
/trunk/include/asterisk/opus.h PRE-CREATION
/trunk/main/channel.c 395904
/trunk/main/format.c 395904
/trunk/main/frame.c 395904
/trunk/main/rtp_engine.c 395904
/trunk/res/res_format_attr_opus.c PRE-CREATION
/trunk/res/res_rtp_asterisk.c 395904
Diff: https://reviewboard.asterisk.org/r/2723/diff/
Testing
-------
Thanks,
Matt Jordan
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2723/">https://reviewboard.asterisk.org/r/2723/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Asterisk Developers, Joshua Colp and Mark Michelson.</div>
<div>By Matt Jordan.</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21981">ASTERISK-21981</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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?)</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/channels/chan_pjsip.c <span style="color: grey">(395904)</span></li>
<li>/trunk/channels/chan_sip.c <span style="color: grey">(395904)</span></li>
<li>/trunk/channels/sip/include/sip.h <span style="color: grey">(395904)</span></li>
<li>/trunk/include/asterisk/format.h <span style="color: grey">(395904)</span></li>
<li>/trunk/include/asterisk/opus.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>/trunk/main/channel.c <span style="color: grey">(395904)</span></li>
<li>/trunk/main/format.c <span style="color: grey">(395904)</span></li>
<li>/trunk/main/frame.c <span style="color: grey">(395904)</span></li>
<li>/trunk/main/rtp_engine.c <span style="color: grey">(395904)</span></li>
<li>/trunk/res/res_format_attr_opus.c <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>/trunk/res/res_rtp_asterisk.c <span style="color: grey">(395904)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2723/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic