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 16:08:27 +0000
Message-ID: <OSZP286MB2173B3CB6DCA15E38C25A8E6CA259@OSZP286MB2173.JPNP286.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <58ed0f9c-5c1d-885a-ba12-c6a9afeefdf1@gmail.com>

James Almer<mailto:jamrial@gmail.com> wrote:
> On 10/13/2022 12:48 PM, Wu Jianhua wrote:
>>> 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.
>
> Then you should probably use imgutils.h functions for that, and/or
> AVPixFmtDescriptor from pixdesc.h.

Great! Really thanks for the details. I will try to take a look at how to do that better.

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