[FFmpeg-devel] [PATCH] lavc/ac3: set channel_layout when downmixing.

[Michael Niedermayer]

On Sat, Aug 10, 2013 at 12:50:05PM +0200, Nicolas George wrote:
> Le decadi 20 thermidor, an CCXXI, Michael Niedermayer a écrit :
> > > @@ -90,6 +90,7 @@ get_next:
> > >                  (avctx->codec_id == AV_CODEC_ID_AC3 ||
> > >                   avctx->codec_id == AV_CODEC_ID_EAC3)))) {
> > >              avctx->channels = avctx->request_channels;
> > > +            avctx->channel_layout = av_get_default_channel_layout(avctx->channels);
> > >          } else {
> > >              avctx->channels = s->channels;
> > >              avctx->channel_layout = s->channel_layout;
> > 
> > iam not sure if setting this from the parser is completely safe
> > the parser could even be used with a binary decoder which might behave
> > differently
> 
> I do not understand in what case it can be unsafe. As you can see, the
> parser sets avctx->channels in both branches (request_channels set or not),
> and it sets avctx->channel_layout in one of the branches. If three corners
> of the square are safe, how could the fourth not be?

maybe the 3 are not safe
the parser is run from the demuxer, and demuxer and decoder can be run
in seperate threads
if the parser sets values like channels just once or just when they
where not set previously then this should work out saftely (because
the decoder cannot run before the demuxer has returned something)

and of course as long as decoder & parser always set the same value
its fine too but with demuxer -> fifo-> decoder that would
implicate that they can never change

also in practice its not so big problem because the decoder
outputs these values in AVFrames and these should be used
and probably almost all applications dont share the AVCodecContext
between layers as this is generally inconvenient for applications
that are not based on ffmpegs libs.
Further with multithreaded decoding the AVframe values are more
reliable too
so the problem really is limited to applications that use both
libavcodec and libavformat
that share the context between them
that use these variables instead of AVFrame variables
that do run demuxer and decoder  with a fifo
between them or use decoders that set different values per frame than
parsers

So maybe we could get away with just documenting the issue and
recommanding applications to use AVFrame fields when they do all of
the above, i dont know.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20130810/c21feeaa/attachment.asc>