Re: [FFmpeg-devel] [PATCH 4/8] lavfi/vf_vpp_qsv: add vpp_preinit callback

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Xiang, Haihao:
> On Do, 2023-01-12 at 05:11 +0100, Andreas Rheinhardt wrote:
>> Xiang, Haihao:
>>> From: Haihao Xiang <haihao.xiang@intel.com>
>>>
>>> Set the expected default value for options in this callback, hence we
>>> have the right values even if these options are not included in the
>>> option arrray.
>>>
>>> This is in preparation for reusing the code for other QSV filters.
>>>
>>> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
>>> ---
>>>  libavfilter/vf_vpp_qsv.c | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
>>> index cf11cd7fdc..2a7b06fa33 100644
>>> --- a/libavfilter/vf_vpp_qsv.c
>>> +++ b/libavfilter/vf_vpp_qsv.c
>>> @@ -259,6 +259,19 @@ release:
>>>      return ret;
>>>  }
>>>  
>>> +static av_cold int vpp_preinit(AVFilterContext *ctx)
>>> +{
>>> +    VPPContext  *vpp  = ctx->priv;
>>> +    /* For AV_OPT_TYPE_STRING options, NULL is handled in other way so
>>> +     * we needn't set default value here
>>> +     */
>>> +    vpp->saturation= 1.0;
>>> +    vpp->contrast = 1.0;
>>> +    vpp->transpose = -1;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  static av_cold int vpp_init(AVFilterContext *ctx)
>>>  {
>>>      VPPContext  *vpp  = ctx->priv;
>>> @@ -683,6 +696,7 @@ const AVFilter ff_vf_vpp_qsv = {
>>>      .name          = "vpp_qsv",
>>>      .description   = NULL_IF_CONFIG_SMALL("Quick Sync Video VPP."),
>>>      .priv_size     = sizeof(VPPContext),
>>> +    .preinit       = vpp_preinit,
>>>      .init          = vpp_init,
>>>      .uninit        = vpp_uninit,
>>>      FILTER_INPUTS(vpp_inputs),
>>
>> I do not get the point of this at all: None of these options are of type
>> AV_OPT_TYPE_STRING. You are merely setting the default values that
>> av_opt_set_defaults() would set lateron anyway.
> 
>>
>> I also fail to see how this would facilitate reusing this code for other
>> QSV filters.
> 
> Actually this is a part of an old patchset, see 
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4467, I want to use use
> functions and data structures defined in vf_vpp_qsv.c and qsvvpp.c to implement
> scale_qsv and deinterlace_qsv filters, hence we may remove vf_scale_qsv.c and
> vf_deinterlace_qsv.c 
> 
> E.g.
> saturation option is used in vpp_qsv filter but not in other qsv filters, so
> av_opt_set_defaults() works for saturation in vpp_qsv filter but not for other
> filters, we need to set a default value to vpp->saturation for other filters,
> otherwise vpp->saturation is 0 for other other filters. 
>  

If you need this for other filters, then why do you add and use a
preinit callback for the filter that doesn't need it?

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".