[FFmpeg-devel] TrueHD track in EVO not playable/testable with ffplay

[Ramiro Polla]

On Thu, Jul 9, 2009 at 10:42 PM, Baptiste
Coudurier<baptiste.coudurier at gmail.com> wrote:
> On 07/09/2009 05:38 PM, Ramiro Polla wrote:
>> 2009/7/9 M?ns Rullg?rd<mans at mansr.com>:
>>> Michael Niedermayer<michaelni at gmx.at> ?writes:
>>>> On Wed, Jul 08, 2009 at 11:01:57PM -0700, Baptiste Coudurier wrote:
>>>>> On 07/08/2009 07:47 PM, Michael Niedermayer wrote:
>>>>>> my tired mind says we need a
>>>>>> coded_channels representing the number of channels stored in the
>>>>>> stream
>>>>>> and a seperate field that represents the decoder output channels
>>>>>
>>>>> I suggest to keep ->channels as the coded channels and to use another
>>>>> field
>>>>> like output_channels.
>>
>> Wouldn't this break API? Programs currently check avctx->channels to
>> know how much data there is. If we make them check
>> avctx->output_channels we'd need to bump major (and break something
>> else so people know they need to update and we hope they stumble upon
>> a place that explains this change too).
>
> It breaks the API anyway since libavformat sets ->channels and in this case
> must not set it. libavformat is libavcodec user too :)
>
> IMHO ->channels is more obvious to people and avoids changing demuxer and
> muxer code.
>
> Also there was no guarantee that ->channels was set to request_channels, MLP
> is a proof.
>
> ->output_channels is clear and obvious IMHO.
>
>>>> I wouldnt mind, if we wouldnt have
>>>> coded_width& ?coded_height in there
>>>
>>> This is different. ?Unless I am mistaken, coded_width and coded_height
>>> indicate the encoded width/height of a video including any padding
>>> (e.g. mod-16) required by the codec but never meant to be displayed.
>>>
>>> In the case at hand, we have a number of audio channels, all of which
>>> are meaningful, but which may be downmixed before output.
>>
>> By looking at the doxy in avcodec.h I don't think coded_ is the best too.
>>
>> I attached a patch to see if this is what you mean by adding a new
>> field (I named it coded_channels like michael suggested but I don't
>> really like the name as I pointed out).
>>
>> Now would this mean that all decoders should be changed to
>> "avctx->coded_channels = avctx->channels =<number of channels>"? Or
>> only the decoders that support downmixing
>
> Decoders supporting downmixing should set output_channels to what is output.

Let me see if I understood what you mean. In this case,
avctx->channels should be set to 6 in the parser, and output_channels
set to the downmixed numbed of channels in the decoder only
if(request_channels), otherwise it's left as 0. av_find_stream_info()
should read frames while(!channels || (request_channels &&
!output_channels)).

Then users are expected to always use output_channels if they set
request_channels, and channels otherwise. Or we could make
av_find_stream_info() always set output_channels at the end, and we
expect users to always use output_channels from now on.

Am I close?

Ramiro Polla