[FFmpeg-devel] [PATCH] histogram filter
Stefano Sabatini
stefasab at gmail.com
Sat Feb 9 00:33:18 CET 2013
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"
[...]
--
FFmpeg = Fiendish Fabulous Most Pure Enlightening Guru
More information about the ffmpeg-devel
mailing list