> From: Lynne<mailto:d...@lynne.ee> > Sent: 2022年10月15日 13:16 > To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] avcodec: add D3D12VA hardware accelerated > H264, HEVC, VP9, and AV1 decoding > >Oct 14, 2022, 14:32 by toq...@outlook.com: > >> Lynne<mailto:d...@lynne.ee> wrote: >> >>> Sent: 2022年10月14日 6:28 >>> To: FFmpeg development discussions and >>> patches<mailto:ffmpeg-devel@ffmpeg.org> >>> Subject: Re: [FFmpeg-devel] [PATCH] avcodec: add D3D12VA hardware >>> accelerated H264, HEVC, VP9, and AV1 decoding >>> >>> Oct 13, 2022, 17:48 by toq...@outlook.com: >>> >>>>> Lynne<mailto:d...@lynne.ee> wrote: >>>>> >>>>> Oct 12, 2022, 13:09 by toq...@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. >>>> >>> >>> Yes, you do, because it's not my design, it's the design of the entire >>> hwcontext system. If API users cannot initialize a hwcontext with >>> their own, it's breakage. It's not optional. >>> Locking primitives are a part of this. >>> Either fix this or this is simply not getting merged. >>> >> >> As I learn from the docs of Direct3D12, it is a multithreading-friendly API, >> which means that the only mandatory field, device, set by the user can be >> accessed and used without any lock. Why do the API users need locking >> primitives to init the hwcontext_d3d12va? Have you got the initialization >> failed? If so, could you share the runtime error details here? >> >> And I checked the other hwcontexts, they didn't have a locking primitive >> also. >> The hwcontext_d3d11va has a locking used to protect accesses to >> device_context >> and video_context calls, which d3d12 doesn't need. So why the API users >> cannot >> initialize the hwcontext with their own? Is there any real failure case? >> Please pardon >> I don't quite understand what situation you are talking about, could you >> elaborate >> further if I get it wrong? >> >>>>> 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? >>>> >>> In the comments. Look at how it's done elsewhere. >>> >> There is already a comment like what d3d11va wrote: >> /** >> * 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; >> >> Is there anything else missing? >> > > What about the frames context? Frame contexts are also user-settable.
Yeah. The comment is missing. I will add them. Thanks. >>>>> Also, struct names in the public context lack an AV prefix. >>>>> >>>> Will fix. And which struct? Could you add the reference? >>>> >>> >>> In the main public context. >>> >> I check the codes and found that AVD3D12VASyncContext, >> AVD3D12VADeviceContext, >> AVD3D12FrameDescriptor and AVD3D12VAFramesContext are the structs in >> the hwcontext_d3d12va. Is there any other struct without AV prefix? Could you >> paste the name here? >> >>>>> 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? >>>> >>> >>> The whole way the hwcontext is designed is sketchy. Why are >>> you keeping all texture information in an array (texture_infos)?Frames are >>> already pooled (it's called AVFramePool), so you're >>> not doing anything by adding a second layer on top of this other >>> than complexity. >>> The initial_pool_size parameter was a hack added to support >>> hwcontexts which cannot allocate surfaces dynamically, you don't >>> need to support this at all, you can just let users allocate >>> frames as they need to rather than preinitializing. >>> >> >> It’s the same implementation as d3d11va. The static initial_pool_size is >> needed by the decoder heap to initialize and the input stream needs it >> as well The feature that allows the user to allocate frames is not the basic >> functionalities of decoding. I recommend the user who needs the feature >> implement it and contribute the codes. >> > > It'll limit the hwcontext to just decoding, but whatever. > > Are there any devices that don't support Vulkan but support d3d12? Maybe those device only care about one thing, like a games console or something. Just a guess. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
