diff mbox series

[FFmpeg-devel,v2] lavc/qsv: adding DX11 support

Message ID 20200123151813.31739-1-artem.galin@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2] lavc/qsv: adding DX11 support | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

galinart Jan. 23, 2020, 3:18 p.m. UTC
This enables DX11 support for QSV with higher priority than DX9.
In case of multiple GPUs configuration, DX9 API does not allow to get
access to QSV device in some cases - headless.
Implementation based on DX11 resolves that restriction by enumerating list of available GPUs and finding device with QSV support.

Signed-off-by: Artem Galin <artem.galin@gmail.com>
---
 libavcodec/qsv.c              |  38 ++++----
 libavcodec/qsv_internal.h     |   5 +
 libavcodec/version.h          |   2 +-
 libavfilter/qsvvpp.c          |  19 +---
 libavutil/hwcontext_d3d11va.c |  57 +++++++++++-
 libavutil/hwcontext_qsv.c     | 169 +++++++++++++++++++++++++++++-----
 libavutil/hwcontext_qsv.h     |  13 ++-
 libavutil/version.h           |   2 +-
 8 files changed, 242 insertions(+), 63 deletions(-)

Comments

Mark Thompson Jan. 24, 2020, 12:23 a.m. UTC | #1
On 23/01/2020 15:18, Artem Galin wrote:
> This enables DX11 support for QSV with higher priority than DX9.
> In case of multiple GPUs configuration, DX9 API does not allow to get
> access to QSV device in some cases - headless.
> Implementation based on DX11 resolves that restriction by enumerating list of available GPUs and finding device with QSV support.
> 
> Signed-off-by: Artem Galin <artem.galin@gmail.com>
> ---
>  libavcodec/qsv.c              |  38 ++++----
>  libavcodec/qsv_internal.h     |   5 +
>  libavcodec/version.h          |   2 +-
>  libavfilter/qsvvpp.c          |  19 +---
>  libavutil/hwcontext_d3d11va.c |  57 +++++++++++-
>  libavutil/hwcontext_qsv.c     | 169 +++++++++++++++++++++++++++++-----
>  libavutil/hwcontext_qsv.h     |  13 ++-
>  libavutil/version.h           |   2 +-
>  8 files changed, 242 insertions(+), 63 deletions(-)

This should be split into separate commits for the three libraries you touch.

> ...
> diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
> index 6670c47579..a08479fd96 100644
> --- a/libavutil/hwcontext_d3d11va.c
> +++ b/libavutil/hwcontext_d3d11va.c
> @@ -244,7 +244,7 @@ static int d3d11va_frames_init(AVHWFramesContext *ctx)
>          return AVERROR(EINVAL);
>      }
>  
> -    texDesc = (D3D11_TEXTURE2D_DESC){
> +    texDesc = (D3D11_TEXTURE2D_DESC) {

Unrelated whitespace change.

>          .Width      = ctx->width,
>          .Height     = ctx->height,
>          .MipLevels  = 1,
> @@ -510,6 +510,46 @@ static void d3d11va_device_uninit(AVHWDeviceContext *hwdev)
>      }
>  }
>  
> +static int d3d11va_device_find_qsv_adapter(AVHWDeviceContext *ctx, UINT creationFlags)
> +{
> +    HRESULT hr;
> +    IDXGIAdapter *adapter = NULL;
> +    int adapter_id = 0;
> +    int vendor_id = 0x8086;
> +    IDXGIFactory2 *factory;
> +    hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void **)&factory);
> +    while (IDXGIFactory2_EnumAdapters(factory, adapter_id++, &adapter) != DXGI_ERROR_NOT_FOUND)
> +    {
> +        ID3D11Device* device = NULL;
> +        DXGI_ADAPTER_DESC adapter_desc;
> +
> +        hr = mD3D11CreateDevice(adapter, D3D_DRIVER_TYPE_UNKNOWN, NULL, creationFlags, NULL, 0, D3D11_SDK_VERSION, &device, NULL, NULL);
> +        if (FAILED(hr)) {
> +            av_log(ctx, AV_LOG_ERROR, "D3D11CreateDevice returned error\n");
> +            continue;
> +        }
> +
> +        hr = IDXGIAdapter2_GetDesc(adapter, &adapter_desc);
> +        if (FAILED(hr)) {
> +            av_log(ctx, AV_LOG_ERROR, "IDXGIAdapter2_GetDesc returned error\n");
> +            continue;
> +        }
> +
> +        if(device)
> +            ID3D11Device_Release(device);
> +
> +        if (adapter)
> +            IDXGIAdapter_Release(adapter);
> +
> +        if (adapter_desc.VendorId == vendor_id) {
> +            IDXGIFactory2_Release(factory);
> +            return adapter_id - 1;
> +        }
> +    }
> +    IDXGIFactory2_Release(factory);
> +    return -1;
> +}
> +
>  static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
>                                   AVDictionary *opts, int flags)
>  {
> @@ -519,7 +559,9 @@ static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
>      IDXGIAdapter           *pAdapter = NULL;
>      ID3D10Multithread      *pMultithread;
>      UINT creationFlags = D3D11_CREATE_DEVICE_VIDEO_SUPPORT;
> +    int adapter = -1;
>      int is_debug       = !!av_dict_get(opts, "debug", NULL, 0);
> +    int is_qsv         = !!av_dict_get(opts, "d3d11va_qsv", NULL, 0);

The only constraint you actually check here is the vendor ID, right?  I think it would make more sense to add code which goes looking for a device given the vendor ID rather than hard-coding a special function doing this specific case in here - compare with how VAAPI does exactly the same thing.

(That is, make "ffmpeg -init_hw_device d3d11va=,vendor=0x8086" do what you  would expect rather than hacking in a special libmfx case here.)

>      int ret;
>  
>      // (On UWP we can't check this.)
> @@ -538,11 +580,22 @@ static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
>          return AVERROR_UNKNOWN;
>      }
>  
> +    if (is_qsv) {
> +        adapter = d3d11va_device_find_qsv_adapter(ctx, creationFlags);
> +        if (adapter < 0) {
> +            av_log(ctx, AV_LOG_ERROR, "Failed to find DX11 adapter with QSV support\n");
> +            return AVERROR_UNKNOWN;
> +        }
> +    }
> +
>      if (device) {
> +        adapter = atoi(device);
> +    }
> +
> +    if (adapter >= 0) {
>          IDXGIFactory2 *pDXGIFactory;
>          hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void **)&pDXGIFactory);
>          if (SUCCEEDED(hr)) {
> -            int adapter = atoi(device);
>              if (FAILED(IDXGIFactory2_EnumAdapters(pDXGIFactory, adapter, &pAdapter)))
>                  pAdapter = NULL;
>              IDXGIFactory2_Release(pDXGIFactory);
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index b1b67400de..05b2ed3333 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -27,9 +27,13 @@
>  #include <pthread.h>
>  #endif
>  
> +#define COBJMACROS
>  #if CONFIG_VAAPI
>  #include "hwcontext_vaapi.h"
>  #endif
> +#if CONFIG_D3D11VA
> +#include "hwcontext_d3d11va.h"
> +#endif
>  #if CONFIG_DXVA2
>  #include "hwcontext_dxva2.h"
>  #endif
> @@ -89,6 +93,9 @@ static const struct {
>  #if CONFIG_VAAPI
>      { MFX_HANDLE_VA_DISPLAY,          AV_HWDEVICE_TYPE_VAAPI, AV_PIX_FMT_VAAPI },
>  #endif
> +#if CONFIG_D3D11VA
> +    { MFX_HANDLE_D3D11_DEVICE, AV_HWDEVICE_TYPE_D3D11VA, AV_PIX_FMT_D3D11 },

Please maintain the alignment.

> +#endif
>  #if CONFIG_DXVA2
>      { MFX_HANDLE_D3D9_DEVICE_MANAGER, AV_HWDEVICE_TYPE_DXVA2, AV_PIX_FMT_DXVA2_VLD },
>  #endif
> @@ -124,18 +131,21 @@ static int qsv_device_init(AVHWDeviceContext *ctx)
>      int i;
>  
>      for (i = 0; supported_handle_types[i].handle_type; i++) {
> -        err = MFXVideoCORE_GetHandle(hwctx->session, supported_handle_types[i].handle_type,
> -                                     &s->handle);
> -        if (err == MFX_ERR_NONE) {
> -            s->handle_type       = supported_handle_types[i].handle_type;
> -            s->child_device_type = supported_handle_types[i].device_type;
> -            s->child_pix_fmt     = supported_handle_types[i].pix_fmt;
> -            break;
> +        if (supported_handle_types[i].handle_type == hwctx->handle_type) {
> +            err = MFXVideoCORE_GetHandle(hwctx->session, supported_handle_types[i].handle_type,
> +                                        &s->handle);
> +            if (err == MFX_ERR_NONE) {
> +                s->handle_type       = supported_handle_types[i].handle_type;
> +                s->child_device_type = supported_handle_types[i].device_type;
> +                s->child_pix_fmt     = supported_handle_types[i].pix_fmt;
> +                break;
> +            }
>          }
>      }
>      if (!s->handle) {
>          av_log(ctx, AV_LOG_VERBOSE, "No supported hw handle could be retrieved "
>                 "from the session\n");
> +        return AVERROR_UNKNOWN;

Why has this become an error when it wasn't previously?

>      }
>  
>      err = MFXQueryIMPL(hwctx->session, &s->impl);
> @@ -229,9 +239,17 @@ static int qsv_init_child_ctx(AVHWFramesContext *ctx)
>          child_device_hwctx->display = (VADisplay)device_priv->handle;
>      }
>  #endif
> +#if CONFIG_D3D11VA
> +    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
> +        AVD3D11VADeviceContext *child_device_hwctx = child_device_ctx->hwctx;
> +        ID3D11Device_AddRef((ID3D11Device*)device_priv->handle);
> +        child_device_hwctx->device = (ID3D11Device*)device_priv->handle;
> +    }
> +#endif
>  #if CONFIG_DXVA2
>      if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
>          AVDXVA2DeviceContext *child_device_hwctx = child_device_ctx->hwctx;
> +        IDirect3DDeviceManager9_AddRef((IDirect3DDeviceManager9*)device_priv->handle);

This looks like you're making a subtle change to something unrelated to the patch.

>          child_device_hwctx->devmgr = (IDirect3DDeviceManager9*)device_priv->handle;
>      }
>  #endif
> @@ -255,6 +273,16 @@ static int qsv_init_child_ctx(AVHWFramesContext *ctx)
>      child_frames_ctx->width             = FFALIGN(ctx->width, 16);
>      child_frames_ctx->height            = FFALIGN(ctx->height, 16);
>  
> +#if CONFIG_D3D11VA
> +    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
> +        AVD3D11VAFramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
> +        child_frames_hwctx->MiscFlags |= D3D11_RESOURCE_MISC_SHARED;
> +        if (hwctx->frame_type & MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET)
> +            child_frames_hwctx->BindFlags = D3D11_BIND_RENDER_TARGET ;
> +        else
> +            child_frames_hwctx->BindFlags = D3D11_BIND_DECODER;
> +    }
> +#endif
>  #if CONFIG_DXVA2
>      if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
>          AVDXVA2FramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
> @@ -279,6 +307,18 @@ static int qsv_init_child_ctx(AVHWFramesContext *ctx)
>          hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
>      }
>  #endif
> +#if CONFIG_D3D11VA
> +    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
> +        AVD3D11VAFramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
> +        hwctx->texture = child_frames_hwctx->texture;
> +        for (i = 0; i < ctx->initial_pool_size; i++)
> +            s->surfaces_internal[i].Data.MemId = (mfxMemId)(int64_t)i;
> +        if (child_frames_hwctx->BindFlags == D3D11_BIND_DECODER)
> +            hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
> +        else
> +            hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET;
> +    }
> +#endif
>  #if CONFIG_DXVA2
>      if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
>          AVDXVA2FramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
> @@ -421,7 +461,16 @@ static mfxStatus frame_unlock(mfxHDL pthis, mfxMemId mid, mfxFrameData *ptr)
>  
>  static mfxStatus frame_get_hdl(mfxHDL pthis, mfxMemId mid, mfxHDL *hdl)
>  {
> -    *hdl = mid;
> +    AVHWFramesContext    *ctx = pthis;
> +    AVQSVFramesContext *hwctx = ctx->hwctx;
> +
> +    if (hwctx->texture) {
> +        mfxHDLPair *pair  =  (mfxHDLPair*)hdl;
> +        pair->first  = hwctx->texture;
> +        pair->second = mid;
> +    } else {
> +        *hdl = mid;
> +    }
>      return MFX_ERR_NONE;
>  }
>  
> @@ -492,7 +541,7 @@ static int qsv_init_internal_session(AVHWFramesContext *ctx,
>  
>      err = MFXVideoVPP_Init(*session, &par);
>      if (err != MFX_ERR_NONE) {
> -        av_log(ctx, AV_LOG_VERBOSE, "Error opening the internal VPP session."
> +        av_log(ctx, AV_LOG_ERROR, "Error opening the internal VPP session."
>                 "Surface upload/download will not be possible\n");

Why is this now an error where it wasn't previously?

>          MFXClose(*session);
>          *session = NULL;
> @@ -668,6 +717,11 @@ static int qsv_map_from(AVHWFramesContext *ctx,
>          child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)surf->Data.MemId;
>          break;
>  #endif
> +#if CONFIG_D3D11VA
> +    case AV_HWDEVICE_TYPE_D3D11VA:
> +        child_data = surf->Data.MemId;
> +        break;
> +#endif
>  #if CONFIG_DXVA2
>      case AV_HWDEVICE_TYPE_DXVA2:
>          child_data = surf->Data.MemId;
> @@ -972,6 +1026,27 @@ static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
>          }
>          break;
>  #endif
> +#if CONFIG_D3D11VA
> +    case AV_HWDEVICE_TYPE_D3D11VA:
> +        {
> +            AVD3D11VAFramesContext *src_hwctx = src_ctx->hwctx;
> +            s->surfaces_internal = av_mallocz_array(src_ctx->initial_pool_size,
> +                                                    sizeof(*s->surfaces_internal));
> +            if (!s->surfaces_internal)
> +                return AVERROR(ENOMEM);
> +            dst_hwctx->texture = src_hwctx->texture;
> +            for (i = 0; i < src_ctx->initial_pool_size; i++) {
> +                qsv_init_surface(dst_ctx, &s->surfaces_internal[i]);
> +                s->surfaces_internal[i].Data.MemId = (mfxMemId)(int64_t)i;
> +            }
> +            dst_hwctx->nb_surfaces = src_ctx->initial_pool_size;
> +            if (src_hwctx->BindFlags == D3D11_BIND_DECODER)
> +                dst_hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
> +            else
> +                dst_hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET;
> +        }
> +        break;
> +#endif
>  #if CONFIG_DXVA2
>      case AV_HWDEVICE_TYPE_DXVA2:
>          {
> @@ -1004,19 +1079,37 @@ static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
>  static int qsv_map_to(AVHWFramesContext *dst_ctx,
>                        AVFrame *dst, const AVFrame *src, int flags)
>  {
> -    AVQSVFramesContext *hwctx = dst_ctx->hwctx;
> +    AVHWFramesContext *src_frames;
>      int i, err;
> +    enum AVHWDeviceType type = AV_HWDEVICE_TYPE_NONE;
> +    AVQSVFramesContext *hwctx = dst_ctx->hwctx;
> +    if (src->hw_frames_ctx) {
> +        src_frames = (AVHWFramesContext*)src->hw_frames_ctx->data;
> +        type = src_frames->internal->hw_type->type;
> +    }

src->format would be a more direct way to find the format of the source frame.

>  
>      for (i = 0; i < hwctx->nb_surfaces; i++) {
>  #if CONFIG_VAAPI
> -        if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
> -            (VASurfaceID)(uintptr_t)src->data[3])
> -            break;
> +        if (AV_HWDEVICE_TYPE_VAAPI == type) {
> +            if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
> +                (VASurfaceID)(uintptr_t)src->data[3])
> +                break;
> +        }
> +#endif
> +#if CONFIG_D3D11VA
> +        if (AV_HWDEVICE_TYPE_D3D11VA == type) {
> +            if ((hwctx->texture == (ID3D11Texture2D*)(uintptr_t)src->data[0]) &&
> +                ((ID3D11Texture2D*)hwctx->surfaces[i].Data.MemId ==
> +                (ID3D11Texture2D*)(uintptr_t)src->data[1]))
> +                break;
> +        }
>  #endif
>  #if CONFIG_DXVA2
> -        if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
> -            (IDirect3DSurface9*)(uintptr_t)src->data[3])
> -            break;
> +        if (AV_HWDEVICE_TYPE_DXVA2 == type) {
> +            if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
> +                (IDirect3DSurface9*)(uintptr_t)src->data[3])
> +                break;
> +        }
>  #endif
>      }
>      if (i >= hwctx->nb_surfaces) {
> @@ -1074,7 +1167,7 @@ static void qsv_device_free(AVHWDeviceContext *ctx)
>      av_freep(&priv);
>  }
>  
> -static mfxIMPL choose_implementation(const char *device)
> +static mfxIMPL choose_implementation(const char *device, enum AVHWDeviceType child_device_type)
>  {
>      static const struct {
>          const char *name;
> @@ -1103,6 +1196,10 @@ static mfxIMPL choose_implementation(const char *device)
>              impl = strtol(device, NULL, 0);
>      }
>  
> +    if ( (child_device_type == AV_HWDEVICE_TYPE_D3D11VA) && (impl != MFX_IMPL_SOFTWARE) ) {
> +        impl |= MFX_IMPL_VIA_D3D11;
> +    }

If this is needed it probably shouldn't be in this function.  This is specifically the string name -> implementation type mapping function.

> +
>      return impl;
>  }
>  
> @@ -1129,6 +1226,15 @@ static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
>          }
>          break;
>  #endif
> +#if CONFIG_D3D11VA
> +    case AV_HWDEVICE_TYPE_D3D11VA:
> +        {
> +            AVD3D11VADeviceContext *child_device_hwctx = child_device_ctx->hwctx;
> +            handle_type = MFX_HANDLE_D3D11_DEVICE;
> +            handle = (mfxHDL)child_device_hwctx->device;
> +        }
> +        break;
> +#endif
>  #if CONFIG_DXVA2
>      case AV_HWDEVICE_TYPE_DXVA2:
>          {
> @@ -1180,6 +1286,7 @@ static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
>          goto fail;
>      }
>  
> +    hwctx->handle_type = handle_type;
>      return 0;
>  
>  fail:
> @@ -1191,7 +1298,9 @@ fail:
>  static int qsv_device_derive(AVHWDeviceContext *ctx,
>                               AVHWDeviceContext *child_device_ctx, int flags)
>  {
> -    return qsv_device_derive_from_child(ctx, MFX_IMPL_HARDWARE_ANY,
> +    mfxIMPL impl;
> +    impl = choose_implementation("hw_any", child_device_ctx->type);
> +    return qsv_device_derive_from_child(ctx, impl,
>                                          child_device_ctx, flags);
>  }
>  
> @@ -1226,23 +1335,35 @@ static int qsv_device_create(AVHWDeviceContext *ctx, const char *device,
>          // possible, even when multiple devices and drivers are available.
>          av_dict_set(&child_device_opts, "kernel_driver", "i915", 0);
>          av_dict_set(&child_device_opts, "driver",        "iHD",  0);
> -    } else if (CONFIG_DXVA2)
> +    } else if (CONFIG_D3D11VA) {
> +        child_device_type = AV_HWDEVICE_TYPE_D3D11VA;
> +        av_dict_set(&child_device_opts, "d3d11va_qsv",   "enabled",  0);
> +    } else if (CONFIG_DXVA2) {
>          child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> -    else {
> +    } else {
>          av_log(ctx, AV_LOG_ERROR, "No supported child device type is enabled\n");
>          return AVERROR(ENOSYS);
>      }

Allowing a user-set child device type seems like a good idea (see the original patch).

>  
>      ret = av_hwdevice_ctx_create(&priv->child_device_ctx, child_device_type,
>                                   e ? e->value : NULL, child_device_opts, 0);
> -
>      av_dict_free(&child_device_opts);
> -    if (ret < 0)
> -        return ret;
> +    if (ret < 0) {
> +        if (CONFIG_DXVA2 && (child_device_type == AV_HWDEVICE_TYPE_D3D11VA)) {
> +            // in case of d3d11va fail, try one more chance to create device via dxva2
> +            child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> +            child_device_opts = NULL;

Leaks the dictionary.

> +            ret = av_hwdevice_ctx_create(&priv->child_device_ctx, child_device_type,
> +                            e ? e->value : NULL, child_device_opts, 0);
> +        }
> +        if (ret < 0) {
> +            return ret;
> +        }
> +    }
>  
>      child_device = (AVHWDeviceContext*)priv->child_device_ctx->data;
>  
> -    impl = choose_implementation(device);
> +    impl = choose_implementation(device, child_device_type);
>  
>      return qsv_device_derive_from_child(ctx, impl, child_device, 0);
>  }
> diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h
> index b98d611cfc..3a322037e5 100644
> --- a/libavutil/hwcontext_qsv.h
> +++ b/libavutil/hwcontext_qsv.h
> @@ -34,6 +34,15 @@
>   */
>  typedef struct AVQSVDeviceContext {
>      mfxSession session;
> +    /**
> +     * Need to store actual handle type that session uses
> +     * MFXVideoCORE_GetHandle() function returns mfxHandleType
> +     * always equal to MFX_HANDLE_D3D9_DEVICE_MANAGER
> +     * even when MFX_HANDLE_D3D11_DEVICE was set as handle before by
> +     * MFXVideoCORE_SetHandle() to mfx session.
> +     * Fixed already but will be available only with latest driver.
> +     */
> +    mfxHandleType handle_type;

This incompatibly changes the ABI because you've made this field required where it didn't previously exist.

Not having it at all is clearly best, because the session really does know what the handle type is.  If there are some broken drivers where the type can't be retrieved maybe we could unconditionally use the existing D3D9 code on them, which at least wouldn't regress?

>  } AVQSVDeviceContext;
>  
>  /**
> @@ -42,11 +51,11 @@ typedef struct AVQSVDeviceContext {
>  typedef struct AVQSVFramesContext {
>      mfxFrameSurface1 *surfaces;
>      int            nb_surfaces;
> -
>      /**
>       * A combination of MFX_MEMTYPE_* describing the frame pool.
>       */
> -    int frame_type;
> +    int             frame_type;
> +    void              *texture;

Why do you need to add this?  Each frame already contains the texture pointer in data[0].

Also, in existing D3D11 code the texture need not be the same for all frames in a frames context (it is when using the default creation with a fixed pool size, but not otherwise).  Are you enforcing this because libmfx is unable to work with multiple textures, or is there some other reason?  (If the former then I think you need to be more careful in banning D3D11 frames contexts of this form earlier, because otherwise you're going to cause weird problems by trying to access higher indices of non-array textures.)

>  } AVQSVFramesContext;
>  
>  #endif /* AVUTIL_HWCONTEXT_QSV_H */

When the original patch for this was written the reason it didn't work was because the second parameter (array index) in the HDLpair was silently ignored in all driver versions back then.  (So for example the encoder always encoded the first texture in the array, which is a fun effect but not really desirable.)

How do we distinguish that case now?  Setting D3D11 as the default without being able to verify that it actually works is going to fail in a horrible way for everyone with a slightly old driver.

Thanks,

- Mark
galinart Jan. 24, 2020, 7:37 p.m. UTC | #2
On Fri, 24 Jan 2020 at 00:46, Mark Thompson <sw@jkqxz.net> wrote:

> On 23/01/2020 15:18, Artem Galin wrote:
> > This enables DX11 support for QSV with higher priority than DX9.
> > In case of multiple GPUs configuration, DX9 API does not allow to get
> > access to QSV device in some cases - headless.
> > Implementation based on DX11 resolves that restriction by enumerating
> list of available GPUs and finding device with QSV support.
> >
> > Signed-off-by: Artem Galin <artem.galin@gmail.com>
> > ---
> >  libavcodec/qsv.c              |  38 ++++----
> >  libavcodec/qsv_internal.h     |   5 +
> >  libavcodec/version.h          |   2 +-
> >  libavfilter/qsvvpp.c          |  19 +---
> >  libavutil/hwcontext_d3d11va.c |  57 +++++++++++-
> >  libavutil/hwcontext_qsv.c     | 169 +++++++++++++++++++++++++++++-----
> >  libavutil/hwcontext_qsv.h     |  13 ++-
> >  libavutil/version.h           |   2 +-
> >  8 files changed, 242 insertions(+), 63 deletions(-)
>
> This should be split into separate commits for the three libraries you
> touch.
>
> These changes are logically one piece of code and do not work
independently.

> ...
> > diff --git a/libavutil/hwcontext_d3d11va.c
> b/libavutil/hwcontext_d3d11va.c
> > index 6670c47579..a08479fd96 100644
> > --- a/libavutil/hwcontext_d3d11va.c
> > +++ b/libavutil/hwcontext_d3d11va.c
> > @@ -244,7 +244,7 @@ static int d3d11va_frames_init(AVHWFramesContext
> *ctx)
> >          return AVERROR(EINVAL);
> >      }
> >
> > -    texDesc = (D3D11_TEXTURE2D_DESC){
> > +    texDesc = (D3D11_TEXTURE2D_DESC) {
>
> Unrelated whitespace change.
>
> >          .Width      = ctx->width,
> >          .Height     = ctx->height,
> >          .MipLevels  = 1,
> > @@ -510,6 +510,46 @@ static void d3d11va_device_uninit(AVHWDeviceContext
> *hwdev)
> >      }
> >  }
> >
> > +static int d3d11va_device_find_qsv_adapter(AVHWDeviceContext *ctx, UINT
> creationFlags)
> > +{
> > +    HRESULT hr;
> > +    IDXGIAdapter *adapter = NULL;
> > +    int adapter_id = 0;
> > +    int vendor_id = 0x8086;
> > +    IDXGIFactory2 *factory;
> > +    hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void **)&factory);
> > +    while (IDXGIFactory2_EnumAdapters(factory, adapter_id++, &adapter)
> != DXGI_ERROR_NOT_FOUND)
> > +    {
> > +        ID3D11Device* device = NULL;
> > +        DXGI_ADAPTER_DESC adapter_desc;
> > +
> > +        hr = mD3D11CreateDevice(adapter, D3D_DRIVER_TYPE_UNKNOWN, NULL,
> creationFlags, NULL, 0, D3D11_SDK_VERSION, &device, NULL, NULL);
> > +        if (FAILED(hr)) {
> > +            av_log(ctx, AV_LOG_ERROR, "D3D11CreateDevice returned
> error\n");
> > +            continue;
> > +        }
> > +
> > +        hr = IDXGIAdapter2_GetDesc(adapter, &adapter_desc);
> > +        if (FAILED(hr)) {
> > +            av_log(ctx, AV_LOG_ERROR, "IDXGIAdapter2_GetDesc returned
> error\n");
> > +            continue;
> > +        }
> > +
> > +        if(device)
> > +            ID3D11Device_Release(device);
> > +
> > +        if (adapter)
> > +            IDXGIAdapter_Release(adapter);
> > +
> > +        if (adapter_desc.VendorId == vendor_id) {
> > +            IDXGIFactory2_Release(factory);
> > +            return adapter_id - 1;
> > +        }
> > +    }
> > +    IDXGIFactory2_Release(factory);
> > +    return -1;
> > +}
> > +
> >  static int d3d11va_device_create(AVHWDeviceContext *ctx, const char
> *device,
> >                                   AVDictionary *opts, int flags)
> >  {
> > @@ -519,7 +559,9 @@ static int d3d11va_device_create(AVHWDeviceContext
> *ctx, const char *device,
> >      IDXGIAdapter           *pAdapter = NULL;
> >      ID3D10Multithread      *pMultithread;
> >      UINT creationFlags = D3D11_CREATE_DEVICE_VIDEO_SUPPORT;
> > +    int adapter = -1;
> >      int is_debug       = !!av_dict_get(opts, "debug", NULL, 0);
> > +    int is_qsv         = !!av_dict_get(opts, "d3d11va_qsv", NULL, 0);
>
> The only constraint you actually check here is the vendor ID, right?  I
> think it would make more sense to add code which goes looking for a device
> given the vendor ID rather than hard-coding a special function doing this
> specific case in here - compare with how VAAPI does exactly the same thing.
>
> Agree to change interface to support given vendor id.


> (That is, make "ffmpeg -init_hw_device d3d11va=,vendor=0x8086" do what
> you  would expect rather than hacking in a special libmfx case here.)
>

Agree that your proposal to have option to choose vendor by given vendor id
via command line is nice to have optionally and could be added later. This
patch is about enabling DX11 for qsv at the moment.


> >      int ret;
> >
> >      // (On UWP we can't check this.)
> > @@ -538,11 +580,22 @@ static int d3d11va_device_create(AVHWDeviceContext
> *ctx, const char *device,
> >          return AVERROR_UNKNOWN;
> >      }
> >
> > +    if (is_qsv) {
> > +        adapter = d3d11va_device_find_qsv_adapter(ctx, creationFlags);
> > +        if (adapter < 0) {
> > +            av_log(ctx, AV_LOG_ERROR, "Failed to find DX11 adapter with
> QSV support\n");
> > +            return AVERROR_UNKNOWN;
> > +        }
> > +    }
> > +
> >      if (device) {
> > +        adapter = atoi(device);
> > +    }
> > +
> > +    if (adapter >= 0) {
> >          IDXGIFactory2 *pDXGIFactory;
> >          hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void
> **)&pDXGIFactory);
> >          if (SUCCEEDED(hr)) {
> > -            int adapter = atoi(device);
> >              if (FAILED(IDXGIFactory2_EnumAdapters(pDXGIFactory,
> adapter, &pAdapter)))
> >                  pAdapter = NULL;
> >              IDXGIFactory2_Release(pDXGIFactory);
> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > index b1b67400de..05b2ed3333 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -27,9 +27,13 @@
> >  #include <pthread.h>
> >  #endif
> >
> > +#define COBJMACROS
> >  #if CONFIG_VAAPI
> >  #include "hwcontext_vaapi.h"
> >  #endif
> > +#if CONFIG_D3D11VA
> > +#include "hwcontext_d3d11va.h"
> > +#endif
> >  #if CONFIG_DXVA2
> >  #include "hwcontext_dxva2.h"
> >  #endif
> > @@ -89,6 +93,9 @@ static const struct {
> >  #if CONFIG_VAAPI
> >      { MFX_HANDLE_VA_DISPLAY,          AV_HWDEVICE_TYPE_VAAPI,
> AV_PIX_FMT_VAAPI },
> >  #endif
> > +#if CONFIG_D3D11VA
> > +    { MFX_HANDLE_D3D11_DEVICE, AV_HWDEVICE_TYPE_D3D11VA,
> AV_PIX_FMT_D3D11 },
>
> Please maintain the alignment.
>
Agree.


>
> > +#endif
> >  #if CONFIG_DXVA2
> >      { MFX_HANDLE_D3D9_DEVICE_MANAGER, AV_HWDEVICE_TYPE_DXVA2,
> AV_PIX_FMT_DXVA2_VLD },
> >  #endif
> > @@ -124,18 +131,21 @@ static int qsv_device_init(AVHWDeviceContext *ctx)
> >      int i;
> >
> >      for (i = 0; supported_handle_types[i].handle_type; i++) {
> > -        err = MFXVideoCORE_GetHandle(hwctx->session,
> supported_handle_types[i].handle_type,
> > -                                     &s->handle);
> > -        if (err == MFX_ERR_NONE) {
> > -            s->handle_type       =
> supported_handle_types[i].handle_type;
> > -            s->child_device_type =
> supported_handle_types[i].device_type;
> > -            s->child_pix_fmt     = supported_handle_types[i].pix_fmt;
> > -            break;
> > +        if (supported_handle_types[i].handle_type ==
> hwctx->handle_type) {
> > +            err = MFXVideoCORE_GetHandle(hwctx->session,
> supported_handle_types[i].handle_type,
> > +                                        &s->handle);
> > +            if (err == MFX_ERR_NONE) {
> > +                s->handle_type       =
> supported_handle_types[i].handle_type;
> > +                s->child_device_type =
> supported_handle_types[i].device_type;
> > +                s->child_pix_fmt     =
> supported_handle_types[i].pix_fmt;
> > +                break;
> > +            }
> >          }
> >      }
> >      if (!s->handle) {
> >          av_log(ctx, AV_LOG_VERBOSE, "No supported hw handle could be
> retrieved "
> >                 "from the session\n");
> > +        return AVERROR_UNKNOWN;
>
> Why has this become an error when it wasn't previously?
>
I presume previously it was wrong, but never be the case. We can't go
further if handle is null.

>
> >      }
> >
> >      err = MFXQueryIMPL(hwctx->session, &s->impl);
> > @@ -229,9 +239,17 @@ static int qsv_init_child_ctx(AVHWFramesContext
> *ctx)
> >          child_device_hwctx->display = (VADisplay)device_priv->handle;
> >      }
> >  #endif
> > +#if CONFIG_D3D11VA
> > +    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
> > +        AVD3D11VADeviceContext *child_device_hwctx =
> child_device_ctx->hwctx;
> > +        ID3D11Device_AddRef((ID3D11Device*)device_priv->handle);
> > +        child_device_hwctx->device = (ID3D11Device*)device_priv->handle;
> > +    }
> > +#endif
> >  #if CONFIG_DXVA2
> >      if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
> >          AVDXVA2DeviceContext *child_device_hwctx =
> child_device_ctx->hwctx;
> > +
> IDirect3DDeviceManager9_AddRef((IDirect3DDeviceManager9*)device_priv->handle);
>
> This looks like you're making a subtle change to something unrelated to
> the patch.
>
Agree.


>
> >          child_device_hwctx->devmgr =
> (IDirect3DDeviceManager9*)device_priv->handle;
> >      }
> >  #endif
> > @@ -255,6 +273,16 @@ static int qsv_init_child_ctx(AVHWFramesContext
> *ctx)
> >      child_frames_ctx->width             = FFALIGN(ctx->width, 16);
> >      child_frames_ctx->height            = FFALIGN(ctx->height, 16);
> >
> > +#if CONFIG_D3D11VA
> > +    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
> > +        AVD3D11VAFramesContext *child_frames_hwctx =
> child_frames_ctx->hwctx;
> > +        child_frames_hwctx->MiscFlags |= D3D11_RESOURCE_MISC_SHARED;
> > +        if (hwctx->frame_type &
> MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET)
> > +            child_frames_hwctx->BindFlags = D3D11_BIND_RENDER_TARGET ;
> > +        else
> > +            child_frames_hwctx->BindFlags = D3D11_BIND_DECODER;
> > +    }
> > +#endif
> >  #if CONFIG_DXVA2
> >      if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
> >          AVDXVA2FramesContext *child_frames_hwctx =
> child_frames_ctx->hwctx;
> > @@ -279,6 +307,18 @@ static int qsv_init_child_ctx(AVHWFramesContext
> *ctx)
> >          hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
> >      }
> >  #endif
> > +#if CONFIG_D3D11VA
> > +    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
> > +        AVD3D11VAFramesContext *child_frames_hwctx =
> child_frames_ctx->hwctx;
> > +        hwctx->texture = child_frames_hwctx->texture;
> > +        for (i = 0; i < ctx->initial_pool_size; i++)
> > +            s->surfaces_internal[i].Data.MemId = (mfxMemId)(int64_t)i;
> > +        if (child_frames_hwctx->BindFlags == D3D11_BIND_DECODER)
> > +            hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
> > +        else
> > +            hwctx->frame_type =
> MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET;
> > +    }
> > +#endif
> >  #if CONFIG_DXVA2
> >      if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
> >          AVDXVA2FramesContext *child_frames_hwctx =
> child_frames_ctx->hwctx;
> > @@ -421,7 +461,16 @@ static mfxStatus frame_unlock(mfxHDL pthis,
> mfxMemId mid, mfxFrameData *ptr)
> >
> >  static mfxStatus frame_get_hdl(mfxHDL pthis, mfxMemId mid, mfxHDL *hdl)
> >  {
> > -    *hdl = mid;
> > +    AVHWFramesContext    *ctx = pthis;
> > +    AVQSVFramesContext *hwctx = ctx->hwctx;
> > +
> > +    if (hwctx->texture) {
> > +        mfxHDLPair *pair  =  (mfxHDLPair*)hdl;
> > +        pair->first  = hwctx->texture;
> > +        pair->second = mid;
> > +    } else {
> > +        *hdl = mid;
> > +    }
> >      return MFX_ERR_NONE;
> >  }
> >
> > @@ -492,7 +541,7 @@ static int
> qsv_init_internal_session(AVHWFramesContext *ctx,
> >
> >      err = MFXVideoVPP_Init(*session, &par);
> >      if (err != MFX_ERR_NONE) {
> > -        av_log(ctx, AV_LOG_VERBOSE, "Error opening the internal VPP
> session."
> > +        av_log(ctx, AV_LOG_ERROR, "Error opening the internal VPP
> session."
> >                 "Surface upload/download will not be possible\n");
>
> Why is this now an error where it wasn't previously?
>
I presume previously it was wrong. It should be an error level.

>
> >          MFXClose(*session);
> >          *session = NULL;
> > @@ -668,6 +717,11 @@ static int qsv_map_from(AVHWFramesContext *ctx,
> >          child_data =
> (uint8_t*)(intptr_t)*(VASurfaceID*)surf->Data.MemId;
> >          break;
> >  #endif
> > +#if CONFIG_D3D11VA
> > +    case AV_HWDEVICE_TYPE_D3D11VA:
> > +        child_data = surf->Data.MemId;
> > +        break;
> > +#endif
> >  #if CONFIG_DXVA2
> >      case AV_HWDEVICE_TYPE_DXVA2:
> >          child_data = surf->Data.MemId;
> > @@ -972,6 +1026,27 @@ static int qsv_frames_derive_to(AVHWFramesContext
> *dst_ctx,
> >          }
> >          break;
> >  #endif
> > +#if CONFIG_D3D11VA
> > +    case AV_HWDEVICE_TYPE_D3D11VA:
> > +        {
> > +            AVD3D11VAFramesContext *src_hwctx = src_ctx->hwctx;
> > +            s->surfaces_internal =
> av_mallocz_array(src_ctx->initial_pool_size,
> > +
> sizeof(*s->surfaces_internal));
> > +            if (!s->surfaces_internal)
> > +                return AVERROR(ENOMEM);
> > +            dst_hwctx->texture = src_hwctx->texture;
> > +            for (i = 0; i < src_ctx->initial_pool_size; i++) {
> > +                qsv_init_surface(dst_ctx, &s->surfaces_internal[i]);
> > +                s->surfaces_internal[i].Data.MemId =
> (mfxMemId)(int64_t)i;
> > +            }
> > +            dst_hwctx->nb_surfaces = src_ctx->initial_pool_size;
> > +            if (src_hwctx->BindFlags == D3D11_BIND_DECODER)
> > +                dst_hwctx->frame_type =
> MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
> > +            else
> > +                dst_hwctx->frame_type =
> MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET;
> > +        }
> > +        break;
> > +#endif
> >  #if CONFIG_DXVA2
> >      case AV_HWDEVICE_TYPE_DXVA2:
> >          {
> > @@ -1004,19 +1079,37 @@ static int
> qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
> >  static int qsv_map_to(AVHWFramesContext *dst_ctx,
> >                        AVFrame *dst, const AVFrame *src, int flags)
> >  {
> > -    AVQSVFramesContext *hwctx = dst_ctx->hwctx;
> > +    AVHWFramesContext *src_frames;
> >      int i, err;
> > +    enum AVHWDeviceType type = AV_HWDEVICE_TYPE_NONE;
> > +    AVQSVFramesContext *hwctx = dst_ctx->hwctx;
> > +    if (src->hw_frames_ctx) {
> > +        src_frames = (AVHWFramesContext*)src->hw_frames_ctx->data;
> > +        type = src_frames->internal->hw_type->type;
> > +    }
>
> src->format would be a more direct way to find the format of the source
> frame.
>
Agree, thanks!


>
> >
> >      for (i = 0; i < hwctx->nb_surfaces; i++) {
> >  #if CONFIG_VAAPI
> > -        if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
> > -            (VASurfaceID)(uintptr_t)src->data[3])
> > -            break;
> > +        if (AV_HWDEVICE_TYPE_VAAPI == type) {
> > +            if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
> > +                (VASurfaceID)(uintptr_t)src->data[3])
> > +                break;
> > +        }
> > +#endif
> > +#if CONFIG_D3D11VA
> > +        if (AV_HWDEVICE_TYPE_D3D11VA == type) {
> > +            if ((hwctx->texture ==
> (ID3D11Texture2D*)(uintptr_t)src->data[0]) &&
> > +                ((ID3D11Texture2D*)hwctx->surfaces[i].Data.MemId ==
> > +                (ID3D11Texture2D*)(uintptr_t)src->data[1]))
> > +                break;
> > +        }
> >  #endif
> >  #if CONFIG_DXVA2
> > -        if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
> > -            (IDirect3DSurface9*)(uintptr_t)src->data[3])
> > -            break;
> > +        if (AV_HWDEVICE_TYPE_DXVA2 == type) {
> > +            if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
> > +                (IDirect3DSurface9*)(uintptr_t)src->data[3])
> > +                break;
> > +        }
> >  #endif
> >      }
> >      if (i >= hwctx->nb_surfaces) {
> > @@ -1074,7 +1167,7 @@ static void qsv_device_free(AVHWDeviceContext *ctx)
> >      av_freep(&priv);
> >  }
> >
> > -static mfxIMPL choose_implementation(const char *device)
> > +static mfxIMPL choose_implementation(const char *device, enum
> AVHWDeviceType child_device_type)
> >  {
> >      static const struct {
> >          const char *name;
> > @@ -1103,6 +1196,10 @@ static mfxIMPL choose_implementation(const char
> *device)
> >              impl = strtol(device, NULL, 0);
> >      }
> >
> > +    if ( (child_device_type == AV_HWDEVICE_TYPE_D3D11VA) && (impl !=
> MFX_IMPL_SOFTWARE) ) {
> > +        impl |= MFX_IMPL_VIA_D3D11;
> > +    }
>
> If this is needed it probably shouldn't be in this function.  This is
> specifically the string name -> implementation type mapping function.
>
In case of DX11, we always have to have MFX_IMPL_HARDWARE_* flag with
combination with MFX_IMPL_VIA_D3D11. Otherwise it will lead to errors,
because of unsupported DX11 handle type.

>
> > +
> >      return impl;
> >  }
> >
> > @@ -1129,6 +1226,15 @@ static int
> qsv_device_derive_from_child(AVHWDeviceContext *ctx,
> >          }
> >          break;
> >  #endif
> > +#if CONFIG_D3D11VA
> > +    case AV_HWDEVICE_TYPE_D3D11VA:
> > +        {
> > +            AVD3D11VADeviceContext *child_device_hwctx =
> child_device_ctx->hwctx;
> > +            handle_type = MFX_HANDLE_D3D11_DEVICE;
> > +            handle = (mfxHDL)child_device_hwctx->device;
> > +        }
> > +        break;
> > +#endif
> >  #if CONFIG_DXVA2
> >      case AV_HWDEVICE_TYPE_DXVA2:
> >          {
> > @@ -1180,6 +1286,7 @@ static int
> qsv_device_derive_from_child(AVHWDeviceContext *ctx,
> >          goto fail;
> >      }
> >
> > +    hwctx->handle_type = handle_type;
> >      return 0;
> >
> >  fail:
> > @@ -1191,7 +1298,9 @@ fail:
> >  static int qsv_device_derive(AVHWDeviceContext *ctx,
> >                               AVHWDeviceContext *child_device_ctx, int
> flags)
> >  {
> > -    return qsv_device_derive_from_child(ctx, MFX_IMPL_HARDWARE_ANY,
> > +    mfxIMPL impl;
> > +    impl = choose_implementation("hw_any", child_device_ctx->type);
> > +    return qsv_device_derive_from_child(ctx, impl,
> >                                          child_device_ctx, flags);
> >  }
> >
> > @@ -1226,23 +1335,35 @@ static int qsv_device_create(AVHWDeviceContext
> *ctx, const char *device,
> >          // possible, even when multiple devices and drivers are
> available.
> >          av_dict_set(&child_device_opts, "kernel_driver", "i915", 0);
> >          av_dict_set(&child_device_opts, "driver",        "iHD",  0);
> > -    } else if (CONFIG_DXVA2)
> > +    } else if (CONFIG_D3D11VA) {
> > +        child_device_type = AV_HWDEVICE_TYPE_D3D11VA;
> > +        av_dict_set(&child_device_opts, "d3d11va_qsv",   "enabled",  0);
> > +    } else if (CONFIG_DXVA2) {
> >          child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> > -    else {
> > +    } else {
> >          av_log(ctx, AV_LOG_ERROR, "No supported child device type is
> enabled\n");
> >          return AVERROR(ENOSYS);
> >      }
>
> Allowing a user-set child device type seems like a good idea (see the
> original patch).
>
I will be happy if you share the link to original patch.


>
> >
> >      ret = av_hwdevice_ctx_create(&priv->child_device_ctx,
> child_device_type,
> >                                   e ? e->value : NULL,
> child_device_opts, 0);
> > -
> >      av_dict_free(&child_device_opts);
> > -    if (ret < 0)
> > -        return ret;
> > +    if (ret < 0) {
> > +        if (CONFIG_DXVA2 && (child_device_type ==
> AV_HWDEVICE_TYPE_D3D11VA)) {
> > +            // in case of d3d11va fail, try one more chance to create
> device via dxva2
> > +            child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> > +            child_device_opts = NULL;
>
> Leaks the dictionary.
>
> > +            ret = av_hwdevice_ctx_create(&priv->child_device_ctx,
> child_device_type,
> > +                            e ? e->value : NULL, child_device_opts, 0);
> > +        }
> > +        if (ret < 0) {
> > +            return ret;
> > +        }
> > +    }
> >
> >      child_device = (AVHWDeviceContext*)priv->child_device_ctx->data;
> >
> > -    impl = choose_implementation(device);
> > +    impl = choose_implementation(device, child_device_type);
> >
> >      return qsv_device_derive_from_child(ctx, impl, child_device, 0);
> >  }
> > diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h
> > index b98d611cfc..3a322037e5 100644
> > --- a/libavutil/hwcontext_qsv.h
> > +++ b/libavutil/hwcontext_qsv.h
> > @@ -34,6 +34,15 @@
> >   */
> >  typedef struct AVQSVDeviceContext {
> >      mfxSession session;
> > +    /**
> > +     * Need to store actual handle type that session uses
> > +     * MFXVideoCORE_GetHandle() function returns mfxHandleType
> > +     * always equal to MFX_HANDLE_D3D9_DEVICE_MANAGER
> > +     * even when MFX_HANDLE_D3D11_DEVICE was set as handle before by
> > +     * MFXVideoCORE_SetHandle() to mfx session.
> > +     * Fixed already but will be available only with latest driver.
> > +     */
> > +    mfxHandleType handle_type;
>
> This incompatibly changes the ABI because you've made this field required
> where it didn't previously exist.
>
> Not having it at all is clearly best, because the session really does know
> what the handle type is.  If there are some broken drivers where the type
> can't be retrieved maybe we could unconditionally use the existing D3D9
> code on them, which at least wouldn't regress?
>
In fact until now, session always returns DXVA handle type on Windows. Now
we have to differentiate between DXVA2  and DX11 sessions somehow. MFX
Implementation with version 1.30 or higher support correct behavior.
What is your suggestion? Using the D3D9 if not the latest driver even we
are able to keep the device type?

>
> >  } AVQSVDeviceContext;
> >
> >  /**
> > @@ -42,11 +51,11 @@ typedef struct AVQSVDeviceContext {
> >  typedef struct AVQSVFramesContext {
> >      mfxFrameSurface1 *surfaces;
> >      int            nb_surfaces;
> > -
> >      /**
> >       * A combination of MFX_MEMTYPE_* describing the frame pool.
> >       */
> > -    int frame_type;
> > +    int             frame_type;
> > +    void              *texture;
>
> Why do you need to add this?  Each frame already contains the texture
> pointer in data[0].
>
> I added texture member for the cases where AVFrame with data[0] is not
available.
It is just a only one case, for example where we use surfaces array:

@@ -452,6 +456,7 @@  static AVBufferRef *qsv_create_mids(AVBufferRef
*hw_frames_ref)
     for (i = 0; i < nb_surfaces; i++) {
         QSVMid *mid = &mids[i];
         mid->handle        = frames_hwctx->surfaces[i].Data.MemId;+
     mid->texture       = frames_hwctx->texture;
         mid->hw_frames_ref = hw_frames_ref1;

Previously in DXVA2 MemId was D3D9 tesxture, for DX11 we have to store one
more parameter index inside the texture.
Where do you suggest to store texture and index if we have only one member
MemId?


> Also, in existing D3D11 code the texture need not be the same for all
> frames in a frames context (it is when using the default creation with a
> fixed pool size, but not otherwise).  Are you enforcing this because libmfx
> is unable to work with multiple textures, or is there some other reason?
> (If the former then I think you need to be more careful in banning D3D11
> frames contexts of this form earlier, because otherwise you're going to
> cause weird problems by trying to access higher indices of non-array
> textures.)
>
Could you help me to reproduce this unusual behavior where texture need not
be the same for all frames in a frames context?


> >  } AVQSVFramesContext;
> >
> >  #endif /* AVUTIL_HWCONTEXT_QSV_H */
>
> When the original patch for this was written the reason it didn't work was
> because the second parameter (array index) in the HDLpair was silently
> ignored in all driver versions back then.  (So for example the encoder
> always encoded the first texture in the array, which is a fun effect but
> not really desirable.)
>
Driver was changed and supports it. Please see discussion here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20191223183104.61202-1-artem.galin@gmail.com/

>
> How do we distinguish that case now?  Setting D3D11 as the default without
> being able to verify that it actually works is going to fail in a horrible
> way for everyone with a slightly old driver.
>
Agree, it make sense to check driver version.

Thanks,
Artem.
Mark Thompson Feb. 1, 2020, 2:54 p.m. UTC | #3
On 24/01/2020 19:37, Artem Galin wrote:
> On Fri, 24 Jan 2020 at 00:46, Mark Thompson <sw@jkqxz.net> wrote:
> 
>> On 23/01/2020 15:18, Artem Galin wrote:
>>> This enables DX11 support for QSV with higher priority than DX9.
>>> In case of multiple GPUs configuration, DX9 API does not allow to get
>>> access to QSV device in some cases - headless.
>>> Implementation based on DX11 resolves that restriction by enumerating
>> list of available GPUs and finding device with QSV support.
>>>
>>> Signed-off-by: Artem Galin <artem.galin@gmail.com>
>>> ---
>>>  libavcodec/qsv.c              |  38 ++++----
>>>  libavcodec/qsv_internal.h     |   5 +
>>>  libavcodec/version.h          |   2 +-
>>>  libavfilter/qsvvpp.c          |  19 +---
>>>  libavutil/hwcontext_d3d11va.c |  57 +++++++++++-
>>>  libavutil/hwcontext_qsv.c     | 169 +++++++++++++++++++++++++++++-----
>>>  libavutil/hwcontext_qsv.h     |  13 ++-
>>>  libavutil/version.h           |   2 +-
>>>  8 files changed, 242 insertions(+), 63 deletions(-)
>>
>> This should be split into separate commits for the three libraries you
>> touch.
>>
>> These changes are logically one piece of code and do not work
> independently.

I don't understand what you mean by this comment.  libavutil does not depend on the other libraries, and incompatible changes to libavutil are not allowed.

>> ...
>>>
>>> +static int d3d11va_device_find_qsv_adapter(AVHWDeviceContext *ctx, UINT
>> creationFlags)
>>> +{
>>> +    HRESULT hr;
>>> +    IDXGIAdapter *adapter = NULL;
>>> +    int adapter_id = 0;
>>> +    int vendor_id = 0x8086;
>>> +    IDXGIFactory2 *factory;
>>> +    hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void **)&factory);
>>> +    while (IDXGIFactory2_EnumAdapters(factory, adapter_id++, &adapter)
>> != DXGI_ERROR_NOT_FOUND)
>>> +    {
>>> +        ID3D11Device* device = NULL;
>>> +        DXGI_ADAPTER_DESC adapter_desc;
>>> +
>>> +        hr = mD3D11CreateDevice(adapter, D3D_DRIVER_TYPE_UNKNOWN, NULL,
>> creationFlags, NULL, 0, D3D11_SDK_VERSION, &device, NULL, NULL);
>>> +        if (FAILED(hr)) {
>>> +            av_log(ctx, AV_LOG_ERROR, "D3D11CreateDevice returned
>> error\n");
>>> +            continue;
>>> +        }
>>> +
>>> +        hr = IDXGIAdapter2_GetDesc(adapter, &adapter_desc);
>>> +        if (FAILED(hr)) {
>>> +            av_log(ctx, AV_LOG_ERROR, "IDXGIAdapter2_GetDesc returned
>> error\n");
>>> +            continue;
>>> +        }
>>> +
>>> +        if(device)
>>> +            ID3D11Device_Release(device);
>>> +
>>> +        if (adapter)
>>> +            IDXGIAdapter_Release(adapter);
>>> +
>>> +        if (adapter_desc.VendorId == vendor_id) {
>>> +            IDXGIFactory2_Release(factory);
>>> +            return adapter_id - 1;
>>> +        }
>>> +    }
>>> +    IDXGIFactory2_Release(factory);
>>> +    return -1;
>>> +}
>>> +
>>>  static int d3d11va_device_create(AVHWDeviceContext *ctx, const char
>> *device,
>>>                                   AVDictionary *opts, int flags)
>>>  {
>>> @@ -519,7 +559,9 @@ static int d3d11va_device_create(AVHWDeviceContext
>> *ctx, const char *device,
>>>      IDXGIAdapter           *pAdapter = NULL;
>>>      ID3D10Multithread      *pMultithread;
>>>      UINT creationFlags = D3D11_CREATE_DEVICE_VIDEO_SUPPORT;
>>> +    int adapter = -1;
>>>      int is_debug       = !!av_dict_get(opts, "debug", NULL, 0);
>>> +    int is_qsv         = !!av_dict_get(opts, "d3d11va_qsv", NULL, 0);
>>
>> The only constraint you actually check here is the vendor ID, right?  I
>> think it would make more sense to add code which goes looking for a device
>> given the vendor ID rather than hard-coding a special function doing this
>> specific case in here - compare with how VAAPI does exactly the same thing.
>>
>> Agree to change interface to support given vendor id.
> 
> 
>> (That is, make "ffmpeg -init_hw_device d3d11va=,vendor=0x8086" do what
>> you  would expect rather than hacking in a special libmfx case here.)
>>
> 
> Agree that your proposal to have option to choose vendor by given vendor id
> via command line is nice to have optionally and could be added later. This
> patch is about enabling DX11 for qsv at the moment.

The point of adding the option is then to use it in the libmfx code rather than dumping ad-hoc libmfx code in the D3D11 file.

>>> ...
>>> +#endif
>>>  #if CONFIG_DXVA2
>>>      { MFX_HANDLE_D3D9_DEVICE_MANAGER, AV_HWDEVICE_TYPE_DXVA2,
>> AV_PIX_FMT_DXVA2_VLD },
>>>  #endif
>>> @@ -124,18 +131,21 @@ static int qsv_device_init(AVHWDeviceContext *ctx)
>>>      int i;
>>>
>>>      for (i = 0; supported_handle_types[i].handle_type; i++) {
>>> -        err = MFXVideoCORE_GetHandle(hwctx->session,
>> supported_handle_types[i].handle_type,
>>> -                                     &s->handle);
>>> -        if (err == MFX_ERR_NONE) {
>>> -            s->handle_type       =
>> supported_handle_types[i].handle_type;
>>> -            s->child_device_type =
>> supported_handle_types[i].device_type;
>>> -            s->child_pix_fmt     = supported_handle_types[i].pix_fmt;
>>> -            break;
>>> +        if (supported_handle_types[i].handle_type ==
>> hwctx->handle_type) {
>>> +            err = MFXVideoCORE_GetHandle(hwctx->session,
>> supported_handle_types[i].handle_type,
>>> +                                        &s->handle);
>>> +            if (err == MFX_ERR_NONE) {
>>> +                s->handle_type       =
>> supported_handle_types[i].handle_type;
>>> +                s->child_device_type =
>> supported_handle_types[i].device_type;
>>> +                s->child_pix_fmt     =
>> supported_handle_types[i].pix_fmt;
>>> +                break;
>>> +            }
>>>          }
>>>      }
>>>      if (!s->handle) {
>>>          av_log(ctx, AV_LOG_VERBOSE, "No supported hw handle could be
>> retrieved "
>>>                 "from the session\n");
>>> +        return AVERROR_UNKNOWN;
>>
>> Why has this become an error when it wasn't previously?
>>
> I presume previously it was wrong, but never be the case. We can't go
> further if handle is null.

Well, it certainly breaks the software cases.

>>> ...
>>> @@ -492,7 +541,7 @@ static int
>> qsv_init_internal_session(AVHWFramesContext *ctx,
>>>
>>>      err = MFXVideoVPP_Init(*session, &par);
>>>      if (err != MFX_ERR_NONE) {
>>> -        av_log(ctx, AV_LOG_VERBOSE, "Error opening the internal VPP
>> session."
>>> +        av_log(ctx, AV_LOG_ERROR, "Error opening the internal VPP
>> session."
>>>                 "Surface upload/download will not be possible\n");
>>
>> Why is this now an error where it wasn't previously?
>>
> I presume previously it was wrong. It should be an error level.

Why?  Surface upload/download is not a required feature.

>>> ...
>>>
>>>      for (i = 0; i < hwctx->nb_surfaces; i++) {
>>>  #if CONFIG_VAAPI
>>> -        if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
>>> -            (VASurfaceID)(uintptr_t)src->data[3])
>>> -            break;
>>> +        if (AV_HWDEVICE_TYPE_VAAPI == type) {
>>> +            if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
>>> +                (VASurfaceID)(uintptr_t)src->data[3])
>>> +                break;
>>> +        }
>>> +#endif
>>> +#if CONFIG_D3D11VA
>>> +        if (AV_HWDEVICE_TYPE_D3D11VA == type) {
>>> +            if ((hwctx->texture ==
>> (ID3D11Texture2D*)(uintptr_t)src->data[0]) &&
>>> +                ((ID3D11Texture2D*)hwctx->surfaces[i].Data.MemId ==
>>> +                (ID3D11Texture2D*)(uintptr_t)src->data[1]))
>>> +                break;
>>> +        }
>>>  #endif
>>>  #if CONFIG_DXVA2
>>> -        if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
>>> -            (IDirect3DSurface9*)(uintptr_t)src->data[3])
>>> -            break;
>>> +        if (AV_HWDEVICE_TYPE_DXVA2 == type) {
>>> +            if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
>>> +                (IDirect3DSurface9*)(uintptr_t)src->data[3])
>>> +                break;
>>> +        }
>>>  #endif
>>>      }
>>>      if (i >= hwctx->nb_surfaces) {
>>> @@ -1074,7 +1167,7 @@ static void qsv_device_free(AVHWDeviceContext *ctx)
>>>      av_freep(&priv);
>>>  }
>>>
>>> -static mfxIMPL choose_implementation(const char *device)
>>> +static mfxIMPL choose_implementation(const char *device, enum
>> AVHWDeviceType child_device_type)
>>>  {
>>>      static const struct {
>>>          const char *name;
>>> @@ -1103,6 +1196,10 @@ static mfxIMPL choose_implementation(const char
>> *device)
>>>              impl = strtol(device, NULL, 0);
>>>      }
>>>
>>> +    if ( (child_device_type == AV_HWDEVICE_TYPE_D3D11VA) && (impl !=
>> MFX_IMPL_SOFTWARE) ) {
>>> +        impl |= MFX_IMPL_VIA_D3D11;
>>> +    }
>>
>> If this is needed it probably shouldn't be in this function.  This is
>> specifically the string name -> implementation type mapping function.
>>
> In case of DX11, we always have to have MFX_IMPL_HARDWARE_* flag with
> combination with MFX_IMPL_VIA_D3D11. Otherwise it will lead to errors,
> because of unsupported DX11 handle type.

So why is this special for D3D11?

>>> ...
>>> @@ -1226,23 +1335,35 @@ static int qsv_device_create(AVHWDeviceContext
>> *ctx, const char *device,
>>>          // possible, even when multiple devices and drivers are
>> available.
>>>          av_dict_set(&child_device_opts, "kernel_driver", "i915", 0);
>>>          av_dict_set(&child_device_opts, "driver",        "iHD",  0);
>>> -    } else if (CONFIG_DXVA2)
>>> +    } else if (CONFIG_D3D11VA) {
>>> +        child_device_type = AV_HWDEVICE_TYPE_D3D11VA;
>>> +        av_dict_set(&child_device_opts, "d3d11va_qsv",   "enabled",  0);
>>> +    } else if (CONFIG_DXVA2) {
>>>          child_device_type = AV_HWDEVICE_TYPE_DXVA2;
>>> -    else {
>>> +    } else {
>>>          av_log(ctx, AV_LOG_ERROR, "No supported child device type is
>> enabled\n");
>>>          return AVERROR(ENOSYS);
>>>      }
>>
>> Allowing a user-set child device type seems like a good idea (see the
>> original patch).
>>
> I will be happy if you share the link to original patch.

I managed to find <http://ixia.jkqxz.net/~mrt/aa6effaae834d9d1734d5d52fff6a461/0001-qsv-Add-support-for-D3D11.patch>.  I'm not sure if it's the most recent one though, Maxim might have something different?

>>>
>>>      ret = av_hwdevice_ctx_create(&priv->child_device_ctx,
>> child_device_type,
>>>                                   e ? e->value : NULL,
>> child_device_opts, 0);
>>> -
>>>      av_dict_free(&child_device_opts);
>>> -    if (ret < 0)
>>> -        return ret;
>>> +    if (ret < 0) {
>>> +        if (CONFIG_DXVA2 && (child_device_type ==
>> AV_HWDEVICE_TYPE_D3D11VA)) {
>>> +            // in case of d3d11va fail, try one more chance to create
>> device via dxva2
>>> +            child_device_type = AV_HWDEVICE_TYPE_DXVA2;
>>> +            child_device_opts = NULL;
>>
>> Leaks the dictionary.
>>
>>> +            ret = av_hwdevice_ctx_create(&priv->child_device_ctx,
>> child_device_type,
>>> +                            e ? e->value : NULL, child_device_opts, 0);
>>> +        }
>>> +        if (ret < 0) {
>>> +            return ret;
>>> +        }
>>> +    }
>>>
>>>      child_device = (AVHWDeviceContext*)priv->child_device_ctx->data;
>>>
>>> -    impl = choose_implementation(device);
>>> +    impl = choose_implementation(device, child_device_type);
>>>
>>>      return qsv_device_derive_from_child(ctx, impl, child_device, 0);
>>>  }
>>> diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h
>>> index b98d611cfc..3a322037e5 100644
>>> --- a/libavutil/hwcontext_qsv.h
>>> +++ b/libavutil/hwcontext_qsv.h
>>> @@ -34,6 +34,15 @@
>>>   */
>>>  typedef struct AVQSVDeviceContext {
>>>      mfxSession session;
>>> +    /**
>>> +     * Need to store actual handle type that session uses
>>> +     * MFXVideoCORE_GetHandle() function returns mfxHandleType
>>> +     * always equal to MFX_HANDLE_D3D9_DEVICE_MANAGER
>>> +     * even when MFX_HANDLE_D3D11_DEVICE was set as handle before by
>>> +     * MFXVideoCORE_SetHandle() to mfx session.
>>> +     * Fixed already but will be available only with latest driver.
>>> +     */
>>> +    mfxHandleType handle_type;
>>
>> This incompatibly changes the ABI because you've made this field required
>> where it didn't previously exist.
>>
>> Not having it at all is clearly best, because the session really does know
>> what the handle type is.  If there are some broken drivers where the type
>> can't be retrieved maybe we could unconditionally use the existing D3D9
>> code on them, which at least wouldn't regress?
>>
> In fact until now, session always returns DXVA handle type on Windows. Now
> we have to differentiate between DXVA2  and DX11 sessions somehow. MFX
> Implementation with version 1.30 or higher support correct behavior.
> What is your suggestion? Using the D3D9 if not the latest driver even we
> are able to keep the device type?

If there isn't any other way then that sounds like it would work without breaking anything existing.

>>>  } AVQSVDeviceContext;
>>>
>>>  /**
>>> @@ -42,11 +51,11 @@ typedef struct AVQSVDeviceContext {
>>>  typedef struct AVQSVFramesContext {
>>>      mfxFrameSurface1 *surfaces;
>>>      int            nb_surfaces;
>>> -
>>>      /**
>>>       * A combination of MFX_MEMTYPE_* describing the frame pool.
>>>       */
>>> -    int frame_type;
>>> +    int             frame_type;
>>> +    void              *texture;
>>
>> Why do you need to add this?  Each frame already contains the texture
>> pointer in data[0].
>>
>> I added texture member for the cases where AVFrame with data[0] is not
> available.
> It is just a only one case, for example where we use surfaces array:
> 
> @@ -452,6 +456,7 @@  static AVBufferRef *qsv_create_mids(AVBufferRef
> *hw_frames_ref)
>      for (i = 0; i < nb_surfaces; i++) {
>          QSVMid *mid = &mids[i];
>          mid->handle        = frames_hwctx->surfaces[i].Data.MemId;+
>      mid->texture       = frames_hwctx->texture;
>          mid->hw_frames_ref = hw_frames_ref1;
> 
> Previously in DXVA2 MemId was D3D9 tesxture, for DX11 we have to store one
> more parameter index inside the texture.
> Where do you suggest to store texture and index if we have only one member
> MemId?

It's just a pointer, let it point to some other structure.

>> Also, in existing D3D11 code the texture need not be the same for all
>> frames in a frames context (it is when using the default creation with a
>> fixed pool size, but not otherwise).  Are you enforcing this because libmfx
>> is unable to work with multiple textures, or is there some other reason?
>> (If the former then I think you need to be more careful in banning D3D11
>> frames contexts of this form earlier, because otherwise you're going to
>> cause weird problems by trying to access higher indices of non-array
>> textures.)
>>
> Could you help me to reproduce this unusual behavior where texture need not
> be the same for all frames in a frames context?

It's the default behaviour of a D3D11 frames context; it only makes a single array texture when you set a fixed size at the start (specifically for the decode use-case which requires a single array texture).  So for example d3d11 + hwupload filter will do this.

>>> ...

- Mark
galinart March 10, 2020, 4:10 p.m. UTC | #4
Any comments on updated patch by link below:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200306130954.8938-1-artem.galin@gmail.com/


Thanks,
Artem.

On Sat, 1 Feb 2020 at 14:55, Mark Thompson <sw@jkqxz.net> wrote:

> On 24/01/2020 19:37, Artem Galin wrote:
> > On Fri, 24 Jan 2020 at 00:46, Mark Thompson <sw@jkqxz.net> wrote:
> >
> >> On 23/01/2020 15:18, Artem Galin wrote:
> >>> This enables DX11 support for QSV with higher priority than DX9.
> >>> In case of multiple GPUs configuration, DX9 API does not allow to get
> >>> access to QSV device in some cases - headless.
> >>> Implementation based on DX11 resolves that restriction by enumerating
> >> list of available GPUs and finding device with QSV support.
> >>>
> >>> Signed-off-by: Artem Galin <artem.galin@gmail.com>
> >>> ---
> >>>  libavcodec/qsv.c              |  38 ++++----
> >>>  libavcodec/qsv_internal.h     |   5 +
> >>>  libavcodec/version.h          |   2 +-
> >>>  libavfilter/qsvvpp.c          |  19 +---
> >>>  libavutil/hwcontext_d3d11va.c |  57 +++++++++++-
> >>>  libavutil/hwcontext_qsv.c     | 169 +++++++++++++++++++++++++++++-----
> >>>  libavutil/hwcontext_qsv.h     |  13 ++-
> >>>  libavutil/version.h           |   2 +-
> >>>  8 files changed, 242 insertions(+), 63 deletions(-)
> >>
> >> This should be split into separate commits for the three libraries you
> >> touch.
> >>
> >> These changes are logically one piece of code and do not work
> > independently.
>
> I don't understand what you mean by this comment.  libavutil does not
> depend on the other libraries, and incompatible changes to libavutil are
> not allowed.
>
> >> ...
> >>>
> >>> +static int d3d11va_device_find_qsv_adapter(AVHWDeviceContext *ctx,
> UINT
> >> creationFlags)
> >>> +{
> >>> +    HRESULT hr;
> >>> +    IDXGIAdapter *adapter = NULL;
> >>> +    int adapter_id = 0;
> >>> +    int vendor_id = 0x8086;
> >>> +    IDXGIFactory2 *factory;
> >>> +    hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void **)&factory);
> >>> +    while (IDXGIFactory2_EnumAdapters(factory, adapter_id++, &adapter)
> >> != DXGI_ERROR_NOT_FOUND)
> >>> +    {
> >>> +        ID3D11Device* device = NULL;
> >>> +        DXGI_ADAPTER_DESC adapter_desc;
> >>> +
> >>> +        hr = mD3D11CreateDevice(adapter, D3D_DRIVER_TYPE_UNKNOWN,
> NULL,
> >> creationFlags, NULL, 0, D3D11_SDK_VERSION, &device, NULL, NULL);
> >>> +        if (FAILED(hr)) {
> >>> +            av_log(ctx, AV_LOG_ERROR, "D3D11CreateDevice returned
> >> error\n");
> >>> +            continue;
> >>> +        }
> >>> +
> >>> +        hr = IDXGIAdapter2_GetDesc(adapter, &adapter_desc);
> >>> +        if (FAILED(hr)) {
> >>> +            av_log(ctx, AV_LOG_ERROR, "IDXGIAdapter2_GetDesc returned
> >> error\n");
> >>> +            continue;
> >>> +        }
> >>> +
> >>> +        if(device)
> >>> +            ID3D11Device_Release(device);
> >>> +
> >>> +        if (adapter)
> >>> +            IDXGIAdapter_Release(adapter);
> >>> +
> >>> +        if (adapter_desc.VendorId == vendor_id) {
> >>> +            IDXGIFactory2_Release(factory);
> >>> +            return adapter_id - 1;
> >>> +        }
> >>> +    }
> >>> +    IDXGIFactory2_Release(factory);
> >>> +    return -1;
> >>> +}
> >>> +
> >>>  static int d3d11va_device_create(AVHWDeviceContext *ctx, const char
> >> *device,
> >>>                                   AVDictionary *opts, int flags)
> >>>  {
> >>> @@ -519,7 +559,9 @@ static int d3d11va_device_create(AVHWDeviceContext
> >> *ctx, const char *device,
> >>>      IDXGIAdapter           *pAdapter = NULL;
> >>>      ID3D10Multithread      *pMultithread;
> >>>      UINT creationFlags = D3D11_CREATE_DEVICE_VIDEO_SUPPORT;
> >>> +    int adapter = -1;
> >>>      int is_debug       = !!av_dict_get(opts, "debug", NULL, 0);
> >>> +    int is_qsv         = !!av_dict_get(opts, "d3d11va_qsv", NULL, 0);
> >>
> >> The only constraint you actually check here is the vendor ID, right?  I
> >> think it would make more sense to add code which goes looking for a
> device
> >> given the vendor ID rather than hard-coding a special function doing
> this
> >> specific case in here - compare with how VAAPI does exactly the same
> thing.
> >>
> >> Agree to change interface to support given vendor id.
> >
> >
> >> (That is, make "ffmpeg -init_hw_device d3d11va=,vendor=0x8086" do what
> >> you  would expect rather than hacking in a special libmfx case here.)
> >>
> >
> > Agree that your proposal to have option to choose vendor by given vendor
> id
> > via command line is nice to have optionally and could be added later.
> This
> > patch is about enabling DX11 for qsv at the moment.
>
> The point of adding the option is then to use it in the libmfx code rather
> than dumping ad-hoc libmfx code in the D3D11 file.
>
> >>> ...
> >>> +#endif
> >>>  #if CONFIG_DXVA2
> >>>      { MFX_HANDLE_D3D9_DEVICE_MANAGER, AV_HWDEVICE_TYPE_DXVA2,
> >> AV_PIX_FMT_DXVA2_VLD },
> >>>  #endif
> >>> @@ -124,18 +131,21 @@ static int qsv_device_init(AVHWDeviceContext
> *ctx)
> >>>      int i;
> >>>
> >>>      for (i = 0; supported_handle_types[i].handle_type; i++) {
> >>> -        err = MFXVideoCORE_GetHandle(hwctx->session,
> >> supported_handle_types[i].handle_type,
> >>> -                                     &s->handle);
> >>> -        if (err == MFX_ERR_NONE) {
> >>> -            s->handle_type       =
> >> supported_handle_types[i].handle_type;
> >>> -            s->child_device_type =
> >> supported_handle_types[i].device_type;
> >>> -            s->child_pix_fmt     = supported_handle_types[i].pix_fmt;
> >>> -            break;
> >>> +        if (supported_handle_types[i].handle_type ==
> >> hwctx->handle_type) {
> >>> +            err = MFXVideoCORE_GetHandle(hwctx->session,
> >> supported_handle_types[i].handle_type,
> >>> +                                        &s->handle);
> >>> +            if (err == MFX_ERR_NONE) {
> >>> +                s->handle_type       =
> >> supported_handle_types[i].handle_type;
> >>> +                s->child_device_type =
> >> supported_handle_types[i].device_type;
> >>> +                s->child_pix_fmt     =
> >> supported_handle_types[i].pix_fmt;
> >>> +                break;
> >>> +            }
> >>>          }
> >>>      }
> >>>      if (!s->handle) {
> >>>          av_log(ctx, AV_LOG_VERBOSE, "No supported hw handle could be
> >> retrieved "
> >>>                 "from the session\n");
> >>> +        return AVERROR_UNKNOWN;
> >>
> >> Why has this become an error when it wasn't previously?
> >>
> > I presume previously it was wrong, but never be the case. We can't go
> > further if handle is null.
>
> Well, it certainly breaks the software cases.
>
> >>> ...
> >>> @@ -492,7 +541,7 @@ static int
> >> qsv_init_internal_session(AVHWFramesContext *ctx,
> >>>
> >>>      err = MFXVideoVPP_Init(*session, &par);
> >>>      if (err != MFX_ERR_NONE) {
> >>> -        av_log(ctx, AV_LOG_VERBOSE, "Error opening the internal VPP
> >> session."
> >>> +        av_log(ctx, AV_LOG_ERROR, "Error opening the internal VPP
> >> session."
> >>>                 "Surface upload/download will not be possible\n");
> >>
> >> Why is this now an error where it wasn't previously?
> >>
> > I presume previously it was wrong. It should be an error level.
>
> Why?  Surface upload/download is not a required feature.
>
> >>> ...
> >>>
> >>>      for (i = 0; i < hwctx->nb_surfaces; i++) {
> >>>  #if CONFIG_VAAPI
> >>> -        if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
> >>> -            (VASurfaceID)(uintptr_t)src->data[3])
> >>> -            break;
> >>> +        if (AV_HWDEVICE_TYPE_VAAPI == type) {
> >>> +            if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
> >>> +                (VASurfaceID)(uintptr_t)src->data[3])
> >>> +                break;
> >>> +        }
> >>> +#endif
> >>> +#if CONFIG_D3D11VA
> >>> +        if (AV_HWDEVICE_TYPE_D3D11VA == type) {
> >>> +            if ((hwctx->texture ==
> >> (ID3D11Texture2D*)(uintptr_t)src->data[0]) &&
> >>> +                ((ID3D11Texture2D*)hwctx->surfaces[i].Data.MemId ==
> >>> +                (ID3D11Texture2D*)(uintptr_t)src->data[1]))
> >>> +                break;
> >>> +        }
> >>>  #endif
> >>>  #if CONFIG_DXVA2
> >>> -        if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
> >>> -            (IDirect3DSurface9*)(uintptr_t)src->data[3])
> >>> -            break;
> >>> +        if (AV_HWDEVICE_TYPE_DXVA2 == type) {
> >>> +            if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
> >>> +                (IDirect3DSurface9*)(uintptr_t)src->data[3])
> >>> +                break;
> >>> +        }
> >>>  #endif
> >>>      }
> >>>      if (i >= hwctx->nb_surfaces) {
> >>> @@ -1074,7 +1167,7 @@ static void qsv_device_free(AVHWDeviceContext
> *ctx)
> >>>      av_freep(&priv);
> >>>  }
> >>>
> >>> -static mfxIMPL choose_implementation(const char *device)
> >>> +static mfxIMPL choose_implementation(const char *device, enum
> >> AVHWDeviceType child_device_type)
> >>>  {
> >>>      static const struct {
> >>>          const char *name;
> >>> @@ -1103,6 +1196,10 @@ static mfxIMPL choose_implementation(const char
> >> *device)
> >>>              impl = strtol(device, NULL, 0);
> >>>      }
> >>>
> >>> +    if ( (child_device_type == AV_HWDEVICE_TYPE_D3D11VA) && (impl !=
> >> MFX_IMPL_SOFTWARE) ) {
> >>> +        impl |= MFX_IMPL_VIA_D3D11;
> >>> +    }
> >>
> >> If this is needed it probably shouldn't be in this function.  This is
> >> specifically the string name -> implementation type mapping function.
> >>
> > In case of DX11, we always have to have MFX_IMPL_HARDWARE_* flag with
> > combination with MFX_IMPL_VIA_D3D11. Otherwise it will lead to errors,
> > because of unsupported DX11 handle type.
>
> So why is this special for D3D11?
>
> >>> ...
> >>> @@ -1226,23 +1335,35 @@ static int qsv_device_create(AVHWDeviceContext
> >> *ctx, const char *device,
> >>>          // possible, even when multiple devices and drivers are
> >> available.
> >>>          av_dict_set(&child_device_opts, "kernel_driver", "i915", 0);
> >>>          av_dict_set(&child_device_opts, "driver",        "iHD",  0);
> >>> -    } else if (CONFIG_DXVA2)
> >>> +    } else if (CONFIG_D3D11VA) {
> >>> +        child_device_type = AV_HWDEVICE_TYPE_D3D11VA;
> >>> +        av_dict_set(&child_device_opts, "d3d11va_qsv",   "enabled",
> 0);
> >>> +    } else if (CONFIG_DXVA2) {
> >>>          child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> >>> -    else {
> >>> +    } else {
> >>>          av_log(ctx, AV_LOG_ERROR, "No supported child device type is
> >> enabled\n");
> >>>          return AVERROR(ENOSYS);
> >>>      }
> >>
> >> Allowing a user-set child device type seems like a good idea (see the
> >> original patch).
> >>
> > I will be happy if you share the link to original patch.
>
> I managed to find <
> http://ixia.jkqxz.net/~mrt/aa6effaae834d9d1734d5d52fff6a461/0001-qsv-Add-support-for-D3D11.patch>.
> I'm not sure if it's the most recent one though, Maxim might have something
> different?
>
> >>>
> >>>      ret = av_hwdevice_ctx_create(&priv->child_device_ctx,
> >> child_device_type,
> >>>                                   e ? e->value : NULL,
> >> child_device_opts, 0);
> >>> -
> >>>      av_dict_free(&child_device_opts);
> >>> -    if (ret < 0)
> >>> -        return ret;
> >>> +    if (ret < 0) {
> >>> +        if (CONFIG_DXVA2 && (child_device_type ==
> >> AV_HWDEVICE_TYPE_D3D11VA)) {
> >>> +            // in case of d3d11va fail, try one more chance to create
> >> device via dxva2
> >>> +            child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> >>> +            child_device_opts = NULL;
> >>
> >> Leaks the dictionary.
> >>
> >>> +            ret = av_hwdevice_ctx_create(&priv->child_device_ctx,
> >> child_device_type,
> >>> +                            e ? e->value : NULL, child_device_opts,
> 0);
> >>> +        }
> >>> +        if (ret < 0) {
> >>> +            return ret;
> >>> +        }
> >>> +    }
> >>>
> >>>      child_device = (AVHWDeviceContext*)priv->child_device_ctx->data;
> >>>
> >>> -    impl = choose_implementation(device);
> >>> +    impl = choose_implementation(device, child_device_type);
> >>>
> >>>      return qsv_device_derive_from_child(ctx, impl, child_device, 0);
> >>>  }
> >>> diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h
> >>> index b98d611cfc..3a322037e5 100644
> >>> --- a/libavutil/hwcontext_qsv.h
> >>> +++ b/libavutil/hwcontext_qsv.h
> >>> @@ -34,6 +34,15 @@
> >>>   */
> >>>  typedef struct AVQSVDeviceContext {
> >>>      mfxSession session;
> >>> +    /**
> >>> +     * Need to store actual handle type that session uses
> >>> +     * MFXVideoCORE_GetHandle() function returns mfxHandleType
> >>> +     * always equal to MFX_HANDLE_D3D9_DEVICE_MANAGER
> >>> +     * even when MFX_HANDLE_D3D11_DEVICE was set as handle before by
> >>> +     * MFXVideoCORE_SetHandle() to mfx session.
> >>> +     * Fixed already but will be available only with latest driver.
> >>> +     */
> >>> +    mfxHandleType handle_type;
> >>
> >> This incompatibly changes the ABI because you've made this field
> required
> >> where it didn't previously exist.
> >>
> >> Not having it at all is clearly best, because the session really does
> know
> >> what the handle type is.  If there are some broken drivers where the
> type
> >> can't be retrieved maybe we could unconditionally use the existing D3D9
> >> code on them, which at least wouldn't regress?
> >>
> > In fact until now, session always returns DXVA handle type on Windows.
> Now
> > we have to differentiate between DXVA2  and DX11 sessions somehow. MFX
> > Implementation with version 1.30 or higher support correct behavior.
> > What is your suggestion? Using the D3D9 if not the latest driver even we
> > are able to keep the device type?
>
> If there isn't any other way then that sounds like it would work without
> breaking anything existing.
>
> >>>  } AVQSVDeviceContext;
> >>>
> >>>  /**
> >>> @@ -42,11 +51,11 @@ typedef struct AVQSVDeviceContext {
> >>>  typedef struct AVQSVFramesContext {
> >>>      mfxFrameSurface1 *surfaces;
> >>>      int            nb_surfaces;
> >>> -
> >>>      /**
> >>>       * A combination of MFX_MEMTYPE_* describing the frame pool.
> >>>       */
> >>> -    int frame_type;
> >>> +    int             frame_type;
> >>> +    void              *texture;
> >>
> >> Why do you need to add this?  Each frame already contains the texture
> >> pointer in data[0].
> >>
> >> I added texture member for the cases where AVFrame with data[0] is not
> > available.
> > It is just a only one case, for example where we use surfaces array:
> >
> > @@ -452,6 +456,7 @@  static AVBufferRef *qsv_create_mids(AVBufferRef
> > *hw_frames_ref)
> >      for (i = 0; i < nb_surfaces; i++) {
> >          QSVMid *mid = &mids[i];
> >          mid->handle        = frames_hwctx->surfaces[i].Data.MemId;+
> >      mid->texture       = frames_hwctx->texture;
> >          mid->hw_frames_ref = hw_frames_ref1;
> >
> > Previously in DXVA2 MemId was D3D9 tesxture, for DX11 we have to store
> one
> > more parameter index inside the texture.
> > Where do you suggest to store texture and index if we have only one
> member
> > MemId?
>
> It's just a pointer, let it point to some other structure.
>
> >> Also, in existing D3D11 code the texture need not be the same for all
> >> frames in a frames context (it is when using the default creation with a
> >> fixed pool size, but not otherwise).  Are you enforcing this because
> libmfx
> >> is unable to work with multiple textures, or is there some other reason?
> >> (If the former then I think you need to be more careful in banning D3D11
> >> frames contexts of this form earlier, because otherwise you're going to
> >> cause weird problems by trying to access higher indices of non-array
> >> textures.)
> >>
> > Could you help me to reproduce this unusual behavior where texture need
> not
> > be the same for all frames in a frames context?
>
> It's the default behaviour of a D3D11 frames context; it only makes a
> single array texture when you set a fixed size at the start (specifically
> for the decode use-case which requires a single array texture).  So for
> example d3d11 + hwupload filter will do this.
>
> >>> ...
>
> - Mark
> _______________________________________________
> 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".
Soft Works March 10, 2020, 9:36 p.m. UTC | #5
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Artem Galin
> Sent: Tuesday, March 10, 2020 5:10 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: adding DX11 support
> 
> Any comments on updated patch by link below:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200306130954.8938-
> 1-artem.galin@gmail.com/

I don't see any comments there.

[...]

> > >>
> > > Could you help me to reproduce this unusual behavior where texture
> > > need
> > not
> > > be the same for all frames in a frames context?
> >
> > It's the default behaviour of a D3D11 frames context; it only makes a
> > single array texture when you set a fixed size at the start
> > (specifically for the decode use-case which requires a single array
> > texture).  So for example d3d11 + hwupload filter will do this.

The D3D11 hardware context needs to be extended to support non-array 
texture-pools. Otherwise a significant range of graphics devices would fail
to work (practically all that don’t have drivers supporting the most recent SDK 
versions.

softworkz
Max Dmitrichenko March 11, 2020, 11:48 a.m. UTC | #6
On Tue, Mar 10, 2020 at 10:36 PM Soft Works <softworkz@hotmail.com> wrote:

>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Artem Galin
> > Sent: Tuesday, March 10, 2020 5:10 PM
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: adding DX11 support
> >
> > Any comments on updated patch by link below:
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200306130954.8938-
> > 1-artem.galin@gmail.com/
>
> I don't see any comments there.
>
>
I can help commenting below:
> Allow selection of a specific DXGI adapter
QSV can make use only one Intel's adapter
for currently available HW,
if changed in the future - this comment should be addressed.

> Change filters to support mfxHandlePair
Filters should come in next patches,
This is initial implementation which will establish base for later filters
change(s).

> Allow deriving from D3D11VA hw context
please explain more to this comment.


> [...]
>
> > > >>
> > > > Could you help me to reproduce this unusual behavior where texture
> > > > need
> > > not
> > > > be the same for all frames in a frames context?
> > >
> > > It's the default behaviour of a D3D11 frames context; it only makes a
> > > single array texture when you set a fixed size at the start
> > > (specifically for the decode use-case which requires a single array
> > > texture).  So for example d3d11 + hwupload filter will do this.
>
> The D3D11 hardware context needs to be extended to support non-array
> texture-pools. Otherwise a significant range of graphics devices would fail
> to work (practically all that don’t have drivers supporting the most
> recent SDK
> versions.
>
>
preferences of ffmpeg is to keep using array-texture
and therefore goal of patch to add such implementation,

Current QSV HW and drivers already support array-texture,
no issue here.


> softworkz
>

regards
Max
Soft Works March 11, 2020, 9:58 p.m. UTC | #7
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Max Dmitrichenko
> Sent: Wednesday, March 11, 2020 12:49 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: adding DX11 support
> 
> On Tue, Mar 10, 2020 at 10:36 PM Soft Works <softworkz@hotmail.com>
> wrote:
> 
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Artem Galin
> > > Sent: Tuesday, March 10, 2020 5:10 PM
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: adding DX11 support
> > >
> > > Any comments on updated patch by link below:
> > >
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200306130954.893
> > > 8-
> > > 1-artem.galin@gmail.com/
> >
> > I don't see any comments there.
> >
> >
> I can help commenting below:
> > Allow selection of a specific DXGI adapter
> QSV can make use only one Intel's adapter for currently available HW, if
> changed in the future - this comment should be addressed.

1. It wil change this year because Intel is about to release discrete gpu boards
2. Users can have other GPU boars installed so Intel's GPU is not always the first
DXGI/D3D11 adapter

> > Change filters to support mfxHandlePair
> Filters should come in next patches,
> This is initial implementation which will establish base for later filters
> change(s).

I don't think it is a good idea to submit a path that will break filtering
functionality.

The submitted patch is far away from being complete.  I know it
because I've already done a full implementation of D3D11
support for QuickSync (but not yet submitted).

softworkz
Max Dmitrichenko March 12, 2020, 8:56 a.m. UTC | #8
On Wed, Mar 11, 2020 at 10:58 PM Soft Works <softworkz@hotmail.com> wrote:

>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Max Dmitrichenko
> > Sent: Wednesday, March 11, 2020 12:49 PM
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: adding DX11 support
> >
> > On Tue, Mar 10, 2020 at 10:36 PM Soft Works <softworkz@hotmail.com>
> > wrote:
> >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Artem Galin
> > > > Sent: Tuesday, March 10, 2020 5:10 PM
> > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsv: adding DX11 support
> > > >
> > > > Any comments on updated patch by link below:
> > > >
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200306130954.893
> > > > 8-
> > > > 1-artem.galin@gmail.com/
> > >
> > > I don't see any comments there.
> > >
> > >
> > I can help commenting below:
> > > Allow selection of a specific DXGI adapter
> > QSV can make use only one Intel's adapter for currently available HW, if
> > changed in the future - this comment should be addressed.
>
> 1. It wil change this year because Intel is about to release discrete gpu
> boards
>

we have plans to address it
when appropriate


> 2. Users can have other GPU boars installed so Intel's GPU is not always
> the first
> DXGI/D3D11 adapter
>
>
see d3d11va_device_find_adapter_by_vendor_id() related changes


> > > Change filters to support mfxHandlePair
> > Filters should come in next patches,
> > This is initial implementation which will establish base for later
> filters
> > change(s).
>
> I don't think it is a good idea to submit a path that will break filtering
> functionality.
>
>
it is not possible to break DX11 filtering functionality
when DX11 support doesnt exist at the first place.


> The submitted patch is far away from being complete.  I know it
> because I've already done a full implementation of D3D11
> support for QuickSync (but not yet submitted).
>
>
DX11 support has certain and strong benefits.

we have this patch for DX11 support already
no clear understanding if and when any other implementation(s) to expect.

softworkz
>

regards
Max
(max.dmitrichenko@intel.com)
Hendrik Leppkes March 12, 2020, 10:21 a.m. UTC | #9
On Wed, Mar 11, 2020 at 10:58 PM Soft Works <softworkz@hotmail.com> wrote:
>
> The submitted patch is far away from being complete.  I know it
> because I've already done a full implementation of D3D11
> support for QuickSync (but not yet submitted).
>

Do note that only supporting array-textures in d3d11va was an
intentional choice, and its unlikely that a patch that would vastly
increase the complexity in dealing with d3d11va by introducing an
alternate mode for only legacy hardware would likely not see much
agreement.

- Hendrik
Soft Works March 12, 2020, 9:17 p.m. UTC | #10
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Hendrik Leppkes
> On Wed, Mar 11, 2020 at 10:58 PM Soft Works <softworkz@hotmail.com>
> wrote:
> >
> > The submitted patch is far away from being complete.  I know it
> > because I've already done a full implementation of D3D11 support for
> > QuickSync (but not yet submitted).
> >
> 
> Do note that only supporting array-textures in d3d11va was an intentional
> choice, and its unlikely that a patch that would vastly increase the complexity
> in dealing with d3d11va by introducing an alternate mode for only legacy
> hardware would likely not see much agreement.

Hendrik,

In this case, "legacy hardware" does not mean some age-old CPUs but a significant
share of Intel CPUs that are in use today.

softworkz
Soft Works March 12, 2020, 9:25 p.m. UTC | #11
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Max Dmitrichenko
> Sent: Thursday, March 12, 2020 9:57 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> >
> DX11 support has certain and strong benefits.
> 
> we have this patch for DX11 support already no clear understanding if and
> when any other implementation(s) to expect.

Hi Max,

My approach is more complete but still not perfect, that's why I haven't 
submitted it yet.

I'll contact you off-list so we can maybe work together to get a good result.

sotworkz
diff mbox series

Patch

diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index db98c75073..df622d9c7d 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -362,7 +362,11 @@  static int ff_qsv_set_display_handle(AVCodecContext *avctx, QSVSession *qs)
 int ff_qsv_init_internal_session(AVCodecContext *avctx, QSVSession *qs,
                                  const char *load_plugins, int gpu_copy)
 {
+#ifdef AVCODEC_QSV_LINUX_SESSION_HANDLE
     mfxIMPL          impl = MFX_IMPL_AUTO_ANY;
+#else
+    mfxIMPL          impl = MFX_IMPL_AUTO_ANY | MFX_IMPL_VIA_D3D11;
+#endif
     mfxVersion        ver = { { QSV_VERSION_MINOR, QSV_VERSION_MAJOR } };
     mfxInitParam init_par = { MFX_IMPL_AUTO_ANY };
 
@@ -452,6 +456,7 @@  static AVBufferRef *qsv_create_mids(AVBufferRef *hw_frames_ref)
     for (i = 0; i < nb_surfaces; i++) {
         QSVMid *mid = &mids[i];
         mid->handle        = frames_hwctx->surfaces[i].Data.MemId;
+        mid->texture       = frames_hwctx->texture;
         mid->hw_frames_ref = hw_frames_ref1;
     }
 
@@ -661,7 +666,13 @@  static mfxStatus qsv_frame_unlock(mfxHDL pthis, mfxMemId mid, mfxFrameData *ptr)
 static mfxStatus qsv_frame_get_hdl(mfxHDL pthis, mfxMemId mid, mfxHDL *hdl)
 {
     QSVMid *qsv_mid = (QSVMid*)mid;
-    *hdl = qsv_mid->handle;
+    if (qsv_mid->texture) {
+        mfxHDLPair *pair  =  (mfxHDLPair*)hdl;
+        pair->first  = qsv_mid->texture;
+        pair->second = qsv_mid->handle;
+    } else {
+        *hdl = qsv_mid->handle;
+    }
     return MFX_ERR_NONE;
 }
 
@@ -669,14 +680,10 @@  int ff_qsv_init_session_device(AVCodecContext *avctx, mfxSession *psession,
                                AVBufferRef *device_ref, const char *load_plugins,
                                int gpu_copy)
 {
-    static const mfxHandleType handle_types[] = {
-        MFX_HANDLE_VA_DISPLAY,
-        MFX_HANDLE_D3D9_DEVICE_MANAGER,
-        MFX_HANDLE_D3D11_DEVICE,
-    };
     AVHWDeviceContext    *device_ctx = (AVHWDeviceContext*)device_ref->data;
     AVQSVDeviceContext *device_hwctx = device_ctx->hwctx;
     mfxSession        parent_session = device_hwctx->session;
+    mfxHandleType parent_handle_type = device_hwctx->handle_type;
     mfxInitParam            init_par = { MFX_IMPL_AUTO_ANY };
     mfxHDL                    handle = NULL;
 
@@ -686,7 +693,7 @@  int ff_qsv_init_session_device(AVCodecContext *avctx, mfxSession *psession,
     mfxHandleType handle_type;
     mfxStatus err;
 
-    int i, ret;
+    int ret;
 
     err = MFXQueryIMPL(parent_session, &impl);
     if (err == MFX_ERR_NONE)
@@ -695,17 +702,12 @@  int ff_qsv_init_session_device(AVCodecContext *avctx, mfxSession *psession,
         return ff_qsv_print_error(avctx, err,
                                   "Error querying the session attributes");
 
-    for (i = 0; i < FF_ARRAY_ELEMS(handle_types); i++) {
-        err = MFXVideoCORE_GetHandle(parent_session, handle_types[i], &handle);
-        if (err == MFX_ERR_NONE) {
-            handle_type = handle_types[i];
-            break;
-        }
-        handle = NULL;
-    }
-    if (!handle) {
-        av_log(avctx, AV_LOG_VERBOSE, "No supported hw handle could be retrieved "
-               "from the session\n");
+    err = MFXVideoCORE_GetHandle(parent_session, parent_handle_type, &handle);
+    if (err == MFX_ERR_NONE) {
+        handle_type = parent_handle_type;
+    } else {
+        return ff_qsv_print_error(avctx, err,
+            "Error no supported hw handle could be retrieved from the session");
     }
 
 #if QSV_VERSION_ATLEAST(1, 16)
diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h
index 6489836a67..c985f227ec 100644
--- a/libavcodec/qsv_internal.h
+++ b/libavcodec/qsv_internal.h
@@ -65,6 +65,11 @@  typedef struct QSVMid {
     AVFrame *locked_frame;
     AVFrame *hw_frame;
     mfxFrameSurface1 surf;
+    /**
+     * ID3D11Texture2D texture in which the frame is located for D3D11VA device. 
+     * Null in case of DXVA2 device.
+     */
+    void *texture;
 } QSVMid;
 
 typedef struct QSVFrame {
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 6cf333eeb6..2fba26e8d0 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@ 
 #include "libavutil/version.h"
 
 #define LIBAVCODEC_VERSION_MAJOR  58
-#define LIBAVCODEC_VERSION_MINOR  66
+#define LIBAVCODEC_VERSION_MINOR  67
 #define LIBAVCODEC_VERSION_MICRO 100
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
index 8d5ff2eb65..298c10718e 100644
--- a/libavfilter/qsvvpp.c
+++ b/libavfilter/qsvvpp.c
@@ -68,12 +68,6 @@  struct QSVVPPContext {
     int                 nb_ext_buffers;
 };
 
-static const mfxHandleType handle_types[] = {
-    MFX_HANDLE_VA_DISPLAY,
-    MFX_HANDLE_D3D9_DEVICE_MANAGER,
-    MFX_HANDLE_D3D11_DEVICE,
-};
-
 static const AVRational default_tb = { 1, 90000 };
 
 /* functions for frameAlloc */
@@ -497,15 +491,10 @@  static int init_vpp_session(AVFilterContext *avctx, QSVVPPContext *s)
         return AVERROR_UNKNOWN;
     }
 
-    for (i = 0; i < FF_ARRAY_ELEMS(handle_types); i++) {
-        ret = MFXVideoCORE_GetHandle(device_hwctx->session, handle_types[i], &handle);
-        if (ret == MFX_ERR_NONE) {
-            handle_type = handle_types[i];
-            break;
-        }
-    }
-
-    if (ret != MFX_ERR_NONE) {
+    ret = MFXVideoCORE_GetHandle(device_hwctx->session, device_hwctx->handle_type, &handle);
+    if (ret == MFX_ERR_NONE) {
+        handle_type = device_hwctx->handle_type;
+    } else {
         av_log(avctx, AV_LOG_ERROR, "Error getting the session handle\n");
         return AVERROR_UNKNOWN;
     }
diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
index 6670c47579..a08479fd96 100644
--- a/libavutil/hwcontext_d3d11va.c
+++ b/libavutil/hwcontext_d3d11va.c
@@ -244,7 +244,7 @@  static int d3d11va_frames_init(AVHWFramesContext *ctx)
         return AVERROR(EINVAL);
     }
 
-    texDesc = (D3D11_TEXTURE2D_DESC){
+    texDesc = (D3D11_TEXTURE2D_DESC) {
         .Width      = ctx->width,
         .Height     = ctx->height,
         .MipLevels  = 1,
@@ -510,6 +510,46 @@  static void d3d11va_device_uninit(AVHWDeviceContext *hwdev)
     }
 }
 
+static int d3d11va_device_find_qsv_adapter(AVHWDeviceContext *ctx, UINT creationFlags)
+{
+    HRESULT hr;
+    IDXGIAdapter *adapter = NULL;
+    int adapter_id = 0;
+    int vendor_id = 0x8086;
+    IDXGIFactory2 *factory;
+    hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void **)&factory);
+    while (IDXGIFactory2_EnumAdapters(factory, adapter_id++, &adapter) != DXGI_ERROR_NOT_FOUND)
+    {
+        ID3D11Device* device = NULL;
+        DXGI_ADAPTER_DESC adapter_desc;
+
+        hr = mD3D11CreateDevice(adapter, D3D_DRIVER_TYPE_UNKNOWN, NULL, creationFlags, NULL, 0, D3D11_SDK_VERSION, &device, NULL, NULL);
+        if (FAILED(hr)) {
+            av_log(ctx, AV_LOG_ERROR, "D3D11CreateDevice returned error\n");
+            continue;
+        }
+
+        hr = IDXGIAdapter2_GetDesc(adapter, &adapter_desc);
+        if (FAILED(hr)) {
+            av_log(ctx, AV_LOG_ERROR, "IDXGIAdapter2_GetDesc returned error\n");
+            continue;
+        }
+
+        if(device)
+            ID3D11Device_Release(device);
+
+        if (adapter)
+            IDXGIAdapter_Release(adapter);
+
+        if (adapter_desc.VendorId == vendor_id) {
+            IDXGIFactory2_Release(factory);
+            return adapter_id - 1;
+        }
+    }
+    IDXGIFactory2_Release(factory);
+    return -1;
+}
+
 static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
                                  AVDictionary *opts, int flags)
 {
@@ -519,7 +559,9 @@  static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
     IDXGIAdapter           *pAdapter = NULL;
     ID3D10Multithread      *pMultithread;
     UINT creationFlags = D3D11_CREATE_DEVICE_VIDEO_SUPPORT;
+    int adapter = -1;
     int is_debug       = !!av_dict_get(opts, "debug", NULL, 0);
+    int is_qsv         = !!av_dict_get(opts, "d3d11va_qsv", NULL, 0);
     int ret;
 
     // (On UWP we can't check this.)
@@ -538,11 +580,22 @@  static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
         return AVERROR_UNKNOWN;
     }
 
+    if (is_qsv) {
+        adapter = d3d11va_device_find_qsv_adapter(ctx, creationFlags);
+        if (adapter < 0) {
+            av_log(ctx, AV_LOG_ERROR, "Failed to find DX11 adapter with QSV support\n");
+            return AVERROR_UNKNOWN;
+        }
+    }
+
     if (device) {
+        adapter = atoi(device);
+    }
+
+    if (adapter >= 0) {
         IDXGIFactory2 *pDXGIFactory;
         hr = mCreateDXGIFactory(&IID_IDXGIFactory2, (void **)&pDXGIFactory);
         if (SUCCEEDED(hr)) {
-            int adapter = atoi(device);
             if (FAILED(IDXGIFactory2_EnumAdapters(pDXGIFactory, adapter, &pAdapter)))
                 pAdapter = NULL;
             IDXGIFactory2_Release(pDXGIFactory);
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index b1b67400de..05b2ed3333 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -27,9 +27,13 @@ 
 #include <pthread.h>
 #endif
 
+#define COBJMACROS
 #if CONFIG_VAAPI
 #include "hwcontext_vaapi.h"
 #endif
+#if CONFIG_D3D11VA
+#include "hwcontext_d3d11va.h"
+#endif
 #if CONFIG_DXVA2
 #include "hwcontext_dxva2.h"
 #endif
@@ -89,6 +93,9 @@  static const struct {
 #if CONFIG_VAAPI
     { MFX_HANDLE_VA_DISPLAY,          AV_HWDEVICE_TYPE_VAAPI, AV_PIX_FMT_VAAPI },
 #endif
+#if CONFIG_D3D11VA
+    { MFX_HANDLE_D3D11_DEVICE, AV_HWDEVICE_TYPE_D3D11VA, AV_PIX_FMT_D3D11 },
+#endif
 #if CONFIG_DXVA2
     { MFX_HANDLE_D3D9_DEVICE_MANAGER, AV_HWDEVICE_TYPE_DXVA2, AV_PIX_FMT_DXVA2_VLD },
 #endif
@@ -124,18 +131,21 @@  static int qsv_device_init(AVHWDeviceContext *ctx)
     int i;
 
     for (i = 0; supported_handle_types[i].handle_type; i++) {
-        err = MFXVideoCORE_GetHandle(hwctx->session, supported_handle_types[i].handle_type,
-                                     &s->handle);
-        if (err == MFX_ERR_NONE) {
-            s->handle_type       = supported_handle_types[i].handle_type;
-            s->child_device_type = supported_handle_types[i].device_type;
-            s->child_pix_fmt     = supported_handle_types[i].pix_fmt;
-            break;
+        if (supported_handle_types[i].handle_type == hwctx->handle_type) {
+            err = MFXVideoCORE_GetHandle(hwctx->session, supported_handle_types[i].handle_type,
+                                        &s->handle);
+            if (err == MFX_ERR_NONE) {
+                s->handle_type       = supported_handle_types[i].handle_type;
+                s->child_device_type = supported_handle_types[i].device_type;
+                s->child_pix_fmt     = supported_handle_types[i].pix_fmt;
+                break;
+            }
         }
     }
     if (!s->handle) {
         av_log(ctx, AV_LOG_VERBOSE, "No supported hw handle could be retrieved "
                "from the session\n");
+        return AVERROR_UNKNOWN;
     }
 
     err = MFXQueryIMPL(hwctx->session, &s->impl);
@@ -229,9 +239,17 @@  static int qsv_init_child_ctx(AVHWFramesContext *ctx)
         child_device_hwctx->display = (VADisplay)device_priv->handle;
     }
 #endif
+#if CONFIG_D3D11VA
+    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
+        AVD3D11VADeviceContext *child_device_hwctx = child_device_ctx->hwctx;
+        ID3D11Device_AddRef((ID3D11Device*)device_priv->handle);
+        child_device_hwctx->device = (ID3D11Device*)device_priv->handle;
+    }
+#endif
 #if CONFIG_DXVA2
     if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
         AVDXVA2DeviceContext *child_device_hwctx = child_device_ctx->hwctx;
+        IDirect3DDeviceManager9_AddRef((IDirect3DDeviceManager9*)device_priv->handle);
         child_device_hwctx->devmgr = (IDirect3DDeviceManager9*)device_priv->handle;
     }
 #endif
@@ -255,6 +273,16 @@  static int qsv_init_child_ctx(AVHWFramesContext *ctx)
     child_frames_ctx->width             = FFALIGN(ctx->width, 16);
     child_frames_ctx->height            = FFALIGN(ctx->height, 16);
 
+#if CONFIG_D3D11VA
+    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
+        AVD3D11VAFramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
+        child_frames_hwctx->MiscFlags |= D3D11_RESOURCE_MISC_SHARED;
+        if (hwctx->frame_type & MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET)
+            child_frames_hwctx->BindFlags = D3D11_BIND_RENDER_TARGET ;
+        else
+            child_frames_hwctx->BindFlags = D3D11_BIND_DECODER;
+    }
+#endif
 #if CONFIG_DXVA2
     if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
         AVDXVA2FramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
@@ -279,6 +307,18 @@  static int qsv_init_child_ctx(AVHWFramesContext *ctx)
         hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
     }
 #endif
+#if CONFIG_D3D11VA
+    if (child_device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
+        AVD3D11VAFramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
+        hwctx->texture = child_frames_hwctx->texture;
+        for (i = 0; i < ctx->initial_pool_size; i++)
+            s->surfaces_internal[i].Data.MemId = (mfxMemId)(int64_t)i;
+        if (child_frames_hwctx->BindFlags == D3D11_BIND_DECODER)
+            hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
+        else
+            hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET;
+    }
+#endif
 #if CONFIG_DXVA2
     if (child_device_ctx->type == AV_HWDEVICE_TYPE_DXVA2) {
         AVDXVA2FramesContext *child_frames_hwctx = child_frames_ctx->hwctx;
@@ -421,7 +461,16 @@  static mfxStatus frame_unlock(mfxHDL pthis, mfxMemId mid, mfxFrameData *ptr)
 
 static mfxStatus frame_get_hdl(mfxHDL pthis, mfxMemId mid, mfxHDL *hdl)
 {
-    *hdl = mid;
+    AVHWFramesContext    *ctx = pthis;
+    AVQSVFramesContext *hwctx = ctx->hwctx;
+
+    if (hwctx->texture) {
+        mfxHDLPair *pair  =  (mfxHDLPair*)hdl;
+        pair->first  = hwctx->texture;
+        pair->second = mid;
+    } else {
+        *hdl = mid;
+    }
     return MFX_ERR_NONE;
 }
 
@@ -492,7 +541,7 @@  static int qsv_init_internal_session(AVHWFramesContext *ctx,
 
     err = MFXVideoVPP_Init(*session, &par);
     if (err != MFX_ERR_NONE) {
-        av_log(ctx, AV_LOG_VERBOSE, "Error opening the internal VPP session."
+        av_log(ctx, AV_LOG_ERROR, "Error opening the internal VPP session."
                "Surface upload/download will not be possible\n");
         MFXClose(*session);
         *session = NULL;
@@ -668,6 +717,11 @@  static int qsv_map_from(AVHWFramesContext *ctx,
         child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)surf->Data.MemId;
         break;
 #endif
+#if CONFIG_D3D11VA
+    case AV_HWDEVICE_TYPE_D3D11VA:
+        child_data = surf->Data.MemId;
+        break;
+#endif
 #if CONFIG_DXVA2
     case AV_HWDEVICE_TYPE_DXVA2:
         child_data = surf->Data.MemId;
@@ -972,6 +1026,27 @@  static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
         }
         break;
 #endif
+#if CONFIG_D3D11VA
+    case AV_HWDEVICE_TYPE_D3D11VA:
+        {
+            AVD3D11VAFramesContext *src_hwctx = src_ctx->hwctx;
+            s->surfaces_internal = av_mallocz_array(src_ctx->initial_pool_size,
+                                                    sizeof(*s->surfaces_internal));
+            if (!s->surfaces_internal)
+                return AVERROR(ENOMEM);
+            dst_hwctx->texture = src_hwctx->texture;
+            for (i = 0; i < src_ctx->initial_pool_size; i++) {
+                qsv_init_surface(dst_ctx, &s->surfaces_internal[i]);
+                s->surfaces_internal[i].Data.MemId = (mfxMemId)(int64_t)i;
+            }
+            dst_hwctx->nb_surfaces = src_ctx->initial_pool_size;
+            if (src_hwctx->BindFlags == D3D11_BIND_DECODER)
+                dst_hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
+            else
+                dst_hwctx->frame_type = MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET;
+        }
+        break;
+#endif
 #if CONFIG_DXVA2
     case AV_HWDEVICE_TYPE_DXVA2:
         {
@@ -1004,19 +1079,37 @@  static int qsv_frames_derive_to(AVHWFramesContext *dst_ctx,
 static int qsv_map_to(AVHWFramesContext *dst_ctx,
                       AVFrame *dst, const AVFrame *src, int flags)
 {
-    AVQSVFramesContext *hwctx = dst_ctx->hwctx;
+    AVHWFramesContext *src_frames;
     int i, err;
+    enum AVHWDeviceType type = AV_HWDEVICE_TYPE_NONE;
+    AVQSVFramesContext *hwctx = dst_ctx->hwctx;
+    if (src->hw_frames_ctx) {
+        src_frames = (AVHWFramesContext*)src->hw_frames_ctx->data;
+        type = src_frames->internal->hw_type->type;
+    }
 
     for (i = 0; i < hwctx->nb_surfaces; i++) {
 #if CONFIG_VAAPI
-        if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
-            (VASurfaceID)(uintptr_t)src->data[3])
-            break;
+        if (AV_HWDEVICE_TYPE_VAAPI == type) {
+            if (*(VASurfaceID*)hwctx->surfaces[i].Data.MemId ==
+                (VASurfaceID)(uintptr_t)src->data[3])
+                break;
+        }
+#endif
+#if CONFIG_D3D11VA
+        if (AV_HWDEVICE_TYPE_D3D11VA == type) {
+            if ((hwctx->texture == (ID3D11Texture2D*)(uintptr_t)src->data[0]) &&
+                ((ID3D11Texture2D*)hwctx->surfaces[i].Data.MemId ==
+                (ID3D11Texture2D*)(uintptr_t)src->data[1]))
+                break;
+        }
 #endif
 #if CONFIG_DXVA2
-        if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
-            (IDirect3DSurface9*)(uintptr_t)src->data[3])
-            break;
+        if (AV_HWDEVICE_TYPE_DXVA2 == type) {
+            if ((IDirect3DSurface9*)hwctx->surfaces[i].Data.MemId ==
+                (IDirect3DSurface9*)(uintptr_t)src->data[3])
+                break;
+        }
 #endif
     }
     if (i >= hwctx->nb_surfaces) {
@@ -1074,7 +1167,7 @@  static void qsv_device_free(AVHWDeviceContext *ctx)
     av_freep(&priv);
 }
 
-static mfxIMPL choose_implementation(const char *device)
+static mfxIMPL choose_implementation(const char *device, enum AVHWDeviceType child_device_type)
 {
     static const struct {
         const char *name;
@@ -1103,6 +1196,10 @@  static mfxIMPL choose_implementation(const char *device)
             impl = strtol(device, NULL, 0);
     }
 
+    if ( (child_device_type == AV_HWDEVICE_TYPE_D3D11VA) && (impl != MFX_IMPL_SOFTWARE) ) {
+        impl |= MFX_IMPL_VIA_D3D11;
+    }
+
     return impl;
 }
 
@@ -1129,6 +1226,15 @@  static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
         }
         break;
 #endif
+#if CONFIG_D3D11VA
+    case AV_HWDEVICE_TYPE_D3D11VA:
+        {
+            AVD3D11VADeviceContext *child_device_hwctx = child_device_ctx->hwctx;
+            handle_type = MFX_HANDLE_D3D11_DEVICE;
+            handle = (mfxHDL)child_device_hwctx->device;
+        }
+        break;
+#endif
 #if CONFIG_DXVA2
     case AV_HWDEVICE_TYPE_DXVA2:
         {
@@ -1180,6 +1286,7 @@  static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
         goto fail;
     }
 
+    hwctx->handle_type = handle_type;
     return 0;
 
 fail:
@@ -1191,7 +1298,9 @@  fail:
 static int qsv_device_derive(AVHWDeviceContext *ctx,
                              AVHWDeviceContext *child_device_ctx, int flags)
 {
-    return qsv_device_derive_from_child(ctx, MFX_IMPL_HARDWARE_ANY,
+    mfxIMPL impl;
+    impl = choose_implementation("hw_any", child_device_ctx->type);
+    return qsv_device_derive_from_child(ctx, impl,
                                         child_device_ctx, flags);
 }
 
@@ -1226,23 +1335,35 @@  static int qsv_device_create(AVHWDeviceContext *ctx, const char *device,
         // possible, even when multiple devices and drivers are available.
         av_dict_set(&child_device_opts, "kernel_driver", "i915", 0);
         av_dict_set(&child_device_opts, "driver",        "iHD",  0);
-    } else if (CONFIG_DXVA2)
+    } else if (CONFIG_D3D11VA) {
+        child_device_type = AV_HWDEVICE_TYPE_D3D11VA;
+        av_dict_set(&child_device_opts, "d3d11va_qsv",   "enabled",  0);
+    } else if (CONFIG_DXVA2) {
         child_device_type = AV_HWDEVICE_TYPE_DXVA2;
-    else {
+    } else {
         av_log(ctx, AV_LOG_ERROR, "No supported child device type is enabled\n");
         return AVERROR(ENOSYS);
     }
 
     ret = av_hwdevice_ctx_create(&priv->child_device_ctx, child_device_type,
                                  e ? e->value : NULL, child_device_opts, 0);
-
     av_dict_free(&child_device_opts);
-    if (ret < 0)
-        return ret;
+    if (ret < 0) {
+        if (CONFIG_DXVA2 && (child_device_type == AV_HWDEVICE_TYPE_D3D11VA)) {
+            // in case of d3d11va fail, try one more chance to create device via dxva2
+            child_device_type = AV_HWDEVICE_TYPE_DXVA2;
+            child_device_opts = NULL;
+            ret = av_hwdevice_ctx_create(&priv->child_device_ctx, child_device_type,
+                            e ? e->value : NULL, child_device_opts, 0);
+        }
+        if (ret < 0) {
+            return ret;
+        }
+    }
 
     child_device = (AVHWDeviceContext*)priv->child_device_ctx->data;
 
-    impl = choose_implementation(device);
+    impl = choose_implementation(device, child_device_type);
 
     return qsv_device_derive_from_child(ctx, impl, child_device, 0);
 }
diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h
index b98d611cfc..3a322037e5 100644
--- a/libavutil/hwcontext_qsv.h
+++ b/libavutil/hwcontext_qsv.h
@@ -34,6 +34,15 @@ 
  */
 typedef struct AVQSVDeviceContext {
     mfxSession session;
+    /**
+     * Need to store actual handle type that session uses
+     * MFXVideoCORE_GetHandle() function returns mfxHandleType
+     * always equal to MFX_HANDLE_D3D9_DEVICE_MANAGER
+     * even when MFX_HANDLE_D3D11_DEVICE was set as handle before by
+     * MFXVideoCORE_SetHandle() to mfx session.
+     * Fixed already but will be available only with latest driver.
+     */
+    mfxHandleType handle_type;
 } AVQSVDeviceContext;
 
 /**
@@ -42,11 +51,11 @@  typedef struct AVQSVDeviceContext {
 typedef struct AVQSVFramesContext {
     mfxFrameSurface1 *surfaces;
     int            nb_surfaces;
-
     /**
      * A combination of MFX_MEMTYPE_* describing the frame pool.
      */
-    int frame_type;
+    int             frame_type;
+    void              *texture;
 } AVQSVFramesContext;
 
 #endif /* AVUTIL_HWCONTEXT_QSV_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index af8f614aff..2bc1b98615 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  38
+#define LIBAVUTIL_VERSION_MINOR  39
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \