[FFmpeg-devel] [PATCH] lavfi: add geq filter.

Stefano Sabatini stefasab at gmail.com
Sat Nov 10 18:16:38 CET 2012


On date Wednesday 2012-11-07 22:46:25 +0100, Clément Bœsch encoded:
> On Thu, Nov 01, 2012 at 08:20:13PM +0100, Stefano Sabatini wrote:
> [...]
> > > + at section geq
> > > +
> > > +The filter takes one, two or three equations as parameter, separated by ':'.
> > 
> > > +The first equation is mandatory and applies to the luma plane. The two
> > > +following are respectively for chroma blue and chroma red planes. If one
> > > +equation is not specified, it is using the same as the previous one.
> > 
> > named options?
> > 
> 
> Documentation extended.
> 
> > > +
> > > +The expressions can use the following variables:
> > 
> > ... and functions.
> > 
> 
> Added.
> 
> > > +
> > > + at table @option
> > > + at item X, Y
> > > +The coordinates of the current sample
> > 
> > nit: don't capitalize if it is not a complete sentence
> > 
> 
> There are sentences in that table so it looks weird to me to mix them…
> 
> > > +
> > > + at item W, H
> > > +The width and height of the image
> > > +
> > > + at item SW, SH
> > > +Width and height scale depending on the currently filtered plane, e.g. 1,1 and
> > > +0.5,0.5 for YUV 4:2:0.
> > 
> > "scale" is not defined, I have no idea what they mean from this
> > description.
> > 
> 
> This is taken from the MPlayer manual, what do you suggest? I have trouble
> wording it differently.

See below.

> > > +
> > > + at item p(x, y)
> > 
> > > +Return the value of the pixel at location (@var{x};@var{y}) of the current
> > > +plane.
> > 
> > nit: "(@var{x}, @var{y})" seems more mathematician-friendly, same below.
> > 
> 
> Changed.
> 
> > Also, what happens if the (x,y) value is not contained in the image?
> > 
> 
> Added a note about the clipping (and BTW, I had to change the original
> code to clip to w-2 and h-2 instead of w-1 and h-1 to avoid overreads).

Another idea would be to wrap around the values, but that would
involve a modulo operation so should be optional.

> > > +
> > > + at item lum(x, y)
> > > +Return the value of the pixel at location (@var{x};@var{y}) of the luminance
> > > +plane.
> > > +
> > > + at item cb(x, y)
> > > +Return the value of the pixel at location (@var{x};@var{y}) of the
> > > +blue-difference chroma plane.
> > > +
> > > + at item cr(x, y)
> > > +Return the value of the pixel at location (@var{x};@var{y}) of the
> > > +red-difference chroma plane.
> > 
> > Do you think would be possible to extend the filter to take the RGB
> > values, and in general support the RGB colorspace? Alternatively we
> > may have geqyuv and geqrgb. Also supporting alpha would be cool.
> > 
> 

> I'm not sure. It might get clumsy in the same filter, especially because
> of the different default expression fallbacks. A geqrgb is indeed a better
> idea, and if we happen to add it, we could add an alias geqyuv -> geq.

Yes it was what I thought as well.

> [...]
> > > +/**
> > > + * @file
> > > + * Generic equation change filter
> > > + * Originally written by Michael Niedermayer for the MPlayer project, and
> > > + * ported by Clément Bœsch for FFmpeg.
> > > + */
> > > +
> > 
> > > +#include "libavcodec/avcodec.h"
> > 
> > what's this good for?
> > 
> 
> A cursed copy/paste leftover, now removed.
> 
> > > +#include "libavutil/eval.h"
> > > +#include "libavutil/avstring.h"
> > > +#include "libavutil/pixdesc.h"
> > > +#include "internal.h"
> > > +#include "video.h"
> > > +
> > > +typedef struct {
> > > +    AVExpr *e[3];               ///< expressions for each plane
> > > +    int framenum;               ///< frame counter
> > > +    AVFilterBufferRef *picref;  ///< current input buffer
> > > +    int hsub, vsub;
> > > +} GEQContext;
> > > +
> > 
> > > +static inline double getpix(void *priv, double x, double y, int plane)
> > 
> > I suggest to avoid the reference to the private context, and pass
> > either the picref or data+linesize to get a more generic function.
> > 
> 
> It doesn't look like a good idea, I would have to pass [vh]sub as well.
> 
> > > +{
> > > +    int xi, yi;
> > > +    GEQContext *geq = priv;
> > > +    AVFilterBufferRef *picref = geq->picref;
> > > +    const uint8_t *src = picref->data[plane];
> > > +    const int linesize = picref->linesize[plane];
> > > +    const int w = picref->video->w >> (plane ? geq->hsub : 0);
> > > +    const int h = picref->video->h >> (plane ? geq->vsub : 0);
> > > +
> > > +    xi = x = FFMIN(FFMAX(x, 0), w - 1);
> > > +    yi = y = FFMIN(FFMAX(y, 0), h - 1);
> > > +
> > > +    x -= xi;
> > > +    y -= yi;
> > > +
> > > +    return (1-y)*((1-x)*src[xi +  yi    * linesize] + x*src[xi + 1 +  yi    * linesize])
> > > +          +   y *((1-x)*src[xi + (yi+1) * linesize] + x*src[xi + 1 + (yi+1) * linesize]);
> > > +}
> > > +
> > 
> > > +//TODO: cubic interpolate
> > > +//TODO: keep the last few frames
> > 
> > > +static double lum(void *priv, double x, double y) { return getpix(priv, x, y, 0); }
> > > +static double  cb(void *priv, double x, double y) { return getpix(priv, x, y, 1); }
> > > +static double  cr(void *priv, double x, double y) { return getpix(priv, x, y, 2); }
> > > +
> > > +static const char *const var_names[] = {   "X",   "Y",   "W",   "H",   "N",   "SW",   "SH",        NULL };
> > > +enum                                   { VAR_X, VAR_Y, VAR_W, VAR_H, VAR_N, VAR_SW, VAR_SH, VAR_VARS_NB };
> > > +
> > > +static av_cold int geq_init(AVFilterContext *ctx, const char *args)
> > > +{
> > > +    GEQContext *geq = ctx->priv;
> > > +    char *args1 = av_strdup(args);
> > > +    char *bufptr = NULL;
> > > +    char *arg = args1;
> > > +    const char *expr[3] = {0};
> > > +    int plane, ret = 0;
> > > +
> > > +    if (!args1) {
> > > +        ret = AVERROR(ENOMEM);
> > > +        goto end;
> > > +    }
> > > +
> > > +    for (plane = 0; plane < 3; plane++, arg = NULL) {
> > > +        static double (*p[])(void *, double, double) = { lum, cb, cr };
> > > +        static const char *const func2_names[]    = { "lum", "cb", "cr", "p", NULL };
> > > +        double (*func2[])(void *, double, double) = { lum, cb, cr, p[plane], NULL };
> > > +
> > > +        expr[plane] = av_strtok(arg, ":", &bufptr);
> > > +        if (!expr[plane]) {
> > > +            if (plane == 0) {
> > > +                av_log(ctx, AV_LOG_ERROR, "At least one expression must be specified\n");
> > > +                ret = AVERROR(EINVAL);
> > > +                goto end;
> > > +            }
> > > +            expr[plane] = expr[plane - 1];
> > > +        }
> > > +        ret = av_expr_parse(&geq->e[plane], expr[plane], var_names, NULL, NULL,
> > > +                            func2_names, func2, 0, ctx);
> > > +        if (ret < 0)
> > > +            goto end;
> > > +    }
> > 
> > av_opt_set_from_string() may help in many respects (more flexible
> > syntax, introspection, supported options from ffmpeg -h full, etc.
> > 
> 
> Good point, changed.
> 
> > > +
> > > +end:
> > > +    av_free(args1);
> > > +    return ret;
> > > +}
> > > +
> > > +static int geq_query_formats(AVFilterContext *ctx)
> > > +{
> > > +    static const enum PixelFormat pix_fmts[] = {
> > > +        AV_PIX_FMT_YUV444P,  AV_PIX_FMT_YUV422P,  AV_PIX_FMT_YUV420P,
> > > +        AV_PIX_FMT_YUV411P,  AV_PIX_FMT_YUV410P,  AV_PIX_FMT_YUV440P,
> > > +        AV_PIX_FMT_YUVA420P,
> > > +        AV_PIX_FMT_NONE
> > > +    };
> > > +    ff_set_common_formats(ctx, ff_make_format_list(pix_fmts));
> > > +    return 0;
> > > +}
> > > +
> > > +static int geq_config_props(AVFilterLink *inlink)
> > > +{
> > > +    GEQContext *geq = inlink->dst->priv;
> > > +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
> > > +
> > > +    geq->hsub = desc->log2_chroma_w;
> > > +    geq->vsub = desc->log2_chroma_h;
> > > +    return 0;
> > > +}
> > > +
> > > +static int geq_end_frame(AVFilterLink *inlink)
> > > +{
> > > +    int ret, plane;
> > > +    GEQContext *geq = inlink->dst->priv;
> > > +    AVFilterLink *outlink = inlink->dst->outputs[0];
> > > +    AVFilterBufferRef *outpicref = outlink->out_buf;
> > 
> > > +    double values[VAR_VARS_NB] = {
> > > +        [VAR_N] = geq->framenum++,
> > > +    };
> > 
> > shouldn't this be set in the context?
> > 
> 
> Why? The whole array change at each call.
> 
> > > +
> > > +    geq->picref = inlink->cur_buf;
> > > +
> > > +    for (plane = 0; plane < 3; plane++) {
> > > +        int x, y;
> > > +        uint8_t *dst = outpicref->data[plane];
> > > +        const int linesize = outpicref->linesize[plane];
> > > +        const int w = inlink->w >> (plane ? geq->hsub : 0);
> > > +        const int h = inlink->h >> (plane ? geq->vsub : 0);
> > > +
> > > +        values[VAR_W]  = w;
> > > +        values[VAR_H]  = h;
> > > +        values[VAR_SW] = w / (double)inlink->w;
> > > +        values[VAR_SH] = h / (double)inlink->h;
> > > +
> > > +        for (y = 0; y < h; y++) {
> > > +            values[VAR_Y] = y;
> > > +            for (x = 0; x < w; x++) {
> > > +                values[VAR_X] = x;
> > > +                dst[x] = av_expr_eval(geq->e[plane], values, geq);
> > > +            }
> > > +            dst += linesize;
> > > +        }
> > > +    }
> > > +
> > > +    if ((ret = ff_draw_slice(outlink, 0, outlink->h, 1)) < 0 ||
> > > +        (ret = ff_end_frame(outlink)) < 0)
> > > +        return ret;
> > > +    return 0;
> > > +}
> > > +
> > > +static av_cold void geq_uninit(AVFilterContext *ctx)
> > > +{
> > > +    int i;
> > > +    GEQContext *geq = ctx->priv;
> > > +
> > > +    for (i = 0; i < FF_ARRAY_ELEMS(geq->e); i++)
> > > +        av_expr_free(geq->e[i]);
> > > +}
> > > +
> > > +static int null_draw_slice(AVFilterLink *link, int y, int h, int slice_dir) { return 0; }
> > > +
> > 
> > > +static const AVFilterPad avfilter_vf_geq_inputs[] = {
> > 
> > you can drop the avfilter_vf_ prefix, since it is local, same below
> > 
> 
> It was for consistency, but I also prefer without this prefix so removed.
> 
> > > +    {
> > > +        .name         = "default",
> > > +        .type         = AVMEDIA_TYPE_VIDEO,
> > > +        .draw_slice   = null_draw_slice,
> > > +        .config_props = geq_config_props,
> > > +        .end_frame    = geq_end_frame,
> > > +        .min_perms    = AV_PERM_READ,
> > > +    },
> > > +    { NULL }
> > > +};
> > > +
> > > +static const AVFilterPad avfilter_vf_geq_outputs[] = {
> > > +    {
> > > +        .name = "default",
> > > +        .type = AVMEDIA_TYPE_VIDEO,
> > > +    },
> > > +    { NULL }
> > > +};
> > > +
> > > +AVFilter avfilter_vf_geq = {
> > > +    .name          = "geq",
> > 
> > > +    .description   = NULL_IF_CONFIG_SMALL("Generic equation change filter"),
> > 
> > I'll never be tired of saying, description is a complete sentence, so
> > it should sound like "Apply generic equation to each pixel.".
> > 
> 
> :)
> 
> Changed.
> 
> > [...]
> > 
> > Thanks for working on this :).
> 
> New patch attached.
> 
> -- 
> Clément B.

> From 0247dddd6f61d22a62eab062dc0875e9485587ef Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Mon, 14 May 2012 19:03:19 +0200
> Subject: [PATCH] lavfi: add geq filter.
> 
> TODO: bump minor
> ---
>  Changelog                |   1 +
>  LICENSE                  |   1 +
>  configure                |   1 +
>  doc/filters.texi         |  70 ++++++++++++++
>  libavfilter/Makefile     |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/vf_geq.c     | 237 +++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 312 insertions(+)
>  create mode 100644 libavfilter/vf_geq.c
> 
> diff --git a/Changelog b/Changelog
> index b6bff7b..6b9d302 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -21,6 +21,7 @@ version <next>:
>  - metadata (info chunk) support in CAF muxer
>  - field filter ported from libmpcodecs
>  - AVR demuxer
> +- geq filter ported from libmpcodecs
>  
>  
>  version 1.0:
> diff --git a/LICENSE b/LICENSE
> index f38fc16..a1204f4 100644
> --- a/LICENSE
> +++ b/LICENSE
> @@ -30,6 +30,7 @@ Specifically, the GPL parts of FFmpeg are
>      - vf_cropdetect.c
>      - vf_decimate.c
>      - vf_delogo.c
> +    - vf_geq.c
>      - vf_hqdn3d.c
>      - vf_hue.c
>      - vf_mp.c
> diff --git a/configure b/configure
> index ae11f5d..5802cb9 100755
> --- a/configure
> +++ b/configure
> @@ -1939,6 +1939,7 @@ frei0r_filter_deps="frei0r dlopen"
>  frei0r_filter_extralibs='$ldl'
>  frei0r_src_filter_deps="frei0r dlopen"
>  frei0r_src_filter_extralibs='$ldl'
> +geq_filter_deps="gpl"
>  hqdn3d_filter_deps="gpl"
>  hue_filter_deps="gpl"
>  movie_filter_deps="avcodec avformat"
> diff --git a/doc/filters.texi b/doc/filters.texi
> index eaf0f42..e77ec1d 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2364,6 +2364,76 @@ frei0r=perspective:0.2/0.2:0.8/0.2
>  For more information see:
>  @url{http://frei0r.dyne.org}
>  
> + at section geq
> +
> +The filter takes one, two or three equations as parameter, separated by ':'.
> +The first equation is mandatory and applies to the luma plane. The two
> +following are respectively for chroma blue and chroma red planes.
> +
> +The filter syntax allows named parameters:
> +
> + at table @option
> + at item lum_expr
> +the luminance expression
> + at item cb_expr
> +the chrominance blue expression
> + at item cr_expr
> +the chrominance red expression
> + at end table
> +

> +If one of the chrominance expression isn't defined, it fall backs on the other
> +one. If none of them are specified, they will evaluate the luminance
> +expression.

Nit: maybe just mention the named options, and suggest the alternative
shorthand, this way it is easier to update the docs in case of
additions.

Nit++: isn't => is not

> +
> +The expressions can use the following variables and functions:
> +

> + at table @option
> + at item X, Y
> +The coordinates of the current sample
> +
> + at item W, H
> +The width and height of the image
> +
> + at item SW, SH
> +Width and height scale depending on the currently filtered plane, e.g. 1,1 and
> +0.5,0.5 for YUV 4:2:0.

nit++: inconsistent final dot

Also "width and height scale" is not very clear/explicit at all. I'd
suggest something like:

Width and height scale depending on the currently filtered plane. It
is the ratio between the corresponding luma plane number of pixels and
the current plane ones. E.g. for YUV4:2:0 the values are 1,1 for the luma
plane, and 0.5,0.5 for chroma planes.

...

I also wonder how are they useful.

> +
> + at item p(x, y)
> +Return the value of the pixel at location (@var{x}, at var{y}) of the current
> +plane.
> +
> + at item lum(x, y)
> +Return the value of the pixel at location (@var{x}, at var{y}) of the luminance
> +plane.
> +
> + at item cb(x, y)
> +Return the value of the pixel at location (@var{x}, at var{y}) of the
> +blue-difference chroma plane.
> +
> + at item cr(x, y)
> +Return the value of the pixel at location (@var{x}, at var{y}) of the
> +red-difference chroma plane.
> + at end table
> +
> +For functions, if @var{x} and @var{y} are outside the area, the value will be
> +automatically clipped to the closer edge.
> +
> +Some examples follow:
> +
> + at itemize
> + at item
> +Flip the image horizontally:
> + at example
> +geq=p(W-X\,Y)
> + at end example
> +

> + at item
> +Fancy enigmatic moving light:

Nit++: Create/Generate fancy...

> + at example
> +color=s=256x256,geq=random(1)/hypot(X-cos(N*0.07)*W/2-W/2\,Y-sin(N*0.09)*H/2-H/2)^2*1000000*sin(N*0.02):128:128
> + at end example
> + at end itemize
> +
>  @section gradfun
>  
>  Fix the banding artifacts that are sometimes introduced into nearly flat
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 0ab0633..dfd5200 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -105,6 +105,7 @@ OBJS-$(CONFIG_FORMAT_FILTER)                 += vf_format.o
>  OBJS-$(CONFIG_FRAMESTEP_FILTER)              += vf_framestep.o
>  OBJS-$(CONFIG_FPS_FILTER)                    += vf_fps.o
>  OBJS-$(CONFIG_FREI0R_FILTER)                 += vf_frei0r.o
> +OBJS-$(CONFIG_GEQ_FILTER)                    += vf_geq.o
>  OBJS-$(CONFIG_GRADFUN_FILTER)                += vf_gradfun.o
>  OBJS-$(CONFIG_HFLIP_FILTER)                  += vf_hflip.o
>  OBJS-$(CONFIG_HQDN3D_FILTER)                 += vf_hqdn3d.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index fef40e9..88e8d68 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -97,6 +97,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER (FPS,         fps,         vf);
>      REGISTER_FILTER (FRAMESTEP,   framestep,   vf);
>      REGISTER_FILTER (FREI0R,      frei0r,      vf);
> +    REGISTER_FILTER (GEQ,         geq,         vf);
>      REGISTER_FILTER (GRADFUN,     gradfun,     vf);
>      REGISTER_FILTER (HFLIP,       hflip,       vf);
>      REGISTER_FILTER (HQDN3D,      hqdn3d,      vf);
> diff --git a/libavfilter/vf_geq.c b/libavfilter/vf_geq.c
> new file mode 100644
> index 0000000..e0d7716
> --- /dev/null
> +++ b/libavfilter/vf_geq.c
> @@ -0,0 +1,237 @@
> +/*
> + * Copyright (C) 2006 Michael Niedermayer <michaelni at gmx.at>
> + * Copyright (C) 2012 Clément Bœsch <ubitux at gmail.com>
> + *
> + * This file is part of MPlayer.

s/MPlayer/FFmpeg, here and below.

> + *
> + * MPlayer is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * MPlayer 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with MPlayer; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +/**
> + * @file
> + * Generic equation change filter
> + * Originally written by Michael Niedermayer for the MPlayer project, and
> + * ported by Clément Bœsch for FFmpeg.
> + */
> +
> +#include "libavutil/avstring.h"
> +#include "libavutil/eval.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "internal.h"
> +
> +typedef struct {
> +    const AVClass *class;
> +    AVExpr *e[3];               ///< expressions for each plane
> +    char *expr_str[3];          ///< expression strings for each plane
> +    int framenum;               ///< frame counter
> +    AVFilterBufferRef *picref;  ///< current input buffer
> +    int hsub, vsub;             ///< chroma subsampling
> +} GEQContext;
> +
> +#define OFFSET(x) offsetof(GEQContext, x)
> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
> +
> +static const AVOption geq_options[] = {
> +    { "lum_expr",   "set luminance expression",   OFFSET(expr_str),                   AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> +    { "cb_expr",    "set chroma blue expression", OFFSET(expr_str) +   sizeof(char*), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> +    { "cr_expr",    "set chroma red expression",  OFFSET(expr_str) + 2*sizeof(char*), AV_OPT_TYPE_STRING, {.str=NULL}, CHAR_MIN, CHAR_MAX, FLAGS },
> +    {NULL},
> +};
> +
> +AVFILTER_DEFINE_CLASS(geq);
> +
> +static inline double getpix(void *priv, double x, double y, int plane)
> +{
> +    int xi, yi;
> +    GEQContext *geq = priv;
> +    AVFilterBufferRef *picref = geq->picref;
> +    const uint8_t *src = picref->data[plane];
> +    const int linesize = picref->linesize[plane];
> +    const int w = picref->video->w >> (plane ? geq->hsub : 0);
> +    const int h = picref->video->h >> (plane ? geq->vsub : 0);
> +

> +    xi = x = av_clipf(x, 0, w - 2);
> +    yi = y = av_clipf(y, 0, h - 2);
> +
> +    x -= xi;
> +    y -= yi;
> +
> +    return (1-y)*((1-x)*src[xi +  yi    * linesize] + x*src[xi + 1 +  yi    * linesize])
> +          +   y *((1-x)*src[xi + (yi+1) * linesize] + x*src[xi + 1 + (yi+1) * linesize]);
> +}
> +
> +//TODO: cubic interpolate
> +//TODO: keep the last few frames
> +static double lum(void *priv, double x, double y) { return getpix(priv, x, y, 0); }
> +static double  cb(void *priv, double x, double y) { return getpix(priv, x, y, 1); }
> +static double  cr(void *priv, double x, double y) { return getpix(priv, x, y, 2); }
> +
> +static const char *const var_names[] = {   "X",   "Y",   "W",   "H",   "N",   "SW",   "SH",        NULL };
> +enum                                   { VAR_X, VAR_Y, VAR_W, VAR_H, VAR_N, VAR_SW, VAR_SH, VAR_VARS_NB };
> +
> +static av_cold int geq_init(AVFilterContext *ctx, const char *args)
> +{
> +    GEQContext *geq = ctx->priv;
> +    int plane, ret = 0;
> +    static const char *shorthand[] = { "lum_expr", "cb_expr", "cr_expr", NULL };
> +
> +    geq->class = &geq_class;
> +    av_opt_set_defaults(geq);
> +
> +    if ((ret = av_opt_set_from_string(geq, args, shorthand, "=", ":")) < 0)
> +        return ret;
> +
> +    if (!geq->expr_str[0]) {
> +        av_log(ctx, AV_LOG_ERROR, "Luminance expression is mandatory\n");
> +        ret = AVERROR(EINVAL);
> +        goto end;
> +    }
> +
> +    if (!geq->expr_str[1] && !geq->expr_str[2]) {
> +        /* No chroma at all: fallback on luma */
> +        geq->expr_str[1] = av_strdup(geq->expr_str[0]);
> +        geq->expr_str[2] = av_strdup(geq->expr_str[0]);
> +    } else {
> +        /* One chroma unspecified, fallback on the other */
> +        if (!geq->expr_str[1]) geq->expr_str[1] = av_strdup(geq->expr_str[2]);
> +        if (!geq->expr_str[2]) geq->expr_str[2] = av_strdup(geq->expr_str[1]);
> +    }
> +
> +    if (!geq->expr_str[1] || !geq->expr_str[2]) {
> +        ret = AVERROR(ENOMEM);
> +        goto end;
> +    }
> +
> +    for (plane = 0; plane < 3; plane++) {
> +        static double (*p[])(void *, double, double) = { lum, cb, cr };
> +        static const char *const func2_names[]    = { "lum", "cb", "cr", "p", NULL };
> +        double (*func2[])(void *, double, double) = { lum, cb, cr, p[plane], NULL };
> +
> +        ret = av_expr_parse(&geq->e[plane], geq->expr_str[plane], var_names,
> +                            NULL, NULL, func2_names, func2, 0, ctx);
> +        if (ret < 0)
> +            break;
> +    }
> +
> +end:
> +    av_opt_free(geq);
> +    return ret;
> +}
> +
> +static int geq_query_formats(AVFilterContext *ctx)
> +{
> +    static const enum PixelFormat pix_fmts[] = {
> +        AV_PIX_FMT_YUV444P,  AV_PIX_FMT_YUV422P,  AV_PIX_FMT_YUV420P,
> +        AV_PIX_FMT_YUV411P,  AV_PIX_FMT_YUV410P,  AV_PIX_FMT_YUV440P,
> +        AV_PIX_FMT_YUVA420P,
> +        AV_PIX_FMT_NONE
> +    };
> +    ff_set_common_formats(ctx, ff_make_format_list(pix_fmts));
> +    return 0;
> +}
> +
> +static int geq_config_props(AVFilterLink *inlink)
> +{
> +    GEQContext *geq = inlink->dst->priv;
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
> +
> +    geq->hsub = desc->log2_chroma_w;
> +    geq->vsub = desc->log2_chroma_h;
> +    return 0;
> +}
> +
> +static int geq_end_frame(AVFilterLink *inlink)
> +{
> +    int ret, plane;
> +    GEQContext *geq = inlink->dst->priv;
> +    AVFilterLink *outlink = inlink->dst->outputs[0];
> +    AVFilterBufferRef *outpicref = outlink->out_buf;
> +    double values[VAR_VARS_NB] = {
> +        [VAR_N] = geq->framenum++,
> +    };
> +
> +    geq->picref = inlink->cur_buf;
> +
> +    for (plane = 0; plane < 3; plane++) {
> +        int x, y;
> +        uint8_t *dst = outpicref->data[plane];
> +        const int linesize = outpicref->linesize[plane];
> +        const int w = inlink->w >> (plane ? geq->hsub : 0);
> +        const int h = inlink->h >> (plane ? geq->vsub : 0);
> +
> +        values[VAR_W]  = w;
> +        values[VAR_H]  = h;
> +        values[VAR_SW] = w / (double)inlink->w;
> +        values[VAR_SH] = h / (double)inlink->h;
> +
> +        for (y = 0; y < h; y++) {
> +            values[VAR_Y] = y;
> +            for (x = 0; x < w; x++) {
> +                values[VAR_X] = x;
> +                dst[x] = av_expr_eval(geq->e[plane], values, geq);

Unrelated, but a clipping mechanism (like in lut*) may be useful,
maybe a warning in case of NaN result.

> +            }
> +            dst += linesize;
> +        }
> +    }
> +
> +    if ((ret = ff_draw_slice(outlink, 0, outlink->h, 1)) < 0 ||
> +        (ret = ff_end_frame(outlink)) < 0)
> +        return ret;
> +    return 0;
> +}
> +
> +static av_cold void geq_uninit(AVFilterContext *ctx)
> +{
> +    int i;
> +    GEQContext *geq = ctx->priv;
> +
> +    for (i = 0; i < FF_ARRAY_ELEMS(geq->e); i++)
> +        av_expr_free(geq->e[i]);

av_opt_free()? I think it could be useful to keep the original
expressions in the context, for debugging reasons.

> +}
> +
> +static int null_draw_slice(AVFilterLink *link, int y, int h, int slice_dir) { return 0; }
> +
> +static const AVFilterPad geq_inputs[] = {
> +    {
> +        .name         = "default",
> +        .type         = AVMEDIA_TYPE_VIDEO,
> +        .draw_slice   = null_draw_slice,
> +        .config_props = geq_config_props,
> +        .end_frame    = geq_end_frame,
> +        .min_perms    = AV_PERM_READ,
> +    },
> +    { NULL }
> +};
> +
> +static const AVFilterPad geq_outputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_VIDEO,
> +    },
> +    { NULL }
> +};
> +
> +AVFilter avfilter_vf_geq = {
> +    .name          = "geq",
> +    .description   = NULL_IF_CONFIG_SMALL("Apply generic equation to each pixel."),
> +    .priv_size     = sizeof(GEQContext),
> +    .init          = geq_init,
> +    .uninit        = geq_uninit,
> +    .query_formats = geq_query_formats,
> +    .inputs        = geq_inputs,
> +    .outputs       = geq_outputs,
> +    .priv_class    = &geq_class,
> +};

LGTM otherwise.
-- 
FFmpeg = Friendly & Fundamental Mystic Pacific Exxagerate Game


More information about the ffmpeg-devel mailing list