[FFmpeg-devel] [PATCH 1/2] libavfilter: add colormatrix filter

Clément Bœsch ubitux at gmail.com
Sat Mar 10 13:01:14 CET 2012


On Sat, Mar 10, 2012 at 08:26:06AM +0100, Michael Niedermayer wrote:
> From: multiple authors <multiple at multiple.x>
> 
> Ported by: Baptiste Coudurier
> For detailed authorship of the original code please see avisynth
> ---
>  libavfilter/Makefile         |    1 +
>  libavfilter/allfilters.c     |    1 +
>  libavfilter/vf_colormatrix.c |  406 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 408 insertions(+), 0 deletions(-)
>  create mode 100644 libavfilter/vf_colormatrix.c
> 

I can't comment much on the colorspace stuff, so here is a nit & overall
review.

> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index e9c9a4b..3645d32 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -51,6 +51,7 @@ OBJS-$(CONFIG_ASS_FILTER)                    += vf_ass.o
>  OBJS-$(CONFIG_BLACKDETECT_FILTER)            += vf_blackdetect.o
>  OBJS-$(CONFIG_BLACKFRAME_FILTER)             += vf_blackframe.o
>  OBJS-$(CONFIG_BOXBLUR_FILTER)                += vf_boxblur.o
> +OBJS-$(CONFIG_COLORMATRIX_FILTER)            += vf_colormatrix.o
>  OBJS-$(CONFIG_COPY_FILTER)                   += vf_copy.o
>  OBJS-$(CONFIG_CROP_FILTER)                   += vf_crop.o
>  OBJS-$(CONFIG_CROPDETECT_FILTER)             += vf_cropdetect.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index f0a7f8b..b35822c 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -59,6 +59,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER (BLACKDETECT, blackdetect, vf);
>      REGISTER_FILTER (BLACKFRAME,  blackframe,  vf);
>      REGISTER_FILTER (BOXBLUR,     boxblur,     vf);
> +    REGISTER_FILTER (COLORMATRIX, colormatrix, vf);
>      REGISTER_FILTER (COPY,        copy,        vf);
>      REGISTER_FILTER (CROP,        crop,        vf);
>      REGISTER_FILTER (CROPDETECT,  cropdetect,  vf);
> diff --git a/libavfilter/vf_colormatrix.c b/libavfilter/vf_colormatrix.c
> new file mode 100644
> index 0000000..0f19189
> --- /dev/null
> +++ b/libavfilter/vf_colormatrix.c
> @@ -0,0 +1,406 @@
> +/*
> + * ColorMatrix v2.2 for Avisynth 2.5.x
> + *
> + * ColorMatrix 2.0 is based on the original ColorMatrix filter by Wilbert
> + * khof.  It adds the ability to convert between any of: Rec.709, FCC,
> + * .601, and SMPTE 240M. It also makes pre and post clipping optional,
> + * s an option to use scaled or non-scaled coefficients, and more...
> + *

Shoudln't this be moved to a @file doxy below?

> + * Copyright (C) 2006-2007 Kevin Stone
> + *
> + * ColorMatrix 1.x is Copyright (C) Wilbert Dijkhof
> + *
> + * This program 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.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * OUT 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 this program; if not, write to the Free Software Foundation,
> + * Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

GPL, so we may need to add a dep in the configure?

> + */
> +
> +#include <strings.h>
> +#include <float.h>
> +#include "avfilter.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/avstring.h"
> +
> +#define NS(n) n < 0 ? (int)(n*65536.0-0.5+DBL_EPSILON) : (int)(n*65536.0+0.5)

No use of epsilon for the uprounding?

> +#define CB(n) av_clip_uint8(n)
> +
> +static const double yuv_coeff[4][3][3] = {
> +    { { +0.7152, +0.0722, +0.2126 }, // Rec.709 (0)
> +      { -0.3850, +0.5000, -0.1150 },
> +      { -0.4540, -0.0460, +0.5000 } },
> +    { { +0.5900, +0.1100, +0.3000 }, // FCC (1)
> +      { -0.3310, +0.5000, -0.1690 },
> +      { -0.4210, -0.0790, +0.5000 } },
> +    { { +0.5870, +0.1140, +0.2990 }, // Rec.601 (ITU-R BT.470-2/SMPTE 170M) (2)
> +      { -0.3313, +0.5000, -0.1687 },
> +      { -0.4187, -0.0813, +0.5000 } },
> +    { { +0.7010, +0.0870, +0.2120 }, // SMPTE 240M (3)
> +      { -0.3840, +0.5000, -0.1160 },
> +      { -0.4450, -0.0550, +0.5000 } },
> +};
> +
> +typedef struct {
> +    int yuv_convert[16][3][3];
> +    int interlaced;
> +    int source, dest, mode;
> +    char src[256];
> +    char dst[256];
> +    int hsub, vsub;
> +} ColorMatrixContext;
> +
> +#define ma m[0][0]
> +#define mb m[0][1]
> +#define mc m[0][2]
> +#define md m[1][0]
> +#define me m[1][1]
> +#define mf m[1][2]
> +#define mg m[2][0]
> +#define mh m[2][1]
> +#define mi m[2][2]
> +
> +#define ima im[0][0]
> +#define imb im[0][1]
> +#define imc im[0][2]
> +#define imd im[1][0]
> +#define ime im[1][1]
> +#define imf im[1][2]
> +#define img im[2][0]
> +#define imh im[2][1]
> +#define imi im[2][2]
> +
> +static void inverse3x3(double im[3][3], const double m[3][3])
> +{
> +    double det = ma * (me * mi - mf * mh) - mb * (md * mi - mf * mg) + mc * (md * mh - me * mg);
> +    det = 1.0 / det;
> +    ima = det * (me * mi - mf * mh);
> +    imb = det * (mc * mh - mb * mi);
> +    imc = det * (mb * mf - mc * me);
> +    imd = det * (mf * mg - md * mi);
> +    ime = det * (ma * mi - mc * mg);
> +    imf = det * (mc * md - ma * mf);
> +    img = det * (md * mh - me * mg);
> +    imh = det * (mb * mg - ma * mh);
> +    imi = det * (ma * me - mb * md);
> +}
> +
> +static void solve_coefficients(double cm[3][3], double rgb[3][3], const double yuv[3][3])
> +{
> +    int i, j;
> +    for (i = 0; i < 3; ++i)
> +        for (j = 0; j < 3; ++j)
> +            cm[i][j] = yuv[i][0] * rgb[0][j] + yuv[i][1] * rgb[1][j] + yuv[i][2] * rgb[2][j];
> +}
> +
> +static void calc_coefficients(AVFilterContext *ctx)
> +{
> +    ColorMatrixContext *color = ctx->priv;
> +    double rgb_coeffd[4][3][3];
> +    double yuv_convertd[16][3][3];
> +    int v = 0;
> +    int i, j, k;
> +
> +    for (i = 0; i < 4; ++i)
> +        inverse3x3(rgb_coeffd[i], yuv_coeff[i]);
> +    for (i = 0; i < 4; ++i) {
> +        for (j = 0; j < 4; ++j) {
> +            solve_coefficients(yuv_convertd[v], rgb_coeffd[i], yuv_coeff[j]);
> +            for (k = 0; k < 3; ++k) {
> +                color->yuv_convert[v][k][0] = NS(yuv_convertd[v][k][0]);
> +                color->yuv_convert[v][k][1] = NS(yuv_convertd[v][k][1]);
> +                color->yuv_convert[v][k][2] = NS(yuv_convertd[v][k][2]);
> +            }
> +            if (color->yuv_convert[v][0][0] != 65536 || color->yuv_convert[v][1][0] != 0 ||
> +                color->yuv_convert[v][2][0] != 0) {
> +                av_log(ctx, AV_LOG_ERROR, "error calculating conversion coefficients\n");
> +            }
> +            ++v;

nit: these pre-increment are meaningless in C

> +        }
> +    }
> +}
> +
> +static void check_mode(ColorMatrixContext *color)
> +{
> +    color->source = color->dest = -1;
> +    if (!av_strcasecmp(color->src, "bt709") && !av_strcasecmp(color->dst, "FCC")) {
> +        color->source = 0;
> +        color->dest = 1;
> +    } else if (!av_strcasecmp(color->src, "bt709") && !av_strcasecmp(color->dst, "bt601")) {
> +        color->source = 0;
> +        color->dest = 2;
> +    } else if (!av_strcasecmp(color->src, "bt709") && !av_strcasecmp(color->dst, "smpte240m")) {
> +        color->source = 0;
> +        color->dest = 3;
> +    } else if (!av_strcasecmp(color->src, "FCC") && !av_strcasecmp(color->dst, "bt709")) {
> +        color->source = 1;
> +        color->dest = 0;
> +    } else if (!av_strcasecmp(color->src, "FCC") && !av_strcasecmp(color->dst, "bt601")) {
> +        color->source = 1;
> +        color->dest = 2;
> +    } else if (!av_strcasecmp(color->src, "FCC") && !av_strcasecmp(color->dst, "smpte240m")) {
> +        color->source = 1;
> +        color->dest = 3;
> +    } else if (!av_strcasecmp(color->src, "bt601") && !av_strcasecmp(color->dst, "bt709")) {
> +        color->source = 2;
> +        color->dest = 0;
> +    } else if (!av_strcasecmp(color->src, "bt601") && !av_strcasecmp(color->dst, "FCC")) {
> +        color->source = 2;
> +        color->dest = 1;
> +    } else if (!av_strcasecmp(color->src, "bt601") && !av_strcasecmp(color->dst, "smpte240m")) {
> +        color->source = 2;
> +        color->dest = 3;
> +    } else if (!av_strcasecmp(color->src, "smpte240m") && !av_strcasecmp(color->dst, "bt709")) {
> +        color->source = 3;
> +        color->dest = 0;
> +    } else if (!av_strcasecmp(color->src, "smpte240m") && !av_strcasecmp(color->dst, "FCC")) {
> +        color->source = 3;
> +        color->dest = 1;
> +    }  else if (!av_strcasecmp(color->src, "smpte240m") && !av_strcasecmp(color->dst, "bt601")) {
> +        color->source = 3;
> +        color->dest = 2;
> +    }
> +}
> +

It seems all the modes are supported, so it can be simplified with a
static const char *color_modes[] = {"bt709", "bt601", "FCC", "smpte240m"}
and a loop over them to get the source and dest indexes.

Additionally a support matrix can be added later if needed, which will be
IMO way more explicit than the current code.

> +static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> +{
> +    ColorMatrixContext *color = ctx->priv;
> +
> +    if (!args)
> +        goto usage;
> +    if (sscanf(args, "%255[^:]:%255[^:]", color->src, color->dst) != 2) {
> +    usage:

if (!args || sscanf(...) != 2) ?

> +        av_log(ctx, AV_LOG_ERROR, "usage: <src>:<dst>\n");
> +        av_log(ctx, AV_LOG_ERROR, "possible options: bt709,bt601,smpte240m,fcc\n");
> +        return -1;
> +    }
> +
> +    check_mode(color);
> +
> +    if (color->source == -1 || color->dest == -1) {
> +        av_log(ctx, AV_LOG_ERROR, "invalid mode");
> +        return -1;
> +    }
> +
> +    color->mode = color->source * 4 + color->dest;
> +
> +    calc_coefficients(ctx);
> +
> +    return 0;
> +}
> +
> +static void process_frame_uyvy422(ColorMatrixContext *color,
> +                                  AVFilterBufferRef *dst, AVFilterBufferRef *src)
> +{
> +    const unsigned char *srcp = src->data[0];

nit: const uint8_t *?

> +    const int src_pitch = src->linesize[0];
> +    const int height = src->video->h;
> +    const int width = src->video->w*2;
> +    unsigned char *dstp = dst->data[0];

nit: uint8_t *?

> +    const int dst_pitch = dst->linesize[0];
> +    const int c2 = color->yuv_convert[color->mode][0][1];
> +    const int c3 = color->yuv_convert[color->mode][0][2];
> +    const int c4 = color->yuv_convert[color->mode][1][1];
> +    const int c5 = color->yuv_convert[color->mode][1][2];
> +    const int c6 = color->yuv_convert[color->mode][2][1];
> +    const int c7 = color->yuv_convert[color->mode][2][2];
> +    int x, y;
> +
> +    for (y = 0; y < height; ++y) {
> +        for (x = 0; x < width; x += 4) {
> +            const int u = srcp[x + 0] - 128;
> +            const int v = srcp[x + 2] - 128;
> +            const int uvval = c2 * u + c3 * v + 1081344;
> +            dstp[x + 0] = CB((c4 * u + c5 * v + 8421376) >> 16);
> +            dstp[x + 1] = CB((65536 * (srcp[x + 1] - 16) + uvval) >> 16);
> +            dstp[x + 2] = CB((c6 * u + c7 * v + 8421376) >> 16);
> +            dstp[x + 3] = CB((65536 * (srcp[x + 3] - 16) + uvval) >> 16);
> +        }
> +        srcp += src_pitch;
> +        dstp += dst_pitch;
> +    }
> +}
> +
> +static void process_frame_yuv422p(ColorMatrixContext *color,
> +                                  AVFilterBufferRef *dst, AVFilterBufferRef *src)
> +{
> +    const unsigned char *srcpU = src->data[1];
> +    const unsigned char *srcpV = src->data[2];
> +    const unsigned char *srcpY = src->data[0];

ditto

> +    const int src_pitchY  = src->linesize[0];
> +    const int src_pitchUV = src->linesize[1];
> +    const int height = src->video->h;
> +    const int width = src->video->w;
> +    unsigned char *dstpU = dst->data[1];
> +    unsigned char *dstpV = dst->data[2];
> +    unsigned char *dstpY = dst->data[0];

ditto, and several other instances below

[...]
> +static void null_draw_slice(AVFilterLink *link, int y, int h, int slice_dir) { }
> +
> +AVFilter avfilter_vf_colormatrix = {
> +    .name          = "colormatrix",
> +    .description   = NULL_IF_CONFIG_SMALL("Color matrix conversion"),
> +
> +    .priv_size     = sizeof(ColorMatrixContext),
> +    .init          = init,
> +    .query_formats = query_formats,
> +
> +    .inputs    = (AVFilterPad[]) {{ .name             = "default",
> +                                    .type             = AVMEDIA_TYPE_VIDEO,
> +                                    .config_props     = config_input,
> +                                    .start_frame      = start_frame,
> +                                    .get_video_buffer = get_video_buffer,
> +                                    .draw_slice       = null_draw_slice,
> +                                    .end_frame        = end_frame, },
> +                                  { .name = NULL}},

Weird spacing after the NULL :)

> +
> +    .outputs   = (AVFilterPad[]) {{ .name             = "default",
> +                                    .type             = AVMEDIA_TYPE_VIDEO, },
> +                                  { .name = NULL}},
> +};

ditto

-- 
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/20120310/a82103fe/attachment.asc>


More information about the ffmpeg-devel mailing list