[FFmpeg-devel] [PATCH 1/3] lavfi: add haldclutsrc filter.

[Clément Bœsch]

On Sun, May 26, 2013 at 08:03:18PM +0000, Paul B Mahol wrote:
[...]
> > +static int haldclutsrc_config_props(AVFilterLink *outlink)
> > +{
> > +    AVFilterContext *ctx = outlink->src;
> > +    TestSourceContext *s = ctx->priv;
> > +
> > +    if (outlink->w || outlink->h) {
> > +        av_log(ctx, AV_LOG_ERROR, "output size must be configured with a
> > level option\n");
> > +        return AVERROR(EINVAL);
> > +    }
> 
> I find this ugly. If filter does not allow user to change w/h, options for
> changing w/h should not be available.
> 

I'll possibly change this to guess the level. Removing the size from the
common options will likely make things a bit ugly.

Also, after a talk with the author of the Hald CLUT, it was suggested to
add some padding at the bottom (or right) to add some extra information
ignored later (typically for a demonstration of the effect of the CLUT, so
users could watch the effect by opening the CLUT image himself)

> Also if something else sets w/h, it should be ignored/replaced.
> 
> > +    s->w = s->h = s->level * s->level * s->level;
> > +    return config_props(outlink);
> > +}
> > +
> > +static const AVFilterPad haldclutsrc_outputs[] = {
> > +    {
> > +        .name          = "default",
> > +        .type          = AVMEDIA_TYPE_VIDEO,
> > +        .request_frame = request_frame,
> > +        .config_props  = haldclutsrc_config_props,
> > +    },
> > +    {  NULL }
> > +};
> > +
> > +AVFilter avfilter_vsrc_haldclutsrc = {
> > +    .name            = "haldclutsrc",
> > +    .description     = NULL_IF_CONFIG_SMALL("Provide an identity Hald
> > CLUT."),
> > +    .priv_class      = &haldclutsrc_class,
> > +    .priv_size       = sizeof(TestSourceContext),
> > +    .init            = haldclutsrc_init,
> > +    .uninit          = uninit,
> > +    .query_formats   = haldclutsrc_query_formats,
> > +    .inputs          = NULL,
> > +    .outputs         = haldclutsrc_outputs,
> > +};
> > +#endif /* CONFIG_HALDCLUTSRC_FILTER */
> > +
> >  #if CONFIG_NULLSRC_FILTER
> >
> >  #define nullsrc_options options
> > --
> > 1.8.2.3
> 
> Rest lgtm.
> 
> I wonder if something similar could be done for yuvs.
> 

I don't think so. Interpolating with R, G and B works because they have
the same weight. I doubt you can do that easily with YUVs.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130526/559ac185/attachment.asc>