[FFmpeg-devel] [PATCH] [SOCIS] [WIP] libavfilter: add vf_hue.c

Stefano Sabatini stefasab at gmail.com
Mon Aug 6 17:25:41 CEST 2012


On date Monday 2012-08-06 14:39:42 +0200, Jeremy Tran encoded:
> Hi,
> 
> Here is an updated patch.
> 
> Regards,
> 
> -- 
> Jérémy Tran
> ACU 2013
> EPITA GISTRE 2013

> From cd77dfda9cc188313d2bdea859b6d2e82fdeb774 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Je=CC=81re=CC=81my=20Tran?= <tran.jeremy.av at gmail.com>
> Date: Mon, 6 Aug 2012 14:19:18 +0200
> Subject: [PATCH] libavfilter: add vf_hue.c
> 
> This is a port of the MPlayer hue filter (libmpcodecs/vf_hue.c)
> ---
>  libavfilter/vf_hue.c |  190 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 190 insertions(+)
>  create mode 100644 libavfilter/vf_hue.c

Missing Makefile, allfilters.c, and documentation changes.
 
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> new file mode 100644
> index 0000000..0aaf601
> --- /dev/null
> +++ b/libavfilter/vf_hue.c
> @@ -0,0 +1,190 @@
> +/*
> + * Copyright (c) 2012 Michael Niedermayer
> + * Copyright (c) 2012 Jeremy Tran
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU 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.
> + */
> +
> +/**
> + * @file
> + * Apply a hue filter to the input video
> + * Ported from MPlayer libmpcodecs/vf_hue.c
> + */
> +

> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +#include "libavutil/common.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/pixdesc.h"

Nit: external headers includes, then internal

> +
> +typedef struct {
> +    float   hue;
> +    float   saturation;
> +    int     hsub;
> +    int     vsub;
> +} HueContext;
> +

> +static av_cold int init(AVFilterContext *ctx, const char *args)
> +{
> +    HueContext *hue = ctx->priv;
> +    float h, s;
> +
> +    if (args)
> +        sscanf(args, "%f:%f", &h, &s);

I don't know if we should use named options and support this syntax
for keeping backward compatibility with mp filter. In general I'd
prefer to drop syntax compatibility in favor of internal lavfi
consistency and absolute convenience.

> +    if (h < 0 || h > 360 || s < 0 || s > 2) {
> +        av_log(ctx, AV_LOG_ERROR,
> +               "Invalid value for h:%0.1f or s:%0.1f\n",
> +               h, s);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    /* Convert angle from degree to radian */
> +    hue->hue = h * M_PI / 180;
> +    hue->saturation = s;
> +
> +    return 0;
> +}
> +

> +static int query_formats(AVFilterContext *ctx)
> +{
> +    static const enum PixelFormat pix_fmts[] = {

> +        PIX_FMT_YUV420P16LE,  PIX_FMT_YUV420P16BE,
> +        PIX_FMT_YUV422P16LE,  PIX_FMT_YUV422P16BE,
> +        PIX_FMT_YUV444P16LE,  PIX_FMT_YUV444P16BE,

Did you check that these formats are supported (I don't think so given
the code in process_chrominance() works with 8-bits values)?

> +        PIX_FMT_YUV444P,      PIX_FMT_YUV422P,
> +        PIX_FMT_YUV420P,      PIX_FMT_YUV411P,
> +        PIX_FMT_YUV410P,      PIX_FMT_YUV440P,
> +        PIX_FMT_YUVA420P,
> +        PIX_FMT_NONE
> +    };
> +
> +    ff_set_common_formats(ctx, ff_make_format_list(pix_fmts));
> +
> +    return 0;
> +}
> +

> +/*
> + * If we consider U and V as the components of a 2D vector then its angle
> + * is the hue and the norm is the saturation
> + */
> +static void process_chrominance(uint8_t *udst, uint8_t *vdst, int dst_linesize,
> +                                uint8_t *usrc, uint8_t *vsrc, int src_linesize,
> +                                int w, int h,
> +                                float hue, float sat)
> +{
> +    int i;
> +    int u, v;
> +    int new_u, new_v;

The following code may benefit from a bit of exegesis.
Basically what is doing:

Compute sine and cosine of the hue angle, multiply for sat and scale
them for 1<<16:

> +    const int s = rint(sin(hue) * (1 << 16) * sat);
> +    const int c = rint(cos(hue) * (1 << 16) * sat);

int -> uint32_t seems more appropriate


> +    while (h--) {
> +        for (i = 0; i < w; i++) {

Convert u,v values to values in normalized values in the range [-112,
112]. These new values represent cosine and sine of the angle
representing the original values (u, v). Call this angle angle(u, v).

> +            u = usrc[i] - 128;
> +            v = vsrc[i] - 128;

Apply cosine and sine addition formulas for computing the sine and
cosine of the new angle obtained by adding hue and angle(u, v).

This is the part:
new_u = (c * u) - (s * v)
new_v = (s * u) + (c * v)

Note that these new values are already multiplied by sat, and are
scaled for 1<<16.

Then add back 128 (which need to be scaled as well), and finally get
back to the unscaled values range (the +(1<<15) approximates the
values to the best approximation in the unscaled values range).

> +            new_u = ((c * u) - (s * v) + (1 << 15) + (128 << 16)) >> 16;
> +            new_v = ((s * u) + (c * v) + (1 << 15) + (128 << 16)) >> 16;
> +

> +            if (new_u & 768)
> +                new_u = (-new_u) >> 31;
> +            if (new_v & 768)
> +                new_v = (-new_v) >> 31;

I'm not sure about this: these should handle overflow (right now you
have 16 bits values which need to be expressed in the 8-bits range).

Indeed what this seems to do is to convert the overflowing values to
0, but maybe I'm wrong.

Michael, can you explain?

> +
> +            udst[i] = new_u;
> +            vdst[i] = new_v;
> +        }
> +
> +        usrc += src_linesize;
> +        vsrc += src_linesize;
> +        udst += dst_linesize;
> +        vdst += dst_linesize;
> +    }
> +}
> +

> +static void copy_plane(uint8_t *dst, int dst_linesize,
> +                       uint8_t *src, int src_linesize,
> +                       int h, int w)
> +{
> +    int i, j;
> +
> +    for (i = 0; i < h; i++) {
> +        for (j = 0; j < w; j++)
> +            dst[j] = src[j];
> +        dst += dst_linesize;
> +        src += src_linesize;
> +    }
> +}

av_image_copy_plane()

> +
> +static int draw_slice(AVFilterLink *inlink, int y, int h, int slice_dir)
> +{
> +    HueContext *hue = inlink->dst->priv;
> +    AVFilterBufferRef *inpic = inlink->cur_buf;
> +    AVFilterBufferRef *outpic = inlink->dst->outputs[0]->out_buf;
> +    uint8_t *inrow[3], *outrow[3]; // 1 : Y, 2 : U, 3 : V
> +    int plane;
> +

> +    for (plane = 0; plane < 3; plane++) {
> +        inrow[plane] = inpic->data[plane] + (y >> hue->vsub) * inpic->linesize[plane];
> +        outrow[plane] = outpic->data[plane] + (y >> hue->vsub) * outpic->linesize[plane];
> +    }

        inrow [0] = inpic ->data[0] + y * inpic ->linesize[0];
        outrow[0] = outpic->data[0] + y * outpic->linesize[0];
        for (plane = 1; plane < 3; plane++) {
             inrow [plane] = inpic ->data[plane] + (y >> hue->vsub) * inpic ->linesize[plane];
             outrow[plane] = outpic->data[plane] + (y >> hue->vsub) * outpic->linesize[plane];
        }

Should be more correct/less confusing.

> +
> +    copy_plane(outrow[0], outpic->linesize[0],
> +               inrow[0], inpic->linesize[0],
> +               inlink->h,
> +               inlink->w);
> +
> +    process_chrominance(outrow[1], outrow[2], outpic->linesize[1],
> +                        inrow[1], inrow[2], inpic->linesize[1],
> +                        inlink->w >> hue->hsub, inlink->h >> hue->vsub,
> +                        hue->hue, hue->saturation);
> +
> +    return ff_draw_slice(inlink->dst->outputs[0], y, h, slice_dir);
> +}
> +

> +static int config_props(AVFilterLink *inlink)
> +{
> +    HueContext *hue = inlink->dst->priv;
> +    const AVPixFmtDescriptor *desc = &av_pix_fmt_descriptors[inlink->format];
> +
> +    hue->hsub = desc->log2_chroma_w;
> +    hue->vsub = desc->log2_chroma_h;
> +
> +    return 0;
> +}

Nit: move this just after query_formats()

[...]

Note: other possible extensions (to be implemented further patch, this
one is just a port):

- support for dynamic h/v expression evaluation
- support for reconfiguration events (check drawtext for an example,
  this is totally undocumented and we need to implement some easy way
  to test it, e.g. through an event auto-injecting filter).
-- 
FFmpeg = Faithless & Formidable Maxi Pitiless Extensive Gigant


More information about the ffmpeg-devel mailing list