Skip to content

Commit e021d1f

Browse files
krajunvzlatinski
authored andcommitted
Fix a corner case in DPB management logic.
Additional details and rationale are documented directly in the code comments.
1 parent 1d9f0d8 commit e021d1f

File tree

1 file changed

+51
-0
lines changed

1 file changed

+51
-0
lines changed

vk_video_encoder/libs/VkVideoEncoder/VkEncoderDpbH264.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,56 @@ int8_t VkEncDpbH264::DpbPictureEnd(const PicInfoH264 *pPicInfo,
318318
if (i < MAX_DPB_SLOTS) {
319319
DpbBumping(false);
320320
} else {
321+
// If we reach this point, it means, DPB is full and no slot is available for the current picture.
322+
// Vulkan video encoding requires a DPB slot for the current picture whenever VkVideoEncodeFrameInfo::pSetupReferenceSlot
323+
// is being set. This application always populates this field, and without a valid DPB slot the API cannot be
324+
// configured correctly.
325+
326+
// Corner case: DpbBumping() attempts to free a DPB slot when certain conditions are met.
327+
// One of the condition checked is whether the picture is marked as "needed for output".
328+
// As part of this process, it first outputs the picture by marking the it as "not needed for output".
329+
// Before actually clearing the slot, it checks whether the picture is "unused for reference".
330+
// If so, the slot is emptied. However, if the picture is still "used for reference", the function
331+
// only marks the picture as "not needed for output" and stops there.
332+
//
333+
// The issue arises when a picture is still "used for reference" at the moment DpbBumping() process, but later
334+
// during subsequent DPB management, it becomes "unused for reference". Because the picture was already
335+
// marked as "not needed for output" at the time last DpbBumping() process, it will no longer be considered by
336+
// the bumping logic when the next DpbBumping() process is called. This leaves the slot stuck in the DPB
337+
// even though it is no longer needed for output and not needed for reference.
338+
//
339+
// To avoid leaving such stale entries in the DPB, perform an explicit check for any slot that is
340+
// both "not needed for output" and "unused for reference", and remove it when encountered.
341+
//
342+
//
343+
// Because there is no reference‑counting tied to the "needed for output"/"used for reference"
344+
// and "not needed for output"/"unused for reference", simply marking a picture as "unused for reference"
345+
// does not immediately make its dpbImageView safe to reuse. Pictures still in the encode queue may
346+
// reference it, so releasing the dpbImageView too early can lead to invalid references.
347+
// Moving this cleanup logic into DpbBumping() can therefore cause problems if the picture is still
348+
// referenced at that moment. To avoid such issues, execute this logic only when it is clear that no empty
349+
// DPB slot can be found for the current picture.
350+
//
351+
// TODO: As an enhacement,
352+
// 1. the dpb slot with lowest PicOrderCnt value can be emptied first.
353+
// 2. tie the reference count to the markings "needed for output"/"used for reference"
354+
// and "not needed for output"/"unused for reference" for cleaner implementation.
355+
int32_t i = 0;
356+
for (i = 0; i < MAX_DPB_SLOTS; i++) {
357+
if ((m_DPB[i].state & DPB_TOP) && !m_DPB[i].top_needed_for_output && (m_DPB[i].top_field_marking == MARKING_UNUSED) &&
358+
(m_DPB[i].state & DPB_BOTTOM) && !m_DPB[i].bottom_needed_for_output && (m_DPB[i].bottom_field_marking == MARKING_UNUSED)) {
359+
360+
m_DPB[i].state = DPB_EMPTY;
361+
ReleaseFrame(m_DPB[i].dpbImageView); // release the image resource
362+
break;
363+
}
364+
}
365+
if (i >= MAX_DPB_SLOTS) {
366+
// Reaching this point indicates an error in the DPB management logic.
367+
assert(!"Failed to find a free DPB slot");
368+
break; // exit while (1)
369+
}
370+
#if 0
321371
// DPB is full, current has lowest value of PicOrderCnt
322372
if (!pPicInfo->field_pic_flag) {
323373
// frame: output current picture immediately
@@ -334,6 +384,7 @@ int8_t VkEncDpbH264::DpbPictureEnd(const PicInfoH264 *pPicInfo,
334384
}
335385

336386
break; // exit while (1)
387+
#endif
337388
}
338389
} else {
339390
for (m_currDpbIdx = 0; m_currDpbIdx < MAX_DPB_SLOTS; m_currDpbIdx++) {

0 commit comments

Comments
 (0)