iter4 partial fix: DPB fill matches FFmpeg semantics

Two empirically-validated correctness fixes from comparing our h264.c
fill_dpb against FFmpeg's libavcodec/v4l2_request_h264.c::fill_dpb on
the iter3 test rig (Firefox-fourier on ohm RK3568, bbb_1080p30 H.264):

1. Set dpb[].fields = V4L2_H264_FRAME_REF for every valid entry. The
   kernel's v4l2_h264_init_reflist_builder iterates dpb[] and skips
   entries with fields == 0 — they count as "no field reference"
   regardless of VALID/ACTIVE flags. Without this, P-slices that
   need to walk the reference list (first one in BBB is at frame 11)
   hit "no valid refs" and S_EXT_CTRLS rejects the request with
   EINVAL (error_idx == count = kernel's "application bug" sentinel).

2. Skip entries with valid=true but used=false. dpb_update() clears
   `used` for all entries then re-marks only those in the current
   ReferenceFrames[] list. Stale entries (frames the consumer has
   retired from its DPB) were being included, growing the V4L2 dpb[]
   monotonically until H264_DPB_SIZE while SPS.max_num_ref_frames
   may be 4. FFmpeg iterates h->short_ref[] / h->long_ref[] only —
   the currently-referenced set.

Empirical: from "10 frames decode, frame-11 P-slice EINVAL" to
"~20 frames decode, then a different EINVAL on later frames."
Confirms both fixes are correctness improvements but Track A is not
yet fully resolved — at least one more bug remains. iter4 stays open.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-05 13:44:53 +00:00
parent a12d29937c
commit 74d8dd134a
+41 -1
View File
@@ -242,7 +242,26 @@ static void h264_fill_dpb(struct request_data *data,
SURFACE(data, entry->pic.picture_id);
uint64_t timestamp;
if (!entry->valid)
/*
* Skip entries no longer referenced by the consumer's
* VAPictureParameterBufferH264.ReferenceFrames[]. dpb_update()
* clears `used` for all entries then re-marks only those in the
* current ReferenceFrames list; entries with valid=true but
* used=false are stale (a frame the libva consumer has retired
* from its DPB).
*
* Without this skip, our V4L2 dpb[] grows monotonically until
* H264_DPB_SIZE; by frame_num=10 it carries 7+ entries while
* SPS.max_num_ref_frames may be 4. The kernel reflist builder /
* cluster validator rejects the request with EINVAL once the
* count exceeds the SPS contract — which iter1+iter2+iter3
* surfaced as the "frame-11 EINVAL" carryover. iter4 fix:
* report only currently-used entries to match FFmpeg's
* libavcodec/v4l2_request_h264.c::fill_dpb behaviour (which
* iterates h->short_ref[] / h->long_ref[] — exactly the
* currently-referenced set).
*/
if (!entry->valid || !entry->used)
continue;
if (surface) {
@@ -300,6 +319,27 @@ static void h264_fill_dpb(struct request_data *data,
if (entry->pic.flags & VA_PICTURE_H264_LONG_TERM_REFERENCE)
dpb->flags |= V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM;
/*
* Mark this DPB entry as a frame reference (both top + bottom
* fields). The kernel's v4l2_h264_init_reflist_builder iterates
* dpb[] and skips entries whose `fields` member is zero — they
* count as "no valid field reference for this entry." For
* frame-coded streams (BBB and most desktop H.264) every
* reference is a frame reference; per UAPI doc
* (ext-ctrls-codec-stateless.rst), fields must be set to
* V4L2_H264_FRAME_REF (= TOP|BOTTOM) for frames.
*
* Cross-reference: FFmpeg libavcodec/v4l2_request_h264.c::
* fill_dpb_entry sets entry->fields from pic->reference; for
* frames pic->reference includes V4L2_H264_FRAME_REF. Without
* this, P-slices that need to walk the reference list (the
* first one in BBB is at frame 11) hit "no valid refs" inside
* the kernel's reflist builder and S_EXT_CTRLS rejects the
* whole request with EINVAL (error_idx == count, the kernel's
* "application bug" sentinel).
*/
dpb->fields = V4L2_H264_FRAME_REF;
}
}