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:
+41
-1
@@ -242,7 +242,26 @@ static void h264_fill_dpb(struct request_data *data,
|
|||||||
SURFACE(data, entry->pic.picture_id);
|
SURFACE(data, entry->pic.picture_id);
|
||||||
uint64_t timestamp;
|
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;
|
continue;
|
||||||
|
|
||||||
if (surface) {
|
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)
|
if (entry->pic.flags & VA_PICTURE_H264_LONG_TERM_REFERENCE)
|
||||||
dpb->flags |= V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM;
|
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;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user