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

[Justin Ruggles]

James Darnley wrote:

> On 2 June 2010 11:03, Robert Swain <robert.swain at gmail.com> wrote:
>> 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.
>>
> 
> Where would you like me to put a new table with the correct input
> mapping?  Is vorbis_data.c, next to the existing _offset array okay?
> If the vorbis encoder ever supports more than 2 channels this would be
> a logical place to have it.
> 
> I didn't notice the incorrect mapping because I only have stereo headphones.


There are some patches on the issue tracker that might be useful.

https://roundup.ffmpeg.org/issue1325

This was only for 5.1 channels though.  The vorbis spec has been
modified since then to support up to 7.1 I think... or more?

-Justin