[FFmpeg-devel] [PATCH] lavfi: support unknown channel layouts.

[Nicolas George]

Le tridi 13 nivôse, an CCXXI, Stefano Sabatini a écrit :
> > -        if (!link->in_channel_layouts->nb_channel_layouts) {
> > +        if (link->in_channel_layouts->all_layouts) {
> >              av_log(link->src, AV_LOG_ERROR, "Cannot select channel layout for"
> >                     "the link between filters %s and %s.\n", link->src->name,
> >                     link->dst->name);
> Again I think a check (!all && count == 0) would be safer/more
> readable (see below).

I think you are misreading this part of the code. It is not complaining that
it has no choice because the list is empty (empty lists are not possible,
they would have been replaced by a convert filter), it is complaining that
it has too much choice: the list is infinite, there is absolutely nothing to
tell us the best format.

> I don't like this, to me all+1 = all. If I understand the logic
> correctly, AVFilterFormats now adopts the convenction all = none, and
> adopts one or the other interpretation according to the context (which
> calls for bug and confused code). Since AVFilterChannelLayouts allows
> to distinguish between all and none we should make the distinction
> effective.

You are (almost) right: this is inconsistent, the inconsistency (or API
abuse) was present in the original code, but that is no excuse since it is
being rewritten. I will rework that slightly.

(I wrote "almost" because you speak of empty lists: you should remember that
empty lists are not supported, they make no sense; it is only an
happenstance that the new AVFilterChannelLayouts can encode them.)

> I'll review the rest of the patch later, sorry for the many
> iterations.

This is a complex patch, it requires careful scrutiny. Thanks.

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/20130103/66feca29/attachment.asc>