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

[Michael Niedermayer]

On Sun, Oct 02, 2011 at 11:03:44PM +0200, Marton Balint wrote:
> 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.

merged, thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111003/9deef39b/attachment.asc>