[FFmpeg-soc] [RFC] Apply function to pixel filter: vf_applyfn

[Stefano Sabatini]

On date Sunday 2009-05-24 20:19:34 +0200, zeger at customsoft.nl encoded:
> Hi,
> 
> A lot of image operations like gamma adjustment, color inversion or
> contrast enhancement can be expressed as a simple function applied 
> to each pixel. I have written a YUV(A) only filter to apply these
> functions.
> 
> For example: color inversion becomes applyfn=255-Y:255-U:255-V.
> If the pow function would be available a gamma correction would 
> look like this: applyfn=255*pow(Y/255\,0.25):U:V

I believe pow *is* supported out of the box by the eval code.
 
> Zeger Knops
> Index: Makefile
> ===================================================================
> --- Makefile    (revision 4305)
> +++ Makefile    (working copy)
> @@ -14,6 +14,7 @@
>         parseutils.o \
>  
>  
> +OBJS-$(CONFIG_APPLYFN_FILTER)    += vf_applyfn.o
>  OBJS-$(CONFIG_CROP_FILTER)       += vf_crop.o
>  OBJS-$(CONFIG_DRAWBOX_FILTER)    += vf_drawbox.o
>  OBJS-$(CONFIG_FPS_FILTER)        += vf_fps.o
> Index: allfilters.c
> ===================================================================
> --- allfilters.c    (revision 4305)
> +++ allfilters.c    (working copy)
> @@ -34,6 +34,7 @@
>          return;
>      initialized = 1;
>  
> +    REGISTER_FILTER(APPLYFN,applyfn,vf);
>      REGISTER_FILTER(CROP,crop,vf);
>      REGISTER_FILTER(DRAWBOX,drawbox,vf);
>      REGISTER_FILTER(FIFO,fifo,vf);
> Index: vf_applyfn.c
> ===================================================================
> --- vf_applyfn.c    (revision 0)
> +++ vf_applyfn.c    (revision 0)
> @@ -0,0 +1,227 @@
> +/*
> + * applyfn filter
> + * copyright (c) 2009 Zeger Knops
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * Assume pow is available
> + * Gamma(g): applyfn=255*pow(Y/255\,g):U:V
> + *
> + * Assume the floor fn is available we can draw cartoons:
> + * applyfn=floor(Y/64)*64:floor(U/64)*64:floor(V/64)*64
> + *
> + * Assume the modulo fn is availabvle we can draw checkerboars
> + * applyfn=(modulo(floor(x/64)\,2)+modulo(floor(y/64)\,2))*Y
> + *
> + * B&W
> + * applyfn=Y:128:128
> + */

Docs and example go to doc/vfilters.texi.

> +#include "libavcodec/eval.h"
> +#include "avfilter.h"
> +#include "libavutil/avstring.h"
> +
> +enum PosOfValue {
> +    POV_PI,
> +    POV_E,
> +    POV_N,
> +    POV_W,
> +    POV_H,
> +    POV_x,
> +    POV_y,
> +    POV_Y,
> +    POV_U,
> +    POV_V,
> +    POV_A,
> +    POV_NULL
> +};

PosOfValue is not a good name, I believe:
enum VarName {
     ...
     VARS_NB
};

is better.

And a PTS var may be useful too.

> +static const char *applyfn_symbols[POV_NULL+1]={
                                              ^^
Why +1?

> +    "PI",
> +    "E",
> +    "N",         ///< frame number (starting at zero)
> +    "W",         ///< frame width
> +    "H",         ///< frame height
> +    "x",         ///< x position of current pixel
> +    "y",         ///< y position of current pixel
> +    "Y",         ///< Y value of current pixel
> +    "U",         ///< U value of current pixel
> +    "V",         ///< V value of current pixel
> +    "A",         ///< A value of current pixel
> +    NULL
> +};
>
> +typedef struct
> +{
> +    AVEvalExpr *value_Y, *value_U, *value_V, *value_A;
> +    int hsub, vsub;
> +    double values[POV_NULL+1];
> +} ApplyfnContext;
> +
> +static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> +{
> +        ApplyfnContext *applyfn = ctx->priv;
> +        char Y_expr[256], U_expr[256], V_expr[256], A_expr[256];
> +       const char *error;

Weird indent.

> +       applyfn->values[POV_PI      ] = M_PI;
> +       applyfn->values[POV_E       ] = M_E;
> +       applyfn->values[POV_N       ] = 0.0;
> +       applyfn->values[POV_NULL    ] = 0.0;
> +
> +       av_strlcpy(Y_expr, "Y", sizeof(Y_expr));
> +        av_strlcpy(U_expr, "U", sizeof(U_expr));
> +        av_strlcpy(V_expr, "V", sizeof(V_expr));
> +        av_strlcpy(A_expr, "A", sizeof(V_expr));
> +
> +       if (args)
> +           sscanf(args, "%255[^:]:%255[^:]:%255[^:]:%255[^:]", Y_expr, U_expr, V_expr, A_expr)

I believe here it would be nice to use av_set_options_string, check
vf_pad.c in the archive for an usage example.

> +
> +       applyfn->value_Y = ff_parse(Y_expr, applyfn_symbols,
> +                               NULL, NULL, NULL, NULL, &error);
> +       if (!applyfn->value_Y )
> +           av_log(ctx, AV_LOG_ERROR,
> +                  "init() cannot parse Y expression due to %s\n", error);
> +
> +        applyfn->value_U = ff_parse(U_expr, applyfn_symbols,
> +                                NULL, NULL, NULL, NULL, &error);
> +        if (!applyfn->value_U )
> +            av_log(ctx, AV_LOG_ERROR,
> +                   "init() cannot parse U expression due to %s\n", error);
> +
> +        applyfn->value_V = ff_parse(V_expr, applyfn_symbols,
> +                                NULL, NULL, NULL, NULL, &error);
> +        if (!applyfn->value_V )
> +            av_log(ctx, AV_LOG_ERROR,
> +                   "init() cannot parse V expression due to %s\n", error);
> +
> +        applyfn->value_A = ff_parse(A_expr, applyfn_symbols,
> +                                NULL, NULL, NULL, NULL, &error);
> +        if (!applyfn->value_A )
> +            av_log(ctx, AV_LOG_ERROR,
> +                   "init() cannot parse A expression due to %s\n", error);

This can be factorized (check again vf_pad.c to see how), maybe even a
macro may be used.

Also I don't like the name of the function (init() in this case) to be
put in the error message, kinda redundant.

> +   return !applyfn->value_Y || !applyfn->value_U || !applyfn->value_V || !applyfn->value_A;
> +}
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    avfilter_set_common_formats(ctx,
> +        avfilter_make_format_list(10,
> +                PIX_FMT_YUV444P,  PIX_FMT_YUV422P,  PIX_FMT_YUV420P,
> +                PIX_FMT_YUV411P,  PIX_FMT_YUV410P,
> +                PIX_FMT_YUVJ444P, PIX_FMT_YUVJ422P, PIX_FMT_YUVJ420P,
> +                PIX_FMT_YUV440P,  PIX_FMT_YUVJ440P,
> +                PIX_FMT_YUVA420P));
> +    return 0;
> +}
> +
> +static int config_props(AVFilterLink *link)
> +{
> +    ApplyfnContext *applyfn = link->dst->priv;
> +
> +    avcodec_get_chroma_sub_sample(link->format, &applyfn->hsub, &applyfn->vsub);
> +
> +    return 0;
> +}
> +
> +static void draw_slice(AVFilterLink *link, int y, int h)
> +{
> +    ApplyfnContext *applyfn = link->dst->priv;
> +    AVFilterPicRef *in  = link->cur_pic;
> +    AVFilterPicRef *out = link->dst->outputs[0]->outpic;
> +    uint8_t *inrow_0, *outrow_0, *alpharow, *outrow_1, *outrow_2, *inrow_1, *inrow_2;
> +    int i, j, i_sub, j_sub, outrow_1_off, outrow_2_off, inrow_1_off, inrow_2_off;
> +
> +    applyfn->values[POV_N] += 1.0;

Ouch, this is wrong, as draw_slice() is called for every slice(), and
you don't know how many slices there are. This update should be done
in start_frame() / end_frame() (same for W and H, since they're not
supposed to change between a slice and another one).

> +    applyfn->values[POV_W] = link->w;
> +    applyfn->values[POV_H] = h;
[...]

Regards and happy filtering.