[FFmpeg-devel] [PATCH 3/4] oggdec: add support for Opus codec.

[Gregory Maxwell]

On Wed, Jul 4, 2012 at 7:57 AM, Nicolas George
<nicolas.george at normalesup.org> wrote:
> +        /*st->codec->skip_data = AV_RL16(packet + 10);*/
> +        st->codec->sample_rate = AV_RL32(packet + 12);
> +        /*gain                 = AV_RL16(packet + 16);*/
> +        /*channel_map          = AV_RL8 (packet + 18);*/
> +        if (os->psize > OPUS_HEAD_SIZE) {

You MUST read the channel map field to determine if there is a map to
read, thats what its there for. Don't just guess based on the size.
If the specification was unclear on this please help me understand
where it threw you off so it can be fixed.

If you enforce any maximum header size at all it should be conditioned
on the major version = and the minor version = 0 or 1.  If it turns
out something important was omitted from the header it may be added in
a minor version 2 for example.  Reject headers with major version>0 is
probably a good idea.

Without reading the skip data you will not be able to get the length
right, and you will decode incorrectly with and offset potentially
annoying sounds at the start, you will also get the timestamps wrong.

Likewise if you do not read the gain field you will decode files
incorrectly and they may blast out at loud levels are be nearly
in-audible.

Thanks for working on this.

(I'll gladly do a bunch of hands on testing of this code as soon as
its in a state where it will pass the test-vectors[1]! :) )


[1] http://opus-codec.org/docs/#testvectors