Thanks for the review Andreas, that's indeed helpful.
Allow me some time, I'll come back bit later with revised patch.
________________________________
От: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> от имени Andreas Rheinhardt
<andreas.rheinha...@outlook.com>
Отправлено: 4 октября 2021 г. 11:59
Кому: ffmpeg-devel@ffmpeg.org <ffmpeg-devel@ffmpeg.org>
Тема: Re: [FFmpeg-devel] [PATCH] avfilter: add scale2ref_npp video filter
External email: Use caution opening links or attachments
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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmpeg-devel&data=04%7C01%7Crarzumanyan%40nvidia.com%7C2d27e43ba73646720eb408d9871549ad%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637689348213495919%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=lcu8tWfQOz0ROcat8ipL9kYqp%2FJc5iw8EldqrG6EjdA%3D&reserved=0
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
