[FFmpeg-devel] [PATCH] avfilter: add nlmeans filter

Benoit Fouet benoit.fouet at free.fr
Wed Sep 21 15:39:55 EEST 2016


Hi,


On 20/09/2016 21:52, Clément Bœsch wrote:
> Fixes Ticket #4910
> ---
> I actually tried to implement the better defaults suggestion from ipol (see
> @todo) but it wasn't convincing; probably because of different scales, so I
> need to investigate.
>
> Also, integral is still inplace in the filter for now as I didn't find a clean
> way of testing it outside the filter without a long trip in dependency hell. I
> think it can wait until the SIMD are implemented and the need to expose it
> comes up.
>
> I've made several changes from the initial WIP. The most important one is the
> fix in the patch distance calculation, followed by the the addition of chroma
> parameters.
>
> I believe the filter is ready for integration as a first version.
>
> Two interesting examples: http://imgur.com/a/XXhJP
> ---
>   Changelog                    |   1 +
>   doc/filters.texi             |  35 +++
>   libavfilter/Makefile         |   3 +-
>   libavfilter/allfilters.c     |   1 +
>   libavfilter/tests/integral.c |  92 ++++++++
>   libavfilter/version.h        |   2 +-
>   libavfilter/vf_nlmeans.c     | 548 +++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 680 insertions(+), 2 deletions(-)
>   create mode 100644 libavfilter/tests/integral.c
>   create mode 100644 libavfilter/vf_nlmeans.c
>
> diff --git a/Changelog b/Changelog
> index 2d0a449..a5282b4 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -31,6 +31,7 @@ version <next>:
>   - MediaCodec HEVC decoding
>   - TrueHD encoder
>   - Meridian Lossless Packing (MLP) encoder
> +- nlmeans filter (denoiser)
>   

The full name could be used here: Non-Local Means (nlmeans) denoising filter

>   
>   version 3.1:
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 070e57d..7e9ab60 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -9695,6 +9695,41 @@ Negate input video.
>   It accepts an integer in input; if non-zero it negates the
>   alpha component (if available). The default value in input is 0.
>   
> + at section nlmeans
> +
> +Denoise frames using Non-Local Means algorithm.
> +
> +Each pixel is adjusted by looking for other pixels with similar contexts. This
> +context similarity is defined by their surrounding patch of size

"is defined by comparing their surrounding patches" ?

> + at option{p}x at option{p}. Patches are researched in an area of
> + at option{r}x at option{r} surrouding the pixel.
> +

surrounding, or even simply "around"
Also "research" sounds weird (I'd use "search"), but maybe wait for 
someone native to comment

[...]

> diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
> new file mode 100644
> index 0000000..f923f80
> --- /dev/null
> +++ b/libavfilter/vf_nlmeans.c
> @@ -0,0 +1,548 @@
> +/*
> + * Copyright (c) 2016 Clément Bœsch <u pkh me>
> + *
> + * 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
> + */
> +
> +/**
> + * @todo
> + * - SIMD for compute_safe_ssd_integral_image
> + * - SIMD for final weighted averaging
> + * - better automatic defaults? see "Parameters" @ http://www.ipol.im/pub/art/2011/bcm_nlm/
> + * - temporal support (probably doesn't need any displacement according to
> + *   "Denoising image sequences does not require motion estimation")
> + * - bayer support?
> + * - FATE test (probably needs visual threshold test mechanism due to the use of floats)
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +struct weighted_avg {
> +    double total_weight;
> +    double sum;
> +};
> +
> +#define WEIGHT_LUT_NBITS 9
> +#define WEIGHT_LUT_SIZE  (1<<WEIGHT_LUT_NBITS)
> +
> +typedef struct {
> +    const AVClass *class;
> +    int nb_planes;
> +    int chroma_w, chroma_h;
> +    double pdiff_scale;                     // invert of the filtering parameter (sigma*10) squared
> +    double sigma;                           // denoising strength
> +    int patch_size,   patch_hsize;          // patch size and half size
> +    int patch_size_c, patch_hsize_c;        // patch size and half size for chroma planes

nit: could you use _uv instead of _c (the latter has a taste of C vs ASM)?

> +    int research_size,   research_hsize;    // research size and half size
> +    int research_size_c, research_hsize_c;  // research size and half size for chroma planes
> +    uint32_t *ii_orig;                      // integral image
> +    uint32_t *ii;                           // integral image starting after the 0-line and 0-column
> +    int ii_w, ii_h;                         // width and height of the integral image
> +    int ii_lz_32;                           // linesize in 32-bit units of the integral image
> +    struct weighted_avg *wa;                // weighted average of every pixel
> +    int wa_linesize;                        // linesize for wa in struct size unit
> +    double weight_lut[WEIGHT_LUT_SIZE];     // lookup table mapping (scaled) patch differences to their associated weights
> +    double pdiff_lut_scale;                 // scale factor for patch differences before looking into the LUT
> +    int max_meaningful_diff;                // maximum difference considered (if the patch difference is too high we ignore the pixel)
> +} NLMeansContext;
> +
> +#define OFFSET(x) offsetof(NLMeansContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +static const AVOption nlmeans_options[] = {
> +    { "s",  "denoising strength", OFFSET(sigma), AV_OPT_TYPE_DOUBLE, { .dbl = 1.0 }, 1.0, 30.0, FLAGS },
> +    { "p",  "patch size",                   OFFSET(patch_size),   AV_OPT_TYPE_INT, { .i64 = 3*2+1 }, 0, 99, FLAGS },
> +    { "pc", "patch size for chroma planes", OFFSET(patch_size_c), AV_OPT_TYPE_INT, { .i64 = 0 },     0, 99, FLAGS },
> +    { "r",  "research window",                   OFFSET(research_size),   AV_OPT_TYPE_INT, { .i64 = 7*2+1 }, 0, 99, FLAGS },

Nit: if there is a correlation between the default patch size and 
research window size, maybe add defines? Like:
#define NLMEANS_DEFAULT_PATCH_SIZE (3*2+1)
#define NLMEANS_DEFAULT_RESEARCH_WINDOW_SIZE (NLMEANS_DEFAULT_PATCH_SIZE 
+ 1)

> +    { "rc", "research window for chroma planes", OFFSET(research_size_c), AV_OPT_TYPE_INT, { .i64 = 0 },     0, 99, FLAGS },
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(nlmeans);
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    static const enum AVPixelFormat pix_fmts[] = {
> +        AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV411P,
> +        AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P,
> +        AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUV444P,
> +        AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ440P,
> +        AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ420P,
> +        AV_PIX_FMT_YUVJ411P,
> +        AV_PIX_FMT_GRAY8, AV_PIX_FMT_GBRP,
> +        AV_PIX_FMT_NONE
> +    };
> +
> +    AVFilterFormats *fmts_list = ff_make_format_list(pix_fmts);
> +    if (!fmts_list)
> +        return AVERROR(ENOMEM);
> +    return ff_set_common_formats(ctx, fmts_list);
> +}
> +
> +/*
> + * M is a discrete map where every entry contains the sum of all the entries
> + * in the rectangle from the top-left origin of M to its coordinate. In the
> + * following schema, "i" contains the sum of the whole map:
> + *
> + * M = +----------+-----------------+----+
> + *     |          |                 |    |
> + *     |          |                 |    |
> + *     |         a|                b|   c|
> + *     +----------+-----------------+----+
> + *     |          |                 |    |
> + *     |          |                 |    |
> + *     |          |        X        |    |
> + *     |          |                 |    |
> + *     |         d|                e|   f|
> + *     +----------+-----------------+----+
> + *     |          |                 |    |
> + *     |         g|                h|   i|
> + *     +----------+-----------------+----+
> + *
> + * The sum of the X box can be calculated with:
> + *    X = e-d-b+a
> + *
> + * See https://en.wikipedia.org/wiki/Summed_area_table
> + *
> + * The compute*_ssd functions compute the integral image M where every entry
> + * contains the sum of the squared difference of every corresponding pixels of
> + * two input planes of the same size as M.
> + */
> +static inline int get_ssd_patch(const uint32_t *ii, int ii_lz_32, int x, int y, int p)

Actually, this is not really about SSD value here. This function does 
not really care about what has been integrated, it just compute the 
patch value only by knowing the buffer is an integral image.
get_patch_value (maybe too much generic)?
Anyway... this is nitpicking, feel free to just ignore, I just felt I 
needed to explain a bit more why I wanted another name :-)

> +{
> +    const int e = ii[(y + p    ) * ii_lz_32 + (x + p    )];
> +    const int d = ii[(y + p    ) * ii_lz_32 + (x - p - 1)];
> +    const int b = ii[(y - p - 1) * ii_lz_32 + (x + p    )];
> +    const int a = ii[(y - p - 1) * ii_lz_32 + (x - p - 1)];
> +    return e - d - b + a;
> +}
> +
> +/**
> + * Compute squared difference of the safe area (the zone where s1 and s2
> + * overlap). It is likely the largest integral zone, so it is interesting to do
> + * as little checks as possible; contrary to the unsafe version of this
> + * function, we do not need any clipping here.
> + *
> + * The above line and left column of dst are always readable.
> + *

The line above dst and the column to its left are always readable.

> + * This C version computes the SSD integral image using a scalar accumulator,
> + * while for SIMD implementation it is likely more interesting to use the
> + * two-loops algorithm variant.
> + */
> +static void compute_safe_ssd_integral_image_c(uint32_t *dst, int dst_linesize_32,
> +                                              const uint8_t *s1, int linesize1,
> +                                              const uint8_t *s2, int linesize2,
> +                                              int w, int h)
> +{
> +    int x, y;
> +
> +    for (y = 0; y < h; y++) {
> +        uint32_t acc = dst[-1] - dst[-dst_linesize_32 - 1];
> +
> +        for (x = 0; x < w; x++) {
> +            const int d  = s1[x] - s2[x];
> +            acc += d * d;
> +            dst[x] = dst[-dst_linesize_32 + x] + acc;
> +        }
> +        s1  += linesize1;
> +        s2  += linesize2;
> +        dst += dst_linesize_32;
> +    }
> +}
> +
> +/**
> + * Compute squared difference of an unsafe area (the zone nor s1 nor s2 could
> + * be readable).
> + *
> + * On the other hand, the above line and left column of dst are still always
> + * readable.
> + *

same

> + * There is little point in having this function SIMDified as it is likely too
> + * complex and only handle small portions of the image.
> + *

Small portions compared to the overall size of the image. This could 
still be "large" buffers to be handled (e.g. the larger the image width, 
the larger the buffer)

> + * @param dst               integral image
> + * @param dst_linesize_32   integral image linesize (in 32-bit integers unit)
> + * @param startx            integral starting x position
> + * @param starty            integral starting y position
> + * @param src               source plane buffer
> + * @param linesize          source plane linesize
> + * @param offx              source offsetting in x
> + * @param offy              source offsetting in y
> + * @paran r                 absolute maximum source offsetting
> + * @param sw                source width
> + * @param sh                source height
> + * @param w                 width to compute
> + * @param h                 height to compute
> + */
> +static inline void compute_unsafe_ssd_integral_image(uint32_t *dst, int dst_linesize_32,
> +                                                     int startx, int starty,
> +                                                     const uint8_t *src, int linesize,
> +                                                     int offx, int offy, int r, int sw, int sh,
> +                                                     int w, int h)
> +{
> +    int x, y;
> +
> +    for (y = starty; y < starty + h; y++) {
> +        uint32_t acc = dst[y*dst_linesize_32 + startx - 1] - dst[(y-1)*dst_linesize_32 + startx - 1];
> +
> +        for (x = startx; x < startx + w; x++) {
> +            const int s1x = av_clip(x -  r,         0, sw - 1);
> +            const int s2x = av_clip(x - (r + offx), 0, sw - 1);
> +            const int s1y = av_clip(y -  r,         0, sh - 1);
> +            const int s2y = av_clip(y - (r + offy), 0, sh - 1);
> +            const uint8_t v1 = src[s1y*linesize + s1x];
> +            const uint8_t v2 = src[s2y*linesize + s2x];
> +            const int d = v1 - v2;
> +            acc += d * d;
> +            dst[y*dst_linesize_32 + x] = dst[(y-1)*dst_linesize_32 + x] + acc;
> +        }

I can understand this is done on smaller portions, but it would still be 
good to (at least) move the y-only parts out of the x loop.

> +    }
> +}
> +
> +/*
> + * Compute the sum of squared difference integral image
> + * http://www.ipol.im/pub/art/2014/57/
> + * Integral Images for Block Matching - Gabriele Facciolo, Nicolas Limare, Enric Meinhardt-Llopis
> + *
> + * @param ii                integral image of dimension (w+e*2) x (h+e*2) with
> + *                          an additional zeroed top line and column already
> + *                          "applied" to the pointer value
> + * @param ii_linesize_32    integral image linesize (in 32-bit integers unit)
> + * @param src               source plane buffer
> + * @param linesize          source plane linesize
> + * @param offx              x-offsetting ranging in [-e;e]
> + * @param offy              y-offsetting ranging in [-e;e]
> + * @param w                 source width
> + * @param h                 source height
> + * @param e                 research padding edge
> + */
> +static void compute_ssd_integral_image(uint32_t *ii, int ii_linesize_32,
> +                                       const uint8_t *src, int linesize, int offx, int offy,
> +                                       int e, int w, int h)
> +{
> +    // ii has a surrounding padding of thickness "e"
> +    const int ii_w = w + e*2;
> +    const int ii_h = h + e*2;
> +
> +    // we center the first source
> +    const int s1x = e;
> +    const int s1y = e;
> +
> +    // 2nd source is the frame with offsetting
> +    const int s2x = e + offx;
> +    const int s2y = e + offy;
> +
> +    // get the dimension of the overlapping rectangle where it is always safe
> +    // to compare the 2 sources pixels
> +    const int startx_safe = FFMAX(s1x, s2x);
> +    const int starty_safe = FFMAX(s1y, s2y);
> +    const int endx_safe   = FFMIN(s1x + w, s2x + w);
> +    const int endy_safe   = FFMIN(s1y + h, s2y + h);
> +
> +    // top part where only one of s1 and s2 is still readable, or none at all
> +    compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
> +                                      0, 0,
> +                                      src, linesize,
> +                                      offx, offy, e, w, h,
> +                                      ii_w, starty_safe);
> +
> +    // fill the left column integral required to compute the central
> +    // overlapping one
> +    compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
> +                                      0, starty_safe,
> +                                      src, linesize,
> +                                      offx, offy, e, w, h,
> +                                      startx_safe, endy_safe - starty_safe);
> +
> +    // main and safe part of the integral
> +    av_assert1(startx_safe - s1x >= 0); av_assert1(startx_safe - s1x < w);
> +    av_assert1(starty_safe - s1y >= 0); av_assert1(starty_safe - s1y < h);
> +    av_assert1(startx_safe - s2x >= 0); av_assert1(startx_safe - s2x < w);
> +    av_assert1(starty_safe - s2y >= 0); av_assert1(starty_safe - s2y < h);
> +    compute_safe_ssd_integral_image_c(ii + starty_safe*ii_linesize_32 + startx_safe, ii_linesize_32,
> +                                      src + (starty_safe - s1y) * linesize + (startx_safe - s1x), linesize,
> +                                      src + (starty_safe - s2y) * linesize + (startx_safe - s2x), linesize,
> +                                      endx_safe - startx_safe, endy_safe - starty_safe);
> +
> +    // right part of the integral
> +    compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
> +                                      endx_safe, starty_safe,
> +                                      src, linesize,
> +                                      offx, offy, e, w, h,
> +                                      ii_w - endx_safe, endy_safe - starty_safe);
> +
> +    // bottom part where only one of s1 and s2 is still readable, or none at all
> +    compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
> +                                      0, endy_safe,
> +                                      src, linesize,
> +                                      offx, offy, e, w, h,
> +                                      ii_w, ii_h - endy_safe);
> +}
> +
> +static int config_input(AVFilterLink *inlink)
> +{
> +    AVFilterContext *ctx = inlink->dst;
> +    NLMeansContext *s = ctx->priv;
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
> +    const int e = FFMAX(s->research_hsize, s->research_hsize_c)
> +                + FFMAX(s->patch_hsize,    s->patch_hsize_c);
> +
> +    s->chroma_w = FF_CEIL_RSHIFT(inlink->w, desc->log2_chroma_w);
> +    s->chroma_h = FF_CEIL_RSHIFT(inlink->h, desc->log2_chroma_h);
> +    s->nb_planes = av_pix_fmt_count_planes(inlink->format);
> +
> +    /* Allocate the integral image with extra edges of thickness "e"
> +     *
> +     *   +_+-------------------------------+
> +     *   |0|0000000000000000000000000000000|
> +     *   +-x-------------------------------+
> +     *   |0|\    ^                         |
> +     *   |0| ii  | e                       |
> +     *   |0|     v                         |
> +     *   |0|   +-----------------------+   |
> +     *   |0|   |                       |   |
> +     *   |0|<->|                       |   |
> +     *   |0| e |                       |   |
> +     *   |0|   |                       |   |
> +     *   |0|   +-----------------------+   |
> +     *   |0|                               |
> +     *   |0|                               |
> +     *   |0|                               |
> +     *   +-+-------------------------------+
> +     */
> +    s->ii_w = inlink->w + e*2;
> +    s->ii_h = inlink->h + e*2;
> +
> +    // align to 4 the linesize, "+1" is for the space of the left 0-column
> +    s->ii_lz_32 = FFALIGN(s->ii_w + 1, 4);
> +
> +    // "+1" is for the space of the top 0-line
> +    s->ii_orig = av_mallocz_array(s->ii_h + 1, s->ii_lz_32 * sizeof(*s->ii_orig));
> +    if (!s->ii_orig)
> +        return AVERROR(ENOMEM);
> +
> +    // skip top 0-line and left 0-column
> +    s->ii = s->ii_orig + s->ii_lz_32 + 1;
> +
> +    // allocate weighted average for every pixel
> +    s->wa_linesize = inlink->w;
> +    s->wa = av_malloc_array(s->wa_linesize, inlink->h * sizeof(*s->wa));
> +    if (!s->wa)
> +        return AVERROR(ENOMEM);

this leaks s->ii_orig

> +
> +    return 0;
> +}
> +

[...]

> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    NLMeansContext *s = ctx->priv;
> +    av_freep(&s->ii_orig);

s->wa also needs to be freed

[...]

Cheers,
Ben



More information about the ffmpeg-devel mailing list