From 74d8dd134a485e30e1017801e896f2ce067b791a Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Tue, 5 May 2026 13:44:53 +0000 Subject: [PATCH] iter4 partial fix: DPB fill matches FFmpeg semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/h264.c | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/h264.c b/src/h264.c index e800826..ffc6ba2 100644 --- a/src/h264.c +++ b/src/h264.c @@ -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; } }