[FFmpeg-devel] [PATCH] avfilter: add scale2ref_npp video filter

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Mon Oct 4 11:59:22 EEST 2021


Roman Arzumanyan:
> Thanks for the review, Timo.
> 
> Please find revised patch attached.
> Docstring was added, trailing spaces removed, commit message brushed up. Avfilter minor version bump added.

This has lots of duplication with the ordinary scale filter: var_names,
enum var_name and enum eval_mode are identical; check_exprs and
scale_pars_expr are basically the same. You are even copying bugs from
the original scale filter: Its handling of the AVDictionary is buggy,
because it is supposed to return an AVDictionary with all the
unrecognized/unsupported options instead of just taking all the options.
Given that your filter simply ignores the options your handling of it is
worse, but also more easily fixable: Use the init-callback instead of
the init_dict and remove the unused options completely.

> 
> @@ -366,8 +715,8 @@ static int nppscale_config_props(AVFilterLink *outlink)
>             inlink->w, inlink->h, outlink->w, outlink->h);
>  
>      if (inlink->sample_aspect_ratio.num)
> -        outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h*inlink->w,
> -                                                             outlink->w*inlink->h},
> +        outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h * inlink->w,
> +                                                             outlink->w * inlink->h},
>                                                  inlink->sample_aspect_ratio);
>      else
>          outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;

Cosmetic change; should not be in a functional commit.

> 
> @@ -148,7 +483,9 @@ static int nppscale_query_formats(AVFilterContext *ctx)
>      static const enum AVPixelFormat pixel_formats[] = {
>          AV_PIX_FMT_CUDA, AV_PIX_FMT_NONE,
>      };
> -    return ff_set_common_formats_from_list(ctx, pixel_formats);
> +    AVFilterFormats *pix_fmts = ff_make_format_list(pixel_formats);
> +
> +    return ff_set_common_formats(ctx, pix_fmts);
>  }

ff_set_common_formats_from_list(ctx, pixel_formats) is a shortcut for
ff_set_common_formats(ctx, ff_make_format_list(pixel_formats)), so this
is not a functional change; it just adds more code duplication.

- Andreas


More information about the ffmpeg-devel mailing list