diff --git a/phase8_iteration1_close.md b/phase8_iteration1_close.md new file mode 100644 index 0000000..be3f670 --- /dev/null +++ b/phase8_iteration1_close.md @@ -0,0 +1,133 @@ +# Iteration 1 — Phase 8 (close) + +Iteration 1 of the fresnel-fourier campaign closes 2026-05-08 with all five Phase 1 boolean-correctness criteria green. **Second codec passes**: campaign scoreboard advances 1/5 → **2/5** (H.264 from T4, MPEG-2 from iter1). + +## What iter1 fixed + +MPEG-2 hardware-accelerated decode on Rockchip RK3399 / hantro-vpu-dec / libva-v4l2-request-fourier — bit-exact correct against software reference, when read via the cache-coherency-safe DMA-BUF GL import path. + +Three independent bugs (per Phase 2 source-read), all in the libva backend (kernel + driver path was already solid per Phase 0 cross-validator sweep): + +1. **`src/config.c:55-69` fall-through to `default:`** — MPEG-2 case labels existed but lacked `break;`, so `vaCreateConfig(VAProfileMPEG2Main)` returned `VA_STATUS_ERROR_UNSUPPORTED_PROFILE` (= 12). Fix: 5-line `break;` addition. + +2. **`src/mpeg2.c` used staging-era UAPI** — `V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS` (= `V4L2_CID_MPEG_BASE+250` = 0x9909fa) and the combined slice_params struct. Mainline kernel removed these in favor of the split `V4L2_CID_STATELESS_MPEG2_{SEQUENCE,PICTURE,QUANTISATION}` (= 0xa409dc/dd/de) with three separate control structs. Fix: full rewrite (~120 lines) against the new API; one batched `VIDIOC_S_EXT_CTRLS count=3` per frame matching FFmpeg's `libavcodec/v4l2_request_mpeg2.c:130-155` reference. + +3. **`include/mpeg2-ctrls.h` was the staging-era local header** — masking the kernel's modern `` definitions. Fix: delete the local header. (Required dropping `#include` from THREE files — `src/config.c:37`, `src/mpeg2.c:38`, and `src/context.c:42`. Phase 2 grep audit only found the first two; Commit D fix-forward caught the third.) + +## Contract that was followed + +Per `feedback_dev_process.md` Phase 6 contract-before-code, citations from authoritative sources, mapped 1:1 to the patch hunks (full detail in [`phase4_iter1_plan.md`](phase4_iter1_plan.md) Clauses 1–6): + +- Linux mainline UAPI: `include/uapi/linux/v4l2-controls.h:1985-2105` for the new MPEG-2 stateless control IDs, struct layouts, and flag macros. +- FFmpeg Kwiboo branch: `libavcodec/v4l2_request_mpeg2.c:130-155` for the batched submission shape (3 `v4l2_ext_control` entries, single `VIDIOC_S_EXT_CTRLS`). +- Linux mainline driver: `drivers/media/platform/verisilicon/hantro_mpeg2.c::hantro_mpeg2_dec_copy_qtable` — confirms zigzag-to-raster permutation is kernel-side; libva backend's quantisation matrices stay in zigzag order (no permutation in userspace). +- Empirical anchor: Phase 3 Baseline C ([`phase0_evidence/2026-05-07/iter1_phase3/baseline_C_xvalidator/`](phase0_evidence/2026-05-07/iter1_phase3/baseline_C_xvalidator/)) — verbatim 256-byte QUANTISATION payload from ffmpeg-v4l2request decode of the BBB MPEG-2 fixture, used for byte-by-byte transcription of `mpeg2_default_intra_matrix[64]` (per Phase 5 S3 amendment that flagged spec-recall as unreliable). + +## What landed where + +Code commits on `git.reauktion.de/marfrit/libva-v4l2-request-fourier`: + +``` +229d6d1 fresnel-fourier iter1 Phase 6 commit D: drop missed mpeg2-ctrls.h include from context.c (fix-forward) +3aab187 fresnel-fourier iter1 Phase 6 commit C: delete staging-era include/mpeg2-ctrls.h +5fe873c fresnel-fourier iter1 Phase 6 commit B: rewrite mpeg2.c against new V4L2 stateless API +e7dad7a fresnel-fourier iter1 Phase 6 commit A: config.c break for MPEG-2 cases +65969da iter8 Phase 4: tests/run_perf_binding_cell.sh — perf binding cell harness (libva-multiplanar tip) +``` + +All four commits authored as `Claude (noether)` per memory `feedback_gitea_as_claude_noether.md`. + +Campaign documents on `git.reauktion.de/marfrit/fresnel-fourier`: + +``` +ec9133a iter1 Phase 7: verification — all 5 criteria GREEN, second codec PASS +0e2e1c2 iter1 Phase 5: sonnet-architect review — 6 findings, 4 amendments +3e996d0 iter1 Phase 4: plan — contract clauses, diff scope, Phase 7 harness +cc55a6e iter1 Phase 2: situation analysis — three bugs in MPEG-2 path +b9625af iter1 Phase 3: baseline measurements — Phase 2 confirmed empirically +f720c77 iter1 Phase 0 + Phase 1 lock: MPEG-2 boolean correctness on hantro +``` + +(Plus campaign Phase 0 setup commits + this Phase 8 close.) + +## Phase 1 binding-cell verification (Phase 7 verbatim) + +| Criterion | Result | +|---|---| +| 1: vainfo MPEG-2 Simple+Main enumeration | ✅ both profiles with `VAEntrypointVLD` | +| 2: `vaCreateConfig(VAProfileMPEG2Main)` | ✅ `VA_STATUS_SUCCESS` (libva trace verbatim) | +| 3: ffmpeg-hwaccel-vaapi engages, exit 0 | ✅ 5 frames, exit 0, no `Failed to create decode configuration` | +| 4: DMA-BUF GL HW=SW byte-identical (+02s) | ✅ HW1=SW1=`6e7873030dbf...`, HW2=SW2=`ccc7ce08810d...`, frames differ | +| 5: T4 H.264 reference hashes match | ✅ HW + SW frames match `f623d5f7...` and `7d7bc6f2...` | + +Bonus: post-fix `VIDIOC_S_EXT_CTRLS` payload structurally matches Baseline C cross-validator (count=3, ctrl_class=`0xf010000=V4L2_CTRL_CLASS_CODEC_STATELESS`, sizes 12/32/256, QUANTISATION intra_matrix bytes byte-identical). One numerical divergence in `vbv_buffer_size` (1 MB post-fix vs 1.31 MB cross-validator) — kernel-ignored informational field, non-blocking. + +## Lessons (for Phase 8 memory update) + +The 8-phase loop caught five lessons worth distilling. Three become durable memory entries (next section); two stay as campaign-internal backlog notes. + +### Worth a memory entry + +**L1 — Header deletion: completeness check is `git rm` + clean rebuild, not grep alone.** + +Phase 2 source-read used `git grep "mpeg2-ctrls.h"` to enumerate include sites. The grep returned `src/config.c:37` and `src/mpeg2.c:38` — Phase 4 plan and Commit B + C scope expanded only those two files. After Commit C (`git rm include/mpeg2-ctrls.h`), the build broke on `src/context.c:42` — a third include the grep had missed (most likely because of grep tooling quirk + my path filtering). Cost: one fix-forward commit (D) and a moment of confused debugging. + +**Rule**: when removing a header, schedule `git rm` *during* the patch series and verify with a clean rebuild. The compiler enumerates includes authoritatively; grep is best-effort. **Memory entry**: `feedback_header_deletion_check.md`. + +**L2 — When the Phase 5 reviewer flags a numerical discrepancy, lean empirical, not theoretical.** + +Phase 5 reviewer's S2 finding said: "post-fix `vbv_buffer_size` will be `SOURCE_SIZE_MAX` (1 MB), Baseline C shows 0x151800 (1.31 MB) — discrepancy unexplained." I responded with confident source-read reasoning that `slot->size = sizeimage = 1382400` ergo equality holds, and rejected the finding. Phase 7 byte-compare empirically showed: post-fix produces 1 MB, NOT 1.31 MB. The reviewer was numerically right; my source-read was wrong (the actual slot->size resolution path doesn't equal the negotiated sizeimage in our backend's S_FMT setup). + +Operational impact: nil (kernel ignores `vbv_buffer_size` per the doc). But the historical record of "I rejected this finding with the wrong reason" is itself the lesson. + +**Rule**: when a reviewer cites a concrete numerical mismatch between two captured payloads, run the empirical byte-compare in Phase 7 BEFORE rejecting on source-read grounds. Source-read can have gaps; verbatim-vs-verbatim comparison can't lie. **Memory entry**: `feedback_review_empirical_over_theoretical.md`. + +**L3 — On RK3399 (and Rockchip in general?): `vaDeriveImage` cache-stale bug class is real and codec-agnostic.** + +Both H.264 (T4) and MPEG-2 (iter1) decode paths produce all-zero NV12 when read via libva's `vaDeriveImage` + cached mmap (the path that ffmpeg-vaapi `+ hwdownload` uses). The kernel + driver decode is correct (verified by ffmpeg-v4l2request DRM_PRIME path producing matching pixels to SW reference). The bug is in the libva backend's image-export — same iter1 patch-0011 bug class observed on RK3568 (ohm) but pre-fixed there. + +**Rule**: pixel verification on Rockchip platforms must use the DMA-BUF GL import path (`mpv --vo=image` or equivalent), NEVER the cached-mmap readback path. Treat ffmpeg-vaapi+hwdownload's correct-pixel guarantee as suspect on these SoCs until backend gains `VIDIOC_EXPBUF` + `DMA_BUF_IOCTL_SYNC` support. **Memory entry**: `feedback_rockchip_pixel_verify_path.md`. + +### Stays as campaign-internal backlog (not durable memory) + +**B1 — V4L2 `/dev/videoN` numbering shuffles across reboots on RK3399.** Phase 0/3 had `rkvdec=video3+media1, hantro=video5+media2`; this boot has `rkvdec=video1+media0, hantro=video3+media1`. Hardcoded env-var paths in Phase 1 binding cells are fragile. Backend should probe `/dev/media*` for `driver=hantro-vpu`/`rkvdec` rather than rely on env var stability. **Iter2+ Phase 4 cross-cutting candidate.** + +**B2 — mpv `--hwdec=vaapi-copy` silently filters MPEG-2 out** before libva is loaded. mpv's hwdec policy or ffmpeg-mpv glue layer rejects MPEG-2 from the vaapi-copy candidates, so the binding cell never engaged the libva backend. mpv `--hwdec=vaapi` (DMA-BUF, not -copy) DOES engage. Phase 1 criterion 3 anchored on ffmpeg-direct as a result; mpv-vaapi-copy-MPEG-2 is a separate follow-up task (mpv-side investigation, possibly an `--hwdec-codecs` filter override). + +### Latent bugs surfaced but deferred + +**B3 — `src/picture.c:287` `params.h264.matrix_set = false` writes byte 240 of the surface's `params` union**, which (per offsetof verification on fresnel via gcc + libva) lands inside `mpeg2.iqmatrix.chroma_intra_quantiser_matrix[20]`. For ffmpeg-vaapi-driven flows this byte is overwritten by the per-frame `VAIQMatrixBufferType` (which `codec_store_buffer:140-146` copies wholesale). For VAAPI clients that reuse a surface without re-sending the IQMatrix on every cycle, the corruption persists. Latent bug; not exercised by iter1's binding cells. **Iter2+ candidate** — the fix is making the BeginPicture matrix_set reset profile-aware, or simpler `memset(¶ms, 0, sizeof params)`. + +**B4 — `src/context.c:142-155` H.264 device-init runs unconditionally on every CreateContext** regardless of profile. Returns EINVAL on hantro-vpu-dec (which has no H.264 controls). Intentional best-effort behavior — return cast to `(void)` and discarded — but `src/v4l2.c:484: request_log("Unable to set control(s): %s\n", strerror(errno))` still fires unconditionally, so every MPEG-2 decode session prints `v4l2-request: Unable to set control(s): Invalid argument` once at init. Cosmetic only; iter2+ cleanup candidate (suppress the log when caller intent is best-effort silent fail). + +**B5 — `vbv_buffer_size = SOURCE_SIZE_MAX = 1 MB`** rather than the kernel-negotiated `sizeimage`. Informational field, kernel ignores. Low-priority post-iter1 polish. + +## What's next (campaign roadmap) + +Per `phase0_evidence/2026-05-07/cross_validator_traces.md` suggested ordering, the campaign now has: + +| Iter | Codec | Status | +|---|---|---| +| iter1 | MPEG-2 (hantro) | ✅ closed (this iteration) | +| iter2 | HEVC (rkvdec) | open — re-enable `h265.c` in build, audit against rkvdec kernel contract | +| iter3 | VP8 (hantro) | open — implement `vp8.c` | +| iter4 | VP9 (rkvdec) | open — implement `vp9.c` (largest kernel control surface) | +| iterN | Phase 4 cross-cutting | open — vaDeriveImage cache-stale fix; V4L2 device-discovery; B3-B5 polish | + +Phase 8 doesn't pre-lock iter2; that decision is the next operator-driven moment. When iter2 opens, its Phase 0 substrate carries forward iter1's findings (especially B3-B5 if those iterations risk exercising them). + +## Memory updates (next section) + +Three new memory entries land alongside this close: + +- `feedback_header_deletion_check.md` (L1) +- `feedback_review_empirical_over_theoretical.md` (L2) +- `feedback_rockchip_pixel_verify_path.md` (L3) + +Plus an update to `phase5_iter1_review.md` to acknowledge S2 was numerically right (the historical-record correction). + +Plus update to `MEMORY.md` index. + +## Iteration close acknowledgement + +Per `feedback_dev_process.md`'s Phase 8: "Loop terminates here." Iteration 1 of fresnel-fourier closes here. The 8-phase loop ran once end-to-end, no Phase 7 → Phase 4 loopbacks needed (one Phase 6 → Phase 4 mid-implementation surface for Commit D fix-forward). Phase 5 sonnet review caught two amendments worth landing (S3, Q4) plus one finding (S2) my response was wrong about. The campaign is on track.