VkVideoEncoder: add VK_KHR_video_encode_feedback2 support for AV1 - #224
VkVideoEncoder: add VK_KHR_video_encode_feedback2 support for AV1#224elima wants to merge 22 commits into
Conversation
|
Heads-up for @dabrain34, @lolzballs, @srinathkr-nv |
|
you should run the tests with You should not report an issue which is not public |
df3ba4b to
ad8be88
Compare
|
Seems like the CTS build from CI is failing because the headers aren't new enough? @dabrain34 are you able to assist with that? |
|
Oh, the Windows arm64 build is failing because of not new enough headers. The CTS build is failing because it doesn't have the new inls? Not sure what we need to do in this situation, but it seems like it's just an integration issue so maybe we can bypass the CTS check in this case? |
That's also what I think we should do. The problem could happen with other extensions too, seems like an issue inherent to this CI job. We are currently discussing options. |
Right, my bad. Both have been fixed. |
ad8be88 to
ca1adf1
Compare
|
I think that would be great to have a full unit test in |
|
I added new tests in 1.av1_encode_feedback is crashing with
on nvidia On Radv I havent been able to find a working public driver. |
Ok, I will look into it. |
|
I fixed most of the reported review issues. Pushed one commit for each fix, so that is easier to review. Once the branch is clear, I will squash the fixes into the main (original) commits. Please let me know if you prefer squashing them now. |
Added two AV1 cases to encode_samples.json: one covering picture-level, pixel-count, and skipped-pixel feedback, and another covering per-partition feedback with --maxPerPartitionFeedbackEntries. Unsupported hardware capabilities are reported as N/S by the runner. |
|
|
When I run on nvidia driver offering `VK_KHR_video_encode_feedback2 : extension revision 1: I get a Is it expected ? With I get a failure result Can you add more information about your working setup ? |
This is expected that CI runner is reporting |
I get the exact same results on NVidia (GeForce RTX 4060 Ti / NVidia 595.44.00):
On AMD (RADV and a custom, non-public branch of Mesa that supports feedback2), I get:
I'm looking into the NVidia failure. My preliminary assessment is that the driver is outputting the right feedback2 values, but leaving the final status blank (zero), when it should be status-complete. A similar CTS test passes so this needs further investigation. |
VK_KHR_video_encode_feedback2 first appears in Vulkan-Headers v1.4.353. The build pinned the minimum at v1.4.321, which predates the extension, so the fetched headers left every feedback2 type and flag undefined. Bump the minimum patch version to 353 so the encoder builds against headers that provide the extension.
This builds on top of video encode functionality used by CTS for AV1 codec. Support for other H.26X codecs will be added in a follow-up.
These will be used in CTS for validation against decoded bitstream as part of per-partition count tests.
After updating to v1.4.353, the Vulkan-Loader build reads
VulkanHeaders_VERSION from find_package(VulkanHeaders).
When the headers come from FetchContent there is no installed package
to find, the variable is empty and CMake fails with:
CMake Error: if given arguments: "VERSION_GREATER" "1.4.353"
This only happens on hosts without a system Vulkan SDK, such as
Windows CI runners, because only they build the loader from source.
Set VulkanHeaders_VERSION manually before configuring the loader,
matching the version of the fetched headers.
The encode feedback query status was only checked with assert(), which compiles away in Release builds, so a failed or incomplete encode operation would go unnoticed and the bitstream data would be consumed with a garbage offset and size. Test the status explicitly and return VK_ERROR_UNKNOWN when it is not VK_QUERY_RESULT_STATUS_COMPLETE_KHR. Do not assert in the handled failure path, so assert-enabled and Release builds report the same error.
Chain VkVideoEncodeFeedback2CapabilitiesKHR into the encode capabilities query and use it to validate the query pool creation: fail when per-partition feedback is unsupported, clamp the requested entry count to the reported maximum, and mask out unsupported per-partition feedback flags.
…lags VK_ERROR_FEATURE_NOT_PRESENT describes the failure better than VK_ERROR_INITIALIZATION_FAILED and is handled as "unsupported" by IsVideoUnsupportedResult().
Min/max quantization feedback flags were silently masked out when not supported by the implementation. Report the dropped flags instead.
string_VkResult() and string_VkResult_Extended() had no callers.
The values parsed from "--pictureFeedback --params" were never read. Remove them and the related help text.
Use the existing conditional pNext accumulator instead of a separate ternary chain, and check the feature with the same CHECK_VULKAN_FEATURE syntax as the other optional features.
SetVideoEncodeFeedback2Enabled() now adds the required device extension itself, so the feature flag and the extension cannot get out of sync.
Add runner coverage for picture-level QP and pixel-count feedback, skipped-pixel feedback, and per-partition feedback with a configurable maximum entry count.
The options report aggregate pixel counts rather than per-pixel feedback. Rename --pixelFeedback and --skippedPixelFeedback to --pixelCountFeedback and --skippedPixelCountFeedback, and update the corresponding configuration members, help text, and test sample.
The BITSTREAM_HAS_OVERRIDES feedback value was parsed but no CLI option ever requested it, so it was never exercised. Add --hasOverridesFeedback, which requests VK_VIDEO_ENCODE_FEEDBACK_BITSTREAM_HAS_OVERRIDES_BIT_KHR and writes the reported value (overrides=...) to the feedback text output. Exercise it from the av1_encode_feedback test.
The combined check printed the unsupported feedback flags as a single hexadecimal mask, which is hard to map back to a specific request. Loop over the unsupported bits and report each one by name so it is clear exactly which requested feedback the implementation lacks before failing with VK_ERROR_FEATURE_NOT_PRESENT.
Vulkan-Loader reads VulkanHeaders_VERSION to gate part of its configuration (notably the Windows arm64 build), and it is left unset when the headers come from FetchContent instead of an installed package. Document why we seed it here and add a TODO to drop it once Vulkan-Loader derives the version itself as a subproject.
99703bf to
aa03f3d
Compare
Regarding the encode_av1_encode_feedback_per_partition failure on NVidia (595.44.00 / RTX 4060 Ti): we believe this is a driver bug rather than an issue in this PR. The encode itself completes normally — all 15 frames are processed and a non-empty bitstream is produced (e.g. bitstreamStartOffset=0, bitstreamSize=42486) — but the whole-picture query status reads back 0x0 (VK_QUERY_RESULT_STATUS_NOT_READY_KHR) on every frame, so the encoder treats the result as incomplete and aborts, leaving an empty .ivf that then fails decode validation. That status is read only after waiting on the encode-complete fence and with VK_QUERY_RESULT_WAIT_BIT set, with vkGetQueryPoolResults returning VK_SUCCESS, so a NOT_READY status is contradictory: the results are finalized and the preceding offset/size fields read back correctly, which indicates the driver populated the feedback values but never wrote the whole-picture status (leaving it at its reset value) instead of VK_QUERY_RESULT_STATUS_COMPLETE_KHR. Notably the AV1 CTS feedback2_partition_count_* tests request per-partition feedback only, whereas this case also enables --pictureFeedback, so the trigger may be the combination of whole-picture QP feedback with per-partition feedback. @srinathkr-nv can you take a look please, in case we are missing something? |
|
@elima I noticed your above comments just now, after being at-mentioned. Can you try out the latest Vulkan beta driver (Linux 595.44.11, Windows 596.83) from https://developer.nvidia.com/vulkan-driver to see if this issue reproduces with it? |
Hi, Srinath! I also changed the video sample to one with higher resolution, because the original test had 352x288 which I understand NVidia feedback2 doesn't support. But still the code never reaches that point, the extension is not advertised. I have spent several rounds of debugging trying to find a culprit on VVS side, but have exhausted all avenues. Is it possible that later drivers like 610.43.03 (and 595.45.04) don't support (or advertise) encode_feedback2? |
Yes, this is expected. The link to the 595.44.11 beta driver is present in the section "Vulkan 1.4 Developer Beta Driver Downloads" but I'll provide it here directly: https://developer.nvidia.com/downloads/assets/gameworks/downloads/secure/Vulkan_Beta_Drivers/NVIDIA-Linux-x86_64-595.44.11.run |
The 352x288 sample is too small to exercise per-partition and pixel count feedback meaningfully on some encoders. Switch both AV1 encode feedback2 tests to the 1920x1080 8-bit 4:2:0 sample so the reported feedback reflects a realistic encode.
Thanks, Srinath. I had been testing on 595.45.04 and 610.43.03, which don't advertise the extension, but 595.44.11 does indeed :) On 595.44.11 (RTX 4060 Ti) per-partition feedback is now advertised and the test reaches the encode. It reproduces the original failure though: the whole-picture query status reads back 0 (VK_QUERY_RESULT_STATUS_NOT_READY_KHR) on every frame, even though the encode completes and writes a valid bitstream (non-zero offset and size). The status is read with VK_QUERY_RESULT_WAIT_BIT, after also waiting on the encode-complete fence, and vkGetQueryPoolResults returns VK_SUCCESS. So NOT_READY looks wrong here: the result is finalized, and the offset and size fields that precede the status read back correctly, so the whole-picture status field is being left at zero instead of set to COMPLETE. My working assumption is that the per picture partition statuses are correct and only the whole-picture status is affected. Let me know if you want me to share a dump of the per-partition statuses if that helps. |
|
I really should have checked the updates to the test application code in this PR earlier. I noticed just now that this PR has updated only the Naturally, this means that the encode_feedback2 test cases run without enabling the |
Great catch! That was indeed the problem; totally overlooked adding the call to the demo.
Thank you for the reviews! |
Call VulkanDeviceContext::SetVideoEncodeFeedback2Enabled() when any AV1 feedback option is set, mirroring the VulkanVideoEncoder library path, so the demo enables the videoEncodeFeedback2 feature and the VK_KHR_video_encode_feedback2 extension.
3562e09 to
90dca0e
Compare
| "--pictureFeedback", | ||
| "--pixelCountFeedback", | ||
| "--skippedPixelCountFeedback", | ||
| "--hasOverridesFeedback" |
There was a problem hiding this comment.
I would suggest not including --hasOverridesFeedback here, it is not an mandatory feedback with feedback2.
5 participants
Description
Adds encoder support for the
VK_KHR_video_encode_feedback2extension to the AV1 encode path, exposing the richer per-encode feedback the extension provides on top of the baseVK_KHR_video_encode_queuefeedback.When any feedback2 option is enabled, the encoder now requests the
VK_KHR_video_encode_feedback2device extension and thevideoEncodeFeedback2feature, sets up the appropriate query pool, and parses the returned feedback into:These outputs are intended for use by VK-GL-CTS to validate encoded output against the decoded bitstream, in particular for the per-partition count tests.
New encoder CLI options
--pictureFeedback--params--pictureFeedback; otherwise defaults are used)--pixelFeedback--skippedPixelFeedback--pixelFeedback)--enablePerPartitionFeedback--maxPerPartitionFeedbackEntries <n>Build requirement
VK_KHR_video_encode_feedback2first appears in Vulkan-Headers v1.4.353, so the minimum required patch version is bumped from 321 to 353. When the system SDK is older, the build fetches v1.4.353 automatically.VkVSCommon.hnow routes Vulkan throughvulkan_interfaces.hso the encoder library also builds under VKCTS without relying on a system Vulkan installation.Type of change
feature
Tests
AMD Radeon RX 7600 (RADV NAVI33) / radv Mesa 26.1.0-devel (git-16ece32984) / Debian GNU/Linux 13 (trixie)
Total Tests: 83
Passed: 69
Crashed: 0
Failed: 0
Not Supported: 10
Skipped: 4 (in skip list)
Success Rate: 100.0%
NVIDIA GeForce RTX 4060 Ti / NVIDIA 595.44.00 / Debian GNU/Linux 13 (trixie)
Total Tests: 83
Passed: 72
Crashed: 0
Failed: 0
Not Supported: 10
Skipped: 1 (in skip list)
Success Rate: 100.0%