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

[Moritz Barsnick]

On Mon, Jul 03, 2017 at 18:39:38 +0530, Ashish Singh wrote:
> This is ANSNR filter, one of the component filters of VMAF.

Would you please expand the abbreviation in at least one place?
Optimally in doc/filters.texi, the content of which is missing (as it
still is in your VMAF patch). (And perhaps in the header of the .c
file.)

> +++ b/libavfilter/ansnr.h
[...]
> +#ifndef AVFILTER_PSNR_H
> +#define AVFILTER_PSNR_H

Incorrect header guard.

> +const float ansnr_filter2d_dis[5 * 5] = {
> +    2.0 / 571.0,  7.0 / 571.0,  12.0 / 571.0,  7.0 / 571.0,  2.0 / 571.0,
> +    7.0 / 571.0,  31.0 / 571.0, 52.0 / 571.0,  31.0 / 571.0, 7.0 / 571.0,
> +    12.0 / 571.0, 52.0 / 571.0, 127.0 / 571.0, 52.0 / 571.0, 12.0 / 571.0,
> +    7.0 / 571.0,  31.0 / 571.0, 52.0 / 571.0,  31.0 / 571.0, 7.0 / 571.0,
> +    2.0 / 571.0,  7.0 / 571.0,  12.0 / 571.0,  7.0 / 571.0,  2.0 / 571.0

If you can align these along the decimal dots, it's easier to read.

> +    for (i = 0; i < h; ++i) {
> +        for (j = 0; j < w; ++j) {

I believe ffmpeg style prefers "i++".

> +            signal_sum   += pow_2(ref[ref_ind]);
> +            noise_sum += pow_2(ref[ref_ind] - dis[dis_ind]);

Whitespace.

> +    *score = (noise==0) ? (psnr_max) : (10.0 * log10(signal / noise));
> +
> +    *score_psnr = FFMIN(10 * log10(pow_2(peak) * w * h / FFMAX(noise, eps)),
> +                        psnr_max);

Inconsistent: Why 10.0 in the first case, 10 in the second?

> +    double score = 0.0;
> +    double score_psnr = 0;

Inconsistent: Why 0.0 in the first case, 0 in the second?

Moritz