From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id E195C45483 for ; Wed, 1 Mar 2023 15:52:20 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6DD9E68ADF2; Wed, 1 Mar 2023 17:52:17 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E673068ACEF for ; Wed, 1 Mar 2023 17:52:10 +0200 (EET) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 97D9B2404EC for ; Wed, 1 Mar 2023 16:52:10 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id TxNBpKIY6TrK for ; Wed, 1 Mar 2023 16:52:09 +0100 (CET) Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 5065A240178 for ; Wed, 1 Mar 2023 16:52:09 +0100 (CET) Received: by lain.khirnov.net (Postfix, from userid 1000) id 330221601B2; Wed, 1 Mar 2023 16:52:09 +0100 (CET) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: =?utf-8?q?=3COSZP286MB217328A12BC439F5BB31897DCAE99=40OSZP286MB?= =?utf-8?q?2173=2EJPNP286=2EPROD=2EOUTLOOK=2ECOM=3E?= References: =?utf-8?q?=3COSZP286MB217380EBFB2351E3B8B54E25CAE99=40OSZP286MB2?= =?utf-8?q?173=2EJPNP286=2EPROD=2EOUTLOOK=2ECOM=3E_=3COSZP286MB217328A12BC43?= =?utf-8?q?9F5BB31897DCAE99=40OSZP286MB2173=2EJPNP286=2EPROD=2EOUTLOOK=2ECOM?= =?utf-8?q?=3E?= Mail-Followup-To: FFmpeg development discussions and patches Date: Wed, 01 Mar 2023 16:52:09 +0100 Message-ID: <167768592917.10789.405819018051248134@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Quoting Wu Jianhua (2022-12-23 19:01:01) > diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h > index 7ff08c8608..691c49087c 100644 > --- a/libavutil/hwcontext.h > +++ b/libavutil/hwcontext.h > @@ -33,6 +33,7 @@ enum AVHWDeviceType { > AV_HWDEVICE_TYPE_QSV, > AV_HWDEVICE_TYPE_VIDEOTOOLBOX, > AV_HWDEVICE_TYPE_D3D11VA, > + AV_HWDEVICE_TYPE_D3D12VA, You cannot add this in the middle of an enum - it will shift the value of the following identifiers and thus break ABI. Add it at the end. > AV_HWDEVICE_TYPE_DRM, > AV_HWDEVICE_TYPE_OPENCL, > AV_HWDEVICE_TYPE_MEDIACODEC, > diff --git a/libavutil/hwcontext_d3d12va.c b/libavutil/hwcontext_d3d12va.c > new file mode 100644 > index 0000000000..94fb172b4b > --- /dev/null > +++ b/libavutil/hwcontext_d3d12va.c > @@ -0,0 +1,696 @@ > +/* > + * Direct3D 12 HW acceleration. > + * > + * copyright (c) 2022 Wu Jianhua > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include "config.h" > +#include "common.h" > +#include "hwcontext.h" > +#include "hwcontext_d3d12va.h" > +#include "hwcontext_internal.h" > +#include "imgutils.h" > +#include "pixdesc.h" > +#include "pixfmt.h" > +#include "thread.h" > +#include "compat/w32dlfcn.h" > +#include > + > +typedef HRESULT(WINAPI *PFN_CREATE_DXGI_FACTORY2)(UINT Flags, REFIID riid, void **ppFactory); > + > +static AVOnce functions_loaded = AV_ONCE_INIT; > + > +static PFN_CREATE_DXGI_FACTORY2 d3d12va_create_dxgi_factory2; > +static PFN_D3D12_CREATE_DEVICE d3d12va_create_device; > +static PFN_D3D12_GET_DEBUG_INTERFACE d3d12va_get_debug_interface; > + > +static av_cold void load_functions(void) > +{ > + HANDLE d3dlib, dxgilib; > + > + d3dlib = dlopen("d3d12.dll", 0); > + dxgilib = dlopen("dxgi.dll", 0); > + if (!d3dlib || !dxgilib) > + return; > + > + d3d12va_create_device = (PFN_D3D12_CREATE_DEVICE)GetProcAddress(d3dlib, "D3D12CreateDevice"); > + d3d12va_create_dxgi_factory2 = (PFN_CREATE_DXGI_FACTORY2)GetProcAddress(dxgilib, "CreateDXGIFactory2"); > + d3d12va_get_debug_interface = (PFN_D3D12_GET_DEBUG_INTERFACE)GetProcAddress(d3dlib, "D3D12GetDebugInterface"); > +} > + > +typedef struct D3D12VAFramesContext { > + ID3D12Resource *staging_buffer; > + ID3D12CommandQueue *command_queue; > + ID3D12CommandAllocator *command_allocator; > + ID3D12GraphicsCommandList *command_list; > + AVD3D12VASyncContext *sync_ctx; > + int nb_surfaces; > + int nb_surfaces_used; > + DXGI_FORMAT format; > + UINT luma_component_size; > +} D3D12VAFramesContext; > + > +static const struct { > + DXGI_FORMAT d3d_format; > + enum AVPixelFormat pix_fmt; > +} supported_formats[] = { > + { DXGI_FORMAT_NV12, AV_PIX_FMT_NV12 }, > + { DXGI_FORMAT_P010, AV_PIX_FMT_P010 }, > +}; > + > +DXGI_FORMAT av_d3d12va_map_sw_to_hw_format(enum AVPixelFormat pix_fmt) > +{ > + switch (pix_fmt) { > + case AV_PIX_FMT_NV12:return DXGI_FORMAT_NV12; > + case AV_PIX_FMT_P010:return DXGI_FORMAT_P010; > + default: return DXGI_FORMAT_UNKNOWN; > + } > +} > + > +int av_d3d12va_create_sync_context(AVD3D12VADeviceContext *ctx, AVD3D12VASyncContext **sync_ctx) > +{ > + AVD3D12VASyncContext *sync; > + > + *sync_ctx = av_mallocz(sizeof(AVD3D12VASyncContext)); Memory allocations need to be checked. > + sync = *sync_ctx; > + DX_CHECK(ID3D12Device_CreateFence(ctx->device, sync->fence_value, D3D12_FENCE_FLAG_NONE, &IID_ID3D12Fence, &sync->fence)); > + > + sync->event = CreateEvent(NULL, FALSE, FALSE, NULL); > + if (!sync->event) > + goto fail; > + > + return 0; > + > +fail: You should actually free the objects you created here. > diff --git a/libavutil/hwcontext_d3d12va.h b/libavutil/hwcontext_d3d12va.h > new file mode 100644 > index 0000000000..6e1c9750b7 > --- /dev/null > +++ b/libavutil/hwcontext_d3d12va.h > @@ -0,0 +1,229 @@ > +/* > + * Direct3D 12 HW acceleration. > + * > + * copyright (c) 2022 Wu Jianhua > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#ifndef AVUTIL_HWCONTEXT_D3D12VA_H > +#define AVUTIL_HWCONTEXT_D3D12VA_H > + > +/** > + * @def COBJMACROS > + * > + * @brief Enable C style interface for D3D12 > + */ > +#ifndef COBJMACROS > +#define COBJMACROS > +#endif Does this need to be in the public header? > + > +/** > + * @file > + * An API-specific header for AV_HWDEVICE_TYPE_D3D12VA. > + * > + */ > +#include > +#include > +#include > +#include > + > +/** > + * @def DX_CHECK > + * > + * @brief A check macro used by D3D12 functions highly frequently > + */ > +#define DX_CHECK(hr) if (FAILED(hr)) { \ > + av_log(NULL, AV_LOG_ERROR, "D3D12 error occurred! Please enable debug=1 and check the output for more details!\n"); \ av_log() to NULL context must not be used > + goto fail; \ > +} All identifiers declared in public headers should be AV-prefixed. Also, these seem like they should be in a private header instead (i.e. one which is not installed and not usable by our API callers). > + > +/** > + * @def D3D12_OBJECT_RELEASE > + * > + * @brief A release macro used by D3D12 objects highly frequently > + */ > +#define D3D12_OBJECT_RELEASE(pInterface) if (pInterface) { \ > + IUnknown_Release((IUnknown *)pInterface); \ > + pInterface = NULL; \ > +} > + > +/** > + * @def D3D12VA_MAX_SURFACES > + * > + * @brief Maximum number surfaces > + * The reference processing over decoding will be corrupted on some drivers > + * if the max number of reference frames exceeds this. > + */ > +#define D3D12VA_MAX_SURFACES 32 > + > +/** > + * @brief This struct is allocated as AVHWDeviceContext.hwctx > + * > + */ > +typedef struct AVD3D12VADeviceContext { > + /** > + * Device used for objects creation and access. This can also be > + * used to set the libavcodec decoding device. > + * > + * Can be set by the user. This is the only mandatory field - the other > + * device context fields are set from this and are available for convenience. > + * > + * Deallocating the AVHWDeviceContext will always release this interface, > + * and it does not matter whether it was user-allocated. > + */ > + ID3D12Device *device; > + > + /** > + * If unset, this will be set from the device field on init. > + * > + * Deallocating the AVHWDeviceContext will always release this interface, > + * and it does not matter whether it was user-allocated. > + */ > + ID3D12VideoDevice *video_device; > + > + /** > + * Specifed by sync=1 when init d3d12va > + * > + * Execute commands as sync mode > + */ > + int sync; > +} AVD3D12VADeviceContext; > + > +/** > + * @brief This struct is used to sync d3d12 execution > + * > + */ > +typedef struct AVD3D12VASyncContext { > + /** > + * D3D12 fence object > + */ > + ID3D12Fence *fence; > + > + /** > + * A handle to the event object > + */ > + HANDLE event; > + > + /** > + * The fence value used for sync > + */ > + uint64_t fence_value; > +} AVD3D12VASyncContext; > + > +/** > + * @brief D3D12 frame descriptor for pool allocation. > + * > + */ > +typedef struct AVD3D12FrameDescriptor { > + /** > + * The texture in which the frame is located. The reference count is > + * managed by the AVBufferRef, and destroying the reference will release > + * the interface. > + * > + * Normally stored in AVFrame.data[0]. > + */ > + ID3D12Resource *texture; > + > + /** > + * The index into the array texture element representing the frame > + * > + * Normally stored in AVFrame.data[1] (cast from intptr_t). > + */ > + intptr_t index; > + > + /** > + * The sync context for the texture > + * > + * Use av_d3d12va_wait_idle(sync_ctx) to ensure the decoding or encoding have been finised > + * @see: https://learn.microsoft.com/en-us/windows/win32/medfound/direct3d-12-video-overview#directx-12-fences > + * > + * Normally stored in AVFrame.data[2]. > + */ > + AVD3D12VASyncContext *sync_ctx; > +} AVD3D12FrameDescriptor; > + > +/** > + * @brief This struct is allocated as AVHWFramesContext.hwctx > + * > + */ > +typedef struct AVD3D12VAFramesContext { > + /** > + * The same implementation as d3d11va > + * This field is not able to be user-allocated at the present. > + */ > + AVD3D12FrameDescriptor *texture_infos; > +} AVD3D12VAFramesContext; > + > +/** > + * @brief Map sw pixel format to d3d12 format > + * > + * @param pix_fmt All these empy param specifiers are just noise with zero useful information. > + * @return d3d12 specified format > + */ > +DXGI_FORMAT av_d3d12va_map_sw_to_hw_format(enum AVPixelFormat pix_fmt); > + > +/** > + * @brief Create a AVD3D12VASyncContext > + * > + * @param ctx > + * @param sync_ctx > + * @return Error code (ret < 0 if failed) > + */ > +int av_d3d12va_create_sync_context(AVD3D12VADeviceContext *ctx, AVD3D12VASyncContext **sync_ctx); Our standard naming convention is something like av_d3d12va_sync_context_alloc() > + > +/** > + * @brief Release a AVD3D12VASyncContext > + * > + * @param sync_ctx > + * @return Error code (ret < 0 if failed), can be ignored ^^^^^^^^^^^^^^ This seems untrue, as the function will not actually free memory on failure. > + */ > +int av_d3d12va_release_sync_context(AVD3D12VASyncContext *sync_ctx); Our standard naming convention is something like av_d3d12va_sync_context_free(). It should also take a double pointer and write a NULL to it on return. > + > +/** > + * @brief Wait for a specified fence value > + * > + * The execution fence values timeline may be like: > + * #0 -> #1 -> #2 -> #3 ----------------> #n > + * ^ > + * | > + * Call av_d3d12va_wait_for_fence_value(2) will wait #2 execution to be completed. > + * > + * @param sync_ctx > + * @param fence_value > + * @return Error code (ret < 0 if failed) > + */ > +int av_d3d12va_wait_for_fence_value(AVD3D12VASyncContext *sync_ctx, uint64_t fence_value); Does this need to be public? I don't see it being used in any other patches. Then fence_value could also be made private. > + > +/** > + * @brief Wait for the sync context to the idle state > + * > + * @param sync_ctx > + * @return Error code (ret < 0 if failed) > + */ > +int av_d3d12va_wait_idle(AVD3D12VASyncContext *sync_ctx); > + > +/** > + * @brief Wait for a specified command queue to the idle state > + * > + * @param sync_ctx > + * @param command_queue > + * @return Error code (ret < 0 if failed) > + */ > +int av_d3d12va_wait_queue_idle(AVD3D12VASyncContext *sync_ctx, ID3D12CommandQueue *command_queue); > + > +#endif /* AVUTIL_HWCONTEXT_D3D12VA_H */ > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > index 37c2c79e01..1870133bf7 100644 > --- a/libavutil/pixfmt.h > +++ b/libavutil/pixfmt.h > @@ -332,6 +332,15 @@ enum AVPixelFormat { > */ > AV_PIX_FMT_D3D11, > > + /** > + * Hardware surfaces for Direct3D12. > + * > + * data[0] contains a ID3D12Resource pointer. > + * data[1] contains the resource array index of the frame as intptr_t. > + * data[2] contains the sync context for current resource > + */ > + AV_PIX_FMT_D3D12, Same here as for device types, you have to add it at the end, right before AV_PIX_FMT_NB. -- Anton Khirnov _______________________________________________ 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".