[FFmpeg-devel] ffplay: fix order of operations for sdl audio open

[Marton Balint]

On Sun, 2 Oct 2011, Michael Niedermayer wrote:
> On Sun, Oct 02, 2011 at 12:30:41AM +0200, Marton Balint wrote:
>> On Sat, 1 Oct 2011, Michael Niedermayer wrote:
>>
>>> On Sat, Oct 01, 2011 at 09:41:00PM +0200, Marton Balint wrote:
>>>> Hi,
>>>>
>>>> The patch in the subject (3715e675) seems broken in some ways:
>>>>
>>>> - The sample rate / channel count sanity check is not against the
>>>>   actually used sample rate / channel count.
>>>>
>>>> - Avctx->request_channels can affect the number of the channels,
>>>>   therefore downmixing of 5.1 audio to stereo (default for ffplay)
>>>>   currently does not work because of the patch.
>>>>
>>>> Isn't the aac decoder should be fixed instead of ffplay to return
>>>> the proper number of channels after calling avcodec_open2?
>>>
>>> The aac decoder in this bug has incorrect extradata that lists a wrong
>>> channel number, i saw no way it could detect this easily.
>>>
>>> my reasoning that lead to the commited fix was that av_find_stream_info()
>>> figured out the correct channel number by decoding the first frame
>>> and the requent_channel value should be passed to it.
>>
>> How request_channels should be passed to av_find_stream_info? Via options?
>
> see patch below, feel free to apply to your branch if you think its
> usefull
> once you confirm its all working ill merge it from there

Thanks, I applied it along with a few other patches.

>>> If this is done it should be more reliable than trusting the decoder
>>> before it saw the first frame
>>>
>>> if you want to try or have a better idea:
>>> rtsp://media2.lsops.net/live/aljazeer_en_high.sdp
>>
>> Perhaps another approach is if we apply my audio patch, which is
>> capable of downmixing or multiplying the audio channels as they
>> change during decoding, and then set the number of SDL channels to
>> 2.
>
> Please see libswresample/swresample.h
> its simpler & can handle many more channel layouts, planar audio, ...
> also its the codebase on which future work will be done.

I reworked my patch to use libswresample. Since a lot of codecs does not 
set the channel_layout at the moment, I had to create a function which 
returns a default channel layout based on the number of channels.

> one distinctive disadvantage of not using request_channels is though
> that decoders can sometimes return 2 channels with less cpu than
> returning all and than seperately downmixing

For now, I kept the request_channels 2 setting in the code.

>>
>> I set up a github repository, you can find the patches there:
>>
>> https://github.com/cus/ffplay.git
>
> great, i will merge it when you say its ready
>

Ready as it can be.

Regards,
   Marton