[FFmpeg-devel] [PATCH 2/6] lavu/channel_layout: implement unknown layouts.

[Nicolas George]

Le nonidi 9 frimaire, an CCXXI, Michael Niedermayer a écrit :
> IMHO we should decide what is the correct solution and work toward
> that.

I agree, but IMHO, the correct solution is pretty obvious, at least in its
rough lines: deprecate "uint64_t channel_layout" in favour of
"AVChannelSomething channel_something", with a data structure powerful
enough to express "5.1+mono" or "mono[lang=ja]+mono[lang=en]".

But until the deprecation is in full effect, i.e. until the next major bump,
we have to make sense of that "uint64_t channel_layout".

> IMHO the correct solution is to never trust the layout to tell the
> array size. And to just set it to 0 when its unknown.

Yes, but lavfi does not allow us to do that.

> I dont think this breaks ABI/API, an application that works within
> the API that |layout| == channel count will never insert things into
> the filter graph that dont conform to that and the filter framework
> from itself wont produce a mismatch

Once again, you forget lavfi. Consider the following application:

	open_filter(&sink, "movie=file.mp3:s=a, "
	            "aformat=sample_fmts=s16:sample_rates=48000");
	while (av_buffersink_read(sink, &ref) >= 0) {
	    channels = ???;
	    if (channels != old_channels)
		reconfigure_sound_output(16, 48000, channels);
	    output_sound(ref->data, ref->audio->nb_samples);
	}

If this application was written at any time between May and July 2012, then
it must have:

    channels = av_get_channel_layout_nb_channels(ref->audio->channel_layout);

(quite a mouthful), and lavfi must ensure compatibility until the next major
bump.

Therefore, this statement is true:

    At lavfi's outputs, |layout| MUST be equal to nb_channels.

And this will stay until the next major bump.

Now, other places clearly authorize layout = 0; well, some APIs accept NULL
and some do not, this is hardly now.

OTOH, "layout != 0 && |layout| != nb_channels" seems gratuitously
inconsistent and mostly useless.

> and it makes a lot more sense to me than setting a completely random
> layout that matches the channel count and substituting a bit of it
> toward a "this layout is not real and just for counting"

It is not completely random.

> Iam concerned if we go the path of such fake layouts they will leak
> into encoded files, into statistics output and possibly break code
> that doesnt expect such layouts ...

That is a concern. A address it as best as I can in patch 6/6.

The only other solution would be: "You can not handle audio with unknown or
too complex layout with lavfi 3.x, nor with ffmpeg command-line tool."

If we consider rushing a new solution, it may be acceptable.

Regards,

-- 
  Nicolas George
-------------- 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/20121129/b7db3ad9/attachment.asc>