[FFmpeg-devel] [PATCH] lavfi: add histeq filter

Clément Bœsch ubitux at gmail.com
Fri Jan 4 16:14:29 CET 2013


On Fri, Jan 04, 2013 at 03:17:24PM +0100, Stefano Sabatini wrote:
> On date Friday 2013-01-04 14:28:05 +0100, Stefano Sabatini encoded:
> > On date Saturday 2012-10-20 13:37:40 +0200, Stefano Sabatini encoded:
> > > On date Saturday 2012-10-20 07:45:51 +0200, Jérémy Tran encoded:
> > > > This is a port of virtual dub's histogram equalization filter by
> > > > Donald A. Graft.
> > > > 
> > > > Using ff_fill_rgba_map() highlighted the fact that I was incorrectly
> > > > filling the map, that is backwards; the output is now much more better (the
> > > > colors are fine) but many blocks appear here and there and depending
> > > > on the frame, it can get really messy (I'm using the default values).
> > > > I am working on this issue.
> > > 
> > > Can you show an example commandline? Could be due to overflowing issues.
> > 
> > Updated patch. I suppose the problem mentioned by Jeremy is due to
> > images with few colors, like it happens for example with
> > fade-in/fade-out effect, but I'll try to investigate more.
> > 
> > TODO: FATE test.
> 
> Updated with FATE test added.
> -- 
> FFmpeg = Fiendish & Friendly Multimedia Power Explosive Game

> From 9eee855131812d335fc0b4bb8186078ab32b0dce Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Sat, 20 Oct 2012 07:45:51 +0200
> Subject: [PATCH] lavfi: add histeq filter
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> This is a port of virtual dub's histogram equalization filter by Donald
> A. Graft. Based on the work by Jérémy Tran <tran.jeremy.av at gmail.com>,
> done for SOCIS 2012.
> 
> TODO: add changelog entry, bump minor.
> ---
>  LICENSE                   |    1 +
>  configure                 |    1 +
>  doc/filters.texi          |   38 ++++++
>  libavfilter/Makefile      |    1 +
>  libavfilter/allfilters.c  |    1 +
>  libavfilter/vf_histeq.c   |  296 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/fate/avfilter.mak   |    1 +
>  tests/lavfi-regression.sh |    1 +
>  tests/ref/lavfi/histeq    |    6 +
>  9 files changed, 346 insertions(+)
>  create mode 100644 libavfilter/vf_histeq.c
>  create mode 100644 tests/ref/lavfi/histeq
> 
[...]
> +/// Linear Congruential Generator, see "Numerical Recipes"

nit++: /* */ or //

> +#define LCG_A 4096
> +#define LCG_C 150889
> +#define LCG_M 714025
> +#define LCG(x) (((x) * LCG_A + LCG_C) % LCG_M)
> +#define LCG_SEED 739187
> +
> +#define R 0
> +#define G 1
> +#define B 2
> +#define A 3
> +
> +#define GET_RGB_VALUES(r, g, b, src, map) do { \
> +    r = src[x + map[R]];                       \
> +    g = src[x + map[G]];                       \
> +    b = src[x + map[B]];                       \
> +} while (0)
> +
> +static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *inpic)
> +{
> +    AVFilterContext   *ctx     = inlink->dst;
> +    HisteqContext     *histeq  = ctx->priv;
> +    AVFilterLink      *outlink = ctx->outputs[0];
> +    int strength  = histeq->strength  * 1000;
> +    int intensity = histeq->intensity * 1000;
> +    int x, y, i, luthi, lutlo, lut, luma, oluma, m;
> +    AVFilterBufferRef *outpic;
> +    unsigned int r, g, b;
> +    uint8_t *src, *dst;
> +
> +    outpic = ff_get_video_buffer(outlink, AV_PERM_WRITE|AV_PERM_ALIGN, outlink->w, outlink->h);
> +    if (!outpic) {
> +        avfilter_unref_bufferp(&inpic);
> +        return AVERROR(ENOMEM);
> +    }
> +    avfilter_copy_buffer_ref_props(outpic, inpic);
> +
> +    /* Seed random generator for antibanding. */
> +    histeq->jran = LCG_SEED;
> +

This one, and certainly many others, can be declared locally. While it
could make sense to keep large buffers allocated in the context, there is
no point in keeping this one.

> +    /* Calculate and store the luminance and calculate the global histogram
> +       based on the luminance. */
> +    memset(histeq->in_histogram, 0, sizeof(histeq->in_histogram));
> +    src = inpic->data[0];
> +    dst = outpic->data[0];
> +    for (y = 0; y < inlink->h; y++) {
> +        for (x = 0; x < inlink->w * histeq->bpp; x += histeq->bpp) {
> +            GET_RGB_VALUES(r, g, b, src, histeq->rgba_map);
> +            luma = (55 * r + 182 * g + 19 * b) >> 8;
> +            dst[x + histeq->rgba_map[A]] = luma;
> +            histeq->in_histogram[luma]++;
> +        }
> +        src += inpic->linesize[0];
> +        dst += outpic->linesize[0];
> +    }
> +
> +#ifdef DEBUG
> +    for (x = 0; x < 256; x++)
> +        av_dlog(ctx, "in[%d]: %u\n", x, histeq->in_histogram[x]);
> +#endif
> +
> +    /* Calculate the lookup table. */
> +    histeq->LUT[0] = histeq->in_histogram[0];
> +    /* Accumulate */
> +    for (x = 1; x < 256; x++)
> +        histeq->LUT[x] = histeq->LUT[x-1] + histeq->in_histogram[x];
> +
> +    /* Normalize */
> +    for (x = 0; x < 256; x++)
> +        histeq->LUT[x] = (histeq->LUT[x] * intensity) / (inlink->h * inlink->w);
> +
> +    /* Adjust the LUT based on the selected strength. This is an alpha
> +       mix of the calculated LUT and a linear LUT with gain 1. */
> +    for (x = 0; x < 256; x++)
> +        histeq->LUT[x] = (strength * histeq->LUT[x]) / 255 +
> +                         ((255 - strength) * x)      / 255;
> +
> +    /* Output the equalized frame. */
> +    memset(histeq->out_histogram, 0, sizeof(histeq->out_histogram));
> +
> +    src = inpic->data[0];
> +    dst = outpic->data[0];
> +    for (y = 0; y < inlink->h; y++) {
> +        for (x = 0; x < inlink->w * histeq->bpp; x += histeq->bpp) {
> +            luma = dst[x + histeq->rgba_map[A]];
> +            if (luma == 0) {
> +                for (i = 0; i < histeq->bpp; ++i)
> +                    dst[x + i] = 0;
> +                histeq->out_histogram[0]++;
> +            } else {
> +                lut = histeq->LUT[luma];
> +                if (histeq->antibanding != HISTEQ_ANTIBANDING_NONE) {
> +                    if (luma > 0) {
> +                        lutlo = histeq->antibanding == HISTEQ_ANTIBANDING_WEAK ?
> +                                (histeq->LUT[luma] + histeq->LUT[luma - 1]) / 2 :
> +                                 histeq->LUT[luma - 1];
> +                    } else
> +                        lutlo = lut;
> +
> +                    if (luma < 255) {
> +                        luthi = (histeq->antibanding == HISTEQ_ANTIBANDING_WEAK) ?
> +                            (histeq->LUT[luma] + histeq->LUT[luma + 1]) / 2 :
> +                             histeq->LUT[luma + 1];
> +                    } else
> +                        luthi = lut;
> +
> +                    if (lutlo != luthi) {
> +                        histeq->jran = LCG(histeq->jran);
> +                        lut = lutlo + ((luthi - lutlo + 1) * histeq->jran) / LCG_M;
> +                    }
> +                }
> +
> +                GET_RGB_VALUES(r, g, b, src, histeq->rgba_map);
> +                if (((m = FFMAX(r, FFMAX(g, b))) * lut) / luma > 255) {

nit: I'm not sure the double expansion of the FFMAX macro will generate a
sexy asm.

> +                    r = (r * 255) / m;
> +                    g = (g * 255) / m;
> +                    b = (b * 255) / m;
> +                } else {
> +                    r = (r * lut) / luma;
> +                    g = (g * lut) / luma;
> +                    b = (b * lut) / luma;
> +                }
> +                dst[x + histeq->rgba_map[R]] = r;
> +                dst[x + histeq->rgba_map[G]] = g;
> +                dst[x + histeq->rgba_map[B]] = b;
> +                oluma = (55 * r + 182 * g + 19 * b) >> 8;
> +                histeq->out_histogram[oluma]++;
> +            }
> +        }
> +        src += inpic->linesize[0];
> +        dst += outpic->linesize[0];
> +    }
> +#ifdef DEBUG
> +    for (x = 0; x < 256; x++)
> +        av_dlog(ctx, "out[%d]: %u\n", x, histeq->out_histogram[x]);
> +#endif
> +
> +    avfilter_unref_bufferp(&inpic);
> +    return ff_filter_frame(outlink, outpic);
> +}
> +
> +static const AVFilterPad histeq_inputs[] = {
> +    {
> +        .name             = "default",
> +        .type             = AVMEDIA_TYPE_VIDEO,
> +        .config_props     = config_input,
> +        .filter_frame     = filter_frame,
> +        .min_perms        = AV_PERM_WRITE | AV_PERM_READ,

I think write can be removed.

> +        .rej_perms        = AV_PERM_PRESERVE

Is that necessary?

[...]

I've already reviewed previous versions, so this now LGTM assuming my
latest comments.

-- 
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/20130104/80b13777/attachment.asc>


More information about the ffmpeg-devel mailing list