[FFmpeg-devel] [PATCH] histogram filter

Paul B Mahol onemda at gmail.com
Sat Feb 9 12:53:39 CET 2013


On 2/8/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Thursday 2013-02-07 13:01:23 +0000, Paul B Mahol encoded:
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>  doc/filters.texi           |  49 ++++++++
>>  libavfilter/Makefile       |   1 +
>>  libavfilter/allfilters.c   |   1 +
>>  libavfilter/vf_histogram.c | 299
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 350 insertions(+)
>>  create mode 100644 libavfilter/vf_histogram.c
>>
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index e047852..2925cbe 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -3098,6 +3098,55 @@ the histogram. Possible values are @code{none},
>> @code{weak} or
>>  @code{strong}. It defaults to @code{none}.
>>  @end table
>>
>> + at section histogram
>> +
>> +Compute and draw an histogram.
>
> Compute and draw a color distribution histogram for the input video.
>
>> +
>> +Computed histogram is representation of distribution of color components
>
> The computed histogram ... is a representation ...
>
>> +in an image.
>
> You may mention that the histogram is relative to the input format,
> and maybe provide an example showing how to force a format in the
> examples section.
>
>> +
>> +The filter accepts the following named parameters:
>> +
>> + at table @option
>> + at item mode
>> +Set histogram mode.
>> +
>> +It accepts the following values:
>> + at table @samp
>> + at item levels
>> +standard histogram that display color components
>> +distribution in an image.
>> + at item color
>> +chroma values in vectorscope, if brighter more
>> +such chroma values are distributed in image.
>> + at item color2
>> +chroma values in vectorscope, similar as color
>> +but actual values are displayed.
>> + at item waveform
>> +per-line luminance graph.
>> + at end table
>> +Default value is @code{levels}.
>
> I would like a more accurate definition of this, it is really hard to
> understand what they really do if not inspecting the code or the
> output.
>
> Copy&pasting from avisynth docs would be fine, I don't mind to fix
> eventual typos later, feel free to skip if this bores you to death,
> I'll provide docs later.
>
>> +
>> + at item level_height
>> +Set height of level in @code{levels}. Default value is @code{200}.
>> +Allowed range is [50, 2048].
>> +
>> + at item scale_height
>> +Set height of color scale in @code{levels}. Default value is @code{10}.
>> +Allowed range is [0, 40].
>
> BTW do you think would be possible to select just a component?, for
> example if I want to show only one component (that would be possible
> by some weird filter combination, but having this in the filter would
> be simpler for the user).
>
>> +
>> + at subsection Examples
>> +
>> + at itemize
>> +
>> + at item
>> +Calculate and draw histogram:
>> + at example
>> +ffplay -i input -vf histogram
>> + at end example
>> +
>> + at end itemize
>> +
>>  @section hqdn3d
>>
>>  High precision/quality 3d denoise filter. This filter aims to reduce
>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> index 938b183..a9dfc16 100644
>> --- a/libavfilter/Makefile
>> +++ b/libavfilter/Makefile
>> @@ -122,6 +122,7 @@ OBJS-$(CONFIG_GEQ_FILTER)                    +=
>> vf_geq.o
>>  OBJS-$(CONFIG_GRADFUN_FILTER)                += vf_gradfun.o
>>  OBJS-$(CONFIG_HFLIP_FILTER)                  += vf_hflip.o
>>  OBJS-$(CONFIG_HISTEQ_FILTER)                 += vf_histeq.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 47158f9..0def0b4 100644
>> --- a/libavfilter/allfilters.c
>> +++ b/libavfilter/allfilters.c
>> @@ -116,6 +116,7 @@ void avfilter_register_all(void)
>>      REGISTER_FILTER(GRADFUN,        gradfun,        vf);
>>      REGISTER_FILTER(HFLIP,          hflip,          vf);
>>      REGISTER_FILTER(HISTEQ,         histeq,         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..4659bd5
>> --- /dev/null
>> +++ b/libavfilter/vf_histogram.c
>> @@ -0,0 +1,299 @@
>> +/*
>> + * 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/avassert.h"
>> +#include "libavutil/opt.h"
>> +#include "libavutil/parseutils.h"
>> +#include "libavutil/pixdesc.h"
>> +#include "avfilter.h"
>> +#include "formats.h"
>> +#include "internal.h"
>> +#include "video.h"
>> +
>> +enum HistogramMode {
>> +    LEVELS,
>> +    WAVEFORM,
>> +    COLOR,
>> +    COLOR2,
>> +    MODE_NB
>> +};
>
> Nit: I suggest to add some prefix MODE_ to clarify the context, and
> avoid possible future conflicts.
>
>> +
>> +typedef struct HistogramContext {
>> +    const AVClass *class;               ///< AVClass context for log and
>> options purpose
>> +    enum HistogramMode mode;
>
>> +    unsigned       histogram[4][256];
>> +    unsigned       max_hval[4];
>
> a short doxy here may help
>
>> +    int            ncomp;
>> +    const uint8_t  *bg_color;
>> +    const uint8_t  *fg_color;
>> +    int            level_height;
>> +    int            scale_height;
>> +} HistogramContext;
>> +
>> +#define OFFSET(x) offsetof(HistogramContext, x)
>> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
>> +
>> +static const AVOption histogram_options[] = {
>> +    { "mode", "set histogram mode", OFFSET(mode), AV_OPT_TYPE_INT,
>> {.i64=LEVELS}, 0, MODE_NB-1, FLAGS, "mode"},
>> +    { "levels", "standard histogram", 0, AV_OPT_TYPE_CONST,
>> {.i64=LEVELS}, 0, 0, FLAGS, "mode" },
>> +    { "waveform", "per-line luminance graph", 0, AV_OPT_TYPE_CONST,
>> {.i64=WAVEFORM}, 0, 0, FLAGS, "mode" },
>> +    { "color", "chroma values in vectorscope", 0, AV_OPT_TYPE_CONST,
>> {.i64=COLOR}, 0, 0, FLAGS, "mode" },
>> +    { "color2", "chroma values in vectorscope", 0, AV_OPT_TYPE_CONST,
>> {.i64=COLOR2}, 0, 0, FLAGS, "mode" },
>> +    { "level_height", "set level height", OFFSET(level_height),
>> AV_OPT_TYPE_INT, {.i64=200}, 50, 2048, FLAGS},
>> +    { "scale_height", "set scale height", OFFSET(scale_height),
>> AV_OPT_TYPE_INT, {.i64=10}, 0, 40, FLAGS},
>> +    { NULL },
>> +};
>> +
>> +AVFILTER_DEFINE_CLASS(histogram);
>> +
>> +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)
>
> Here you could use av_opt_set_string() and select a shorthand for the
> mode.
>
>> +        return ret;
>> +
>> +    return 0;
>> +}
>> +
>> +static const enum AVPixelFormat color_pix_fmts[] = {
>> +    AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUVJ444P,
>> +    AV_PIX_FMT_NONE
>> +};
>> +
>> +static const enum AVPixelFormat levels_pix_fmts[] = {
>> +    AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUVJ444P,
>> +    AV_PIX_FMT_GRAY8, AV_PIX_FMT_GBRP, AV_PIX_FMT_NONE
>> +};
>> +
>> +static int query_formats(AVFilterContext *ctx)
>> +{
>> +    HistogramContext *h = ctx->priv;
>> +    const enum AVPixelFormat *pix_fmts;
>> +
>> +    switch (h->mode) {
>> +    case LEVELS:
>> +        pix_fmts = levels_pix_fmts;
>> +        break;
>> +    case WAVEFORM:
>> +    case COLOR:
>> +    case COLOR2:
>> +        pix_fmts = color_pix_fmts;
>> +        break;
>> +    default:
>> +        av_assert0(0);
>> +    }
>> +
>> +    ff_set_common_formats(ctx, ff_make_format_list(pix_fmts));
>> +
>> +    return 0;
>> +}
>> +
>> +static const uint8_t black_yuva_color[4] = { 0, 127, 127, 255 };
>> +static const uint8_t black_gbrp_color[4] = { 0, 0, 0, 255 };
>> +static const uint8_t white_yuva_color[4] = { 255, 127, 127, 255 };
>> +static const uint8_t white_gbrp_color[4] = { 255, 255, 255, 255 };
>> +
>> +static int config_input(AVFilterLink *inlink)
>> +{
>> +    HistogramContext *h = inlink->dst->priv;
>> +    const AVPixFmtDescriptor *desc =
>> av_pix_fmt_desc_get(inlink->format);
>> +
>> +    h->ncomp = desc->nb_components;
>> +
>> +    switch (inlink->format) {
>> +    case AV_PIX_FMT_GBRP:
>> +        h->bg_color = black_gbrp_color;
>> +        h->fg_color = white_gbrp_color;
>> +        break;
>> +    default:
>> +        h->bg_color = black_yuva_color;
>> +        h->fg_color = white_yuva_color;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int config_output(AVFilterLink *outlink)
>> +{
>> +    AVFilterContext *ctx = outlink->src;
>> +    HistogramContext *h = ctx->priv;
>> +
>> +    switch (h->mode) {
>> +    case LEVELS:
>> +        outlink->w = 256;
>> +        outlink->h = (h->level_height + h->scale_height) * h->ncomp;
>> +        break;
>> +    case WAVEFORM:
>> +        outlink->w = 256;
>> +        break;
>> +    case COLOR:
>> +    case COLOR2:
>> +        outlink->h = outlink->w = 256;
>> +        break;
>> +    default:
>> +        av_assert0(0);
>> +    }
>
> maybe you could add some consistency checks here in case levels
> options are specified but mode != LEVELS.
>
>> +
>> +    outlink->sample_aspect_ratio = (AVRational){1,1};
>> +
>> +    return 0;
>> +}
>> +
>> +static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *in)
>> +{
>> +    HistogramContext *h   = inlink->dst->priv;
>> +    AVFilterContext *ctx  = inlink->dst;
>> +    AVFilterLink *outlink = ctx->outputs[0];
>> +    AVFilterBufferRef *out;
>> +    const uint8_t *src;
>> +    int i, j, k, l, ret;
>> +
>> +    out = ff_get_video_buffer(outlink, AV_PERM_WRITE, outlink->w,
>> outlink->h);
>> +    if (!out) {
>> +        avfilter_unref_bufferp(&in);
>> +        return AVERROR(ENOMEM);
>> +    }
>> +
>> +    out->pts = in->pts;
>> +    out->pos = in->pos;
>
> avfilter_copy_buffer_ref_props() may be safer in case we add more
> params (e.g. duration), or me may add a shallower macro/function to
> copy only minimal stuff.
>
>> +
>> +    for (k = 0; k < h->ncomp; k++)
>> +        for (i = 0; i < outlink->h; i++)
>> +            memset(out->data[k] + i * out->linesize[k], h->bg_color[k],
>> outlink->w);
>> +
>> +    switch (h->mode) {
>> +    case LEVELS:
>
> please add a note before each block explaining what it does, they are
> really valuable when reading/debugging the code for the first time
>
>> +        for (k = 0; k < h->ncomp; k++) {
>> +            for (i = 0; i < in->video->h; i++) {
>
>> +                int lsize = i * in->linesize[k];
>
> "lsize" is not a linesize, so the name is a bit confusing.
>
>> +                for (j = 0; j < in->video->w; j++) {
>
>> +                    src = in->data[k] + lsize;
>
> is this supposed to change in the inner loop?
>
>> +                    h->histogram[k][src[j]]++;
>> +                }
>
> I suggest (untested):
>
> for (k = 0; k < h->ncomp; k++) {
>     for (i = 0; i < in->video->h; i++) {
>         src = in->data[k] + i * in->linesize[k];
>         for (j = 0; j < in->video->w; j++)
>             h->histogram[k][src[j]]++;
>     }
> }
>
>> +            }
>> +        }
>> +
>> +        for (k = 0; k < h->ncomp; k++)
>> +            for (i = 0; i < 256; i++)
>> +                h->max_hval[k] = FFMAX(h->max_hval[k],
>> h->histogram[k][i]);
>> +
>> +        for (k = 0; k < h->ncomp; k++) {
>> +            int start = k * (h->level_height + h->scale_height);
>> +            for (i = 0; i < outlink->w; i++) {
>> +                int col_height = h->level_height -
>> (float)h->histogram[k][i] / h->max_hval[k] * h->level_height;
>> +
>> +                for (j = h->level_height - 1; j >= col_height; j--)
>> +                    for (l = 0; l < h->ncomp; l++)
>> +                        out->data[l][(j + start) * out->linesize[l] + i]
>> = h->fg_color[l];
>> +                for (j = h->level_height + h->scale_height - 1; j >=
>> h->level_height; j--)
>> +                    out->data[k][(j + start) * out->linesize[0] + i] =
>> i;
>> +            }
>> +        }
>> +
>> +        for (k = 0; k < h->ncomp; k++) {
>> +            memset(h->histogram[k], 0, 256 * sizeof(unsigned));
>> +            h->max_hval[k] = 0;
>> +        }
>
> I suppose you could merge the external loops on k
>
>> +        break;
>> +    case WAVEFORM:
>> +        for (i = 0; i < inlink->h; i++) {
>
>> +            int ow = i * out->linesize[0];
>> +            int iw = i * in->linesize[0];
>> +            for (j = 0; j < inlink->w; j++)
>> +                out->data[0][ow + in->data[0][iw + j]] = 255;
>
> again i'm confused by the variable names choice, i suggest:
>
> for (i = 0; i < inlink->h; i++) {
>    src = in ->data[0] + i *in ->linesize;
>    dst = out->data[0] + i *out->linesize;
>    for (j = 0; j < inlink->w; j++)
>       dst[src[j]] = 255;
>
> Also it would be more interesting if you could also set an intensity
> based on the number of value hits.
>
> [...]
>> +AVFilter avfilter_vf_histogram = {
>> +    .name          = "histogram",
>> +    .description   = NULL_IF_CONFIG_SMALL("Compute and draw an
>> histogram."),
>
> "a histogram"
>
> [...]

I fixed most of things I could and pushed. Feel free to improve it as you like.


More information about the ffmpeg-devel mailing list