[FFmpeg-devel] [RFC] [PATCH] histogram filter

Stefano Sabatini stefasab at gmail.com
Wed Dec 26 16:38:51 CET 2012


On date Wednesday 2012-12-26 13:29:29 +0000, Paul B Mahol encoded:
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  libavfilter/Makefile       |   1 +
>  libavfilter/allfilters.c   |   1 +
>  libavfilter/vf_histogram.c | 276 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 278 insertions(+)
>  create mode 100644 libavfilter/vf_histogram.c

Missing docs.

> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 7595e4c..2b4cde8 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -109,6 +109,7 @@ 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_HISTOGRAM_FILTER)              += vf_histogram.o
>  OBJS-$(CONFIG_HQDN3D_FILTER)                 += vf_hqdn3d.o
>  OBJS-$(CONFIG_HUE_FILTER)                    += vf_hue.o
>  OBJS-$(CONFIG_IDET_FILTER)                   += vf_idet.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 88b0979..94b1aa1 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -100,6 +100,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER (GEQ,         geq,         vf);
>      REGISTER_FILTER (GRADFUN,     gradfun,     vf);
>      REGISTER_FILTER (HFLIP,       hflip,       vf);
> +    REGISTER_FILTER (HISTOGRAM,   histogram,   vf);
>      REGISTER_FILTER (HQDN3D,      hqdn3d,      vf);
>      REGISTER_FILTER (HUE,         hue,         vf);
>      REGISTER_FILTER (IDET,        idet,        vf);
> diff --git a/libavfilter/vf_histogram.c b/libavfilter/vf_histogram.c
> new file mode 100644
> index 0000000..a7e4781
> --- /dev/null
> +++ b/libavfilter/vf_histogram.c
> @@ -0,0 +1,276 @@
> +/*
> + * Copyright (c) 2012 Paul B Mahol
> + *
> + * 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
> + */
> +
> +#include "libavutil/opt.h"
> +#include "libavutil/parseutils.h"
> +#include "libavutil/pixdesc.h"
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "drawutils.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +typedef struct {
> +    const AVClass *class;               ///< AVClass context for log and options purpose
> +    unsigned       histogram[1 << 16];
> +    unsigned       histogram_size;
> +    unsigned       max_hval;

> +    int            comp_yuv[4];
> +    int            comp_rgb[4];

documenting these ones may help reading the code

> +    int            ncomp;
> +    int            do_graph;
> +    int            w, h;
> +    AVFilterBufferRef *outpicref;   ///< output picture reference, updated regularly
> +    FFDrawContext   draw;
> +    char           *fg_color_str, *bg_color_str;
> +    uint8_t         fg_color_rgba[4], bg_color_rgba[4];
> +} HistogramContext;
> +

> +static int config_input(AVFilterLink *link)

nit: inlink

> +{
> +    HistogramContext *h = link->dst->priv;
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> +
> +    h->histogram_size = 1 << desc->comp[0].depth_minus1 + 1;
> +    h->ncomp          = desc->nb_components;
> +
> +    return 0;
> +}
> +

> +static int filter_frame(AVFilterLink *link, AVFilterBufferRef *in)

Nit: inlink, inbuf or inpicref may be more descriptive

> +{
> +    HistogramContext *h = link->dst->priv;
> +    AVFilterContext *ctx = link->dst;
> +    const uint8_t *src;
> +    int i, j, k;
> +
> +    switch (in->format) {

> +    case AV_PIX_FMT_RGB24:
> +    case AV_PIX_FMT_RGBA:

can be easily extended to other packed RGBA 32 bits formats.


> +        src = in->data[0];
> +        for (i = 0; i < in->video->h; i++) {

> +            for (j = 0; j < in->linesize[0]; j += h->ncomp) {

j < in->video->w, or you read uninitialized data.

> +                unsigned v = 0, used = 0;
> +
> +                for (k = 0; k < h->ncomp; k++) {
> +                    if (h->comp_rgb[k]) {
> +                        v += src[j + k];
> +                        used++;
> +                    }
> +                }
> +
> +                if (used)
> +                    h->histogram[v / used]++;
> +            }
> +            src += in->linesize[0];
> +        }
> +        break;
> +    case AV_PIX_FMT_YUVA444P:
> +    case AV_PIX_FMT_YUV444P:
> +    case AV_PIX_FMT_YUVJ444P:
> +    case AV_PIX_FMT_GRAY8:
> +    case AV_PIX_FMT_GRAY8A:
> +        for (i = 0; i < in->video->h; i++) {

> +            for (j = 0; j < in->linesize[0]; j++) {

as above

> +                unsigned v = 0, used = 0;
> +
> +                for (k = 0; k < h->ncomp; k++) {
> +                    if (h->comp_yuv[k]) {
> +                        src = in->data[k] + i * in->linesize[k];
> +                        v += src[j];
> +                        used++;
> +                    }
> +                }
> +
> +                if (used)
> +                    h->histogram[v / used]++;
> +            }
> +        }
> +        break;
> +    }
> +
> +    for (i = 0; i < h->histogram_size; i++) {
> +        h->max_hval = FFMAX(h->max_hval, h->histogram[i]);
> +        if (h->histogram[i])
> +            av_log(ctx, h->do_graph ? AV_LOG_VERBOSE : AV_LOG_INFO, "i:%d c:%d\n", i, h->histogram[i]);
> +    }
> +
> +    if (h->do_graph) {
> +        AVFilterBufferRef *out = h->outpicref;
> +        FFDrawColor bg, fg;
> +        int ret, col_width = h->w / h->histogram_size;
> +
> +        out->pts = in->pts;
> +        out->pos = in->pos;
> +
> +        ff_draw_color(&h->draw, &bg, h->bg_color_rgba);
> +        ff_draw_color(&h->draw, &fg, h->fg_color_rgba);
> +        ff_fill_rectangle(&h->draw, &bg, out->data, out->linesize, 0, 0, h->w, h->h);
> +        for (i = 0; i < h->histogram_size; i++) {
> +            int col_height = (float) h->histogram[i] / h->max_hval * h->h;
> +
> +            ff_fill_rectangle(&h->draw, &fg, out->data, out->linesize,
> +                              i * col_width,
> +                              h->h - col_height,
> +                              col_width,
> +                              col_height);
> +        }
> +
> +        ret = ff_filter_frame(ctx->outputs[0], avfilter_ref_buffer(out, ~AV_PERM_WRITE));
> +        if (ret < 0)
> +            return ret;
> +    } else {
> +        ff_filter_frame(link->dst->outputs[0], in);

missing return check, also link->dst -> ctx.

> +    }
> +    memset(h->histogram, 0, h->histogram_size * sizeof(*h->histogram));
> +    h->max_hval = 0;
> +    avfilter_unref_buffer(in);
> +
> +    return 0;
> +}
> +
> +#define Y 0
> +#define U 1
> +#define V 2
> +#define R 0
> +#define G 1
> +#define B 2
> +#define A 3
> +
> +#define OFFSET(x) offsetof(HistogramContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +
> +static const AVOption histogram_options[] = {
> +    { "graph", "show histogram graph", OFFSET(do_graph), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
> +    { "size", "set histogram graph video size", OFFSET(w), AV_OPT_TYPE_IMAGE_SIZE, {.str = "256x200"}, 0, 0, FLAGS },
> +    { "fg", "set histogram foreground color", OFFSET(fg_color_str), AV_OPT_TYPE_STRING, {.str = "white"}, CHAR_MIN, CHAR_MAX, FLAGS },
> +    { "bg", "set histogram background color", OFFSET(bg_color_str), AV_OPT_TYPE_STRING, {.str = "black"}, CHAR_MIN, CHAR_MAX, FLAGS },
> +    { "y", "use Y component", OFFSET(comp_yuv[Y]), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
> +    { "u", "use U component", OFFSET(comp_yuv[U]), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS},
> +    { "v", "use V component", OFFSET(comp_yuv[V]), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS},
> +    { "a", "use A component", OFFSET(comp_yuv[A]), AV_OPT_TYPE_INT, {.i64=0}, 0, 1, FLAGS},
> +    { "r", "use R component", OFFSET(comp_rgb[R]), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
> +    { "g", "use G component", OFFSET(comp_rgb[G]), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
> +    { "b", "use B component", OFFSET(comp_rgb[B]), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
> +    { NULL },
> +};
> +
> +AVFILTER_DEFINE_CLASS(histogram);
> +
> +static int config_output(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    HistogramContext *h = ctx->priv;
> +
> +    if (!h->do_graph)
> +        return 0;
> +
> +    outlink->w = h->w;
> +    outlink->h = h->h;
> +
> +    avfilter_unref_bufferp(&h->outpicref);

> +    h->outpicref = ff_get_video_buffer(outlink, AV_PERM_WRITE|AV_PERM_PRESERVE|AV_PERM_REUSE2,
> +                            outlink->w, outlink->h);

is PRESERVE required at all?

> +    if (!h->outpicref)
> +        return AVERROR(ENOMEM);
> +    outlink->sample_aspect_ratio = (AVRational){1,1};
> +
> +    ff_draw_init(&h->draw, outlink->format, 0);
> +
> +    return 0;
> +}
> +
> +static av_cold int init(AVFilterContext *ctx, const char *args)
> +{
> +    HistogramContext *h = ctx->priv;
> +    int ret;
> +
> +    h->class = &histogram_class;
> +    av_opt_set_defaults(h);
> +
> +    if ((ret = (av_set_options_string(h, args, "=", ":"))) < 0)
> +        return ret;
> +

> +    if ((ret = av_parse_color(h->fg_color_rgba, h->fg_color_str, -1, ctx)) < 0 ||
> +        (ret = av_parse_color(h->bg_color_rgba, h->bg_color_str, -1, ctx)) < 0 )
> +        return ret;

Note: we may start to think about a color option

> +
> +    h->comp_rgb[A] = h->comp_yuv[A];

> +    if (!h->comp_rgb[R] && !h->comp_rgb[G] && !h->comp_rgb[B] && !h->comp_rgb[A] &&
> +        !h->comp_yuv[Y] && !h->comp_yuv[U] && !h->comp_yuv[V] && !h->comp_yuv[A])
> +        return AVERROR(EINVAL);

Add an explanation in case of failure.

> +
> +    return 0;
> +}
> +
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    HistogramContext *h = ctx->priv;
> +
> +    avfilter_unref_bufferp(&h->outpicref);
> +    av_opt_free(h);
> +}
> +

> +static int query_formats(AVFilterContext *ctx)
> +{
> +    static const enum AVPixelFormat pix_fmts[] = {
> +        AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUVJ444P,
> +        AV_PIX_FMT_RGB24, AV_PIX_FMT_RGBA,
> +        AV_PIX_FMT_GRAY8, AV_PIX_FMT_GRAY8A,
> +        AV_PIX_FMT_NONE
> +    };
> +
> +    ff_set_common_formats(ctx, ff_make_format_list(pix_fmts));
> +
> +    return 0;
> +}

I'd find more logical if the functions would be defined in the order
they are used, that is: init, query_formats, config_*, filter_frame,
uninit.
-- 
FFmpeg = Fast and Fierce Mortal Prodigious EnGine


More information about the ffmpeg-devel mailing list