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