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

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

[Wu Jianhua]

[-- Attachment #1: Type: text/plain, Size: 115 bytes --]

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

Patches attached.




[-- Attachment #2: 0004-avcodec-add-D3D12VA-hardware-accelerated-VP9-decodin.patch --]
[-- Type: application/octet-stream, Size: 15188 bytes --]

[-- Attachment #3: 0005-avcodec-add-D3D12VA-hardware-accelerated-AV1-decodin.patch --]
[-- Type: application/octet-stream, Size: 24714 bytes --]

[-- Attachment #4: 0006-Changelog-add-D3D12VA-hardware-accelerated-H264-HEVC.patch --]
[-- Type: application/octet-stream, Size: 1215 bytes --]

[-- Attachment #5: 0001-libavutil-add-hwcontext_d3d12va-and-AV_PIX_FMT_D3D12.patch --]
[-- Type: application/octet-stream, Size: 40484 bytes --]

[-- Attachment #6: 0002-avcodec-add-D3D12VA-hardware-accelerated-H264-decodi.patch --]
[-- Type: application/octet-stream, Size: 45738 bytes --]

[-- Attachment #7: 0003-avcodec-add-D3D12VA-hardware-accelerated-HEVC-decodi.patch --]
[-- Type: application/octet-stream, Size: 22298 bytes --]

[-- Attachment #8: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[Wu, Tong1]

> [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9,
> and AV1 decoding
> 
> Patches attached.

>-		      HWACCEL_D3D11VA2(h264),
>+                               HWACCEL_D3D11VA(h264),

This shouldn't be changed. It will block d3d11va h264dec.

>+#endif
>+#if CONFIG_H264_D3D12DEC_HWACCEL
>+                               HWACCEL_D3D12DEC(h264),


_______________________________________________
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".

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

[Hendrik Leppkes]

On Fri, Dec 23, 2022 at 7:01 PM Wu Jianhua <toqsxw@outlook.com> wrote:
>
> [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
>
> Patches attached.
>

The naming scheme on this seems to be rather inconsistent. Both
"d3d12dec" and "d3d12va" seem to be used in different places - please
standardize it all on "d3d12va", it matches d3d11va and remains
consistent with itself.

Additionally, why are you not supporting all codecs that D3D11
supports? Are there any limitations on what it supports, for some
reason?

- Hendrik
_______________________________________________
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".

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

[Wu Jianhua]

> From: Wu, Tong1<mailto:tong1.wu-at-intel.com@ffmpeg.org>
> Sent: 2023年2月28日 22:51
> To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
>
>> [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9,
>> and AV1 decoding
>>
>> Patches attached.
>
>>-                    HWACCEL_D3D11VA2(h264),
>>+                               HWACCEL_D3D11VA(h264),
>
>This shouldn't be changed. It will block d3d11va h264dec.

Yes, I should not change it. I will fix it in the next patch.

Thanks.
Jianhua

>
>>+#endif
>>+#if CONFIG_H264_D3D12DEC_HWACCEL
>>+                               HWACCEL_D3D12DEC(h264),


_______________________________________________
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".

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

[Wu Jianhua]

> From: Hendrik Leppkes<mailto:h.leppkes@gmail.com>
> Sent: 2023年3月1日 23:55
> To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
>
> On Fri, Dec 23, 2022 at 7:01 PM Wu Jianhua <toqsxw@outlook.com> wrote:
>>
>> [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
>>
>> Patches attached.
>>
>
> The naming scheme on this seems to be rather inconsistent. Both
> "d3d12dec" and "d3d12va" seem to be used in different places - please
> standardize it all on "d3d12va", it matches d3d11va and remains
> consistent with itself.

Hi Hendrik,

The d3d12 supports video encoding that d3d11 does not, so I cannot use
d3d12va_h264 just like d3d11va_h264. I think to reserve d3d12enc_h264/hevc/vp9/av1
for d3d12 video encoding and use d3d12dec_h264/hevc/vp9/av1 for d3d12 video
decoding now. If we want to use the d3d12va naming scheme, we might have
to use some name like d3d12va_dec_h264 to distinguish decoder and encoder.
What do you think?

>
> Additionally, why are you not supporting all codecs that D3D11
> supports? Are there any limitations on what it supports, for some
> reason?
>

Actually, I have all D3D12 codecs implemented on my local repository.
The reason is fairly simple, I got some issues but I don't have much free
time to fix them recently. Maybe I can send another patch set for them
in the future.

Thanks,
Jianhua

>- Hendrik
>

_______________________________________________
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".

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

[Hendrik Leppkes]

On Tue, Apr 11, 2023 at 6:41 PM Wu Jianhua <toqsxw@outlook.com> wrote:
>
> > From: Hendrik Leppkes<mailto:h.leppkes@gmail.com>
> > Sent: 2023年3月1日 23:55
> > To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
> >
> > On Fri, Dec 23, 2022 at 7:01 PM Wu Jianhua <toqsxw@outlook.com> wrote:
> >>
> >> [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
> >>
> >> Patches attached.
> >>
> >
> > The naming scheme on this seems to be rather inconsistent. Both
> > "d3d12dec" and "d3d12va" seem to be used in different places - please
> > standardize it all on "d3d12va", it matches d3d11va and remains
> > consistent with itself.
>
> Hi Hendrik,
>
> The d3d12 supports video encoding that d3d11 does not, so I cannot use
> d3d12va_h264 just like d3d11va_h264. I think to reserve d3d12enc_h264/hevc/vp9/av1
> for d3d12 video encoding and use d3d12dec_h264/hevc/vp9/av1 for d3d12 video
> decoding now. If we want to use the d3d12va naming scheme, we might have
> to use some name like d3d12va_dec_h264 to distinguish decoder and encoder.
> What do you think?
>

Why do you need codec specific files for decoding?
I would assume the actual information structures remain the same, and
dxva2_<codec>.c can be used, just as its used for both dxva2 and
d3d11va now.
There should be no code duplication here, as that will make it very
easy to add new codecs in the future, one addition rather then
multiple.

This would then easily allow you to have "d3d12va" as the general
module and hwaccel name, and have a singular "d3d12vadec.c" for the
decoder, with any extra encoding files as needed.

- Hendrik
_______________________________________________
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".

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

[Wu Jianhua]

> From: Hendrik Leppkes<mailto:h.leppkes@gmail.com>
> Sent: 2023年4月12日 1:28
> To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
>
> On Tue, Apr 11, 2023 at 6:41 PM Wu Jianhua <toqsxw@outlook.com> wrote:
>>
>> > From: Hendrik Leppkes<mailto:h.leppkes@gmail.com>
>> > Sent: 2023年3月1日 23:55
>> > To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org>
>> > Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
>> >
>> > On Fri, Dec 23, 2022 at 7:01 PM Wu Jianhua <toqsxw@outlook.com> wrote:
>> >>
>> >> [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
>> >>
>> >> Patches attached.
>> >>
>>> >
>> > The naming scheme on this seems to be rather inconsistent. Both
>> > "d3d12dec" and "d3d12va" seem to be used in different places - please
> >> standardize it all on "d3d12va", it matches d3d11va and remains
> >> consistent with itself.
>>
>> Hi Hendrik,
>>
>> The d3d12 supports video encoding that d3d11 does not, so I cannot use
>> d3d12va_h264 just like d3d11va_h264. I think to reserve d3d12enc_h264/hevc/vp9/av1
>> for d3d12 video encoding and use d3d12dec_h264/hevc/vp9/av1 for d3d12 video
>> decoding now. If we want to use the d3d12va naming scheme, we might have
>> to use some name like d3d12va_dec_h264 to distinguish decoder and encoder.
>> What do you think?
>>
>
> Why do you need codec specific files for decoding?
> I would assume the actual information structures remain the same, and
> dxva2_<codec>.c can be used, just as its used for both dxva2 and
> d3d11va now.
>
> There should be no code duplication here, as that will make it very
> easy to add new codecs in the future, one addition rather then
> multiple.
>
> This would then easily allow you to have "d3d12va" as the general
> module and hwaccel name, and have a singular "d3d12vadec.c" for the
> decoder, with any extra encoding files as needed.
>

Yes. I wrote all d3d12 codes to dxva_codec at the first time, but finally,
it gets ugly and had fewer readabilities for more and more #if #endif
and if else introduced. There are already a lot of if and else. The primary
difference between d3d11va and dxva2 is the GetBuffer and BeginFrame
but d3d12 has more change in updating arguments.

Such as the codes in dxva2_h264.c
#if CONFIG_D3D11VA
    if (ff_dxva2_is_d3d11(avctx)) {
        D3D11_VIDEO_DECODER_BUFFER_DESC *dsc11 = bs;
        memset(dsc11, 0, sizeof(*dsc11));
        dsc11->BufferType           = type;
        dsc11->DataSize             = current - dxva_data;
        dsc11->NumMBsInBuffer       = mb_count;

        type = D3D11_VIDEO_DECODER_BUFFER_SLICE_CONTROL;

        av_assert0((dsc11->DataSize & 127) == 0);
    }
#endif
#if CONFIG_DXVA2
    if (avctx->pix_fmt == AV_PIX_FMT_DXVA2_VLD) {
        DXVA2_DecodeBufferDesc *dsc2 = bs;
        memset(dsc2, 0, sizeof(*dsc2));
        dsc2->CompressedBufferType = type;
        dsc2->DataSize             = current - dxva_data;
        dsc2->NumMBsInBuffer       = mb_count;

        type = DXVA2_SliceControlBufferType;

        av_assert0((dsc2->DataSize & 127) == 0);
    }
#endif

Even though the codes are doing the same thing, we still
cannot avoid writing codes like this for the different structure
between D3D11_VIDEO_DECODER_BUFFER_DESC  and
DXVA2_DecodeBufferDesc, right?

How about this solution? The major part of duplicated codes
are fill_picture_parameters, fill_scaling_lists, h264_decode_slice,
and fill_slice_short, and others have fewer codes and fewer similarities.
Can we let d3d12va_h264 call dava2_fill_xxxx to reduce these codes?
And keep h264_d3d12va in d3d12va_h264.c and the codes get more
concise and have better readabilities.

Thanks,
Jianhua


_______________________________________________
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".

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

[Wu Jianhua]

From: Anton Khirnov<mailto:anton@khirnov.net>
Sent: 2023年3月1日 23:52
To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding

Hi Anton,

>> +#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
>> + */
>
> 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).
>

Can I put COBJMACROS, and the DX_CHECK without AV-prefixed in hwcontext_d3d12va_internal.h
and used them in d3d12dec_h264/HEVC/VP9/AV1.cpp and hwcontext_d3d12va.cpp?

>> +
>> +/**
>> + * @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.

It’s provided to the users so they can control the synchronization by themselves.
Both keeping or deleting it is fine because the user can write the codes to do the
same thing. I use it frequently when rendering video frames with D3D12 so I think
it's helpful to make it a public API. What do you think?

All the other comments are very helpful and I will try to fix them in the next patch.
Thanks for your detailed review.

Thanks,
Jianhua



_______________________________________________
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".

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

[Anton Khirnov]

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".