[FFmpeg-devel] [PATCH] libavfilter: add vf_hue.c
Stefano Sabatini
stefasab at gmail.com
Sat Aug 11 01:20:45 CEST 2012
On date Friday 2012-08-10 22:49:06 +0200, Jérémy Tran encoded:
> This is a port of the MPlayer hue filter (libmpcodecs/vf_hue.c)
> ---
> doc/filters.texi | 12 +++
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/vf_hue.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 202 insertions(+)
> create mode 100644 libavfilter/vf_hue.c
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 18be723..ea8263d 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2187,6 +2187,18 @@ a float number which specifies chroma temporal strength, defaults to
> @var{luma_tmp}*@var{chroma_spatial}/@var{luma_spatial}
> @end table
>
> + at section hue
> +
> +Hue and saturation filter. This filter modifies the hue and/or the saturation
First sentence tells what the filter does, not what it is.
> +of the input.
> +
> + at table @option
> + at item hue
> +a float number that specifies the hue.
Range? Default value?
> + at item saturation
> +a float number that specifies the saturation.
Ditto.
Also missing description of the syntax (how to specify hue and
saturation?)
> + at end table
> +
> @section idet
>
> Interlaceing detect filter. This filter tries to detect if the input is
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 727ab4e..66b5d52 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -98,6 +98,7 @@ OBJS-$(CONFIG_FREI0R_FILTER) += vf_frei0r.o
> OBJS-$(CONFIG_GRADFUN_FILTER) += vf_gradfun.o
> OBJS-$(CONFIG_HFLIP_FILTER) += vf_hflip.o
> OBJS-$(CONFIG_HQDN3D_FILTER) += vf_hqdn3d.o
> +OBJS-$(CONFIG_HUE_FILTER) += vf_hue.o
> OBJS-$(CONFIG_IDET_FILTER) += vf_idet.o
> OBJS-$(CONFIG_LUT_FILTER) += vf_lut.o
> OBJS-$(CONFIG_LUTRGB_FILTER) += vf_lut.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 403383d..f097bcf 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -88,6 +88,7 @@ void avfilter_register_all(void)
> REGISTER_FILTER (GRADFUN, gradfun, vf);
> REGISTER_FILTER (HFLIP, hflip, vf);
> REGISTER_FILTER (HQDN3D, hqdn3d, vf);
> + REGISTER_FILTER (HUE, hue, vf);
> REGISTER_FILTER (IDET, idet, vf);
> REGISTER_FILTER (LUT, lut, vf);
> REGISTER_FILTER (LUTRGB, lutrgb, vf);
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> new file mode 100644
> index 0000000..a5bd635
> --- /dev/null
> +++ b/libavfilter/vf_hue.c
> @@ -0,0 +1,188 @@
> +/*
> + * Copyright (c) 2012 Michael Niedermayer
>From the MPlayer SVN log:
r11250 | michael | 2003-10-23 20:36:02 +0200 (Thu, 23 Oct 2003) | 2 lines
vf_hue
I'll retain the original year (and having ancient year is somewhat cooler...)
> + * 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
Apply a hue/saturation filter to the input video
> + * Ported from MPlayer libmpcodecs/vf_hue.c
> + */
> +
> +#include "libavutil/common.h"
> +#include "libavutil/imgutils.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/pixdesc.h"
> +
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +typedef struct {
> + float hue;
> + float saturation;
> + int hsub;
> + int vsub;
> + uint32_t sin;
> + uint32_t cos;
int32_t?
> +} 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 tend to prefer the use of named options, despite the fact of
breaking compatibility with mp=hue, because of the increased
robustness/flexibility/overall consistency.
What's the opinion of other devs?
Alternatively we could try to support both syntaxes (check how it is
done in buffersrc).
> +
> + if (h < 0 || h > 360 || s < 0 || s > 2) {
It could be useful to support negative h values as well. Also I wonder
if we should support angles expressed in radians (with named options
this could be done by using two distinct names h/H).
> + av_log(ctx, AV_LOG_ERROR,
> + "Invalid value for h:%0.1f or s:%0.1f\n",
> + h, s);
> + return AVERROR(EINVAL);
> + }
Adding different error messages for different errors is helpful
toward users, at a modest code complexity price.
> + /* 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_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;
> +}
> +
> +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;
> + /*
> + * Scale the value to the norm of the resulting (U,V) vector, that is
> + * the saturation.
> + * This will be useful in the process_chrominance function.
> + */
> + hue->sin = rint(sin(hue->hue) * (1 << 16) * hue->saturation);
> + hue->cos = rint(cos(hue->hue) * (1 << 16) * hue->saturation);
> +
> + 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
> + */
Move the comment inside the function, otherwise the reader will parse
it as function documentation.
> +static void process_chrominance(uint8_t *udst, uint8_t *vdst, const int dst_linesize,
> + uint8_t *usrc, uint8_t *vsrc, const int src_linesize,
> + int w, int h,
> + const uint32_t c, const uint32_t s)
maybe int32_t since the values may be negative
> +{
> + int i;
> + int u, v;
> + int new_u, new_v;
int32_t should be more meaningful.
> +
> + while (h--) {
> + for (i = 0; i < w; i++) {
> + /* Normalize the components from range [16;140] to [-112;112] */
> + u = usrc[i] - 128;
> + v = vsrc[i] - 128;
> + /*
> + * Apply the rotation of the vector : (c * u) - (s * v)
> + * (s * u) + (c * v)
> + * De-normalize the components (without forgetting to scale 128
> + * by << 16)
> + * Finally scale back the result by >> 16
> + */
> + new_u = ((c * u) - (s * v) + (1 << 15) + (128 << 16)) >> 16;
> + new_v = ((s * u) + (c * v) + (1 << 15) + (128 << 16)) >> 16;
> +
> + /* Prevent a potential overflow */
> + udst[i] = av_clip_uint8_c(new_u);
> + vdst[i] = av_clip_uint8_c(new_v);
> + }
> +
> + usrc += src_linesize;
> + vsrc += src_linesize;
> + udst += dst_linesize;
> + vdst += dst_linesize;
> + }
> +}
[...]
Rest looks fine.
Make sure that the new filter issues the same output as mp=hue (we
have several tools for that, compare output issued with showinfo,
md5/framecrc/framemd5 output formats, md5 protocol, possibly using
lavfi test sources).
--
FFmpeg = Fabulous & Fiendish Most Pure Erotic Game
More information about the ffmpeg-devel
mailing list