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

[Soft Works]

> -----Original Message-----
> From: Xiang, Haihao <haihao.xiang at intel.com>
> Sent: Friday, January 7, 2022 7:48 AM
> To: ffmpeg-devel at ffmpeg.org
> Cc: haihao.xiang-at-intel.com at ffmpeg.org; softworkz at hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v3] avfilter/vpp_qsv: fix regression on
> older api versions (e.g. 1.11)
> 
> On Fri, 2022-01-07 at 03:58 +0000, ffmpegagent wrote:
> > From: softworkz <softworkz at 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 at 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 v3: Use different way to acquire
> >     runtime version
> >
> > Published-As:
> > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> 15%2Fsoftworkz%2Fqsv_vpp_regression-v3
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> > 15/softworkz/qsv_vpp_regression-v3
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> >
> > Range-diff vs v2:
> >
> >  1:  ccede9d840 ! 1:  cbf53d8ef4 avfilter/vpp_qsv: fix regression on older
> api
> > versions (e.g. 1.11)
> >      @@ Commit message
> >           Signed-off-by: softworkz <softworkz at hotmail.com>
> >
> >        ## libavfilter/vf_vpp_qsv.c ##
> >      +@@
> >      + #include "libavutil/opt.h"
> >      + #include "libavutil/eval.h"
> >      + #include "libavutil/hwcontext.h"
> >      ++#include "libavutil/hwcontext_qsv.h"
> >      + #include "libavutil/pixdesc.h"
> >      + #include "libavutil/mathematics.h"
> >      +
> >       @@ libavfilter/vf_vpp_qsv.c: 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 },
> >      @@ libavfilter/vf_vpp_qsv.c: static const AVOption options[] = {
> >            { NULL }
> >        };
> >
> >      +@@ libavfilter/vf_vpp_qsv.c: static int config_input(AVFilterLink
> > *inlink)
> >      +     return 0;
> >      + }
> >      +
> >      ++static int get_mfx_version(const AVFilterContext *ctx, mfxVersion
> > *mfx_version)
> >      ++{
> >      ++    const AVFilterLink *inlink = ctx->inputs[0];
> >      ++    AVBufferRef *device_ref;
> >      ++    AVHWDeviceContext *device_ctx;
> >      ++    AVQSVDeviceContext *device_hwctx;
> >      ++
> >      ++    if (inlink->hw_frames_ctx) {
> >      ++        AVHWFramesContext *frames_ctx = (AVHWFramesContext *)inlink-
> > >hw_frames_ctx->data;
> >      ++        device_ref = frames_ctx->device_ref;
> >      ++    } else if (ctx->hw_device_ctx) {
> >      ++        device_ref = ctx->hw_device_ctx;
> >      ++    } else {
> >      ++        // Unavailable hw context doesn't matter in pass-through
> mode,
> >      ++        // so don't error here but let runtime version checks fail
> by
> > setting to 0.0
> >      ++        mfx_version->Major = 0;
> >      ++        mfx_version->Minor = 0;
> >      ++        return 0;
> >      ++    }
> >      ++
> >      ++    device_ctx   = (AVHWDeviceContext *)device_ref->data;
> >      ++    device_hwctx = device_ctx->hwctx;
> >      ++
> >      ++    return MFXQueryVersion(device_hwctx->session, mfx_version);
> 
> Thanks for the new patch version, it works well for me.
> 
> It would be better to change the return type of get_mfx_version from int to
> mfxStatus if returning MFXQueryVersion(device_hwctx->session, mfx_version)
> here.
> Otherwise map the returned mfx status to ffmpeg status, then return ffmpeg
> status.

Yes, makes sense. Update on the way..

sw