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

From: Wu Jianhua <toqsxw@outlook.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
Date: Thu, 13 Oct 2022 15:48:42 +0000
Message-ID: <OSZP286MB21733DFD32E20AEAB7457E98CA259@OSZP286MB2173.JPNP286.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <NEGLL5h--B-2@lynne.ee>

> Lynne<mailto:dev@lynne.ee> wrote:

> Oct 12, 2022, 13:09 by toqsxw@outlook.com:

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

> The Sync locking functions and the queue locking functions should
> be a function pointer in the device/frame context. Vulkan has
> the same issue, and that's how I did it there. This allows for
> API users to plug their own locking primitives in, which they need
> to in case they initialize their own contexts.

I don’t need to follow your design.

> You should also document which fields API users have to set
> themselves if they plan to use their own context.

Where should I document them? Doesn’t the comments enough?

> Also, struct names in the public context lack an AV prefix.
Will fix. And which struct? Could you add the reference?

> D3D12VA_MAX_SURFACES is a terrible hack. Vendors should
> fix their own drivers rather than users running out of memory.

Not my responsibility as a personal developer. I know nothing
about the drivers. You can ask those vendors to fix them. I don’t
think it’s a `terrible hack`. On my test, The MAX_SURFACES is
enough for the decoder. If there are any docs or the drivers fixed
it, just simply remove it. Why user will run out of memory?

> Also, you have code style issues, don't wrap one-line if statements
> or loops in brackets.
Will fix. And which loop? Could you add the reference?

> ff_d3d12dec_get_suitable_max_bitstream_size is an awful function.
> It does float math for sizes and has a magic mult factor of 1.5.
> You have to calculate this properly.
It simply calculate the size of NV12 and P010. Will add comment.

> On a first look, this is what stands out. Really must be split apart
> in patches.

Already claim that I will split it.

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