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

Paul B Mahol onemda at gmail.com
Sat Jul 29 12:51:36 EEST 2017


On 7/29/17, Moritz Barsnick <barsnick at gmx.net> wrote:
> 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?

Remove this one message.


More information about the ffmpeg-devel mailing list