On Sun, 26 Apr 2020 at 19:54, Mark Thompson <s...@jkqxz.net> wrote: > On 24/04/2020 15:52, artem.ga...@gmail.com wrote: > > From: Artem Galin <artem.ga...@intel.com> > > > > Makes selection of d3d11va device type by default and over DirectX 9, > > which is still supported but requires explicit selection. > > ... which might break users with older drivers/systems. Some comment on > exactly which setups are broken would be helpful here. > > > This enables usage of non-powered/headless GPU, better HDR support. > > Pool of resources is allocated as one texture with array of slices. > > > > Added d3d11va device selection by vendor id. > > Example: --init_hw_device d3d11va:,vendor=0x8086 > > > > DirectX 9 usage. > > Example: --init_hw_device qsv:hw,child_device_type=dxva2 > > > > Signed-off-by: Artem Galin <artem.ga...@intel.com> > > --- > > libavutil/hwcontext_d3d11va.c | 82 +++++++- > > libavutil/hwcontext_d3d11va.h | 8 + > > libavutil/hwcontext_qsv.c | 363 +++++++++++++++++++++++++++------- > > 3 files changed, 379 insertions(+), 74 deletions(-) > > Please split this patch into the different changes. There are at least > three separate things here: > > Done in new patch set https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1165
> - Adding more texture information to the D3D11 hwcontext API. > - Adding the vendor option to D3D11 device creation. > - Supporting D3D11 in libmfx hwcontext. > > Since the first one is an external API change it probably wants more > explanation as to why it is wanted and the effects on compatibility. > > > diff --git a/libavutil/hwcontext_d3d11va.c > b/libavutil/hwcontext_d3d11va.c > > index c8ae58f908..c5e127aadb 100644 > > --- a/libavutil/hwcontext_d3d11va.c > > +++ b/libavutil/hwcontext_d3d11va.c > > @@ -72,7 +72,7 @@ static av_cold void load_functions(void) > > } > > > > typedef struct D3D11VAFramesContext { > > - int nb_surfaces_used; > > + size_t nb_surfaces; > > > > DXGI_FORMAT format; > > > > @@ -112,6 +112,8 @@ static void d3d11va_frames_uninit(AVHWFramesContext > *ctx) > > if (s->staging_texture) > > ID3D11Texture2D_Release(s->staging_texture); > > s->staging_texture = NULL; > > + > > + av_freep(&frames_hwctx->texture_infos); > > } > > > > static int d3d11va_frames_get_constraints(AVHWDeviceContext *ctx, > > @@ -152,8 +154,9 @@ static void free_texture(void *opaque, uint8_t *data) > > av_free(data); > > } > > > > -static AVBufferRef *wrap_texture_buf(ID3D11Texture2D *tex, int index) > > +static AVBufferRef *wrap_texture_buf(AVHWFramesContext *ctx, > ID3D11Texture2D *tex, int index) > > { > > + AVD3D11VAFramesContext *frames_hwctx = ctx->hwctx; > > AVBufferRef *buf; > > AVD3D11FrameDescriptor *desc = av_mallocz(sizeof(*desc)); > > if (!desc) { > > @@ -161,6 +164,10 @@ static AVBufferRef > *wrap_texture_buf(ID3D11Texture2D *tex, int index) > > return NULL; > > } > > > > + frames_hwctx->texture_infos[frames_hwctx->nb_surfaces_used].texture > = tex; > > + frames_hwctx->texture_infos[frames_hwctx->nb_surfaces_used].index = > index; > > + frames_hwctx->nb_surfaces_used++; > > + > > desc->texture = tex; > > desc->index = index; > > > > @@ -199,13 +206,12 @@ static AVBufferRef > *d3d11va_alloc_single(AVHWFramesContext *ctx) > > return NULL; > > } > > > > - return wrap_texture_buf(tex, 0); > > + return wrap_texture_buf(ctx, tex, 0); > > } > > > > static AVBufferRef *d3d11va_pool_alloc(void *opaque, int size) > > { > > AVHWFramesContext *ctx = (AVHWFramesContext*)opaque; > > - D3D11VAFramesContext *s = ctx->internal->priv; > > AVD3D11VAFramesContext *hwctx = ctx->hwctx; > > D3D11_TEXTURE2D_DESC texDesc; > > > > @@ -214,13 +220,13 @@ static AVBufferRef *d3d11va_pool_alloc(void > *opaque, int size) > > > > ID3D11Texture2D_GetDesc(hwctx->texture, &texDesc); > > > > - if (s->nb_surfaces_used >= texDesc.ArraySize) { > > + if (hwctx->nb_surfaces_used >= texDesc.ArraySize) { > > av_log(ctx, AV_LOG_ERROR, "Static surface pool size > exceeded.\n"); > > return NULL; > > } > > > > ID3D11Texture2D_AddRef(hwctx->texture); > > - return wrap_texture_buf(hwctx->texture, s->nb_surfaces_used++); > > + return wrap_texture_buf(ctx, hwctx->texture, > hwctx->nb_surfaces_used); > > } > > > > static int d3d11va_frames_init(AVHWFramesContext *ctx) > > @@ -267,7 +273,7 @@ static int d3d11va_frames_init(AVHWFramesContext > *ctx) > > av_log(ctx, AV_LOG_ERROR, "User-provided texture has > mismatching parameters\n"); > > return AVERROR(EINVAL); > > } > > - } else if (texDesc.ArraySize > 0) { > > + } else if (!(texDesc.BindFlags & D3D11_BIND_RENDER_TARGET) && > texDesc.ArraySize > 0) { > > hr = ID3D11Device_CreateTexture2D(device_hwctx->device, > &texDesc, NULL, &hwctx->texture); > > if (FAILED(hr)) { > > av_log(ctx, AV_LOG_ERROR, "Could not create the texture > (%lx)\n", (long)hr); > > @@ -275,6 +281,12 @@ static int d3d11va_frames_init(AVHWFramesContext > *ctx) > > } > > } > > > > + hwctx->texture_infos = av_mallocz_array(ctx->initial_pool_size, > sizeof(*hwctx->texture_infos)); > > + if (!hwctx->texture_infos) > > + return AVERROR(ENOMEM); > > + > > + s->nb_surfaces = ctx->initial_pool_size; > > + > > ctx->internal->pool_internal = > av_buffer_pool_init2(sizeof(AVD3D11FrameDescriptor), > > ctx, > d3d11va_pool_alloc, NULL); > > if (!ctx->internal->pool_internal) > > @@ -511,15 +523,55 @@ static void > d3d11va_device_uninit(AVHWDeviceContext *hwdev) > > } > > } > > > > +static int d3d11va_device_find_adapter_by_vendor_id(AVHWDeviceContext > *ctx, UINT creationFlags, long int vendor_id) > > +{ > > + HRESULT hr; > > + IDXGIAdapter *adapter = NULL; > > + int adapter_id = 0; > > + 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; > > +} > > This duplicates quite a bit of the function below. Can you rearrange the > code it to avoid doing that? > Refactored. > > > + > > static int d3d11va_device_create(AVHWDeviceContext *ctx, const char > *device, > > AVDictionary *opts, int flags) > > { > > AVD3D11VADeviceContext *device_hwctx = ctx->hwctx; > > > > HRESULT hr; > > + AVDictionaryEntry *e; > > 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 ret; > > > > @@ -539,11 +591,24 @@ static int d3d11va_device_create(AVHWDeviceContext > *ctx, const char *device, > > return AVERROR_UNKNOWN; > > } > > > > + e = av_dict_get(opts, "vendor", NULL, 0); > > + if (e) { > > + long int vendor_id = strtol(e->value, NULL, 0); > > + adapter = d3d11va_device_find_adapter_by_vendor_id(ctx, > creationFlags, vendor_id); > > + if (adapter < 0) { > > + av_log(ctx, AV_LOG_ERROR, "Failed to find d3d11va adapter > by vendor id %s\n", e ? e->value : NULL); > > + 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); > > @@ -568,6 +633,7 @@ static int d3d11va_device_create(AVHWDeviceContext > *ctx, const char *device, > > return AVERROR_UNKNOWN; > > } > > > > + av_log(ctx, AV_LOG_VERBOSE, "Using D3D11 device.\n"); > > This log message is not adding very much. > Fixed. > > > hr = ID3D11Device_QueryInterface(device_hwctx->device, > &IID_ID3D10Multithread, (void **)&pMultithread); > > if (SUCCEEDED(hr)) { > > ID3D10Multithread_SetMultithreadProtected(pMultithread, TRUE); > > diff --git a/libavutil/hwcontext_d3d11va.h > b/libavutil/hwcontext_d3d11va.h > > index 9f91e9b1b6..f3def15bea 100644 > > --- a/libavutil/hwcontext_d3d11va.h > > +++ b/libavutil/hwcontext_d3d11va.h > > @@ -39,6 +39,11 @@ > > #include <d3d11.h> > > #include <stdint.h> > > > > +typedef struct D3D11TextureInfo { > > + ID3D11Texture2D *texture; > > + size_t index; > > +} D3D11TextureInfo; > > + > > /** > > * This struct is allocated as AVHWDeviceContext.hwctx > > */ > > @@ -164,6 +169,9 @@ typedef struct AVD3D11VAFramesContext { > > * This field is ignored/invalid if a user-allocated texture is > provided. > > */ > > UINT MiscFlags; > > + > > + D3D11TextureInfo *texture_infos; > > Please include documentation for this new field. (In particular, explain > what the user should set it to in a user-created context.) > > > + size_t nb_surfaces_used; > > Why is this value added to the public API? You haven't used it outside > hwcontext_d3d11.c. > > > } AVD3D11VAFramesContext; > > > > #endif /* AVUTIL_HWCONTEXT_D3D11VA_H */ > > ... > > - 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-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".