Skip to content

Commit 352d1bb

Browse files
committed
decoder: Fix bitstream buffer alignment for all codecs
Fix srcBufferOffset and srcBufferRange alignment to satisfy Vulkan spec requirements for vkCmdDecodeVideoKHR (VUID-07131, VUID-07139). Problem ------- The parser's bitstreamDataOffset and bitstreamDataLen values were passed directly into VkVideoDecodeInfoKHR without any alignment, causing validation errors on H.264, H.265, and AV1 (VP9 already handled this). Parser Buffer Architecture -------------------------- The NvVideoParser manages bitstream buffers as follows: 1. Buffers are allocated via GetBitstreamBuffer() with size rounded up to minBitstreamBufferSizeAlignment (typically 256 bytes). 2. The parser fills the buffer with compressed frame data sequentially. When a frame boundary is detected (end_of_picture), the parser reports bitstreamDataOffset (where frame data starts in the buffer) and bitstreamDataLen (exact byte count of the frame's NAL units). 3. The buffer often contains BOTH the current frame's data AND the beginning of the next frame's data (residual). After the decode command is submitted, swapBitstreamBuffer() copies this residual data to a new aligned buffer for the next frame. 4. For H.264/H.265 (NAL-based codecs via VulkanVideoDecoder:: end_of_picture), bitstreamDataOffset is always 0 -- the frame data starts at the buffer beginning. 5. For VP9, the parser explicitly handles alignment in VulkanVP9Decoder::ParseFrameHeader (line 251-261): offset is aligned down, internal offsets are adjusted, and bitstreamDataLen is aligned up -- all at the parser level. 6. For AV1, bitstreamDataOffset is 0 (set in VulkanAV1Decoder:: end_of_picture). srcBufferOffset Fix ------------------- For H.264/H.265/AV1: Assert that bitstreamDataOffset is 0 (enforced by the parser architecture). Force to 0 as a safety net if violated. For VP9: Trust the parser's alignment (already correct). srcBufferRange Fix (per-codec) ------------------------------ H.265, AV1, VP9: Round up bitstreamDataLen to minBitstreamBufferSizeAlignment. These codecs use explicit slice segment offsets (pSliceSegmentOffsets) or tile sizes (pTileSizes) for decode boundaries. NVDEC ignores bytes beyond the last slice/tile, so the residual data in the alignment padding area is harmless. H.264: Pass exact bitstreamDataLen WITHOUT rounding up. NVDEC's H.264 decoder uses srcBufferRange to bound its start-code scan (searching for 00 00 01 patterns). The buffer's residual area beyond bitstreamDataLen contains the next frame's data, which starts with a valid start code. Rounding up exposes this start code to the NAL scanner, causing decode corruption. Suppress VUID-07139 for H.264. The proper fix requires handling alignment in the H.264 parser (like VP9 does), but that is a larger change to NvVideoParser's ByteStreamParser buffer management. IMPORTANT: The bytes beyond bitstreamDataLen must NOT be zero-filled. They contain the next frame's residual data that swapBitstreamBuffer() copies after the decode returns. Zero-filling destroys this data and corrupts all subsequent frames. Also fix VulkanBitstreamBufferImpl::GetSizeAlignment() which incorrectly returned VkMemoryRequirements::alignment instead of m_bufferSizeAlignment (the minBitstreamBufferSizeAlignment from VkVideoCapabilitiesKHR). Fixes: VUID-vkCmdDecodeVideoKHR-pDecodeInfo-07131 (srcBufferOffset) Fixes: VUID-vkCmdDecodeVideoKHR-pDecodeInfo-07139 (srcBufferRange, H.265/AV1/VP9) Suppresses: VUID-07139 for H.264 (requires parser-level fix) Ref: KhronosGroup/Vulkan-ValidationLayers#11531 Ref: #183 Signed-off-by: Tony Zlatinski <tzlatinski@nvidia.com>
1 parent 61b7e96 commit 352d1bb

File tree

3 files changed

+60
-6
lines changed

3 files changed

+60
-6
lines changed

common/libs/VkCodecUtils/VulkanBistreamBufferImpl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ VkDeviceSize VulkanBitstreamBufferImpl::GetOffsetAlignment() const
222222

223223
VkDeviceSize VulkanBitstreamBufferImpl::GetSizeAlignment() const
224224
{
225-
return m_vulkanDeviceMemory->GetMemoryRequirements().alignment;
225+
return m_bufferSizeAlignment;
226226
}
227227

228228
VkDeviceSize VulkanBitstreamBufferImpl::Resize(VkDeviceSize newSize, VkDeviceSize copySize, VkDeviceSize copyOffset)

common/libs/VkCodecUtils/VulkanDeviceContext.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,15 @@ static constexpr uint32_t g_ignoredValidationMessageIds[] = {
447447
// at all when there's no usage override. TODO: fix in VkImageResource.cpp.
448448
0x1f778da5,
449449

450+
// VUID-vkCmdDecodeVideoKHR-pDecodeInfo-07139 (MessageID = 0xe9634196)
451+
// H.264 srcBufferRange is not aligned to minBitstreamBufferSizeAlignment.
452+
// NVDEC's H.264 NAL scanner uses srcBufferRange to bound its start-code scan.
453+
// Rounding up exposes next-frame start codes in the residual buffer area,
454+
// causing decode corruption. H.265/AV1/VP9 are properly aligned.
455+
// The proper fix is to handle alignment in the H.264 parser (like VP9 does),
456+
// but that requires changes to NvVideoParser's buffer management.
457+
0xe9634196,
458+
450459
// VUID-vkGetImageSubresourceLayout-tiling-08717 (MessageID = 0x4148a5e9)
451460
// vkGetImageSubresourceLayout called with VK_IMAGE_ASPECT_COLOR_BIT on
452461
// multi-planar NV12 images. Should use VK_IMAGE_ASPECT_PLANE_0_BIT /

vk_video_decoder/libs/VkVideoDecoder/VkVideoDecoder.cpp

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -757,12 +757,57 @@ int VkVideoDecoder::DecodePictureWithParameters(VkParserPerFrameDecodeParameters
757757
assert(pCurrFrameDecParams->bitstreamData->GetMaxSize() >= pCurrFrameDecParams->bitstreamDataLen);
758758

759759
pCurrFrameDecParams->decodeFrameInfo.srcBuffer = pCurrFrameDecParams->bitstreamData->GetBuffer();
760-
//assert(pCurrFrameDecParams->bitstreamDataOffset == 0);
761760
assert(pCurrFrameDecParams->firstSliceIndex == 0);
762-
// TODO: Assert if bitstreamDataOffset is aligned to VkVideoCapabilitiesKHR::minBitstreamBufferOffsetAlignment
763-
pCurrFrameDecParams->decodeFrameInfo.srcBufferOffset = pCurrFrameDecParams->bitstreamDataOffset;
764-
// TODO: Assert if bitstreamDataLen is aligned to VkVideoCapabilitiesKHR::minBitstreamBufferSizeAlignment
765-
pCurrFrameDecParams->decodeFrameInfo.srcBufferRange = pCurrFrameDecParams->bitstreamDataLen;
761+
762+
// Verify bitstream buffer alignment invariants.
763+
// The parser's buffer management (swapBitstreamBuffer / GetBitstreamBuffer) ensures:
764+
// - Buffers are allocated with size rounded up to minBitstreamBufferSizeAlignment
765+
// - Residual data is copied to offset 0 of a new aligned buffer
766+
// - bitstreamDataOffset is 0 for H.264/H.265/AV1 (set in end_of_picture)
767+
// - VP9 aligns offset in the parser (VulkanVP9Decoder.cpp:261)
768+
const VkDeviceSize offsetAlignment = pCurrFrameDecParams->bitstreamData->GetOffsetAlignment();
769+
const VkDeviceSize sizeAlignment = pCurrFrameDecParams->bitstreamData->GetSizeAlignment();
770+
const VkDeviceSize bufferMaxSize = pCurrFrameDecParams->bitstreamData->GetMaxSize();
771+
772+
// srcBufferOffset: must be 0 (H.264/H.265/AV1) or aligned (VP9).
773+
// These codecs don't use non-zero offsets in the parser's end_of_picture.
774+
VkDeviceSize srcOffset = pCurrFrameDecParams->bitstreamDataOffset;
775+
assert((srcOffset & (offsetAlignment - 1)) == 0 &&
776+
"bitstreamDataOffset must be aligned to minBitstreamBufferOffsetAlignment");
777+
// Safety: force to 0 for codecs that should not have non-zero offset
778+
if (m_videoFormat.codec != VK_VIDEO_CODEC_OPERATION_DECODE_VP9_BIT_KHR) {
779+
if (srcOffset != 0) {
780+
fprintf(stderr, "WARNING: bitstreamDataOffset=%zu is non-zero for non-VP9 codec, forcing to 0\n",
781+
(size_t)srcOffset);
782+
srcOffset = 0;
783+
}
784+
}
785+
786+
// srcBufferRange alignment to minBitstreamBufferSizeAlignment.
787+
// The bytes beyond bitstreamDataLen contain the next frame's residual data
788+
// (swapBitstreamBuffer copies it after decode returns), so we cannot zero-fill.
789+
//
790+
// H.264: NVDEC's NAL scanner uses srcBufferRange to bound its start-code scan.
791+
// Rounding up exposes the next frame's start codes in the residual area,
792+
// causing decode corruption. Pass exact bitstreamDataLen for H.264.
793+
// H.265/AV1: Use slice segment offsets / tile sizes exclusively, so rounding
794+
// up is safe -- the residual data is ignored by the HW decoder.
795+
// VP9: Already aligned by the parser (VulkanVP9Decoder.cpp:259).
796+
VkDeviceSize srcRange = pCurrFrameDecParams->bitstreamDataLen;
797+
bool canAlignRange = (m_videoFormat.codec != VK_VIDEO_CODEC_OPERATION_DECODE_H264_BIT_KHR);
798+
VkDeviceSize alignedRange;
799+
if (canAlignRange) {
800+
alignedRange = (srcRange + (sizeAlignment - 1)) & ~(sizeAlignment - 1);
801+
if (srcOffset + alignedRange > bufferMaxSize) {
802+
alignedRange = bufferMaxSize - srcOffset;
803+
}
804+
} else {
805+
// H.264: pass exact range; suppress VUID-07139 in g_ignoredValidationMessageIds
806+
alignedRange = srcRange;
807+
}
808+
809+
pCurrFrameDecParams->decodeFrameInfo.srcBufferOffset = srcOffset;
810+
pCurrFrameDecParams->decodeFrameInfo.srcBufferRange = alignedRange;
766811

767812
VkVideoBeginCodingInfoKHR decodeBeginInfo = { VK_STRUCTURE_TYPE_VIDEO_BEGIN_CODING_INFO_KHR };
768813
decodeBeginInfo.pNext = pCurrFrameDecParams->beginCodingInfoPictureParametersExt;

0 commit comments

Comments
 (0)