[FFmpeg-devel] Correct libvorbis.c behaviour with >2 channels

[Robert Swain]

On 2 June 2010 09:19, Martin Storsj? <martin at martin.st> wrote:
> On Wed, 2 Jun 2010, Michael Niedermayer wrote:
>
>> On Fri, May 28, 2010 at 07:57:54PM +0200, James Darnley wrote:
>> > I fail hard at sending out correct patches. ?New one, git format-patch
>>
>> > From 1325bfbc5240cec90937f3d26f2663073050ccda Mon Sep 17 00:00:00 2001
>> > From: James Darnley <james.darnley at gmail.com>
>> > Date: Fri, 28 May 2010 14:36:53 +0200
>> > Subject: [PATCH 2/3] Fix libvorbis encoding with more than 2 channels
>> >
>> > ---
>> > ?libavcodec/Makefile ? ?| ? ?2 +-
>> > ?libavcodec/libvorbis.c | ? 12 +++++-------
>> > ?2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> if the bug and fix are confirmed then this should be ok
>
> The actual bug with more than one channel is obviously fixed by this, but
> this also remaps the channels, and I'm not really sure if this is the
> correct way.
>
> Robert, Alex, or anyone else that are familiar with mapping of channels in
> these kinds of codecs - care to give a comment on this?

I don't think this is correct. What you're trying to do is map from
wave format extensible order to Vorbis order. The first few channel
layouts match, coincidentally. But, when you get to 5.1 and beyond
there are problems. For 5.1 the input wave format extensible order
will be:

FL FR FC LFE RL RR

If you use the ff_vorbis_channel_layout_offsets table as you are:

{ 0, 2, 1, 5, 3, 4, },

which gives:

FL FC FR RR LFE RL

and what we actually want (Vorbis order for 5.1):

FL FC FR RL RR LFE

You don't need to re-apply the mapping but rather apply the inverse which is:

{ 0, 2, 1, 4, 5, 3, },

This can be constructed from the decoder mapping table by noting the
position of a channel number in the _offsets array[1]. Or we could
just have a table for encoding.

Best regards,
Rob

[1] For clarity of the example - 0 is in the 0th position, 1 is in the
2nd position, 2 is in the 1st position, 3 is in the 4th position, 4 is
in the 5th position, 5 is in the 3rd position.