[FFmpeg-devel] [PATCH v08 09/10] qsv: use a new method to create mfx session when using oneVPL

[Mark Thompson]

On 28/04/2022 10:23, Haihao Xiang wrote:
> In oneVPL, MFXLoad() and MFXCreateSession() are required to create a
> workable mfx session[1]
> 
> Add config filters for D3D9/D3D11 session (galinart)
> 
> The default device is changed to d3d11va for oneVPL when both d3d11va
> and dxva2 are enabled on Microsoft Windows
> 
> This is in preparation for oneVPL support
> 
> [1] https://spec.oneapi.io/versions/latest/elements/oneVPL/source/programming_guide/VPL_prg_session.html#onevpl-dispatcher
> 
> Co-authored-by: galinart <artem.galin at intel.com>
> Signed-off-by: galinart <artem.galin at intel.com>
> ---
>   libavcodec/qsv.c                 | 197 ++++++++++--
>   libavcodec/qsv_internal.h        |   1 +
>   libavcodec/qsvdec.c              |  10 +
>   libavcodec/qsvenc.h              |   3 +
>   libavcodec/qsvenc_h264.c         |   1 -
>   libavcodec/qsvenc_hevc.c         |   1 -
>   libavcodec/qsvenc_jpeg.c         |   1 -
>   libavcodec/qsvenc_mpeg2.c        |   1 -
>   libavcodec/qsvenc_vp9.c          |   1 -
>   libavfilter/qsvvpp.c             | 113 ++++++-
>   libavfilter/qsvvpp.h             |   5 +
>   libavfilter/vf_deinterlace_qsv.c |  14 +-
>   libavfilter/vf_scale_qsv.c       |  12 +-
>   libavutil/hwcontext_d3d11va.c    |   7 +
>   libavutil/hwcontext_qsv.c        | 515 ++++++++++++++++++++++++++++---
>   libavutil/hwcontext_qsv.h        |   1 +
>   libavutil/hwcontext_vaapi.c      |  13 +
>   libavutil/hwcontext_vaapi.h      |   4 +
>   18 files changed, 812 insertions(+), 88 deletions(-)
> 
> ...
> diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
> index 8ab96bad25..fd5fedbdac 100644
> --- a/libavutil/hwcontext_d3d11va.c
> +++ b/libavutil/hwcontext_d3d11va.c
> @@ -525,6 +525,11 @@ static void d3d11va_device_uninit(AVHWDeviceContext *hwdev)
>       }
>   }
>   
> +static void d3d11va_device_free(AVHWDeviceContext *ctx)
> +{
> +    AVD3D11VADeviceContext *hwctx = ctx->hwctx;
> +}
> +
>   static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
>                                    AVDictionary *opts, int flags)
>   {
> @@ -537,6 +542,8 @@ static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
>       int is_debug       = !!av_dict_get(opts, "debug", NULL, 0);
>       int ret;
>   
> +    ctx->free = d3d11va_device_free;
> +
>       // (On UWP we can't check this.)
>   #if !HAVE_UWP
>       if (!LoadLibrary("d3d11_1sdklayers.dll"))

This change doesn't do anything.

> ...
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index c3a98bc4b1..cb714df934 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -1572,6 +1572,7 @@ static void vaapi_device_free(AVHWDeviceContext *ctx)
>       if (priv->drm_fd >= 0)
>           close(priv->drm_fd);
>   
> +    av_free(hwctx->device_name);
>       av_freep(&priv);
>   }
>   
> @@ -1620,6 +1621,7 @@ static int vaapi_device_connect(AVHWDeviceContext *ctx,
>   static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
>                                  AVDictionary *opts, int flags)
>   {
> +    AVVAAPIDeviceContext *hwctx = ctx->hwctx;
>       VAAPIDevicePriv *priv;
>       VADisplay display = NULL;
>       const AVDictionaryEntry *ent;
> @@ -1665,6 +1667,11 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
>                          "DRM device node.\n", device);
>                   break;
>               }
> +
> +            hwctx->device_name = av_strdup(device);
> +
> +            if (!hwctx->device_name)
> +                return AVERROR(ENOMEM);
>           } else {
>               char path[64];
>               int n, max_devices = 8;
> @@ -1705,6 +1712,12 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
>                       av_log(ctx, AV_LOG_VERBOSE, "Trying to use "
>                              "DRM render node for device %d.\n", n);
>                   }
> +
> +                hwctx->device_name = av_strdup(path);
> +
> +                if (!hwctx->device_name)
> +                    return AVERROR(ENOMEM);
> +
>                   break;
>               }
>               if (n >= max_devices)
> diff --git a/libavutil/hwcontext_vaapi.h b/libavutil/hwcontext_vaapi.h
> index 0b2e071cb3..3e0b54f5e9 100644
> --- a/libavutil/hwcontext_vaapi.h
> +++ b/libavutil/hwcontext_vaapi.h
> @@ -78,6 +78,10 @@ typedef struct AVVAAPIDeviceContext {
>        * operations using VAAPI with the same VADisplay.
>        */
>       unsigned int driver_quirks;
> +    /**
> +     * The string for the used device
> +     */
> +    char *device_name;

This new user-facing API field needs a lot more explanation.  The only example you've got is for simple creation via DRM, which sets it to a device path.

There are other ways to make a device:
* Created via X11 (like ":0").
* Derived from a DRM device (e.g. from kmsgrab).
* Manually created by the user (who need not have any path even if they did open via DRM, because they could have given the fd by something else like DRI3).

What happens for existing users who have build against the current API without this field and expect it to continue working?

I am generally very sceptical that hacking the library API like this is sensible.  If you only want it to work in the ffmpeg utility (which is what the current version appears to do), then maybe you would be better off modifying that instead.

If this new libmfx really does want to open a new device from the filesystem then you probably want to add new API to libva to supply that information.

- Mark