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

[Nicolas George]

Le quartidi 14 nivôse, an CCXXI, Stefano Sabatini a écrit :
> > +                /* let the remaining computation run and get 0 */
> confusing: what's the point of computing score if you already knows it
> is 0?

It avoids gratuitous differences with the fork because of reindented or
moved code, and it makes the patch more readable IMHO.

> this could be commented. Also the heuristics seems somehow arbitrary
> (and I'd prefer the score to be computed in a separated function, but
> this is unrelated).

I mostly agree, but as you say, this is unrelated.

> given the logic, naming this as "set_channel_layout()" maybe more
> clear, but feel free to keep the current name for consistency with the
> other misnamed functions.

I changed it like that:

int ff_add_channel_layout(AVFilterChannelLayouts **l, uint64_t channel_layout)
{
    av_assert1(!(*l && (*l)->all_layouts));
    ADD_FORMAT(l, channel_layout, uint64_t, channel_layouts, nb_channel_layouts);
    return 0;   
}               

And changed the relevant calling site into that

            if (fmts->all_layouts) {
                /* Turn the infinite list into a singleton */
                fmts->all_layouts = fmts->all_counts  = 0;
                ff_add_channel_layout(&outlink->in_channel_layouts, fmt);
                break;
            }

> alternatively this could be named COUNT2LAYOUT

Changed to FF_COUNT2LAYOUT and FF_LAYOUT2COUNT.

> > + * Decode a channel count encoded as a channel layout.
> > + * Return 0 is the channel layout was a real one.
> is -> if?

Fixed.

> also:
> Return 0 is the channel layout was a real one, otherwise the number of
> channels | channels count.

I find it gratuitously redundant with the introductory sentence, and
therefore less readable.

Do you want the updated patch? The changes are pretty trivial since last
round.

Also, did you have time to look at the next patches in the series:

cd1a315 ffmpeg: support filtering of unknown channel layouts.
fb1c817 lavfi/af_anull: accept unknown channel layouts.
92bd405 lavfi/af_aresample: accept unknown channel layouts.
f4f05e8f lavfi/af_aformat: accept unknwon channel layouts.
da727c9 lavfi/buffersrc: accept unknown channel layouts.
d880dc6 lavfi/sink_buffer: accept unknown channel layouts.
309df73 lavfi: implement ff_query_formats_all().
bd00f66 lavfi: implement ff_all_channel_counts().
194aaa5 ffmpeg: add -guess_layout_max option.

They are much simpler, but some of them have design decisions in them.

Thanks for the review.

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/20130104/54e20ee4/attachment.asc>