diff mbox series

[FFmpeg-devel,v2] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)

Message ID pull.15.v2.ffstaging.FFmpeg.1641094910660.ffmpegagent@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11) | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

Aman Karmani Jan. 2, 2022, 3:41 a.m. UTC 
From: softworkz <softworkz@hotmail.com>

Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
regression in a way that scaling via vpp_qsv doesn't work any longer
for devices with an MSDK runtime version lower than 1.19. This is true
for older CPUs which are stuck at 1.11.
The commit added checks for the compile-sdk version but it didn't test
for the runtime version.

Signed-off-by: softworkz <softworkz@hotmail.com>
---
    avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
    
    Fix a recently introduced regression when using QSV VPP.
    
    v2: Fixed commit message wrapping

Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-15%2Fsoftworkz%2Fqsv_vpp_regression-v2
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-15/softworkz/qsv_vpp_regression-v2
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15

Range-diff vs v1:

 1:  eea966f702 ! 1:  ccede9d840 avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
     @@ Metadata
       ## Commit message ##
          avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
      
     -    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a regression in a way
     -    that scaling via vpp_qsv didn't work any longer for devices with an MSDK runtime
     -    version lower than 1.19. This is true for older CPUs which are stuck at 1.11.
     -    The commit added checks for the compile-sdk version but it didn't test for the
     -    runtime version.
     +    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
     +    regression in a way that scaling via vpp_qsv doesn't work any longer
     +    for devices with an MSDK runtime version lower than 1.19. This is true
     +    for older CPUs which are stuck at 1.11.
     +    The commit added checks for the compile-sdk version but it didn't test
     +    for the runtime version.
      
          Signed-off-by: softworkz <softworkz@hotmail.com>
      


 libavfilter/vf_vpp_qsv.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)


base-commit: d6b2357eddca392ee137cb2a92ff178a0a7d0cce

Comments

Xiang, Haihao Jan. 4, 2022, 2:16 a.m. UTC | #1 
On Sun, 2022-01-02 at 03:41 +0000, ffmpegagent wrote:
> From: softworkz <softworkz@hotmail.com>
> 
> Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> regression in a way that scaling via vpp_qsv doesn't work any longer
> for devices with an MSDK runtime version lower than 1.19. This is true
> for older CPUs which are stuck at 1.11.
> The commit added checks for the compile-sdk version but it didn't test
> for the runtime version.
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>     avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
>     
>     Fix a recently introduced regression when using QSV VPP.
>     
>     v2: Fixed commit message wrapping
> 
> Published-As: 
> https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-15%2Fsoftworkz%2Fqsv_vpp_regression-v2
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> 15/softworkz/qsv_vpp_regression-v2
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> 
> Range-diff vs v1:
> 
>  1:  eea966f702 ! 1:  ccede9d840 avfilter/vpp_qsv: fix regression on older api
> versions (e.g. 1.11)
>      @@ Metadata
>        ## Commit message ##
>           avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
>       
>      -    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> regression in a way
>      -    that scaling via vpp_qsv didn't work any longer for devices with an
> MSDK runtime
>      -    version lower than 1.19. This is true for older CPUs which are stuck
> at 1.11.
>      -    The commit added checks for the compile-sdk version but it didn't
> test for the
>      -    runtime version.
>      +    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
>      +    regression in a way that scaling via vpp_qsv doesn't work any longer
>      +    for devices with an MSDK runtime version lower than 1.19. This is
> true
>      +    for older CPUs which are stuck at 1.11.
>      +    The commit added checks for the compile-sdk version but it didn't
> test
>      +    for the runtime version.
>       
>           Signed-off-by: softworkz <softworkz@hotmail.com>
>       
> 
> 
>  libavfilter/vf_vpp_qsv.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> index ab58a5777e..09590157e3 100644
> --- a/libavfilter/vf_vpp_qsv.c
> +++ b/libavfilter/vf_vpp_qsv.c
> @@ -139,8 +139,9 @@ static const AVOption options[] = {
>      { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, {
> .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
>      { "format", "Output pixel format", OFFSET(output_format_str),
> AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
>      { "async_depth", "Internal parallelization depth, the higher the value
> the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, INT_MAX, .flags = FLAGS },
> +#ifdef QSV_HAVE_SCALING_CONFIG
>      { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality",
> OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT },
> MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale
> mode" },
> -
> +#endif
>      { NULL }
>  };
>  
> @@ -304,6 +305,7 @@ static int config_output(AVFilterLink *outlink)
>      QSVVPPParam     param = { NULL };
>      QSVVPPCrop      crop  = { 0 };
>      mfxExtBuffer    *ext_buf[ENH_FILTERS_COUNT];
> +    mfxVersion      mfx_version;
>      AVFilterLink    *inlink = ctx->inputs[0];
>      enum AVPixelFormat in_format;
>  
> @@ -317,6 +319,11 @@ static int config_output(AVFilterLink *outlink)
>      param.ext_buf       = ext_buf;
>      param.async_depth   = vpp->async_depth;
>  
> +    if (MFXQueryVersion(vpp->qsv->session, &mfx_version) < 0) {
> +        av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
> +        return AVERROR(EINVAL);
> +    }
> +
>      if (inlink->format == AV_PIX_FMT_QSV) {
>           if (!inlink->hw_frames_ctx || !inlink->hw_frames_ctx->data)
>               return AVERROR(EINVAL);
> @@ -467,19 +474,20 @@ static int config_output(AVFilterLink *outlink)
>  #endif
>      }
>  
> -    if (inlink->w != outlink->w || inlink->h != outlink->h) {
>  #ifdef QSV_HAVE_SCALING_CONFIG
> -        memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> -        vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> -        vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> -        vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> -
> -        param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp->scale_conf;
> -#else
> -        av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> -            "not supported with this MSDK version.\n");
> -#endif
> +    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> +        if (QSV_RUNTIME_VERSION_ATLEAST(mfx_version, 1, 19)) {
> +            memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> +            vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> +            vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> +            vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> +
> +            param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> >scale_conf;
> +        } else
> +            av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> +                "not supported with this MSDK version.\n");
>      }
> +#endif
>  
>      if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise ||
>          vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||

LGTM. I wonder whether you installed libmfx and msdk runtime from 
different packages, so the mfx version is higher than the runtime version? 

Thanks
Haihao


> 
> base-commit: d6b2357eddca392ee137cb2a92ff178a0a7d0cce
Soft Works Jan. 4, 2022, 2:25 a.m. UTC | #2 
> -----Original Message-----
> From: Xiang, Haihao <haihao.xiang@intel.com>
> Sent: Tuesday, January 4, 2022 3:16 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: softworkz@hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression on
> older api versions (e.g. 1.11)
> 
> On Sun, 2022-01-02 at 03:41 +0000, ffmpegagent wrote:
> > From: softworkz <softworkz@hotmail.com>
> >
> > Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > regression in a way that scaling via vpp_qsv doesn't work any longer
> > for devices with an MSDK runtime version lower than 1.19. This is true
> > for older CPUs which are stuck at 1.11.
> > The commit added checks for the compile-sdk version but it didn't test
> > for the runtime version.
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >     avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> >
> >     Fix a recently introduced regression when using QSV VPP.
> >
> >     v2: Fixed commit message wrapping
> >
> > Published-As:
> > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> 15%2Fsoftworkz%2Fqsv_vpp_regression-v2
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> > 15/softworkz/qsv_vpp_regression-v2
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> >
> > Range-diff vs v1:
> >
> >  1:  eea966f702 ! 1:  ccede9d840 avfilter/vpp_qsv: fix regression on older
> api
> > versions (e.g. 1.11)
> >      @@ Metadata
> >        ## Commit message ##
> >           avfilter/vpp_qsv: fix regression on older api versions (e.g.
> 1.11)
> >
> >      -    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > regression in a way
> >      -    that scaling via vpp_qsv didn't work any longer for devices with
> an
> > MSDK runtime
> >      -    version lower than 1.19. This is true for older CPUs which are
> stuck
> > at 1.11.
> >      -    The commit added checks for the compile-sdk version but it didn't
> > test for the
> >      -    runtime version.
> >      +    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> >      +    regression in a way that scaling via vpp_qsv doesn't work any
> longer
> >      +    for devices with an MSDK runtime version lower than 1.19. This is
> > true
> >      +    for older CPUs which are stuck at 1.11.
> >      +    The commit added checks for the compile-sdk version but it didn't
> > test
> >      +    for the runtime version.
> >
> >           Signed-off-by: softworkz <softworkz@hotmail.com>
> >
> >
> >
> >  libavfilter/vf_vpp_qsv.c | 32 ++++++++++++++++++++------------
> >  1 file changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > index ab58a5777e..09590157e3 100644
> > --- a/libavfilter/vf_vpp_qsv.c
> > +++ b/libavfilter/vf_vpp_qsv.c
> > @@ -139,8 +139,9 @@ static const AVOption options[] = {
> >      { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, {
> > .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
> >      { "format", "Output pixel format", OFFSET(output_format_str),
> > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> >      { "async_depth", "Internal parallelization depth, the higher the value
> > the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0
> },
> > 0, INT_MAX, .flags = FLAGS },
> > +#ifdef QSV_HAVE_SCALING_CONFIG
> >      { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality",
> > OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT },
> > MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale
> > mode" },
> > -
> > +#endif
> >      { NULL }
> >  };
> >
> > @@ -304,6 +305,7 @@ static int config_output(AVFilterLink *outlink)
> >      QSVVPPParam     param = { NULL };
> >      QSVVPPCrop      crop  = { 0 };
> >      mfxExtBuffer    *ext_buf[ENH_FILTERS_COUNT];
> > +    mfxVersion      mfx_version;
> >      AVFilterLink    *inlink = ctx->inputs[0];
> >      enum AVPixelFormat in_format;
> >
> > @@ -317,6 +319,11 @@ static int config_output(AVFilterLink *outlink)
> >      param.ext_buf       = ext_buf;
> >      param.async_depth   = vpp->async_depth;
> >
> > +    if (MFXQueryVersion(vpp->qsv->session, &mfx_version) < 0) {
> > +        av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> >      if (inlink->format == AV_PIX_FMT_QSV) {
> >           if (!inlink->hw_frames_ctx || !inlink->hw_frames_ctx->data)
> >               return AVERROR(EINVAL);
> > @@ -467,19 +474,20 @@ static int config_output(AVFilterLink *outlink)
> >  #endif
> >      }
> >
> > -    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> >  #ifdef QSV_HAVE_SCALING_CONFIG
> > -        memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> > -        vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> > -        vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> > -        vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> > -
> > -        param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> >scale_conf;
> > -#else
> > -        av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> > -            "not supported with this MSDK version.\n");
> > -#endif
> > +    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> > +        if (QSV_RUNTIME_VERSION_ATLEAST(mfx_version, 1, 19)) {
> > +            memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> > +            vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> > +            vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> > +            vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> > +
> > +            param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> > >scale_conf;
> > +        } else
> > +            av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> > +                "not supported with this MSDK version.\n");
> >      }
> > +#endif
> >
> >      if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise
> ||
> >          vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
> 
> LGTM. I wonder whether you installed libmfx and msdk runtime from
> different packages, so the mfx version is higher than the runtime version?

It's about a user with a Broadwell CPU on Windows. The latest drivers' MSDK 
version is 1.11 and our ffmpeg is compiled against MSDK 1.35.
(a setup that is basically working fine, just the runtime checks are missing 
at several places)

Thanks,
softworkz
Xiang, Haihao Jan. 4, 2022, 2:37 a.m. UTC | #3 
On Tue, 2022-01-04 at 02:25 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: Xiang, Haihao <haihao.xiang@intel.com>
> > Sent: Tuesday, January 4, 2022 3:16 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: softworkz@hotmail.com
> > Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression on
> > older api versions (e.g. 1.11)
> > 
> > On Sun, 2022-01-02 at 03:41 +0000, ffmpegagent wrote:
> > > From: softworkz <softworkz@hotmail.com>
> > > 
> > > Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > regression in a way that scaling via vpp_qsv doesn't work any longer
> > > for devices with an MSDK runtime version lower than 1.19. This is true
> > > for older CPUs which are stuck at 1.11.
> > > The commit added checks for the compile-sdk version but it didn't test
> > > for the runtime version.
> > > 
> > > Signed-off-by: softworkz <softworkz@hotmail.com>
> > > ---
> > >     avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> > > 
> > >     Fix a recently introduced regression when using QSV VPP.
> > > 
> > >     v2: Fixed commit message wrapping
> > > 
> > > Published-As:
> > > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> > 
> > 15%2Fsoftworkz%2Fqsv_vpp_regression-v2
> > > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> > > 15/softworkz/qsv_vpp_regression-v2
> > > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> > > 
> > > Range-diff vs v1:
> > > 
> > >  1:  eea966f702 ! 1:  ccede9d840 avfilter/vpp_qsv: fix regression on older
> > 
> > api
> > > versions (e.g. 1.11)
> > >      @@ Metadata
> > >        ## Commit message ##
> > >           avfilter/vpp_qsv: fix regression on older api versions (e.g.
> > 
> > 1.11)
> > > 
> > >      -    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > regression in a way
> > >      -    that scaling via vpp_qsv didn't work any longer for devices with
> > 
> > an
> > > MSDK runtime
> > >      -    version lower than 1.19. This is true for older CPUs which are
> > 
> > stuck
> > > at 1.11.
> > >      -    The commit added checks for the compile-sdk version but it
> > > didn't
> > > test for the
> > >      -    runtime version.
> > >      +    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > >      +    regression in a way that scaling via vpp_qsv doesn't work any
> > 
> > longer
> > >      +    for devices with an MSDK runtime version lower than 1.19. This
> > > is
> > > true
> > >      +    for older CPUs which are stuck at 1.11.
> > >      +    The commit added checks for the compile-sdk version but it
> > > didn't
> > > test
> > >      +    for the runtime version.
> > > 
> > >           Signed-off-by: softworkz <softworkz@hotmail.com>
> > > 
> > > 
> > > 
> > >  libavfilter/vf_vpp_qsv.c | 32 ++++++++++++++++++++------------
> > >  1 file changed, 20 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > > index ab58a5777e..09590157e3 100644
> > > --- a/libavfilter/vf_vpp_qsv.c
> > > +++ b/libavfilter/vf_vpp_qsv.c
> > > @@ -139,8 +139,9 @@ static const AVOption options[] = {
> > >      { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, {
> > > .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
> > >      { "format", "Output pixel format", OFFSET(output_format_str),
> > > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> > >      { "async_depth", "Internal parallelization depth, the higher the
> > > value
> > > the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0
> > 
> > },
> > > 0, INT_MAX, .flags = FLAGS },
> > > +#ifdef QSV_HAVE_SCALING_CONFIG
> > >      { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality",
> > > OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT },
> > > MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale
> > > mode" },
> > > -
> > > +#endif
> > >      { NULL }
> > >  };
> > > 
> > > @@ -304,6 +305,7 @@ static int config_output(AVFilterLink *outlink)
> > >      QSVVPPParam     param = { NULL };
> > >      QSVVPPCrop      crop  = { 0 };
> > >      mfxExtBuffer    *ext_buf[ENH_FILTERS_COUNT];
> > > +    mfxVersion      mfx_version;
> > >      AVFilterLink    *inlink = ctx->inputs[0];
> > >      enum AVPixelFormat in_format;
> > > 
> > > @@ -317,6 +319,11 @@ static int config_output(AVFilterLink *outlink)
> > >      param.ext_buf       = ext_buf;
> > >      param.async_depth   = vpp->async_depth;
> > > 
> > > +    if (MFXQueryVersion(vpp->qsv->session, &mfx_version) < 0) {
> > > +        av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
> > > +        return AVERROR(EINVAL);
> > > +    }
> > > +
> > >      if (inlink->format == AV_PIX_FMT_QSV) {
> > >           if (!inlink->hw_frames_ctx || !inlink->hw_frames_ctx->data)
> > >               return AVERROR(EINVAL);
> > > @@ -467,19 +474,20 @@ static int config_output(AVFilterLink *outlink)
> > >  #endif
> > >      }
> > > 
> > > -    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> > >  #ifdef QSV_HAVE_SCALING_CONFIG
> > > -        memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> > > -        vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> > > -        vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> > > -        vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> > > -
> > > -        param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> > > scale_conf;
> > > -#else
> > > -        av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> > > -            "not supported with this MSDK version.\n");
> > > -#endif
> > > +    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> > > +        if (QSV_RUNTIME_VERSION_ATLEAST(mfx_version, 1, 19)) {
> > > +            memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> > > +            vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> > > +            vpp->scale_conf.Header.BufferSz    =
> > > sizeof(mfxExtVPPScaling);
> > > +            vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> > > +
> > > +            param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> > > > scale_conf;
> > > 
> > > +        } else
> > > +            av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> > > +                "not supported with this MSDK version.\n");
> > >      }
> > > +#endif
> > > 
> > >      if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise
> > > > 
> > > 
> > >          vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
> > 
> > LGTM. I wonder whether you installed libmfx and msdk runtime from
> > different packages, so the mfx version is higher than the runtime version?
> 
> It's about a user with a Broadwell CPU on Windows. The latest drivers' MSDK 
> version is 1.11 and our ffmpeg is compiled against MSDK 1.35.
> (a setup that is basically working fine, just the runtime checks are missing 
> at several places)

Thanks for answer, I will apply your patch soon.

BRs
Haihao
Soft Works Jan. 4, 2022, 2:42 a.m. UTC | #4 
> -----Original Message-----
> From: Xiang, Haihao <haihao.xiang@intel.com>
> Sent: Tuesday, January 4, 2022 3:37 AM
> To: ffmpeg-devel@ffmpeg.org; softworkz@hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression on
> older api versions (e.g. 1.11)
> 
> On Tue, 2022-01-04 at 02:25 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: Xiang, Haihao <haihao.xiang@intel.com>
> > > Sent: Tuesday, January 4, 2022 3:16 AM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: softworkz@hotmail.com
> > > Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression
> on
> > > older api versions (e.g. 1.11)
> > >
> > > On Sun, 2022-01-02 at 03:41 +0000, ffmpegagent wrote:
> > > > From: softworkz <softworkz@hotmail.com>
> > > >
> > > > Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > > regression in a way that scaling via vpp_qsv doesn't work any longer
> > > > for devices with an MSDK runtime version lower than 1.19. This is true
> > > > for older CPUs which are stuck at 1.11.
> > > > The commit added checks for the compile-sdk version but it didn't test
> > > > for the runtime version.
> > > >
> > > > Signed-off-by: softworkz <softworkz@hotmail.com>
> > > > ---
> > > >     avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> > > >
> > > >     Fix a recently introduced regression when using QSV VPP.
> > > >
> > > >     v2: Fixed commit message wrapping
> > > >
> > > > Published-As:
> > > > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> > >
> > > 15%2Fsoftworkz%2Fqsv_vpp_regression-v2
> > > > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-
> > > > 15/softworkz/qsv_vpp_regression-v2
> > > > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> > > >
> > > > Range-diff vs v1:
> > > >
> > > >  1:  eea966f702 ! 1:  ccede9d840 avfilter/vpp_qsv: fix regression on
> older
> > >
> > > api
> > > > versions (e.g. 1.11)
> > > >      @@ Metadata
> > > >        ## Commit message ##
> > > >           avfilter/vpp_qsv: fix regression on older api versions (e.g.
> > >
> > > 1.11)
> > > >
> > > >      -    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > > regression in a way
> > > >      -    that scaling via vpp_qsv didn't work any longer for devices
> with
> > >
> > > an
> > > > MSDK runtime
> > > >      -    version lower than 1.19. This is true for older CPUs which
> are
> > >
> > > stuck
> > > > at 1.11.
> > > >      -    The commit added checks for the compile-sdk version but it
> > > > didn't
> > > > test for the
> > > >      -    runtime version.
> > > >      +    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > >      +    regression in a way that scaling via vpp_qsv doesn't work any
> > >
> > > longer
> > > >      +    for devices with an MSDK runtime version lower than 1.19.
> This
> > > > is
> > > > true
> > > >      +    for older CPUs which are stuck at 1.11.
> > > >      +    The commit added checks for the compile-sdk version but it
> > > > didn't
> > > > test
> > > >      +    for the runtime version.
> > > >
> > > >           Signed-off-by: softworkz <softworkz@hotmail.com>
> > > >
> > > >
> > > >
> > > >  libavfilter/vf_vpp_qsv.c | 32 ++++++++++++++++++++------------
> > > >  1 file changed, 20 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > > > index ab58a5777e..09590157e3 100644
> > > > --- a/libavfilter/vf_vpp_qsv.c
> > > > +++ b/libavfilter/vf_vpp_qsv.c
> > > > @@ -139,8 +139,9 @@ static const AVOption options[] = {
> > > >      { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING,
> {
> > > > .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
> > > >      { "format", "Output pixel format", OFFSET(output_format_str),
> > > > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> > > >      { "async_depth", "Internal parallelization depth, the higher the
> > > > value
> > > > the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64
> = 0
> > >
> > > },
> > > > 0, INT_MAX, .flags = FLAGS },
> > > > +#ifdef QSV_HAVE_SCALING_CONFIG
> > > >      { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality",
> > > > OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT
> },
> > > > MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS,
> "scale
> > > > mode" },
> > > > -
> > > > +#endif
> > > >      { NULL }
> > > >  };
> > > >
> > > > @@ -304,6 +305,7 @@ static int config_output(AVFilterLink *outlink)
> > > >      QSVVPPParam     param = { NULL };
> > > >      QSVVPPCrop      crop  = { 0 };
> > > >      mfxExtBuffer    *ext_buf[ENH_FILTERS_COUNT];
> > > > +    mfxVersion      mfx_version;
> > > >      AVFilterLink    *inlink = ctx->inputs[0];
> > > >      enum AVPixelFormat in_format;
> > > >
> > > > @@ -317,6 +319,11 @@ static int config_output(AVFilterLink *outlink)
> > > >      param.ext_buf       = ext_buf;
> > > >      param.async_depth   = vpp->async_depth;
> > > >
> > > > +    if (MFXQueryVersion(vpp->qsv->session, &mfx_version) < 0) {
> > > > +        av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
> > > > +        return AVERROR(EINVAL);
> > > > +    }
> > > > +
> > > >      if (inlink->format == AV_PIX_FMT_QSV) {
> > > >           if (!inlink->hw_frames_ctx || !inlink->hw_frames_ctx->data)
> > > >               return AVERROR(EINVAL);
> > > > @@ -467,19 +474,20 @@ static int config_output(AVFilterLink *outlink)
> > > >  #endif
> > > >      }
> > > >
> > > > -    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> > > >  #ifdef QSV_HAVE_SCALING_CONFIG
> > > > -        memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> > > > -        vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> > > > -        vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> > > > -        vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> > > > -
> > > > -        param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> > > > scale_conf;
> > > > -#else
> > > > -        av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> > > > -            "not supported with this MSDK version.\n");
> > > > -#endif
> > > > +    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> > > > +        if (QSV_RUNTIME_VERSION_ATLEAST(mfx_version, 1, 19)) {
> > > > +            memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> > > > +            vpp->scale_conf.Header.BufferId    =
> MFX_EXTBUFF_VPP_SCALING;
> > > > +            vpp->scale_conf.Header.BufferSz    =
> > > > sizeof(mfxExtVPPScaling);
> > > > +            vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> > > > +
> > > > +            param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> > > > > scale_conf;
> > > >
> > > > +        } else
> > > > +            av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> > > > +                "not supported with this MSDK version.\n");
> > > >      }
> > > > +#endif
> > > >
> > > >      if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp-
> >denoise
> > > > >
> > > >
> > > >          vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
> > >
> > > LGTM. I wonder whether you installed libmfx and msdk runtime from
> > > different packages, so the mfx version is higher than the runtime
> version?
> >
> > It's about a user with a Broadwell CPU on Windows. The latest drivers' MSDK
> > version is 1.11 and our ffmpeg is compiled against MSDK 1.35.
> > (a setup that is basically working fine, just the runtime checks are
> missing
> > at several places)
> 
> Thanks for answer, I will apply your patch soon.


Thanks - just wait what the others will say, whether they want to have it in 5.0
or not..

sw
Xiang, Haihao Jan. 4, 2022, 8:09 a.m. UTC | #5 
On Sun, 2022-01-02 at 03:41 +0000, ffmpegagent wrote:
> From: softworkz <softworkz@hotmail.com>
> 
> Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> regression in a way that scaling via vpp_qsv doesn't work any longer
> for devices with an MSDK runtime version lower than 1.19. This is true
> for older CPUs which are stuck at 1.11.
> The commit added checks for the compile-sdk version but it didn't test
> for the runtime version.
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>     avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
>     
>     Fix a recently introduced regression when using QSV VPP.
>     
>     v2: Fixed commit message wrapping
> 
> Published-As: 
> https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-15%2Fsoftworkz%2Fqsv_vpp_regression-v2
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> 15/softworkz/qsv_vpp_regression-v2
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> 
> Range-diff vs v1:
> 
>  1:  eea966f702 ! 1:  ccede9d840 avfilter/vpp_qsv: fix regression on older api
> versions (e.g. 1.11)
>      @@ Metadata
>        ## Commit message ##
>           avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
>       
>      -    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> regression in a way
>      -    that scaling via vpp_qsv didn't work any longer for devices with an
> MSDK runtime
>      -    version lower than 1.19. This is true for older CPUs which are stuck
> at 1.11.
>      -    The commit added checks for the compile-sdk version but it didn't
> test for the
>      -    runtime version.
>      +    Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
>      +    regression in a way that scaling via vpp_qsv doesn't work any longer
>      +    for devices with an MSDK runtime version lower than 1.19. This is
> true
>      +    for older CPUs which are stuck at 1.11.
>      +    The commit added checks for the compile-sdk version but it didn't
> test
>      +    for the runtime version.
>       
>           Signed-off-by: softworkz <softworkz@hotmail.com>
>       
> 
> 
>  libavfilter/vf_vpp_qsv.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> index ab58a5777e..09590157e3 100644
> --- a/libavfilter/vf_vpp_qsv.c
> +++ b/libavfilter/vf_vpp_qsv.c
> @@ -139,8 +139,9 @@ static const AVOption options[] = {
>      { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, {
> .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
>      { "format", "Output pixel format", OFFSET(output_format_str),
> AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
>      { "async_depth", "Internal parallelization depth, the higher the value
> the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, INT_MAX, .flags = FLAGS },
> +#ifdef QSV_HAVE_SCALING_CONFIG
>      { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality",
> OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT },
> MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale
> mode" },
> -
> +#endif
>      { NULL }
>  };
>  
> @@ -304,6 +305,7 @@ static int config_output(AVFilterLink *outlink)
>      QSVVPPParam     param = { NULL };
>      QSVVPPCrop      crop  = { 0 };
>      mfxExtBuffer    *ext_buf[ENH_FILTERS_COUNT];
> +    mfxVersion      mfx_version;
>      AVFilterLink    *inlink = ctx->inputs[0];
>      enum AVPixelFormat in_format;
>  
> @@ -317,6 +319,11 @@ static int config_output(AVFilterLink *outlink)
>      param.ext_buf       = ext_buf;
>      param.async_depth   = vpp->async_depth;
>  
> +    if (MFXQueryVersion(vpp->qsv->session, &mfx_version) < 0) {

It resulted in segmentation fault in testing because vpp->qsv is NULL here. Note
vpp->qsv is set in ff_qsvvpp_create.

Thanks
Haihao


> +        av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
> +        return AVERROR(EINVAL);
> +    }
> +
>      if (inlink->format == AV_PIX_FMT_QSV) {
>           if (!inlink->hw_frames_ctx || !inlink->hw_frames_ctx->data)
>               return AVERROR(EINVAL);
> @@ -467,19 +474,20 @@ static int config_output(AVFilterLink *outlink)
>  #endif
>      }
>  
> -    if (inlink->w != outlink->w || inlink->h != outlink->h) {
>  #ifdef QSV_HAVE_SCALING_CONFIG
> -        memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> -        vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> -        vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> -        vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> -
> -        param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp->scale_conf;
> -#else
> -        av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> -            "not supported with this MSDK version.\n");
> -#endif
> +    if (inlink->w != outlink->w || inlink->h != outlink->h) {
> +        if (QSV_RUNTIME_VERSION_ATLEAST(mfx_version, 1, 19)) {
> +            memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
> +            vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
> +            vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
> +            vpp->scale_conf.ScalingMode        = vpp->scale_mode;
> +
> +            param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp-
> >scale_conf;
> +        } else
> +            av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
> +                "not supported with this MSDK version.\n");
>      }
> +#endif
>  
>      if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise ||
>          vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
> 
> base-commit: d6b2357eddca392ee137cb2a92ff178a0a7d0cce
diff mbox series

Patch

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index ab58a5777e..09590157e3 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -139,8 +139,9 @@  static const AVOption options[] = {
     { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
     { "format", "Output pixel format", OFFSET(output_format_str), AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
     { "async_depth", "Internal parallelization depth, the higher the value the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, .flags = FLAGS },
+#ifdef QSV_HAVE_SCALING_CONFIG
     { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality", OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT }, MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale mode" },
-
+#endif
     { NULL }
 };
 
@@ -304,6 +305,7 @@  static int config_output(AVFilterLink *outlink)
     QSVVPPParam     param = { NULL };
     QSVVPPCrop      crop  = { 0 };
     mfxExtBuffer    *ext_buf[ENH_FILTERS_COUNT];
+    mfxVersion      mfx_version;
     AVFilterLink    *inlink = ctx->inputs[0];
     enum AVPixelFormat in_format;
 
@@ -317,6 +319,11 @@  static int config_output(AVFilterLink *outlink)
     param.ext_buf       = ext_buf;
     param.async_depth   = vpp->async_depth;
 
+    if (MFXQueryVersion(vpp->qsv->session, &mfx_version) < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
+        return AVERROR(EINVAL);
+    }
+
     if (inlink->format == AV_PIX_FMT_QSV) {
          if (!inlink->hw_frames_ctx || !inlink->hw_frames_ctx->data)
              return AVERROR(EINVAL);
@@ -467,19 +474,20 @@  static int config_output(AVFilterLink *outlink)
 #endif
     }
 
-    if (inlink->w != outlink->w || inlink->h != outlink->h) {
 #ifdef QSV_HAVE_SCALING_CONFIG
-        memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
-        vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
-        vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
-        vpp->scale_conf.ScalingMode        = vpp->scale_mode;
-
-        param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp->scale_conf;
-#else
-        av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
-            "not supported with this MSDK version.\n");
-#endif
+    if (inlink->w != outlink->w || inlink->h != outlink->h) {
+        if (QSV_RUNTIME_VERSION_ATLEAST(mfx_version, 1, 19)) {
+            memset(&vpp->scale_conf, 0, sizeof(mfxExtVPPScaling));
+            vpp->scale_conf.Header.BufferId    = MFX_EXTBUFF_VPP_SCALING;
+            vpp->scale_conf.Header.BufferSz    = sizeof(mfxExtVPPScaling);
+            vpp->scale_conf.ScalingMode        = vpp->scale_mode;
+
+            param.ext_buf[param.num_ext_buf++] = (mfxExtBuffer*)&vpp->scale_conf;
+        } else
+            av_log(ctx, AV_LOG_WARNING, "The QSV VPP Scale option is "
+                "not supported with this MSDK version.\n");
     }
+#endif
 
     if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise ||
         vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||