Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding

[Anton Khirnov <anton@khirnov.net>]

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 <toqsxw@outlook.com>
> + *
> + * 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 <dxgi1_3.h>
> +
> +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 <toqsxw@outlook.com>
> + *
> + * 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 <stdint.h>
> +#include <initguid.h>
> +#include <d3d12.h>
> +#include <d3d12video.h>
> +
> +/**
> + * @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".