Video Encode/Decode#226
Conversation
|
Wow! Magnificent! Thanks a ton for bringing this work to life! I will try to review later today. |
Should
Should we add more formats? We should invent better names :) DXGI has short names, but they are unclear (seems to be a good start):
No D3D11 support? Probably not needed, but, IM, nice to have :) (...to be continued) |
|
Q1 Q2: Q3 |
+1, clarifying comments may be added if needed. They are pretty standard.
No objections here (assuming the interface is implementable in D3D11) |
|
Do we need other formats, like |
|
Please, add this link somewhere in the beginning of |
|
I'm ready to merge it after you are done with polishing. Just let me know. Further improvements can be made in I haven't deeply analyzed the implementation, but the API additions are nice and clear. Also I think that these formats: can be removed and |
Definitely. I'll look into it as soon as I can.
I like that. |
The current set targets the common 4:2:0 format family needed by the decode/encode path: NV12, P010, P016. Y410 is packed 4:4:4 and would add support for it in a follow-up if the need arises. |
|
I don't like that you're using native API structures in NRI public API. Wouldn't be logical to implemented it as a translation layer, not in the NRIWrapper* way? |
- "TextureViewDesc::readonlyPlanes" renamed to "planes". Logic switched to "direct" from "inverse"
- removed special depth-stencil format for shader resource views ("R24_UNORM_X8", "X24_G8_UINT", "R32_SFLOAT_X8_X24" and "X32_G8_UINT_X24"). Use "planes" instead
- explained valid usage
OTHER:
- VK: untangled "planes" (aspect mask) logic to be friendly for subpass inputs
- prerequisite for video support (PR #226)
@Daedie-git I have implemented it. Please, update your code to the latest since it's a prerequisite for the video support. Please, note that I have removed PR #222 since the code is completely different for these lines. Feel free to adjust, re-add or discuss. Thanks in advance! We are moving forward!
Just echoing already said, I absolutely agree with this. GAPI specific structs must be moved to corresponding |
Indeed. I have addressed this locally already. I'll rebase it on your change and push it soon. |
…ueues # Conflicts: # Source/VK/DescriptorVK.hpp
437decc to
4a2960d
Compare
|
I wrapped up my changes. |
|
I found some issues, so I will follow up with another commit. |
|
Additional question: For the sake of testing this, I've been building up quite the test suite in my own repo. Do you want to have it in some form? |
|
May I continue "massaging" the code or better wait for fixes from your side, if there is something in the works? |
Feel free. |
|
Do we really need |
… NRI as a logical entity
… some (not all) error checks to "Val" (they were different in D3D12/VK for some reason)
Not strictly necessary. VideoPicture is the exception: it adapts an NRI texture/subresource into backend-specific video picture state, so keeping CreateVideoPicture in the video interface makes sense I think. The main reason of having them was to have an native passthrough option for things that weren't modelled yet in the abstraction. This would also be the main argument for keeping it: currently unsupported features/options. |
|
Please, allow me to do this:
What do you think? |
Sounds good to me. |
…K_SUPPORT" (it's all or nothing, NRI requires latest known AgilitySDK if it's enabled)
- TODO: allow wrapping of video command lists - TODO: AV1 is incomplete, "1" interface usage is incomplete
- 4 ComPtrs reduced to 2 - added TODOs since the usage is still incomplete - honored "private"
|
I spent many days working on video support. Notes:
It looks like we need an "AI skill" to properly define:
Yeah, it sounds vague but it's a way to go. I have already made tons of changes :( In any case the AI assistant must review the recent. |
AV is indeed incomplete. Somewhat deliberately. Because the PR has already scope crept quite a bit beyond what I initially intended to do. Which, to be clear, is not a problem for me. But I felt compelled to not drag this on indefinitely, which has left things in a bit of a "in between" state. In hindsight I probably should've pulled the original PR and re-open it in a more complete state. That being said. My schedule will be freeing up quite a bit soon, and I can pick up where I left off and iterate this towards a sufficiently complete state. To reduce AI agent entropy, my main recommendations are:
Sorry if I have left you with a frustrating experience 🙂 |
|
I don't use AI coding assistants yet. But I'm planning to setup Claude and after getting familiar with it, yeah, AI-related |
#if NRI_ENABLE_AGILITY_SDK_SUPPORT
commandList->EncodeFrame1(session.GetEncoder(), session.GetEncoderHeap(), &input, &output);
#else
commandList->EncodeFrame(session.GetEncoder(), session.GetEncoderHeap(), &input, &output); // FAILS to compile
#endif
...
#if NRI_ENABLE_AGILITY_SDK_SUPPORT
commandList->ResolveEncoderOutputMetadata1(&resolveInput, &resolveOutput);
#else
commandList->ResolveEncoderOutputMetadata(&resolveInput, &resolveOutput); // FAILS to compile
#endifAs it turned out, real video encoding (i.e.
so, it's "all or nothing". |
The open encode/decode enticed me to try and get VK hardware decoding working in my software (until now, I only had it on the D3D12 side). I expect this will need further changes, just let me know.
I wasn't sure what the right way to handle the D3D12 CommandBuffer was, I settled on the variant for now.
Summary
Adds hardware video queue support and a minimal native-backed video encode/decode extension API.
This branch makes video-capable queues visible through NRI, allows D3D12/Vulkan video command buffers to be created and submitted, adds
basic multi-planar video format support, and exposes
NRIVideoentry points for issuing native encode/decode commands.Changes
Public API
QueueType::VIDEO_DECODEQueueType::VIDEO_ENCODEG8_B8R8_2PLANE_420_UNORMG10X6_B10X6R10X6_2PLANE_420_UNORM_3PACK16G16_B16R16_2PLANE_420_UNORMPLANE_0PLANE_1PLANE_2Include/Extensions/NRIVideo.hVideoInterfaceCmdDecodeVideoCmdEncodeVideoAdapter and Queue Discovery
ID3D12VideoDeviceis availableVkQueueFamilyVideoPropertiesKHRto ensure Vulkan video queues also expose compatible codec operationsD3D12 Backend
D3D12_COMMAND_LIST_TYPE_VIDEO_DECODED3D12_COMMAND_LIST_TYPE_VIDEO_ENCODECommandBufferD3D12to hold graphics, video decode, or video encode command listsCmdDecodeVideoviaID3D12VideoDecodeCommandList::DecodeFrameCmdEncodeVideoviaID3D12VideoEncodeCommandList2::EncodeFrameID3D12CommandList*, since video command lists are not graphics command listsVulkan Backend
VK_KHR_video_queueVK_KHR_video_decode_queueVK_KHR_video_encode_queuevkCmdDecodeVideoKHRandvkCmdEncodeVideoKHRCmdDecodeVideo/CmdEncodeVideoforwarding to Vulkan command buffersValidation / Interface Wiring
VideoInterfacethroughnriGetInterfaceDeviceBase::FillFunctionTable(VideoInterface&)FillFunctionTable(VideoInterface&)implementationsAPI Shape
The new video API is intentionally small and native-backed.
NRI handles queue discovery, command buffer ownership, function table exposure, and command dispatch. Codec/session setup remains
backend-native (initially):
VkVideoDecodeInfoKHR/VkVideoEncodeInfoKHRpayloadsValidation
Built successfully with: