From 7a1bd8ec0a456e934c25c144860bb7da549acba9 Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Thu, 14 May 2026 02:06:37 +0000 Subject: [PATCH] =?UTF-8?q?iter11=20Phase=205:=20=CE=B1-13=20inert;=20pivo?= =?UTF-8?q?t=20to=20=CE=B1-14=20num=5Fentry=5Fpoint=5Foffsets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer empirically read rkvdec-hevc.c on boltzmann kernel-agent tree. sps_max_num_reorder_pics is NOT read by rkvdec. α-13 would match kdirect's wire bytes but produce no behavioural change. CRIT-2: num_entry_point_offsets (libva hardcoded 0 at h265.c:356, kdirect 22 from slice header parse) + PPS UNIFORM_SPACING flag are the live candidates. BBB HEVC uses WPP (ENTROPY_CODING_SYNC flag set in PPS) not tiles; 22 entry points = 23 CTB rows for 720p with 32-pixel CTBs. Decision: land α-13 as wire-correctness hygiene (matches kdirect, no regression risk), then α-14 for the actual fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- phase5_iter11_review.md | 95 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 phase5_iter11_review.md diff --git a/phase5_iter11_review.md b/phase5_iter11_review.md new file mode 100644 index 0000000..b782d97 --- /dev/null +++ b/phase5_iter11_review.md @@ -0,0 +1,95 @@ +# Iteration 11 — Phase 5 Review + +Reviewed 2026-05-14. Empirical anchor: Phase 3 strace, rkvdec-hevc.c on boltzmann +(`build/marfrit-packages/arch/linux-fresnel-fourier/src/linux-7.0`), h265.c in +`libva-v4l2-request-fourier/src/`. + +--- + +## CRIT-1 — rkvdec does not read `sps_max_num_reorder_pics` at all + +**Invalidates the primary mechanism.** + +Exhaustive grep of `rkvdec-hevc.c`: the field `sps_max_num_reorder_pics` is never +read. The SPS fields rkvdec actually maps to hardware registers are geometry, +bitdepth, CTB sizes, PCM config, short/long-term ref counts, temporal-MVP, and flags +— not the buffering/reorder hints. The field is present in `v4l2_ctrl_hevc_sps` for +spec completeness but rkvdec ignores it entirely. + +**Consequence:** α-13 will change wire bytes 10 and produce no behavioural difference +in rkvdec. Phase 7 YUV hash will remain all-zero. + +**Required Phase 6b amendment A:** α-13 must be deprioritised to IMP (harmless +wire-correctness improvement). Bug 5 investigation must shift to the two fields +rkvdec *does* act on that differ between libva and kdirect. + +--- + +## CRIT-2 — `num_entry_point_offsets` is the live suspect and needs a Phase 4b plan + +rkvdec-hevc.c at line 272-284 writes `num_tile_columns_minus1` and +`num_tile_rows_minus1` from the PPS to hardware. kdirect submits +`num_entry_point_offsets = 22`; libva submits 0. BBB HEVC is a tiled stream +(confirmed by 22 entry points from kdirect). Libva's hardcoded 0 tells the kernel +the bitstream has no tile boundaries, which is false. rkvdec writes tile geometry +into hardware from PPS fields — if those fields are also wrong, or if the driver +validates entry-point-offsets count against tile geometry internally, the zero +produces either a wrong decode or a silent error. + +Additionally, the PPS `UNIFORM_SPACING` flag (bit 20) is missing from libva. For +tiled streams this flag controls whether rkvdec computes uniform column/row widths +or reads them from per-column/row arrays. BBB being tiled makes this non-don't-care. + +**Required Phase 6b amendment B:** Add Phase 4b scope covering: +1. `num_entry_point_offsets` — derive from `slice->num_entry_point_offsets` if + VAAPI exposes it, else from tile geometry (num_columns × num_rows per slice). +2. PPS `UNIFORM_SPACING` — set flag when `pic_fields.bits.uniform_spacing_flag` is + set (VAAPI does expose this field in `VAPictureParameterBufferHEVC.pic_fields`). + +--- + +## IMP-1 — `slice_qp_delta` byte 22 is NOT a hardcoded parallel bug + +Phase 4 plan raises this as a concern. Empirical check: h265.c line 368 reads +`slice_params->slice_qp_delta = slice->slice_qp_delta` directly from the VAAPI +buffer — it is not hardcoded. The -1 value (0xff) observed in Phase 3 strace is +what VAAPI delivered for frame 1 of BBB. kdirect's 0x00 (0) comes from its own SPS +NAL parse. This divergence is real but secondary; slice_qp_delta miscalculation +shifts luma values, it does not suppress entire frame output. Not a parallel +hardcoded-zero bug. + +--- + +## IMP-2 — Phase 7 hash check must not be skipped if wire bytes match kdirect's + +Per `feedback_wire_vs_behavior.md` and the Phase 4 plan's own §Phase 7: matching +wire bytes alone is insufficient. Phase 7 must SHA the raw NV12 CAPTURE output +against kdirect's `9340b832…` hash, not just strace-diff the control payloads. +The plan states this correctly; reviewer flags it to ensure it survives any "close +enough" shortcut in execution. + +--- + +## MIN-1 — Comment accuracy in α-13 patch + +The proposed α-13 comment says "blocked B-frame decode." If CRIT-1 stands and +rkvdec ignores the field, this causal claim is wrong. The comment should be scoped +to "wire-level conformance fix; not confirmed to unblock rkvdec output." Amend +before commit to avoid misleading future readers. + +--- + +## Summary + +| # | Severity | Finding | Blocks Phase 6? | +|---|----------|---------|----------------| +| CRIT-1 | CRIT | rkvdec never reads `sps_max_num_reorder_pics`; α-13 is wire-only | Yes — replan required | +| CRIT-2 | CRIT | `num_entry_point_offsets=0` + missing UNIFORM_SPACING are the live suspects for tiled BBB | Yes — Phase 4b needed | +| IMP-1 | IMP | `slice_qp_delta` is not hardcoded; 0xff is VAAPI-supplied, not a parallel bug | No | +| IMP-2 | IMP | Phase 7 must hash YUV not just strace | No | +| MIN-1 | MIN | α-13 comment overclaims causation | No | + +**Recommended Phase 6b scope:** Land α-13 as a wire-correctness commit with +corrected comment (MIN-1). Open Phase 4b for `num_entry_point_offsets` + +`UNIFORM_SPACING` as the primary Bug 5 fix path (CRIT-2). Do not gate Phase 6 on +α-13 unblocking Bug 5 — it won't.