[FFmpeg-devel] [PATCH v2] lavc/qsv: adding DX11 support
Artem Galin
artem.galin at gmail.com
Fri Jan 24 21:37:09 EET 2020
On Fri, 24 Jan 2020 at 00:46, Mark Thompson <sw at 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 at 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.
More information about the ffmpeg-devel
mailing list