Update draft-ietf-codec-opus-update-04 to the finalised rfc8251.txt text
This commit is contained in:
parent
21883978f9
commit
0556c6498e
@ -1,753 +0,0 @@
|
||||
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
|
||||
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
|
||||
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
|
||||
<head profile="http://dublincore.org/documents/2008/08/04/dc-html/">
|
||||
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
|
||||
<meta name="robots" content="index,follow" />
|
||||
<meta name="creator" content="rfcmarkup version 1.120" />
|
||||
<link rel="schema.DC" href="http://purl.org/dc/elements/1.1/" />
|
||||
<meta name="DC.Relation.Replaces" content="draft-valin-codec-opus-update" />
|
||||
<meta name="DC.Identifier" content="urn:ietf:id:ietf-codec-opus-update" />
|
||||
<meta name="DC.Date.Issued" content="2016-10-21" />
|
||||
<meta name="DC.Creator" content="Valin, Jean-Marc" />
|
||||
<meta name="DC.Creator" content="Vos, Koen" />
|
||||
<meta name="DC.Description.Abstract" content="This document addresses minor issues that were found in the
|
||||
specification of the Opus audio codec in RFC 6716 [RFC6716]." />
|
||||
<meta name="DC.Title" content="Updates to the Opus Audio Codec" />
|
||||
|
||||
<link rel="icon" href="/images/id.png" type="image/png" />
|
||||
<link rel="shortcut icon" href="/images/id.png" type="image/png" />
|
||||
<title>draft-ietf-codec-opus-update-04 - Updates to the Opus Audio Codec</title>
|
||||
|
||||
|
||||
<style type="text/css">
|
||||
@media only screen
|
||||
and (min-width: 992px)
|
||||
and (max-width: 1199px) {
|
||||
body { font-size: 14pt; }
|
||||
div.content { width: 96ex; margin: 0 auto; }
|
||||
}
|
||||
@media only screen
|
||||
and (min-width: 768px)
|
||||
and (max-width: 991px) {
|
||||
body { font-size: 14pt; }
|
||||
div.content { width: 96ex; margin: 0 auto; }
|
||||
}
|
||||
@media only screen
|
||||
and (min-width: 480px)
|
||||
and (max-width: 767px) {
|
||||
body { font-size: 11pt; }
|
||||
div.content { width: 96ex; margin: 0 auto; }
|
||||
}
|
||||
@media only screen
|
||||
and (max-width: 479px) {
|
||||
body { font-size: 8pt; }
|
||||
div.content { width: 96ex; margin: 0 auto; }
|
||||
}
|
||||
@media only screen
|
||||
and (min-device-width : 375px)
|
||||
and (max-device-width : 667px) {
|
||||
body { font-size: 9.5pt; }
|
||||
div.content { width: 96ex; margin: 0 1px; }
|
||||
}
|
||||
@media only screen
|
||||
and (min-device-width: 1200px) {
|
||||
body { font-size: 10pt; margin: 0 4em; }
|
||||
div.content { width: 96ex; margin: 0; }
|
||||
}
|
||||
h1, h2, h3, h4, h5, h6, .h1, .h2, .h3, .h4, .h5, .h6 {
|
||||
font-weight: bold;
|
||||
line-height: 0pt;
|
||||
display: inline;
|
||||
white-space: pre;
|
||||
font-family: monospace;
|
||||
font-size: 1em;
|
||||
font-weight: bold;
|
||||
}
|
||||
pre {
|
||||
font-size: 1em;
|
||||
margin-top: 0px;
|
||||
margin-bottom: 0px;
|
||||
}
|
||||
.pre {
|
||||
white-space: pre;
|
||||
font-family: monospace;
|
||||
}
|
||||
.header{
|
||||
font-weight: bold;
|
||||
}
|
||||
.newpage {
|
||||
page-break-before: always;
|
||||
}
|
||||
.invisible {
|
||||
text-decoration: none;
|
||||
color: white;
|
||||
}
|
||||
a.selflink {
|
||||
color: black;
|
||||
text-decoration: none;
|
||||
}
|
||||
@media print {
|
||||
body {
|
||||
font-family: monospace;
|
||||
font-size: 10.5pt;
|
||||
}
|
||||
h1, h2, h3, h4, h5, h6 {
|
||||
font-size: 1em;
|
||||
}
|
||||
|
||||
a:link, a:visited {
|
||||
color: inherit;
|
||||
text-decoration: none;
|
||||
}
|
||||
.noprint {
|
||||
display: none;
|
||||
}
|
||||
}
|
||||
@media screen {
|
||||
.grey, .grey a:link, .grey a:visited {
|
||||
color: #777;
|
||||
}
|
||||
.docinfo {
|
||||
background-color: #EEE;
|
||||
}
|
||||
.top {
|
||||
border-top: 7px solid #EEE;
|
||||
}
|
||||
.bgwhite { background-color: white; }
|
||||
.bgred { background-color: #F44; }
|
||||
.bggrey { background-color: #666; }
|
||||
.bgbrown { background-color: #840; }
|
||||
.bgorange { background-color: #FA0; }
|
||||
.bgyellow { background-color: #EE0; }
|
||||
.bgmagenta{ background-color: #F4F; }
|
||||
.bgblue { background-color: #66F; }
|
||||
.bgcyan { background-color: #4DD; }
|
||||
.bggreen { background-color: #4F4; }
|
||||
|
||||
.legend { font-size: 90%; }
|
||||
.cplate { font-size: 70%; border: solid grey 1px; }
|
||||
}
|
||||
</style>
|
||||
<!--[if IE]>
|
||||
<style>
|
||||
body {
|
||||
font-size: 13px;
|
||||
margin: 10px 10px;
|
||||
}
|
||||
</style>
|
||||
<![endif]-->
|
||||
|
||||
<script type="text/javascript"><!--
|
||||
function addHeaderTags() {
|
||||
var spans = document.getElementsByTagName("span");
|
||||
for (var i=0; i < spans.length; i++) {
|
||||
var elem = spans[i];
|
||||
if (elem) {
|
||||
var level = elem.getAttribute("class");
|
||||
if (level == "h1" || level == "h2" || level == "h3" || level == "h4" || level == "h5" || level == "h6") {
|
||||
elem.innerHTML = "<"+level+">"+elem.innerHTML+"</"+level+">";
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
var legend_html = "Colour legend:<br /> <table> <tr><td>Unknown:</td> <td><span class='cplate bgwhite'> </span></td></tr> <tr><td>Draft:</td> <td><span class='cplate bgred'> </span></td></tr> <tr><td>Informational:</td> <td><span class='cplate bgorange'> </span></td></tr> <tr><td>Experimental:</td> <td><span class='cplate bgyellow'> </span></td></tr> <tr><td>Best Common Practice:</td> <td><span class='cplate bgmagenta'> </span></td></tr> <tr><td>Proposed Standard:</td> <td><span class='cplate bgblue'> </span></td></tr> <tr><td>Draft Standard (old designation):</td> <td><span class='cplate bgcyan'> </span></td></tr> <tr><td>Internet Standard:</td> <td><span class='cplate bggreen'> </span></td></tr> <tr><td>Historic:</td> <td><span class='cplate bggrey'> </span></td></tr> <tr><td>Obsolete:</td> <td><span class='cplate bgbrown'> </span></td></tr> </table>";
|
||||
function showElem(id) {
|
||||
var elem = document.getElementById(id);
|
||||
elem.innerHTML = eval(id+"_html");
|
||||
elem.style.visibility='visible';
|
||||
}
|
||||
function hideElem(id) {
|
||||
var elem = document.getElementById(id);
|
||||
elem.style.visibility='hidden';
|
||||
elem.innerHTML = "";
|
||||
}
|
||||
// -->
|
||||
</script>
|
||||
</head>
|
||||
<body onload="addHeaderTags()">
|
||||
<div class="content">
|
||||
<div style="height: 13px;">
|
||||
<div onmouseover="this.style.cursor='pointer';"
|
||||
onclick="showElem('legend');"
|
||||
onmouseout="hideElem('legend')"
|
||||
style="height: 6px; position: absolute;"
|
||||
class="pre noprint docinfo bgred"
|
||||
title="Click for colour legend." > </div>
|
||||
<div id="legend"
|
||||
class="docinfo noprint pre legend"
|
||||
style="position:absolute; top: 4px; left: 4ex; visibility:hidden; background-color: white; padding: 4px 9px 5px 7px; border: solid #345 1px; "
|
||||
onmouseover="showElem('legend');"
|
||||
onmouseout="hideElem('legend');">
|
||||
</div>
|
||||
</div>
|
||||
<span class="pre noprint docinfo top">[<a href="../html/" title="Document search and retrieval page">Docs</a>] [<a href="https://tools.ietf.org/id/draft-ietf-codec-opus-update-04.txt" title="Plaintext version of this document">txt</a>|<a href="/pdf/draft-ietf-codec-opus-update-04.txt" title="PDF version of this document">pdf</a>] [<a href='https://datatracker.ietf.org/doc/draft-ietf-codec-opus-update' title='IESG Datatracker information for this document'>Tracker</a>] [<a href="../wg/codec" title="The working group handling this document">WG</a>] [<a href="mailto:draft-ietf-codec-opus-update@tools.ietf.org?subject=draft-ietf-codec-opus-update%20" title="Send email to the document authors">Email</a>] [<a href="/rfcdiff?difftype=--hwdiff&url2=draft-ietf-codec-opus-update-04.txt" title="Inline diff (wdiff)">Diff1</a>] [<a href="/rfcdiff?url2=draft-ietf-codec-opus-update-04.txt" title="Side-by-side diff">Diff2</a>] [<a href="/idnits?url=https://tools.ietf.org/id/draft-ietf-codec-opus-update-04.txt" title="Run an idnits check of this document">Nits</a>] </span><br />
|
||||
<span class="pre noprint docinfo"> </span><br />
|
||||
<span class="pre noprint docinfo">Versions: (<a href="./draft-valin-codec-opus-update" title="Precursor">draft-valin-codec-opus-update</a>) <a href="./draft-ietf-codec-opus-update-00">00</a>
|
||||
<a href="./draft-ietf-codec-opus-update-01">01</a> <a href="./draft-ietf-codec-opus-update-02">02</a> <a href="./draft-ietf-codec-opus-update-03">03</a> <a href="./draft-ietf-codec-opus-update-04">04</a> </span><br />
|
||||
<span class="pre noprint docinfo"> </span><br />
|
||||
<pre>
|
||||
Network Working Group JM. Valin
|
||||
Internet-Draft Mozilla Corporation
|
||||
Intended status: Standards Track K. Vos
|
||||
Expires: April 24, 2017 vocTone
|
||||
October 21, 2016
|
||||
|
||||
|
||||
<span class="h1">Updates to the Opus Audio Codec</span>
|
||||
<span class="h1">draft-ietf-codec-opus-update-04</span>
|
||||
|
||||
Abstract
|
||||
|
||||
This document addresses minor issues that were found in the
|
||||
specification of the Opus audio codec in <a href="./rfc6716">RFC 6716</a> [<a href="./rfc6716" title=""Definition of the Opus Audio Codec"">RFC6716</a>].
|
||||
|
||||
Status of This Memo
|
||||
|
||||
This Internet-Draft is submitted in full conformance with the
|
||||
provisions of <a href="./bcp78">BCP 78</a> and <a href="./bcp79">BCP 79</a>.
|
||||
|
||||
Internet-Drafts are working documents of the Internet Engineering
|
||||
Task Force (IETF). Note that other groups may also distribute
|
||||
working documents as Internet-Drafts. The list of current Internet-
|
||||
Drafts is at <a href="http://datatracker.ietf.org/drafts/current/">http://datatracker.ietf.org/drafts/current/</a>.
|
||||
|
||||
Internet-Drafts are draft documents valid for a maximum of six months
|
||||
and may be updated, replaced, or obsoleted by other documents at any
|
||||
time. It is inappropriate to use Internet-Drafts as reference
|
||||
material or to cite them other than as "work in progress."
|
||||
|
||||
This Internet-Draft will expire on April 24, 2017.
|
||||
|
||||
Copyright Notice
|
||||
|
||||
Copyright (c) 2016 IETF Trust and the persons identified as the
|
||||
document authors. All rights reserved.
|
||||
|
||||
This document is subject to <a href="./bcp78">BCP 78</a> and the IETF Trust's Legal
|
||||
Provisions Relating to IETF Documents
|
||||
(<a href="http://trustee.ietf.org/license-info">http://trustee.ietf.org/license-info</a>) in effect on the date of
|
||||
publication of this document. Please review these documents
|
||||
carefully, as they describe your rights and restrictions with respect
|
||||
to this document. Code Components extracted from this document must
|
||||
include Simplified BSD License text as described in Section 4.e of
|
||||
the Trust Legal Provisions and are provided without warranty as
|
||||
described in the Simplified BSD License.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 1]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-2" id="page-2" href="#page-2" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
Table of Contents
|
||||
|
||||
<a href="#section-1">1</a>. Introduction . . . . . . . . . . . . . . . . . . . . . . . . <a href="#page-2">2</a>
|
||||
<a href="#section-2">2</a>. Terminology . . . . . . . . . . . . . . . . . . . . . . . . . <a href="#page-2">2</a>
|
||||
<a href="#section-3">3</a>. Stereo State Reset in SILK . . . . . . . . . . . . . . . . . <a href="#page-2">2</a>
|
||||
<a href="#section-4">4</a>. Parsing of the Opus Packet Padding . . . . . . . . . . . . . <a href="#page-3">3</a>
|
||||
<a href="#section-5">5</a>. Resampler buffer . . . . . . . . . . . . . . . . . . . . . . <a href="#page-4">4</a>
|
||||
<a href="#section-6">6</a>. Integer wrap-around in inverse gain computation . . . . . . . <a href="#page-5">5</a>
|
||||
<a href="#section-7">7</a>. Integer wrap-around in LSF decoding . . . . . . . . . . . . . <a href="#page-6">6</a>
|
||||
<a href="#section-8">8</a>. Cap on Band Energy . . . . . . . . . . . . . . . . . . . . . <a href="#page-6">6</a>
|
||||
<a href="#section-9">9</a>. Hybrid Folding . . . . . . . . . . . . . . . . . . . . . . . <a href="#page-7">7</a>
|
||||
<a href="#section-10">10</a>. Downmix to Mono . . . . . . . . . . . . . . . . . . . . . . . <a href="#page-9">9</a>
|
||||
<a href="#section-11">11</a>. New Test Vectors . . . . . . . . . . . . . . . . . . . . . . <a href="#page-9">9</a>
|
||||
<a href="#section-12">12</a>. IANA Considerations . . . . . . . . . . . . . . . . . . . . . <a href="#page-9">9</a>
|
||||
<a href="#section-13">13</a>. Acknowledgements . . . . . . . . . . . . . . . . . . . . . . <a href="#page-9">9</a>
|
||||
<a href="#section-14">14</a>. References . . . . . . . . . . . . . . . . . . . . . . . . . <a href="#page-10">10</a>
|
||||
Authors' Addresses . . . . . . . . . . . . . . . . . . . . . . . <a href="#page-10">10</a>
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-1" href="#section-1">1</a>. Introduction</span>
|
||||
|
||||
This document addresses minor issues that were discovered in the
|
||||
reference implementation of the Opus codec that serves as the
|
||||
specification in <a href="./rfc6716">RFC 6716</a> [<a href="./rfc6716" title=""Definition of the Opus Audio Codec"">RFC6716</a>]. Only issues affecting the
|
||||
decoder are listed here. An up-to-date implementation of the Opus
|
||||
encoder can be found at <a href="http://opus-codec.org/">http://opus-codec.org/</a>. The updated
|
||||
specification remains fully compatible with the original
|
||||
specification.
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-2" href="#section-2">2</a>. Terminology</span>
|
||||
|
||||
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
|
||||
"SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
|
||||
document are to be interpreted as described in <a href="./rfc2119">RFC 2119</a> [<a href="./rfc2119" title=""Key words for use in RFCs to Indicate Requirement Levels"">RFC2119</a>].
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-3" href="#section-3">3</a>. Stereo State Reset in SILK</span>
|
||||
|
||||
The reference implementation does not reinitialize the stereo state
|
||||
during a mode switch. The old stereo memory can produce a brief
|
||||
impulse (i.e. single sample) in the decoded audio. This can be fixed
|
||||
by changing silk/dec_API.c at line 72:
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 2]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-3" id="page-3" href="#page-3" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
for( n = 0; n < DECODER_NUM_CHANNELS; n++ ) {
|
||||
ret = silk_init_decoder( &channel_state[ n ] );
|
||||
}
|
||||
+ silk_memset(&((silk_decoder *)decState)->sStereo, 0,
|
||||
+ sizeof(((silk_decoder *)decState)->sStereo));
|
||||
+ /* Not strictly needed, but it's cleaner that way */
|
||||
+ ((silk_decoder *)decState)->prev_decode_only_middle = 0;
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
This change affects the normative part of the decoder, although the
|
||||
amount of change is too small to make a significant impact on
|
||||
testvectors.
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-4" href="#section-4">4</a>. Parsing of the Opus Packet Padding</span>
|
||||
|
||||
It was discovered that some invalid packets of very large size could
|
||||
trigger an out-of-bounds read in the Opus packet parsing code
|
||||
responsible for padding. This is due to an integer overflow if the
|
||||
signaled padding exceeds 2^31-1 bytes (the actual packet may be
|
||||
smaller). The code can be fixed by applying the following changes at
|
||||
line 596 of src/opus_decoder.c:
|
||||
|
||||
/* Padding flag is bit 6 */
|
||||
if (ch&0x40)
|
||||
{
|
||||
- int padding=0;
|
||||
int p;
|
||||
do {
|
||||
if (len<=0)
|
||||
return OPUS_INVALID_PACKET;
|
||||
p = *data++;
|
||||
len--;
|
||||
- padding += p==255 ? 254: p;
|
||||
+ len -= p==255 ? 254: p;
|
||||
} while (p==255);
|
||||
- len -= padding;
|
||||
}
|
||||
|
||||
This packet parsing issue is limited to reading memory up to about 60
|
||||
kB beyond the compressed buffer. This can only be triggered by a
|
||||
compressed packet more than about 16 MB long, so it's not a problem
|
||||
for RTP. In theory, it _could_ crash a file decoder (e.g. Opus in
|
||||
Ogg) if the memory just after the incoming packet is out-of-range,
|
||||
but our attempts to trigger such a crash in a production application
|
||||
built using an affected version of the Opus decoder failed.
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 3]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-4" id="page-4" href="#page-4" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-5" href="#section-5">5</a>. Resampler buffer</span>
|
||||
|
||||
The SILK resampler had the following issues:
|
||||
|
||||
1. The calls to memcpy() were using sizeof(opus_int32), but the type
|
||||
of the local buffer was opus_int16.
|
||||
|
||||
2. Because the size was wrong, this potentially allowed the source
|
||||
and destination regions of the memcpy() to overlap. We _believe_
|
||||
that nSamplesIn is at least fs_in_khZ, which is at least 8.
|
||||
Since RESAMPLER_ORDER_FIR_12 is only 8, that should not be a
|
||||
problem once the type size is fixed.
|
||||
|
||||
3. The size of the buffer used RESAMPLER_MAX_BATCH_SIZE_IN, but the
|
||||
data stored in it was actually _twice_ the input batch size
|
||||
(nSamplesIn<<1).
|
||||
|
||||
The fact that the code never produced any error in testing (including
|
||||
when run under the Valgrind memory debugger), suggests that in
|
||||
practice the batch sizes are reasonable enough that none of the
|
||||
issues above was ever a problem. However, proving that is non-
|
||||
obvious.
|
||||
|
||||
The code can be fixed by applying the following changes to line 70 of
|
||||
silk/resampler_private_IIR_FIR.c:
|
||||
|
||||
)
|
||||
{
|
||||
silk_resampler_state_struct *S = \
|
||||
(silk_resampler_state_struct *)SS;
|
||||
opus_int32 nSamplesIn;
|
||||
opus_int32 max_index_Q16, index_increment_Q16;
|
||||
- opus_int16 buf[ RESAMPLER_MAX_BATCH_SIZE_IN + \
|
||||
RESAMPLER_ORDER_FIR_12 ];
|
||||
+ opus_int16 buf[ 2*RESAMPLER_MAX_BATCH_SIZE_IN + \
|
||||
RESAMPLER_ORDER_FIR_12 ];
|
||||
|
||||
/* Copy buffered samples to start of buffer */
|
||||
- silk_memcpy( buf, S->sFIR, RESAMPLER_ORDER_FIR_12 \
|
||||
* sizeof( opus_int32 ) );
|
||||
+ silk_memcpy( buf, S->sFIR, RESAMPLER_ORDER_FIR_12 \
|
||||
* sizeof( opus_int16 ) );
|
||||
|
||||
/* Iterate over blocks of frameSizeIn input samples */
|
||||
index_increment_Q16 = S->invRatio_Q16;
|
||||
while( 1 ) {
|
||||
nSamplesIn = silk_min( inLen, S->batchSize );
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 4]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-5" id="page-5" href="#page-5" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
/* Upsample 2x */
|
||||
silk_resampler_private_up2_HQ( S->sIIR, &buf[ \
|
||||
RESAMPLER_ORDER_FIR_12 ], in, nSamplesIn );
|
||||
|
||||
max_index_Q16 = silk_LSHIFT32( nSamplesIn, 16 + 1 \
|
||||
); /* + 1 because 2x upsampling */
|
||||
out = silk_resampler_private_IIR_FIR_INTERPOL( out, \
|
||||
buf, max_index_Q16, index_increment_Q16 );
|
||||
in += nSamplesIn;
|
||||
inLen -= nSamplesIn;
|
||||
|
||||
if( inLen > 0 ) {
|
||||
/* More iterations to do; copy last part of \
|
||||
filtered signal to beginning of buffer */
|
||||
- silk_memcpy( buf, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
|
||||
+ silk_memmove( buf, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/* Copy last part of filtered signal to the state for \
|
||||
the next call */
|
||||
- silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
|
||||
+ silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
|
||||
}
|
||||
|
||||
Note: due to RFC formatting conventions, lines exceeding the column
|
||||
width in the patch above are split using a backslash character. The
|
||||
backslashes at the end of a line and the white space at the beginning
|
||||
of the following line are not part of the patch. A properly
|
||||
formatted patch including the three changes above is available at
|
||||
<<a href="https://jmvalin.ca/misc_stuff/opus_update.patch">https://jmvalin.ca/misc_stuff/opus_update.patch</a>>. (EDITOR: change
|
||||
to an ietf.org link when ready)
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-6" href="#section-6">6</a>. Integer wrap-around in inverse gain computation</span>
|
||||
|
||||
It was discovered through decoder fuzzing that some bitstreams could
|
||||
produce integer values exceeding 32-bits in
|
||||
LPC_inverse_pred_gain_QA(), causing a wrap-around. Although the
|
||||
error is harmless in practice, the C standard considers the behavior
|
||||
as undefined, so the following patch to line 87 of silk/
|
||||
LPC_inv_pred_gain.c detects values that do not fit in a 32-bit
|
||||
integer and considers the corresponding filters unstable:
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 5]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-6" id="page-6" href="#page-6" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
/* Update AR coefficient */
|
||||
for( n = 0; n < k; n++ ) {
|
||||
- tmp_QA = Aold_QA[ n ] - MUL32_FRAC_Q( \
|
||||
Aold_QA[ k - n - 1 ], rc_Q31, 31 );
|
||||
- Anew_QA[ n ] = MUL32_FRAC_Q( tmp_QA, rc_mult2 , mult2Q );
|
||||
+ opus_int64 tmp64;
|
||||
+ tmp_QA = silk_SUB_SAT32( Aold_QA[ n ], MUL32_FRAC_Q( \
|
||||
Aold_QA[ k - n - 1 ], rc_Q31, 31 ) );
|
||||
+ tmp64 = silk_RSHIFT_ROUND64( silk_SMULL( tmp_QA, \
|
||||
rc_mult2 ), mult2Q);
|
||||
+ if( tmp64 > silk_int32_MAX || tmp64 < silk_int32_MIN ) {
|
||||
+ return 0;
|
||||
+ }
|
||||
+ Anew_QA[ n ] = ( opus_int32 )tmp64;
|
||||
}
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-7" href="#section-7">7</a>. Integer wrap-around in LSF decoding</span>
|
||||
|
||||
It was discovered -- also from decoder fuzzing -- that an integer
|
||||
wrap-around could occur when decoding line spectral frequency
|
||||
coefficients from extreme bitstreams. The end result of the wrap-
|
||||
around is an illegal read access on the stack, which the authors do
|
||||
not believe is exploitable but should nonetheless be fixed. The
|
||||
following patch to line 137 of silk/NLSF_stabilize.c prevents the
|
||||
problem:
|
||||
|
||||
/* Keep delta_min distance between the NLSFs */
|
||||
for( i = 1; i < L; i++ )
|
||||
- NLSF_Q15[i] = silk_max_int( NLSF_Q15[i], \
|
||||
NLSF_Q15[i-1] + NDeltaMin_Q15[i] );
|
||||
+ NLSF_Q15[i] = silk_max_int( NLSF_Q15[i], \
|
||||
silk_ADD_SAT16( NLSF_Q15[i-1], NDeltaMin_Q15[i] ) );
|
||||
|
||||
/* Last NLSF should be no higher than 1 - NDeltaMin[L] */
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-8" href="#section-8">8</a>. Cap on Band Energy</span>
|
||||
|
||||
On extreme bit-streams, it is possible for log-domain band energy
|
||||
levels to exceed the maximum single-precision floating point value
|
||||
once converted to a linear scale. This would later cause the decoded
|
||||
values to be NaN, possibly causing problems in the software using the
|
||||
PCM values. This can be avoided with the following patch to line 552
|
||||
of celt/quant_bands.c:
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 6]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-7" id="page-7" href="#page-7" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
{
|
||||
opus_val16 lg = ADD16(oldEBands[i+c*m->nbEBands],
|
||||
SHL16((opus_val16)eMeans[i],6));
|
||||
+ lg = MIN32(QCONST32(32.f, 16), lg);
|
||||
eBands[i+c*m->nbEBands] = PSHR32(celt_exp2(lg),4);
|
||||
}
|
||||
for (;i<m->nbEBands;i++)
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-9" href="#section-9">9</a>. Hybrid Folding</span>
|
||||
|
||||
When encoding in hybrid mode at low bitrate, we sometimes only have
|
||||
enough bits to code a single CELT band (8 - 9.6 kHz). When that
|
||||
happens, the second band (CELT band 18, from 9.6 to 12 kHz) cannot
|
||||
use folding because it is wider than the amount already coded, and
|
||||
falls back to LCG noise. Because it can also happen on transients
|
||||
(e.g. stops), it can cause audible pre-echo.
|
||||
|
||||
To address the issue, we change the folding behavior so that it is
|
||||
never forced to fall back to LCG due to the first band not containing
|
||||
enough coefficients to fold onto the second band. This is achieved
|
||||
by simply repeating part of the first band in the folding of the
|
||||
second band. This changes the code in celt/bands.c around line 1237:
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 7]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-8" id="page-8" href="#page-8" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
b = 0;
|
||||
}
|
||||
|
||||
- if (resynth && M*eBands[i]-N >= M*eBands[start] && \
|
||||
(update_lowband || lowband_offset==0))
|
||||
+ if (resynth && (M*eBands[i]-N >= M*eBands[start] || \
|
||||
i==start+1) && (update_lowband || lowband_offset==0))
|
||||
lowband_offset = i;
|
||||
|
||||
+ if (i == start+1)
|
||||
+ {
|
||||
+ int n1, n2;
|
||||
+ int offset;
|
||||
+ n1 = M*(eBands[start+1]-eBands[start]);
|
||||
+ n2 = M*(eBands[start+2]-eBands[start+1]);
|
||||
+ offset = M*eBands[start];
|
||||
+ /* Duplicate enough of the first band folding data to \
|
||||
be able to fold the second band.
|
||||
+ Copies no data for CELT-only mode. */
|
||||
+ OPUS_COPY(&norm[offset+n1], &norm[offset+2*n1 - n2], n2-n1);
|
||||
+ if (C==2)
|
||||
+ OPUS_COPY(&norm2[offset+n1], &norm2[offset+2*n1 - n2], \
|
||||
n2-n1);
|
||||
+ }
|
||||
+
|
||||
tf_change = tf_res[i];
|
||||
if (i>=m->effEBands)
|
||||
{
|
||||
|
||||
as well as line 1260:
|
||||
|
||||
fold_start = lowband_offset;
|
||||
while(M*eBands[--fold_start] > effective_lowband);
|
||||
fold_end = lowband_offset-1;
|
||||
- while(M*eBands[++fold_end] < effective_lowband+N);
|
||||
+ while(++fold_end < i && M*eBands[fold_end] < \
|
||||
effective_lowband+N);
|
||||
x_cm = y_cm = 0;
|
||||
fold_i = fold_start; do {
|
||||
x_cm |= collapse_masks[fold_i*C+0];
|
||||
|
||||
|
||||
The fix does not impact compatibility, because the improvement does
|
||||
not depend on the encoder doing anything special. There is also no
|
||||
reasonable way for an encoder to use the original behavior to improve
|
||||
quality over the proposed change.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 8]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-9" id="page-9" href="#page-9" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-10" href="#section-10">10</a>. Downmix to Mono</span>
|
||||
|
||||
The last issue is not strictly a bug, but it is an issue that has
|
||||
been reported when downmixing an Opus decoded stream to mono, whether
|
||||
this is done inside the decoder or as a post-processing step on the
|
||||
stereo decoder output. Opus intensity stereo allows optionally
|
||||
coding the two channels 180-degrees out of phase on a per-band basis.
|
||||
This provides better stereo quality than forcing the two channels to
|
||||
be in phase, but when the output is downmixed to mono, the energy in
|
||||
the affected bands is cancelled sometimes resulting in audible
|
||||
artefacts.
|
||||
|
||||
As a work-around for this issue, the decoder MAY choose not to apply
|
||||
the 180-degree phase shift when the output is meant to be downmixed
|
||||
(inside or outside of the decoder).
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-11" href="#section-11">11</a>. New Test Vectors</span>
|
||||
|
||||
Changes in <a href="#section-9">Section 9</a> and <a href="#section-10">Section 10</a> have sufficient impact on the
|
||||
testvectors to make them fail. For this reason, this document also
|
||||
updates the Opus test vectors. The new test vectors now include two
|
||||
decoded outputs for the same bitstream. The outputs with suffix 'm'
|
||||
do not apply the CELT 180-degree phase shift as allowed in
|
||||
<a href="#section-10">Section 10</a>, while the outputs without the suffix do. An
|
||||
implementation is compliant as long as it passes either set of
|
||||
vectors.
|
||||
|
||||
In addition, any Opus implementation that passes the original test
|
||||
vectors from <a href="./rfc6716">RFC 6716</a> [<a href="./rfc6716" title=""Definition of the Opus Audio Codec"">RFC6716</a>] is still compliant with the Opus
|
||||
specification. However, newer implementations SHOULD be based on the
|
||||
new test vectors rather than the old ones.
|
||||
|
||||
The new test vectors are located at <<a href="https://jmvalin.ca/misc_stuff/opus_newvectors.tar.gz">https://jmvalin.ca/misc_stuff/</a>
|
||||
<a href="https://jmvalin.ca/misc_stuff/opus_newvectors.tar.gz">opus_newvectors.tar.gz</a>>. (EDITOR: change to an ietf.org link when
|
||||
ready)
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-12" href="#section-12">12</a>. IANA Considerations</span>
|
||||
|
||||
This document makes no request of IANA.
|
||||
|
||||
Note to RFC Editor: this section may be removed on publication as an
|
||||
RFC.
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-13" href="#section-13">13</a>. Acknowledgements</span>
|
||||
|
||||
We would like to thank Juri Aedla for reporting the issue with the
|
||||
parsing of the Opus padding.
|
||||
|
||||
|
||||
|
||||
|
||||
<span class="grey">Valin & Vos Expires April 24, 2017 [Page 9]</span></pre>
|
||||
<hr class='noprint' style='width: 96ex;' align='left'/><!--NewPage--><pre class='newpage'><a name="page-10" id="page-10" href="#page-10" class="invisible"> </a>
|
||||
<span class="grey">Internet-Draft Opus Update October 2016</span>
|
||||
|
||||
|
||||
<span class="h2"><a class="selflink" name="section-14" href="#section-14">14</a>. References</span>
|
||||
|
||||
[<a name="ref-RFC2119" id="ref-RFC2119">RFC2119</a>] Bradner, S., "Key words for use in RFCs to Indicate
|
||||
Requirement Levels", <a href="./bcp14">BCP 14</a>, <a href="./rfc2119">RFC 2119</a>,
|
||||
DOI 10.17487/RFC2119, March 1997,
|
||||
<<a href="http://www.rfc-editor.org/info/rfc2119">http://www.rfc-editor.org/info/rfc2119</a>>.
|
||||
|
||||
[<a name="ref-RFC6716" id="ref-RFC6716">RFC6716</a>] Valin, JM., Vos, K., and T. Terriberry, "Definition of the
|
||||
Opus Audio Codec", <a href="./rfc6716">RFC 6716</a>, DOI 10.17487/RFC6716,
|
||||
September 2012, <<a href="http://www.rfc-editor.org/info/rfc6716">http://www.rfc-editor.org/info/rfc6716</a>>.
|
||||
|
||||
Authors' Addresses
|
||||
|
||||
Jean-Marc Valin
|
||||
Mozilla Corporation
|
||||
331 E. Evelyn Avenue
|
||||
Mountain View, CA 94041
|
||||
USA
|
||||
|
||||
Phone: +1 650 903-0800
|
||||
Email: jmvalin@jmvalin.ca
|
||||
|
||||
|
||||
Koen Vos
|
||||
vocTone
|
||||
|
||||
Email: koenvos74@gmail.com
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Expires April 24, 2017 [Page 10]
|
||||
|
||||
</pre><br />
|
||||
<span class="noprint"><small><small>Html markup produced by rfcmarkup 1.120, available from
|
||||
<a href="https://tools.ietf.org/tools/rfcmarkup/">https://tools.ietf.org/tools/rfcmarkup/</a>
|
||||
</small></small></span>
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
@ -10,7 +10,7 @@ URL: https://www.opus-codec.org/
|
||||
Source0: http://downloads.xiph.org/releases/%{name}/%{name}-%{version}%{?candidate:-%{candidate}}.tar.gz
|
||||
# This is the final IETF Working Group RFC
|
||||
Source1: http://tools.ietf.org/rfc/rfc6716.txt
|
||||
Source2: https://tools.ietf.org/html/draft-ietf-codec-opus-update-04
|
||||
Source2: http://tools.ietf.org/rfc/rfc8251.txt
|
||||
|
||||
BuildRequires: doxygen
|
||||
|
||||
@ -57,7 +57,7 @@ make check
|
||||
%{_libdir}/libopus.so.*
|
||||
|
||||
%files devel
|
||||
%doc README doc/html rfc6716.txt draft-ietf-codec-opus-update-04
|
||||
%doc README doc/html rfc6716.txt rfc8251.txt
|
||||
%{_includedir}/opus
|
||||
%{_libdir}/libopus.so
|
||||
%{_libdir}/pkgconfig/opus.pc
|
||||
|
||||
675
rfc8251.txt
Normal file
675
rfc8251.txt
Normal file
@ -0,0 +1,675 @@
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Internet Engineering Task Force (IETF) JM. Valin
|
||||
Request for Comments: 8251 Mozilla Corporation
|
||||
Updates: 6716 K. Vos
|
||||
Category: Standards Track vocTone
|
||||
ISSN: 2070-1721 October 2017
|
||||
|
||||
|
||||
Updates to the Opus Audio Codec
|
||||
|
||||
Abstract
|
||||
|
||||
This document addresses minor issues that were found in the
|
||||
specification of the Opus audio codec in RFC 6716. It updates the
|
||||
normative decoder implementation included in Appendix A of RFC 6716.
|
||||
The changes fix real and potential security-related issues, as well
|
||||
as minor quality-related issues.
|
||||
|
||||
Status of This Memo
|
||||
|
||||
This is an Internet Standards Track document.
|
||||
|
||||
This document is a product of the Internet Engineering Task Force
|
||||
(IETF). It represents the consensus of the IETF community. It has
|
||||
received public review and has been approved for publication by the
|
||||
Internet Engineering Steering Group (IESG). Further information on
|
||||
Internet Standards is available in Section 2 of RFC 7841.
|
||||
|
||||
Information about the current status of this document, any errata,
|
||||
and how to provide feedback on it may be obtained at
|
||||
https://www.rfc-editor.org/info/rfc8251.
|
||||
|
||||
Copyright Notice
|
||||
|
||||
Copyright (c) 2017 IETF Trust and the persons identified as the
|
||||
document authors. All rights reserved.
|
||||
|
||||
This document is subject to BCP 78 and the IETF Trust's Legal
|
||||
Provisions Relating to IETF Documents
|
||||
(https://trustee.ietf.org/license-info) in effect on the date of
|
||||
publication of this document. Please review these documents
|
||||
carefully, as they describe your rights and restrictions with respect
|
||||
to this document. Code Components extracted from this document must
|
||||
include Simplified BSD License text as described in Section 4.e of
|
||||
the Trust Legal Provisions and are provided without warranty as
|
||||
described in the Simplified BSD License.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 1]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
Table of Contents
|
||||
|
||||
1. Introduction . . . . . . . . . . . . . . . . . . . . . . . . 2
|
||||
2. Terminology . . . . . . . . . . . . . . . . . . . . . . . . . 3
|
||||
3. Stereo State Reset in SILK . . . . . . . . . . . . . . . . . 3
|
||||
4. Parsing of the Opus Packet Padding . . . . . . . . . . . . . 4
|
||||
5. Resampler Buffer . . . . . . . . . . . . . . . . . . . . . . 4
|
||||
6. Integer Wrap-Around in Inverse Gain Computation . . . . . . . 6
|
||||
7. Integer Wrap-Around in LSF Decoding . . . . . . . . . . . . . 7
|
||||
8. Cap on Band Energy . . . . . . . . . . . . . . . . . . . . . 7
|
||||
9. Hybrid Folding . . . . . . . . . . . . . . . . . . . . . . . 8
|
||||
10. Downmix to Mono . . . . . . . . . . . . . . . . . . . . . . . 9
|
||||
11. New Test Vectors . . . . . . . . . . . . . . . . . . . . . . 9
|
||||
12. Security Considerations . . . . . . . . . . . . . . . . . . . 11
|
||||
13. IANA Considerations . . . . . . . . . . . . . . . . . . . . . 11
|
||||
14. Normative References . . . . . . . . . . . . . . . . . . . . 11
|
||||
Acknowledgements . . . . . . . . . . . . . . . . . . . . . . . . 11
|
||||
Authors' Addresses . . . . . . . . . . . . . . . . . . . . . . . 12
|
||||
|
||||
1. Introduction
|
||||
|
||||
This document addresses minor issues that were discovered in the
|
||||
reference implementation of the Opus codec. Unlike most IETF
|
||||
specifications, RFC 6716 [RFC6716] defines Opus in terms of a
|
||||
normative reference decoder implementation rather than from the
|
||||
associated text description. Appendix A of that RFC includes the
|
||||
reference decoder implementation, which is why only issues affecting
|
||||
the decoder are listed here. An up-to-date implementation of the
|
||||
Opus encoder can be found at <https://opus-codec.org/>.
|
||||
|
||||
Some of the changes in this document update normative behavior in a
|
||||
way that requires new test vectors. Only the C implementation is
|
||||
affected, not the English text of the specification. This
|
||||
specification remains fully compatible with RFC 6716 [RFC6716].
|
||||
|
||||
Note: Due to RFC formatting conventions, lines exceeding the column
|
||||
width in the patch are split using a backslash character. The
|
||||
backslashes at the end of a line and the white space at the beginning
|
||||
of the following line are not part of the patch. Referenced line
|
||||
numbers are approximations. A properly formatted patch including all
|
||||
changes is available at <https://www.ietf.org/proceedings/98/slides/
|
||||
materials-98-codec-opus-update-00.patch> and has a SHA-1 hash of
|
||||
029e3aa88fc342c91e67a21e7bfbc9458661cd5f.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 2]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
2. Terminology
|
||||
|
||||
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
|
||||
"SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
|
||||
"OPTIONAL" in this document are to be interpreted as described in
|
||||
BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all
|
||||
capitals, as shown here.
|
||||
|
||||
3. Stereo State Reset in SILK
|
||||
|
||||
The reference implementation does not reinitialize the stereo state
|
||||
during a mode switch. The old stereo memory can produce a brief
|
||||
impulse (i.e., single sample) in the decoded audio. This can be
|
||||
fixed by changing silk/dec_API.c around line 72:
|
||||
|
||||
<CODE BEGINS>
|
||||
for( n = 0; n < DECODER_NUM_CHANNELS; n++ ) {
|
||||
ret = silk_init_decoder( &channel_state[ n ] );
|
||||
}
|
||||
+ silk_memset(&((silk_decoder *)decState)->sStereo, 0,
|
||||
+ sizeof(((silk_decoder *)decState)->sStereo));
|
||||
+ /* Not strictly needed, but it's cleaner that way */
|
||||
+ ((silk_decoder *)decState)->prev_decode_only_middle = 0;
|
||||
|
||||
return ret;
|
||||
}
|
||||
<CODE ENDS>
|
||||
|
||||
This change affects the normative output of the decoder, but the
|
||||
amount of change is within the tolerance and is too small to make the
|
||||
test vector check fail.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 3]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
4. Parsing of the Opus Packet Padding
|
||||
|
||||
It was discovered that some invalid packets of a very large size
|
||||
could trigger an out-of-bounds read in the Opus packet parsing code
|
||||
responsible for padding. This is due to an integer overflow if the
|
||||
signaled padding exceeds 2^31-1 bytes (the actual packet may be
|
||||
smaller). The code can be fixed by decrementing the (signed) len
|
||||
value, instead of incrementing a separate padding counter. This is
|
||||
done by applying the following changes around line 596 of
|
||||
src/opus_decoder.c:
|
||||
|
||||
<CODE BEGINS>
|
||||
/* Padding flag is bit 6 */
|
||||
if (ch&0x40)
|
||||
{
|
||||
- int padding=0;
|
||||
int p;
|
||||
do {
|
||||
if (len<=0)
|
||||
return OPUS_INVALID_PACKET;
|
||||
p = *data++;
|
||||
len--;
|
||||
- padding += p==255 ? 254: p;
|
||||
+ len -= p==255 ? 254: p;
|
||||
} while (p==255);
|
||||
- len -= padding;
|
||||
}
|
||||
<CODE ENDS>
|
||||
|
||||
This packet-parsing issue is limited to reading memory up to about 60
|
||||
KB beyond the compressed buffer. This can only be triggered by a
|
||||
compressed packet more than about 16 MB long, so it's not a problem
|
||||
for RTP. In theory, it could crash a file decoder (e.g., Opus in
|
||||
Ogg) if the memory just after the incoming packet is out of range,
|
||||
but our attempts to trigger such a crash in a production application
|
||||
built using an affected version of the Opus decoder failed.
|
||||
|
||||
5. Resampler Buffer
|
||||
|
||||
The SILK resampler had the following issues:
|
||||
|
||||
1. The calls to memcpy() were using sizeof(opus_int32), but the type
|
||||
of the local buffer was opus_int16.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 4]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
2. Because the size was wrong, this potentially allowed the source
|
||||
and destination regions of the memcpy() to overlap on the copy
|
||||
from "buf" to "buf". We believe that nSamplesIn (number of input
|
||||
samples) is at least fs_in_khZ (sampling rate in kHz), which is
|
||||
at least 8. Since RESAMPLER_ORDER_FIR_12 is only 8, that should
|
||||
not be a problem once the type size is fixed.
|
||||
|
||||
3. The size of the buffer used RESAMPLER_MAX_BATCH_SIZE_IN, but the
|
||||
data stored in it was actually twice the input batch size
|
||||
(nSamplesIn<<1).
|
||||
|
||||
The code can be fixed by applying the following changes around line
|
||||
78 of silk/resampler_private_IIR_FIR.c:
|
||||
|
||||
<CODE BEGINS>
|
||||
|
||||
)
|
||||
{
|
||||
silk_resampler_state_struct *S = \
|
||||
(silk_resampler_state_struct *)SS;
|
||||
opus_int32 nSamplesIn;
|
||||
opus_int32 max_index_Q16, index_increment_Q16;
|
||||
- opus_int16 buf[ RESAMPLER_MAX_BATCH_SIZE_IN + \
|
||||
RESAMPLER_ORDER_FIR_12 ];
|
||||
+ opus_int16 buf[ 2*RESAMPLER_MAX_BATCH_SIZE_IN + \
|
||||
RESAMPLER_ORDER_FIR_12 ];
|
||||
|
||||
/* Copy buffered samples to start of buffer */
|
||||
- silk_memcpy( buf, S->sFIR, RESAMPLER_ORDER_FIR_12 \
|
||||
* sizeof( opus_int32 ) );
|
||||
+ silk_memcpy( buf, S->sFIR, RESAMPLER_ORDER_FIR_12 \
|
||||
* sizeof( opus_int16 ) );
|
||||
|
||||
/* Iterate over blocks of frameSizeIn input samples */
|
||||
index_increment_Q16 = S->invRatio_Q16;
|
||||
while( 1 ) {
|
||||
nSamplesIn = silk_min( inLen, S->batchSize );
|
||||
|
||||
/* Upsample 2x */
|
||||
silk_resampler_private_up2_HQ( S->sIIR, &buf[ \
|
||||
RESAMPLER_ORDER_FIR_12 ], in, nSamplesIn );
|
||||
|
||||
max_index_Q16 = silk_LSHIFT32( nSamplesIn, 16 + 1 \
|
||||
); /* + 1 because 2x upsampling */
|
||||
out = silk_resampler_private_IIR_FIR_INTERPOL( out, \
|
||||
buf, max_index_Q16, index_increment_Q16 );
|
||||
in += nSamplesIn;
|
||||
inLen -= nSamplesIn;
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 5]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
if( inLen > 0 ) {
|
||||
/* More iterations to do; copy last part of \
|
||||
filtered signal to beginning of buffer */
|
||||
- silk_memcpy( buf, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
|
||||
+ silk_memmove( buf, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/* Copy last part of filtered signal to the state for \
|
||||
the next call */
|
||||
- silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
|
||||
+ silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], \
|
||||
RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
|
||||
}
|
||||
<CODE ENDS>
|
||||
|
||||
6. Integer Wrap-Around in Inverse Gain Computation
|
||||
|
||||
It was discovered through decoder fuzzing that some bitstreams could
|
||||
produce integer values exceeding 32 bits in
|
||||
LPC_inverse_pred_gain_QA(), causing a wrap-around. The C standard
|
||||
considers this behavior as undefined. The following patch around
|
||||
line 87 of silk/LPC_inv_pred_gain.c detects values that do not fit in
|
||||
a 32-bit integer and considers the corresponding filters unstable:
|
||||
|
||||
<CODE BEGINS>
|
||||
/* Update AR coefficient */
|
||||
for( n = 0; n < k; n++ ) {
|
||||
- tmp_QA = Aold_QA[ n ] - MUL32_FRAC_Q( \
|
||||
Aold_QA[ k - n - 1 ], rc_Q31, 31 );
|
||||
- Anew_QA[ n ] = MUL32_FRAC_Q( tmp_QA, rc_mult2 , mult2Q );
|
||||
+ opus_int64 tmp64;
|
||||
+ tmp_QA = silk_SUB_SAT32( Aold_QA[ n ], MUL32_FRAC_Q( \
|
||||
Aold_QA[ k - n - 1 ], rc_Q31, 31 ) );
|
||||
+ tmp64 = silk_RSHIFT_ROUND64( silk_SMULL( tmp_QA, \
|
||||
rc_mult2 ), mult2Q);
|
||||
+ if( tmp64 > silk_int32_MAX || tmp64 < silk_int32_MIN ) {
|
||||
+ return 0;
|
||||
+ }
|
||||
+ Anew_QA[ n ] = ( opus_int32 )tmp64;
|
||||
}
|
||||
<CODE ENDS>
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 6]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
7. Integer Wrap-Around in LSF Decoding
|
||||
|
||||
It was discovered -- also from decoder fuzzing -- that an integer
|
||||
wrap-around could occur when decoding bitstreams with extremely large
|
||||
values for the high Line Spectral Frequency (LSF) parameters. The
|
||||
end result of the wrap-around is an illegal read access on the stack,
|
||||
which the authors do not believe is exploitable but should
|
||||
nonetheless be fixed. The following patch around line 137 of silk/
|
||||
NLSF_stabilize.c prevents the problem:
|
||||
|
||||
<CODE BEGINS>
|
||||
/* Keep delta_min distance between the NLSFs */
|
||||
for( i = 1; i < L; i++ )
|
||||
- NLSF_Q15[i] = silk_max_int( NLSF_Q15[i], \
|
||||
NLSF_Q15[i-1] + NDeltaMin_Q15[i] );
|
||||
+ NLSF_Q15[i] = silk_max_int( NLSF_Q15[i], \
|
||||
silk_ADD_SAT16( NLSF_Q15[i-1], NDeltaMin_Q15[i] ) );
|
||||
|
||||
/* Last NLSF should be no higher than 1 - NDeltaMin[L] */
|
||||
<CODE ENDS>
|
||||
|
||||
8. Cap on Band Energy
|
||||
|
||||
On extreme bitstreams, it is possible for log-domain band energy
|
||||
levels to exceed the maximum single-precision floating point value
|
||||
once converted to a linear scale. This would later cause the decoded
|
||||
values to be NaN (not a number), possibly causing problems in the
|
||||
software using the PCM values. This can be avoided with the
|
||||
following patch around line 552 of celt/quant_bands.c:
|
||||
|
||||
<CODE BEGINS>
|
||||
{
|
||||
opus_val16 lg = ADD16(oldEBands[i+c*m->nbEBands],
|
||||
SHL16((opus_val16)eMeans[i],6));
|
||||
+ lg = MIN32(QCONST32(32.f, 16), lg);
|
||||
eBands[i+c*m->nbEBands] = PSHR32(celt_exp2(lg),4);
|
||||
}
|
||||
for (;i<m->nbEBands;i++)
|
||||
<CODE ENDS>
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 7]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
9. Hybrid Folding
|
||||
|
||||
When encoding in hybrid mode at low bitrate, we sometimes only have
|
||||
enough bits to code a single Constrained-Energy Lapped Transform
|
||||
(CELT) band (8 - 9.6 kHz). When that happens, the second band (CELT
|
||||
band 18, from 9.6 - 12 kHz) cannot use folding because it is wider
|
||||
than the amount already coded and falls back to white noise. Because
|
||||
it can also happen on transients (e.g., stops), it can cause audible
|
||||
pre-echo.
|
||||
|
||||
To address the issue, we change the folding behavior so that it is
|
||||
never forced to fall back to Linear Congruential Generator (LCG) due
|
||||
to the first band not containing enough coefficients to fold onto the
|
||||
second band. This is achieved by simply repeating part of the first
|
||||
band in the folding of the second band. This changes the code in
|
||||
celt/bands.c around line 1237:
|
||||
|
||||
<CODE BEGINS>
|
||||
b = 0;
|
||||
}
|
||||
|
||||
- if (resynth && M*eBands[i]-N >= M*eBands[start] && \
|
||||
(update_lowband || lowband_offset==0))
|
||||
+ if (resynth && (M*eBands[i]-N >= M*eBands[start] || \
|
||||
i==start+1) && (update_lowband || lowband_offset==0))
|
||||
lowband_offset = i;
|
||||
|
||||
+ if (i == start+1)
|
||||
+ {
|
||||
+ int n1, n2;
|
||||
+ int offset;
|
||||
+ n1 = M*(eBands[start+1]-eBands[start]);
|
||||
+ n2 = M*(eBands[start+2]-eBands[start+1]);
|
||||
+ offset = M*eBands[start];
|
||||
+ /* Duplicate enough of the first band folding data to \
|
||||
be able to fold the second band.
|
||||
+ Copies no data for CELT-only mode. */
|
||||
+ OPUS_COPY(&norm[offset+n1], &norm[offset+2*n1 - n2], n2-n1);
|
||||
+ if (C==2)
|
||||
+ OPUS_COPY(&norm2[offset+n1], &norm2[offset+2*n1 - n2], \
|
||||
n2-n1);
|
||||
+ }
|
||||
+
|
||||
tf_change = tf_res[i];
|
||||
if (i>=m->effEBands)
|
||||
{
|
||||
<CODE ENDS>
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 8]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
as well as around line 1260:
|
||||
|
||||
<CODE BEGINS>
|
||||
fold_start = lowband_offset;
|
||||
while(M*eBands[--fold_start] > effective_lowband);
|
||||
fold_end = lowband_offset-1;
|
||||
- while(M*eBands[++fold_end] < effective_lowband+N);
|
||||
+ while(++fold_end < i && M*eBands[fold_end] < \
|
||||
effective_lowband+N);
|
||||
x_cm = y_cm = 0;
|
||||
fold_i = fold_start; do {
|
||||
x_cm |= collapse_masks[fold_i*C+0];
|
||||
|
||||
<CODE ENDS>
|
||||
|
||||
The fix does not impact compatibility, because the improvement does
|
||||
not depend on the encoder doing anything special. There is also no
|
||||
reasonable way for an encoder to use the original behavior to improve
|
||||
quality over the proposed change.
|
||||
|
||||
10. Downmix to Mono
|
||||
|
||||
The last issue is not strictly a bug, but it is an issue that has
|
||||
been reported when downmixing an Opus decoded stream to mono, whether
|
||||
this is done inside the decoder or as a post-processing step on the
|
||||
stereo decoder output. Opus intensity stereo allows optionally
|
||||
coding the two channels 180 degrees out of phase on a per-band basis.
|
||||
This provides better stereo quality than forcing the two channels to
|
||||
be in phase, but when the output is downmixed to mono, the energy in
|
||||
the affected bands is canceled, sometimes resulting in audible
|
||||
artifacts.
|
||||
|
||||
As a work-around for this issue, the decoder MAY choose not to apply
|
||||
the 180-degree phase shift. This can be useful when downmixing to
|
||||
mono inside or outside of the decoder (e.g., requested explicitly
|
||||
from an API).
|
||||
|
||||
11. New Test Vectors
|
||||
|
||||
Changes in Sections 9 and 10 have sufficient impact on the test
|
||||
vectors to make them fail. For this reason, this document also
|
||||
updates the Opus test vectors. The new test vectors now include two
|
||||
decoded outputs for the same bitstream. The outputs with suffix 'm'
|
||||
do not apply the CELT 180-degree phase shift as allowed in
|
||||
Section 10, while the outputs without the suffix do. An
|
||||
implementation is compliant as long as it passes either set of
|
||||
vectors.
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 9]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
Any Opus implementation that passes either the original test vectors
|
||||
from RFC 6716 [RFC6716] or one of the new sets of test vectors is
|
||||
compliant with the Opus specification. However, newer
|
||||
implementations SHOULD be based on the new test vectors rather than
|
||||
the old ones.
|
||||
|
||||
The new test vectors are located at
|
||||
<https://www.ietf.org/proceedings/98/slides/materials-98-codec-opus-
|
||||
newvectors-00.tar.gz>. The SHA-1 hashes of the test vectors are:
|
||||
|
||||
e49b2862ceec7324790ed8019eb9744596d5be01 testvector01.bit
|
||||
b809795ae1bcd606049d76de4ad24236257135e0 testvector02.bit
|
||||
e0c4ecaeab44d35a2f5b6575cd996848e5ee2acc testvector03.bit
|
||||
a0f870cbe14ebb71fa9066ef3ee96e59c9a75187 testvector04.bit
|
||||
9b3d92b48b965dfe9edf7b8a85edd4309f8cf7c8 testvector05.bit
|
||||
28e66769ab17e17f72875283c14b19690cbc4e57 testvector06.bit
|
||||
bacf467be3215fc7ec288f29e2477de1192947a6 testvector07.bit
|
||||
ddbe08b688bbf934071f3893cd0030ce48dba12f testvector08.bit
|
||||
3932d9d61944dab1201645b8eeaad595d5705ecb testvector09.bit
|
||||
521eb2a1e0cc9c31b8b740673307c2d3b10c1900 testvector10.bit
|
||||
6bc8f3146fcb96450c901b16c3d464ccdf4d5d96 testvector11.bit
|
||||
338c3f1b4b97226bc60bc41038becbc6de06b28f testvector12.bit
|
||||
f5ef93884da6a814d311027918e9afc6f2e5c2c8 testvector01.dec
|
||||
48ac1ff1995250a756e1e17bd32acefa8cd2b820 testvector02.dec
|
||||
d15567e919db2d0e818727092c0af8dd9df23c95 testvector03.dec
|
||||
1249dd28f5bd1e39a66fd6d99449dca7a8316342 testvector04.dec
|
||||
b85675d81deef84a112c466cdff3b7aaa1d2fc76 testvector05.dec
|
||||
55f0b191e90bfa6f98b50d01a64b44255cb4813e testvector06.dec
|
||||
61e8b357ab090b1801eeb578a28a6ae935e25b7b testvector07.dec
|
||||
a58539ee5321453b2ddf4c0f2500e856b3966862 testvector08.dec
|
||||
bb96aad2cde188555862b7bbb3af6133851ef8f4 testvector09.dec
|
||||
1b6cdf0413ac9965b16184b1bea129b5c0b2a37a testvector10.dec
|
||||
b1fff72b74666e3027801b29dbc48b31f80dee0d testvector11.dec
|
||||
98e09bbafed329e341c3b4052e9c4ba5fc83f9b1 testvector12.dec
|
||||
1e7d984ea3fbb16ba998aea761f4893fbdb30157 testvector01m.dec
|
||||
48ac1ff1995250a756e1e17bd32acefa8cd2b820 testvector02m.dec
|
||||
d15567e919db2d0e818727092c0af8dd9df23c95 testvector03m.dec
|
||||
1249dd28f5bd1e39a66fd6d99449dca7a8316342 testvector04m.dec
|
||||
d70b0bad431e7d463bc3da49bd2d49f1c6d0a530 testvector05m.dec
|
||||
6ac1648c3174c95fada565161a6c78bdbe59c77d testvector06m.dec
|
||||
fc5e2f709693738324fb4c8bdc0dad6dda04e713 testvector07m.dec
|
||||
aad2ba397bf1b6a18e8e09b50e4b19627d479f00 testvector08m.dec
|
||||
6feb7a7b9d7cdc1383baf8d5739e2a514bd0ba08 testvector09m.dec
|
||||
1b6cdf0413ac9965b16184b1bea129b5c0b2a37a testvector10m.dec
|
||||
fd3d3a7b0dfbdab98d37ed9aa04b659b9fefbd18 testvector11m.dec
|
||||
98e09bbafed329e341c3b4052e9c4ba5fc83f9b1 testvector12m.dec
|
||||
|
||||
Note that the decoder input bitstream files (.bit) are unchanged.
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 10]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
12. Security Considerations
|
||||
|
||||
This document fixes two security issues reported on Opus that affect
|
||||
the reference implementation in RFC 6716 [RFC6716]: CVE-2013-0899
|
||||
<https://nvd.nist.gov/vuln/detail/CVE-2013-0899> and CVE-2017-0381
|
||||
<https://nvd.nist.gov/vuln/detail/CVE-2017-0381>. CVE-2013-0899
|
||||
theoretically could have caused an information leak. The leaked
|
||||
information would have gone through the decoder process before being
|
||||
accessible to the attacker. The update in Section 4 fixes this.
|
||||
CVE-2017-0381 could have resulted in a 16-bit out-of-bounds read from
|
||||
a fixed location. The update in Section 7 fixes this. Beyond the
|
||||
two fixed Common Vulnerabilities and Exposures (CVEs), this document
|
||||
adds no new security considerations beyond those in RFC 6716
|
||||
[RFC6716].
|
||||
|
||||
13. IANA Considerations
|
||||
|
||||
This document does not require any IANA actions.
|
||||
|
||||
14. Normative References
|
||||
|
||||
[RFC2119] Bradner, S., "Key words for use in RFCs to Indicate
|
||||
Requirement Levels", BCP 14, RFC 2119,
|
||||
DOI 10.17487/RFC2119, March 1997,
|
||||
<https://www.rfc-editor.org/info/rfc2119>.
|
||||
|
||||
[RFC6716] Valin, JM., Vos, K., and T. Terriberry, "Definition of the
|
||||
Opus Audio Codec", RFC 6716, DOI 10.17487/RFC6716,
|
||||
September 2012, <https://www.rfc-editor.org/info/rfc6716>.
|
||||
|
||||
[RFC8174] Leiba, B., "Ambiguity of Uppercase vs Lowercase in RFC
|
||||
2119 Key Words", BCP 14, RFC 8174, DOI 10.17487/RFC8174,
|
||||
May 2017, <https://www.rfc-editor.org/info/rfc8174>.
|
||||
|
||||
Acknowledgements
|
||||
|
||||
We would like to thank Juri Aedla for reporting the issue with the
|
||||
parsing of the Opus padding. Thanks to Felicia Lim for reporting the
|
||||
LSF integer overflow issue. Also, thanks to Tina le Grand, Jonathan
|
||||
Lennox, and Mark Harris for their feedback on this document.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 11]
|
||||
|
||||
RFC 8251 Opus Update October 2017
|
||||
|
||||
|
||||
Authors' Addresses
|
||||
|
||||
Jean-Marc Valin
|
||||
Mozilla Corporation
|
||||
331 E. Evelyn Avenue
|
||||
Mountain View, CA 94041
|
||||
United States of America
|
||||
|
||||
Phone: +1 650 903-0800
|
||||
Email: jmvalin@jmvalin.ca
|
||||
|
||||
|
||||
Koen Vos
|
||||
vocTone
|
||||
|
||||
Email: koenvos74@gmail.com
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Valin & Vos Standards Track [Page 12]
|
||||
|
||||
Loading…
Reference in New Issue
Block a user