[FFmpeg-devel] [PATCH] 8 channel layout fix for Vorbis decoder

[Justin Ruggles]

Michael Niedermayer wrote:

> On Sat, Nov 07, 2009 at 11:08:26AM -0500, Justin Ruggles wrote:
>> Michael Niedermayer wrote:
>>
>>> On Tue, Nov 03, 2009 at 07:39:51PM -0500, Jason Tackaberry wrote:
>>>> Hi,
>>>>
>>>> The attached patch implements sane channel layout behaviour for 7 and 8
>>>> channel Vorbis files.
>>>>
>>>> The Vorbis specification [1] says that the layout for greater than six
>>>> channels is left up to the application.  I think the most sensible and
>>>> consistent layout for 7 and 8 channels is their 5 and 6 channel
>>>> counterparts with back left/right channels appended.
>>> ahh, xiph-alzheimer design again
>>> from all options they succeed in selecting the one that is most different
>>> from everyone else no matter how broken the option is even on its own
>>>
>>>
>>> so what are our options
>>> 1. leave channels as they are ordered
>>>    -> requires each application to have a special case for vorbis because
>>>       this order is never what one expects
>>> 2. do a best effort reordering 
>>>    -> someone will complain that its not correct for his file (that he just
>>>       created)
>>>       and his application could magically do better if only the evil
>>>       ffmpeg didnt reorder channels
>>>
>>> solution:
>>> add a value to request_channel_layout that specifies raw channel order and
>>> layout without any changes and then do the reorder as you do in your patch
>>> if this is not requested.
>>> comments? suggestions? flames?
>> I like it.  So value to use?
>>
>> 0x80000000 already means "all layouts supported" since we're using
> 
> is that documented in the appropriate header?
> btw, what does "all layouts supported" mean for request_channel_layout ?

We do not have it defined or documented.

>> Microsoft's dwChannelMask values (for the low 32 bits at least).  So
>> maybe 0x100000000LL or 0x8000000000000000LL?

We're using Microsoft's channel mask values, but I guess we could
deviate from this if we want since we have not committed to it in the
API documentation.  It just might be confusing to some if we use
dwChannelMask values for the speaker locations but define our own
special-case values that overlap with Microsoft's definition.

-Justin