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

[Wu Jianhua]

> From: Hendrik Leppkes<mailto:h.leppkes at gmail.com>
> Sent: 2023年4月12日 1:28
> To: FFmpeg development discussions and patches<mailto:ffmpeg-devel at 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 at outlook.com> wrote:
>>
>> > From: Hendrik Leppkes<mailto:h.leppkes at gmail.com>
>> > Sent: 2023年3月1日 23:55
>> > To: FFmpeg development discussions and patches<mailto:ffmpeg-devel at 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 at 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