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

Moritz Barsnick barsnick at gmx.net
Sat Jul 29 12:45:58 EEST 2017


On Sat, Jul 29, 2017 at 14:16:29 +0530, Ashish Pratap Singh wrote:

> +Both input videos must have the same resolution and pixel format for
> +this filter to work correctly. Also it assumes that both inputs
> +have the same number of frames, which are compared one by one.

Nit, for all these filters: If the inputs are checked, you should
probably just write "must have the same resolution and pixel format",
or "must have the same resolution and pixel format to work". Since it's
checked, there's no chance of working incorrectly.

> +const int vif_filter1d_width1[4] = { 17, 9, 5, 3 };
> +const float vif_filter1d_table[4][17] = {
> +    { 0x1.e8a77p-8,  0x1.d373b2p-7, 0x1.9a1cf6p-6, 0x1.49fd9ep-5, 0x1.e7092ep-5,

My brain has a hard time interpreting scientific hex notation, but I
guess it's fine. ;-)

> +                num_val = 1.0 - sigma2_sq*sigma_max_inv;
> +                den_val = 1.0;

1.0f to avoid double promotion.

> +    if (*score_den == 0.0) {
> +        *score = 1.0f;

Nit: I know the compiler fixes the right hand side, but since you're
assigning to a double, you shouldn't force the const to a float. "0.0"
or "0" as you chose elsewhere is more consistent.

> +    int i,j; \
[...]
> +    for(i = 0; i < h; i++) { \
> +        for(j = 0; j < w; j++) { \

Minor nit: Whitespace. "for (", "i, j"

> +        av_log(ctx, AV_LOG_ERROR, "Width and height of input videos must be same.\n");

Either "must be the same" or "must be identical".

> +        av_log(ctx, AV_LOG_ERROR, "error: av_malloc failed for data_buf.\n");

Are these messages of any help to anyone?

Moritz


More information about the ffmpeg-devel mailing list