[FFmpeg-devel] [PATCH] avcodec: add D3D12VA hardware accelerated H264, HEVC, VP9, and AV1 decoding
James Almer
jamrial at gmail.com
Thu Oct 13 19:04:21 EEST 2022
On 10/13/2022 12:48 PM, Wu Jianhua wrote:
>> Lynne<mailto:dev at lynne.ee> wrote:
>
>> Oct 12, 2022, 13:09 by toqsxw at 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.
>
>> 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list