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

[Xiang, Haihao]

On Do, 2023-01-12 at 05:49 +0100, Andreas Rheinhardt wrote:
> Xiang, Haihao:
> > On Do, 2023-01-12 at 05:11 +0100, Andreas Rheinhardt wrote:
> > > Xiang, Haihao:
> > > > From: Haihao Xiang <haihao.xiang at 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 at 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?

I want to make sure these filters (include vpp_qsv filter) have the same default
values for options at preinit stage. 

Thanks
Haihao

> 
> - Andreas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".