Pr 228+fix+syncfix#232
Open
zlatinski wants to merge 3 commits into
Open
Conversation
Split VkVideoEncoder::AssembleBitstreamData into ReadbackBitstreamData
and WriteBitstreamToFile so the AV1 path can reuse the same code as
the base encoder and the async-assembly worker thread, and rewrite
VkVideoEncoderAV1::AssembleBitstreamData on top of two new helpers:
- BuildFrameObuSequence: stages sequence-header + frame OBUs into
m_bitstream[frameIdx].
- FlushBatchedTemporalUnit: emits IVF headers, the TD OBU, and all
batched per-frame payloads, then releases the per-slot capacity.
Drive-by cleanups:
- Add encodeInfo.dstBufferOffset to the GPU pointer when reading
back the bitstream so the offset is honored even if it ever
becomes nonzero.
- Hoist the AV1 temporal-delimiter OBU into a class-scope constant
(TD_OBU_HDR / TD_OBU_PAYLOAD_SIZE) instead of redeclaring the
two-byte literal at each call site.
- Drop the noisy "[EncoderConfig] Unknown positional args" stderr
warning in EncoderConfig::ParseArguments.
Signed-off-by: Raju Konda <kraju@nvidia.com>
- Honor encodeInfo.dstBufferOffset in the GPU-pointer math of the base WriteBitstreamToFile mapped path and VkVideoEncoderAV1:: ReadbackBitstreamData: the encode-feedback query's bitstreamStartOffset is relative to the bound bitstream buffer range, not the buffer start. Currently a no-op (dstBufferOffset is always 0) but spec-correct, and implements what the PR description already claimed. - Fail loudly instead of silently succeeding when a frame reaches the synchronous AssembleBitstreamData paths (base and AV1) without a bitstream buffer (readbackDone == false): previously the header was written and the coded payload silently dropped with VK_SUCCESS. The readbackDone=false skip remains valid for the async worker, which handles show-existing frames without buffers. - Route AV1 FlushBatchedTemporalUnit payload writes through WriteDataToFile so --crc covers the coded frame data (previously only IVF headers and the TD OBU were included in the CRC). - Avoid one full bitstream copy per AV1 frame: BuildFrameObuSequence now moves readback.bitstreamCopy into the staging vector when no sequence header needs to be prepended (documented as consuming its readback argument). - Correct the misleading 'Failed to get vcl query pool results' message in both sync paths - ReadbackBitstreamData also fails on fence waits.
With --syncAssembly (and the consumer-thread path without async assembly),
deferred-frame chains were released with ReleaseChildrenFrames(), which
only drops the shared_ptr chain. The frame-info objects are pool-recycled
nodes whose destructors do not run when the last reference drops, so every
frame's resources - DPB setup image, staged input image, output bitstream
buffer, command buffers - stayed pinned inside the returned-to-pool nodes.
With one pool node per encoded frame, nothing was ever reused and reset,
and the DPB image pool ran dry after ~16 frames:
- AV1 --syncAssembly: ProcessDpb failed with VK_ERROR_INITIALIZATION_FAILED
(GetAvailableImage) for every batch after the third; the error was
swallowed upstream, producing a cleanly-exiting but truncated bitstream
(13 of 64 frames decodable).
- H.264 --syncAssembly: the same starvation surfaced as a driver-side
rate-control validation assert (pQualityLevelInfo) that stalls waiting
for input on dev drivers.
Fix: add VkVideoEncodeFrameInfo::ResetAndReleaseFrames(), which walks the
chain and calls Reset(true) on each frame - mirroring what
ReleaseAssemblyItem does per work item on the async path - and use it at
both non-async release sites. The async path is unchanged (frames there
are owned by the assembly queue items).
Verified on RTX 5080 (BasketballDrive 1080p, 64 frames, VBR 5 Mbps, B=3):
av1/h264/hevc --syncAssembly now produce 64/64-frame output, 3/3 runs
byte-identical, and byte-identical to the async-assembly output of the
same build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
encoder: factor bitstream assembly into shared readback/write helpers - original from Raju Konda
encoder: address review findings for bitstream assembly refactor - proposed fixes of the above change
encoder: release per-frame resources on the synchronous assembly path - issue found during testing