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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
