iter1 Phase 2: situation analysis — three bugs in MPEG-2 path
Phase 2 source-read of the libva-v4l2-request-fourier MPEG-2 path
on master tip 65969da identifies three independent bugs, all in
the libva backend (kernel + driver path proven solid by Phase 0
cross-validator sweep).
Bug 1 — fall-through to default in RequestCreateConfig
(src/config.c:55-69):
case VAProfileH264*:
// FIXME
break;
case VAProfileMPEG2Simple:
case VAProfileMPEG2Main:
case VAProfileHEVCMain:
default:
return VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
H.264 cases have a break, MPEG-2 + HEVC fall through to default.
This explains the vaCreateConfig: 12 (UNSUPPORTED_PROFILE) error
observed in Phase 0 cross-validator sweep for both codecs.
Likely history: H.264 was libva-multiplanar focus iter1-iter5;
the FIXME comment suggests profile-specific validation logic was
expected but never landed. MPEG-2 stayed in fall-through bucket.
Fix shape: add break for MPEG-2 cases. HEVC stays in fall-through
(h265.c excluded from build per Phase 0 finding F-C; honest
UNSUPPORTED_PROFILE is correct until h265.c is reinstated in a
later iteration).
Bug 2 — staging-era UAPI in mpeg2.c; mainline kernel removed it:
src/mpeg2.c uses:
V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (V4L2_CID_MPEG_BASE+250 = 0x9909fa)
V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION (V4L2_CID_MPEG_BASE+251 = 0x9909fb)
Mainline kernel UAPI (include/uapi/linux/v4l2-controls.h:1985-2105):
V4L2_CID_STATELESS_MPEG2_SEQUENCE (CODEC_STATELESS_BASE+220 = 0xa409dc)
V4L2_CID_STATELESS_MPEG2_PICTURE (CODEC_STATELESS_BASE+221 = 0xa409dd)
V4L2_CID_STATELESS_MPEG2_QUANTISATION (CODEC_STATELESS_BASE+222 = 0xa409de)
Fresnel V4L2 inventory confirms kernel exposes the new IDs only.
The fork's local include/mpeg2-ctrls.h is the staging-era header
that masks the kernel's modern definitions.
Six structural changes from old to new API:
1. Slice header parsing moved to kernel — bit_size, data_bit_offset,
quantiser_scale_code GONE from new structs.
2. Reference timestamps moved from slice to picture
(forward_ref_ts, backward_ref_ts now in v4l2_ctrl_mpeg2_picture).
3. Boolean fields collapsed into v4l2_ctrl_mpeg2_picture.flags
bitmask (TOP_FIELD_FIRST, FRAME_PRED_DCT, CONCEALMENT_MV,
Q_SCALE_TYPE, INTRA_VLC, ALT_SCAN, REPEAT_FIRST, PROGRESSIVE).
4. progressive_sequence collapsed into
v4l2_ctrl_mpeg2_sequence.flags & V4L2_MPEG2_SEQ_FLAG_PROGRESSIVE.
5. PICTURE_CODING_TYPE renamed to PIC_CODING_TYPE
(V4L2_MPEG2_PICTURE_CODING_TYPE_X → V4L2_MPEG2_PIC_CODING_TYPE_X).
6. Quantisation load_* flags removed; matrices always present;
British spelling — quantiSation not quantiZation.
Quantisation matrix order: kernel doc says zigzag scanning order;
VAAPI VAIQMatrixBufferMPEG2 also stores in zigzag scanning order;
direct memcpy works. Kernel hantro_mpeg2.c does the
zigzag-to-raster permutation kernel-side
(hantro_mpeg2_dec_copy_qtable lines 12-26). No userspace
permutation needed in the libva backend (unlike FFmpeg, which
unwinds its internal idsp.idct_permutation order).
Per-frame submission: FFmpeg reference (libavcodec/
v4l2_request_mpeg2.c:130-155) batches 3 controls in single
VIDIOC_S_EXT_CTRLS. Backend's v4l2_set_controls (src/v4l2.c:475)
already supports batching — used by iter6/7/8 H.264
(src/h264.c:986). MPEG-2 rewrite follows H.264's batched pattern.
Bug 3 — include/mpeg2-ctrls.h is the staging-era local header:
The fork's local include/mpeg2-ctrls.h is the staging-era header
that defines the old (removed) API. config.c:37 + mpeg2.c:38
include it via meson's include_directories('../include'). Should
be deleted (or emptied); rely on kernel <linux/v4l2-controls.h>
pulled transitively via <linux/videodev2.h>.
Things verified NOT to be bugs:
- src/picture.c MPEG-2 dispatch is fully wired:
- codec_store_buffer handles VAPictureParameterBuffer + VAIQMatrix
- codec_set_controls dispatches MPEG-2 to mpeg2_set_controls
- HEVC explicitly UNSUPPORTED_PROFILE (correct for build state)
- src/picture.c:287 unconditional h264.matrix_set=false reset is
benign for MPEG-2 (union aliasing puts it in mpeg2.picture or
.slice region; RenderPicture overwrites that byte before
mpeg2_set_controls reads anything).
- src/mpeg2.c field extraction from VAAPI structs is sound; only
the destination control IDs and struct shape need rewiring.
- src/v4l2.c batching API (v4l2_set_controls) is in place.
Open questions tabled for Phase 3 baseline:
1. Live ftrace of failing libva MPEG-2 attempt post Bug-1-fix
(verify expected EINVAL on VIDIOC_S_EXT_CTRLS for old CID).
2. VAAPI VAIQMatrixBufferMPEG2 matrix order from real mpv decode
(verify zigzag, no pre-permutation).
3. Cross-reference verbatim VIDIOC_S_EXT_CTRLS payload from
ffmpeg-v4l2request cross-validator anchor strace dump.
4. SDDM watchpoint resolution — fresnel SSH No route to host at
Phase 2 start (network event, SDDM regression, or operator
power-state). Resolve before Phase 3.
Predicted iter1 outcome: small mechanical diff (config.c break
+ mpeg2.c rewrite + drop local mpeg2-ctrls.h). Phase 7 verification
should land all 5 Phase 1 boolean checks green on first or second
try. Likely Phase 7 → Phase 4 loopback triggers if any: forgotten
struct padding zero, garbage timestamps on first I-frame, or
device-state precondition we missed in hantro_mpeg2.c.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,229 @@
|
||||
# Iteration 1 — Phase 2 (situation analysis)
|
||||
|
||||
Phase 2 source-read of the MPEG-2 path in libva-v4l2-request-fourier master tip `65969da` (iter8 Phase 4). Three independent bugs identified, none of which require kernel work; all three live in the libva backend.
|
||||
|
||||
## Reset context
|
||||
|
||||
`git status -sb` (campaign repo): `## master`, clean.
|
||||
`git status -sb` (libva-v4l2-request-fourier fork): `## master...origin/master`, one local mod (`tests/run_perf_binding_cell.sh` — pre-iter1, unrelated).
|
||||
fresnel SSH reachability: `ssh fresnel` → `No route to host` at Phase 2 start. Watchpoint per [`phase0_recovery_2026-05-07.md`](phase0_recovery_2026-05-07.md) (SDDM regression, network event, or operator power-state change) — non-blocking for Phase 2 source-read; needs verification before Phase 3 baseline measurement work.
|
||||
|
||||
Source files inspected (line counts):
|
||||
|
||||
```
|
||||
src/config.c 255 lines
|
||||
src/picture.c 403 lines
|
||||
src/mpeg2.c 154 lines
|
||||
src/v4l2.c 579 lines
|
||||
src/surface.h line 90 (params union)
|
||||
include/mpeg2-ctrls.h (full file, fork-local staging-era header)
|
||||
```
|
||||
|
||||
Reference checkouts:
|
||||
|
||||
- Linux mainline at `~/src/libva-multiplanar/references/linux-mainline/` — kernel UAPI (`include/uapi/linux/v4l2-controls.h`) + driver (`drivers/media/platform/verisilicon/hantro_mpeg2.c`).
|
||||
- FFmpeg Kwiboo `v4l2-request-n8.1` branch at `~/src/libva-multiplanar/references/ffmpeg-kwiboo/` — independent V4L2 client implementation (`libavcodec/v4l2_request_mpeg2.c`).
|
||||
|
||||
## Bug 1 — fall-through to default in `RequestCreateConfig`
|
||||
|
||||
`src/config.c:55-69`:
|
||||
|
||||
```c
|
||||
switch (profile) {
|
||||
|
||||
case VAProfileH264Main:
|
||||
case VAProfileH264High:
|
||||
case VAProfileH264ConstrainedBaseline:
|
||||
case VAProfileH264MultiviewHigh:
|
||||
case VAProfileH264StereoHigh:
|
||||
// FIXME
|
||||
break;
|
||||
case VAProfileMPEG2Simple:
|
||||
case VAProfileMPEG2Main:
|
||||
case VAProfileHEVCMain:
|
||||
default:
|
||||
return VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
|
||||
}
|
||||
```
|
||||
|
||||
The H.264 profile case labels have a `// FIXME` comment and a `break;`. The MPEG-2 + HEVC case labels have **no break** — control falls through to `default:` and returns `VA_STATUS_ERROR_UNSUPPORTED_PROFILE` (= 12).
|
||||
|
||||
This explains the `vaCreateConfig: 12 (UNSUPPORTED_PROFILE)` error observed in Phase 0 deliverable #6 ([`phase0_evidence/2026-05-07/cross_validator_traces.md`](phase0_evidence/2026-05-07/cross_validator_traces.md)) for both MPEG-2 and HEVC. The cases were authored as if they would pass through, but the missing `break;` makes them peers of `default:` instead of accepted profiles.
|
||||
|
||||
Likely history: H.264 was the libva-multiplanar focus through iter1–iter5; the `// FIXME` next to H.264's break suggests at some point the switch was expected to gain profile-specific validation logic (V4L2 capability checks, format presence, etc.) that never materialized. MPEG-2 and HEVC stayed in the "not yet enabled" fall-through bucket. iter1 of fresnel-fourier opens that bucket.
|
||||
|
||||
**Fix shape**: add a `break;` for the MPEG-2 cases. HEVC stays in the fall-through (h265.c is excluded from the meson build per Phase 0 finding F-C; honest UNSUPPORTED_PROFILE is the correct rejection until h265.c is reinstated in a later iteration). The fix is the smallest patch this iteration sees:
|
||||
|
||||
```c
|
||||
case VAProfileMPEG2Simple:
|
||||
case VAProfileMPEG2Main:
|
||||
// FIXME — same as H.264, no profile-specific config validation yet
|
||||
break;
|
||||
case VAProfileHEVCMain:
|
||||
/* h265.c excluded from build; vaCreateConfig honestly rejects */
|
||||
default:
|
||||
return VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
|
||||
```
|
||||
|
||||
But this fix alone is not sufficient — see Bug 2.
|
||||
|
||||
## Bug 2 — `mpeg2.c` uses the staging-era UAPI; mainline kernel removed it
|
||||
|
||||
`src/mpeg2.c` calls `v4l2_set_control` with two control IDs:
|
||||
|
||||
- `V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS` (line 121)
|
||||
- `V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION` (line 149)
|
||||
|
||||
Both are defined in the fork's local `include/mpeg2-ctrls.h`:
|
||||
|
||||
```c
|
||||
#define V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (V4L2_CID_MPEG_BASE+250) // 0x009909fa
|
||||
#define V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION (V4L2_CID_MPEG_BASE+251) // 0x009909fb
|
||||
```
|
||||
|
||||
The header's own preamble flags this as staging-era:
|
||||
|
||||
> It turns out that these structs are not stable yet and will undergo more changes. So keep them private until they are stable and ready to become part of the official public API.
|
||||
|
||||
The mainline kernel UAPI (`include/uapi/linux/v4l2-controls.h:1985-2105`) instead defines:
|
||||
|
||||
- `V4L2_CID_STATELESS_MPEG2_SEQUENCE = V4L2_CID_CODEC_STATELESS_BASE+220` (= 0xa409dc)
|
||||
- `V4L2_CID_STATELESS_MPEG2_PICTURE = V4L2_CID_CODEC_STATELESS_BASE+221` (= 0xa409dd)
|
||||
- `V4L2_CID_STATELESS_MPEG2_QUANTISATION = V4L2_CID_CODEC_STATELESS_BASE+222` (= 0xa409de)
|
||||
|
||||
The fresnel V4L2 inventory ([`phase0_evidence/2026-05-07/v4l2_inventory.txt`](phase0_evidence/2026-05-07/v4l2_inventory.txt)) confirms `/dev/video5` exposes the new control IDs (matching `0xa409dc/dd/de`) — `mpeg_2_sequence_header`, `mpeg_2_picture_header`, `mpeg_2_quantisation_matrices`. The old `0x9909fa/fb` IDs are gone from mainline (Linux removed them when stabilizing the API; the staging headers were never accepted upstream as-is, the fields were re-shaped instead).
|
||||
|
||||
**So even with Bug 1 fixed, every MPEG-2 decode would fail at `mpeg2_set_controls` time** with `VIDIOC_S_EXT_CTRLS` returning `EINVAL` because the kernel doesn't recognize CID `0x009909fa`. The contract layer between userspace and kernel is wrong by version; this isn't a parameter-value bug, it's a UAPI-version bug.
|
||||
|
||||
### Old → new struct shape changes
|
||||
|
||||
The mainline split the combined `v4l2_ctrl_mpeg2_slice_params` into 3 controls. Six structural changes:
|
||||
|
||||
1. **Slice header parsing moved to the kernel.** Old API required userspace to provide `bit_size`, `data_bit_offset`, `quantiser_scale_code`. New API: kernel parses these from the bitstream itself (the slice bytes go into the OUTPUT buffer; the kernel scans them). All three fields are **gone** from the new control structs. (Per `v4l2-controls.h:2009-2017` — `v4l2_ctrl_mpeg2_sequence` doesn't have any of them; `v4l2_ctrl_mpeg2_picture` doesn't either.)
|
||||
|
||||
2. **Reference timestamps moved from slice to picture.** Old: `slice_params.forward_ref_ts` + `slice_params.backward_ref_ts`. New: `v4l2_ctrl_mpeg2_picture.forward_ref_ts` + `v4l2_ctrl_mpeg2_picture.backward_ref_ts`.
|
||||
|
||||
3. **Boolean fields collapsed into `flags` bitmask.** Old `picture.{top_field_first, frame_pred_frame_dct, concealment_motion_vectors, q_scale_type, intra_vlc_format, alternate_scan, repeat_first_field, progressive_frame}` (8 separate `__u8`) → New `picture.flags` bitmask with `V4L2_MPEG2_PIC_FLAG_TOP_FIELD_FIRST` (0x01), `_FRAME_PRED_DCT` (0x02), `_CONCEALMENT_MV` (0x04), `_Q_SCALE_TYPE` (0x08), `_INTRA_VLC` (0x10), `_ALT_SCAN` (0x20), `_REPEAT_FIRST` (0x40), `_PROGRESSIVE` (0x80).
|
||||
|
||||
4. **Sequence's `progressive_sequence` collapsed into `flags`.** Old `sequence.progressive_sequence` (`__u8`) → New `sequence.flags` bitmask with `V4L2_MPEG2_SEQ_FLAG_PROGRESSIVE` (0x01).
|
||||
|
||||
5. **Picture-coding-type macro renamed.** Old `V4L2_MPEG2_PICTURE_CODING_TYPE_{I,P,B,D}` → New `V4L2_MPEG2_PIC_CODING_TYPE_{I,P,B,D}` (note `PIC` not `PICTURE`). Values unchanged: I=1, P=2, B=3, D=4.
|
||||
|
||||
6. **Quantisation `load_*` flags removed; matrices always present.** Old `v4l2_ctrl_mpeg2_quantization` had `load_intra_quantiser_matrix`, `load_non_intra_quantiser_matrix`, `load_chroma_intra_quantiser_matrix`, `load_chroma_non_intra_quantiser_matrix` (each `__u8`, 0/1 flags). New `v4l2_ctrl_mpeg2_quantisation` (note **British spelling**, no `z`) just has the four 64-byte matrices unconditionally. The kernel always reads all four; userspace always provides all four. (FFmpeg's `v4l2_request_mpeg2.c` matches — it populates all four matrices unconditionally.)
|
||||
|
||||
### Quantisation matrix order
|
||||
|
||||
Kernel doc (`v4l2-controls.h:2069-2096`):
|
||||
|
||||
> @intra_quantiser_matrix: The quantisation matrix coefficients for intra-coded frames, **in zigzag scanning order**.
|
||||
|
||||
VAAPI's `VAIQMatrixBufferMPEG2` matrices are also in zigzag scanning order (per VAAPI spec, inheriting MPEG-2 bitstream conventions). So the libva backend's MPEG-2 matrix path is a direct memcpy:
|
||||
|
||||
```c
|
||||
for (i = 0; i < 64; i++)
|
||||
quantisation.intra_quantiser_matrix[i] = iqmatrix->intra_quantiser_matrix[i];
|
||||
```
|
||||
|
||||
No de-zigzagging or permutation needed at the libva-backend layer. The kernel `hantro_mpeg2.c` (`hantro_mpeg2_dec_copy_qtable` in the mainline driver, lines 12-26) applies the zigzag-to-raster permutation when copying to the hardware quantisation table — that's a kernel-side concern, transparent to userspace. Confirmed by reading the mainline driver source.
|
||||
|
||||
(Aside: FFmpeg's `v4l2_request_mpeg2.c:110-116` does an extra permutation step `idsp.idct_permutation[ff_zigzag_direct[i]]` because FFmpeg internally stores the matrices in IDCT-permuted order, not zigzag — it has to undo that permutation to put them back in zigzag for the kernel. The libva backend has no equivalent internal permutation, so no equivalent unwinding step.)
|
||||
|
||||
### Per-frame submission shape
|
||||
|
||||
FFmpeg's reference (`libavcodec/v4l2_request_mpeg2.c:130-155`) submits all three MPEG-2 controls in a single batched `VIDIOC_S_EXT_CTRLS` call:
|
||||
|
||||
```c
|
||||
struct v4l2_ext_control control[] = {
|
||||
{ .id = V4L2_CID_STATELESS_MPEG2_SEQUENCE, .ptr = &sequence, .size = sizeof(sequence) },
|
||||
{ .id = V4L2_CID_STATELESS_MPEG2_PICTURE, .ptr = &picture, .size = sizeof(picture) },
|
||||
{ .id = V4L2_CID_STATELESS_MPEG2_QUANTISATION, .ptr = &quantisation, .size = sizeof(quantisation) },
|
||||
};
|
||||
return ff_v4l2_request_decode_frame(avctx, &controls->pic, control, FF_ARRAY_ELEMS(control));
|
||||
```
|
||||
|
||||
The libva backend's `src/v4l2.c:475-489` already supports batched submission via `v4l2_set_controls` (used in iter6/iter7/iter8 H.264 — see `src/h264.c:986-988`). The MPEG-2 path's current single-control wrapper calls (`v4l2_set_control` once per control) are an old pattern. Phase 4 plan should follow H.264's batched pattern: one VIDIOC_S_EXT_CTRLS with 3 controls.
|
||||
|
||||
## Bug 3 — `include/mpeg2-ctrls.h` is the staging-era header
|
||||
|
||||
The fork-local header `include/mpeg2-ctrls.h` redefines the control IDs and structs that should now come from `<linux/v4l2-controls.h>`. The local header was relevant when the API was unstable and the kernel hadn't published these symbols yet. Since the API stabilized (mainline 5.x), the kernel exposes the NEW symbols at the standard path, and the local header now ACTIVELY MASKS them — `config.c:37: #include <mpeg2-ctrls.h>` resolves to the fork's `include/mpeg2-ctrls.h` (because `meson.build` adds `include_directories('../include')` to the include path), bringing in the dead old definitions instead of the kernel's live ones.
|
||||
|
||||
Fix shape: drop `#include <mpeg2-ctrls.h>` from anywhere it's used (`src/config.c:37`, `src/mpeg2.c:38`), rely on the system kernel header `<linux/v4l2-controls.h>` (already pulled in transitively via `<linux/videodev2.h>`). The local `include/mpeg2-ctrls.h` file should either be deleted (cleanest) or kept as legacy documentation if the lock-history of the staging API is operationally useful (probably not — git history has it).
|
||||
|
||||
`hevc-ctrls.h` (also in `include/`) is a parallel concern for HEVC iteration but is out of scope for iter1.
|
||||
|
||||
## Things that are NOT bugs (verified during source-read)
|
||||
|
||||
### picture.c — MPEG-2 dispatch is fully wired
|
||||
|
||||
`src/picture.c:54-176` `codec_store_buffer`: handles `VAPictureParameterBufferType` (lines 87-92) and `VAIQMatrixBufferType` (lines 140-146) for MPEG-2 correctly — copies VAAPI struct into `surface->params.mpeg2.{picture,iqmatrix}`. `VASliceParameterBufferType` is correctly NOT handled for MPEG-2 (kernel parses slice header from the bitstream now; no userspace-supplied slice param needed). `VASliceDataBufferType` (lines 61-83) is profile-agnostic — copies the slice bytes into the OUTPUT buffer for all codecs.
|
||||
|
||||
`src/picture.c:178-213` `codec_set_controls`: dispatches MPEG-2 to `mpeg2_set_controls` (lines 186-191) and HEVC to an explicit UNSUPPORTED_PROFILE (lines 204-206 — honest because h265.c isn't compiled). The dispatcher is correct.
|
||||
|
||||
`src/picture.c:287` resets `surface->params.h264.matrix_set = false` unconditionally in BeginPicture, regardless of the surface's profile. The `params` field is a union (`src/surface.h:90-109` — three-way union of mpeg2/h264/h265 structs). Setting `h264.matrix_set` writes a bool at byte offset 224 of the union (size of `VAIQMatrixBufferH264 = 6×16 + 2×64 = 224`); for an MPEG-2 surface this byte offset lands inside `params.mpeg2.picture` or `.slice`, but `RenderPicture` then overwrites that byte via the `memcpy(¶ms.mpeg2.picture, …)` path before `mpeg2_set_controls` reads any of it. Net effect: benign for MPEG-2 today. (Would be cleaner to make the reset conditional on profile or use `memset(¶ms, 0, sizeof params)` — but that's polish, not iter1-scope.)
|
||||
|
||||
### mpeg2_set_controls field math is mostly right
|
||||
|
||||
`src/mpeg2.c:42-153` does the right things on the value side, modulo the UAPI-version mismatch:
|
||||
|
||||
- `slice_params.sequence.{horizontal_size, vertical_size, profile_and_level_indication, chroma_format}` — populated from VAAPI `picture->{horizontal_size, vertical_size, …}`. Correct.
|
||||
- `slice_params.sequence.vbv_buffer_size = SOURCE_SIZE_MAX` — placeholder; FFmpeg uses `controls->pic.output->size`. Either should work for hantro (vbv check is informational); confirm in Phase 7 verification.
|
||||
- `slice_params.sequence.progressive_sequence = 0` — hardcoded. FFmpeg sets the new `flags |= V4L2_MPEG2_SEQ_FLAG_PROGRESSIVE` only if the bitstream is progressive. For BBB which IS progressive, this matters. New-API rewrite needs to lift the bool from VAAPI's picture-extension bits.
|
||||
- `slice_params.picture.f_code` populated correctly from VAAPI `picture->f_code` (16-bit packed → 4 nibbles). Same packing applies to new API.
|
||||
- `slice_params.picture.{intra_dc_precision, picture_structure, top_field_first, …}` — populated from VAAPI's `picture_coding_extension.bits.*`. New-API rewrite collapses these into `picture.flags` bitmask but the source data is identical.
|
||||
- `slice_params.{forward_ref_ts, backward_ref_ts}` — populated from VAAPI surface object timestamps via `v4l2_timeval_to_ns`. New API moves the timestamps from slice to picture; same population logic, different destination field.
|
||||
- `slice_params.{bit_size, data_bit_offset}` — populated. New API doesn't need these (kernel parses).
|
||||
- `slice_params.quantiser_scale_code = slice->quantiser_scale_code` — populated from VAAPI's slice param. New API doesn't need this either.
|
||||
|
||||
Verdict: the data extraction from VAAPI is sound; only the destination control IDs and struct shape need rewiring. Phase 4 plan should preserve the existing field-extraction logic and route it into the new struct layouts.
|
||||
|
||||
### v4l2.c batching API is already in place
|
||||
|
||||
`src/v4l2.c:475-489` `v4l2_set_controls` accepts `struct v4l2_ext_control *control_array` and `unsigned int num_controls`, calls `VIDIOC_S_EXT_CTRLS` once with the whole batch. Used by iter6/7/8 H.264 path (`src/h264.c:986`). MPEG-2 rewrite uses the same function with 3 controls.
|
||||
|
||||
## Open questions for Phase 3 baseline
|
||||
|
||||
Before Phase 4 plan, Phase 3 should capture:
|
||||
|
||||
1. **Live ftrace of the failing libva MPEG-2 attempt**, post Bug-1-fix. Apply the `break;` patch as a scratch test, then run mpv MPEG-2 and capture which kernel-side ioctl error comes back. Expected: `VIDIOC_S_EXT_CTRLS` returns `EINVAL` because of CID `0x9909fa`. If something else fires first (e.g., `VIDIOC_S_FMT` rejecting `MG2S` with EBUSY because the libva backend was bound to a node that already had a different format set), the situation analysis needs updating. **Fresnel must be back online for this**.
|
||||
2. **VAAPI `VAIQMatrixBufferMPEG2` field shape from a real mpv decode**. Strace `mpv --hwdec=vaapi-copy --frames=1` against `bbb_720p10s_mpeg2.ts` and capture the buffer payload that `vaRenderPicture` passes for the IQMatrixBufferType. Verify the `intra_quantiser_matrix[64]` etc. arrive in zigzag order (per VAAPI spec) and not pre-permuted. If pre-permuted (some VAAPI clients send IDCT-permuted matrices to optimize the encoder side), the libva-backend rewrite needs the equivalent of FFmpeg's `idsp.idct_permutation` unwind. **Likely not needed** since mpv → ffmpeg-vaapi → libva path passes through cleanly, but worth verifying.
|
||||
3. **Cross-reference ffmpeg-v4l2request reference contract** ([`phase0_evidence/2026-05-07/cross_validator/mpeg2/`](phase0_evidence/2026-05-07/cross_validator/mpeg2/) — strace + ftrace from cross-validator sweep). Pull the exact `VIDIOC_S_EXT_CTRLS` payload from the strace dump for the 3 batched controls, verify the field values match what our libva backend's data extraction would produce given the same fixture. This is the contract-anchor for Phase 4.
|
||||
4. **Verify SDDM watchpoint status**. fresnel `No route to host` at Phase 2 start — could be SDDM regression, network event, or operator power-state. Resolve before Phase 3.
|
||||
|
||||
## Phase 4 plan inputs (for the Phase 4 doc, not iter1's situation analysis)
|
||||
|
||||
Once Phase 3 baseline confirms the situation, Phase 4 plan should specify:
|
||||
|
||||
- **Diff scope**: `src/config.c` (1 break), `src/mpeg2.c` (rewrite against new API), `include/mpeg2-ctrls.h` (delete or empty), `src/picture.c` (no changes — verified wired correctly).
|
||||
- **Includes shift**: drop `<mpeg2-ctrls.h>` includes from `src/config.c` and `src/mpeg2.c`. Verify `<linux/videodev2.h>` transitively pulls `<linux/v4l2-controls.h>` (which has the new MPEG-2 control IDs and structs).
|
||||
- **Control batching**: follow H.264's pattern in `src/h264.c:986` — single `v4l2_set_controls` call with 3 `v4l2_ext_control` entries.
|
||||
- **Field mapping table** (per Bug 2): 6 structural changes, all source data already extracted correctly in current `src/mpeg2.c`. The rewrite is mechanical.
|
||||
- **Contract-before-code per `feedback_dev_process.md`**: cite the contract — kernel `v4l2-controls.h:1985-2105` for the new struct layouts and flag definitions, FFmpeg `v4l2_request_mpeg2.c:130-155` for the batched submission shape, kernel `hantro_mpeg2.c::hantro_mpeg2_dec_copy_qtable` for the zigzag-to-raster permutation that's done kernel-side. Verbatim. Per `feedback_dev_process.md`'s "stating the contract explicitly before implementing against it" requirement, the Phase 4 plan + commit message must lead with these citations, not with the patch hunks.
|
||||
- **Verification per Phase 1 lock**: re-run the 5 boolean checks from [`phase0_findings_iter1.md`](phase0_findings_iter1.md). Specifically pin: (a) `vaCreateConfig(VAProfileMPEG2Main)` returns SUCCESS, (b) mpv-vaapi-copy decode exits 0 with no `Failed to create decode configuration`, (c) DMA-BUF GL import HW vs SW pixel hash equality at `--start=00:00:02`, (d) H.264 regression hashes still match T4 reference values.
|
||||
|
||||
## Predicted iter1 outcome
|
||||
|
||||
The three bugs are well-bounded, the kernel + driver path is proven to work (cross-validator sweep), the field-extraction logic in current `mpeg2.c` is already pulling the right data from VAAPI structs. Phase 4 plan should produce a small, mechanical diff. Phase 7 verification should land all 5 Phase 1 boolean checks green on first or second try.
|
||||
|
||||
If Phase 7 misses a check, the most likely culprits in priority order:
|
||||
|
||||
1. **`VIDIOC_S_EXT_CTRLS` returns EINVAL** because the new control struct layout has a padding field (`v4l2_ctrl_mpeg2_picture.reserved[5]`) that needs explicit zeroing. Mitigation: `memset(&picture, 0, sizeof picture)` before populating fields.
|
||||
2. **Frame-1-only "all-zero pixels"** because the kernel needs both forward and backward refs zero-timestamped (no reference frame yet) for the first I-frame, but the rewrite carries garbage timestamp values. Mitigation: explicit zero of forward_ref_ts/backward_ref_ts on I-frames.
|
||||
3. **Cache-stale readback** via vaDeriveImage (the iter1 H.264 finding from T4) — but iter1 binding cells use DMA-BUF GL import, so this won't bite.
|
||||
4. **VAAPI `VAIQMatrixBufferMPEG2` matrices arrive in non-zigzag order** for some pathological encoder. Unlikely for BBB; would surface as garbled-but-recognizable bunny content rather than all-zero, useful diagnostic signal.
|
||||
|
||||
If something far weirder fires (e.g., `VIDIOC_S_FMT` rejects `MG2S` despite the kernel inventory saying it accepts it), Phase 7 → Phase 4 loopback: re-read kernel hantro_mpeg2.c for any device-state preconditions we missed.
|
||||
|
||||
## Predecessor-data discipline (not violated)
|
||||
|
||||
Per `feedback_dev_process.md` and `feedback_replicate_baseline_first.md`, this Phase 2 analysis cites **state** (file:line pointers, struct shapes, control IDs, kernel doc text) — all of which is reproducible from the current state of the source trees. No data is being carried forward from libva-multiplanar's iter1–iter8 (since libva-multiplanar never tested MPEG-2 end-to-end). The cross-validator anchor in `phase0_evidence/2026-05-07/cross_validator/mpeg2/` is in-session-acquired on fresnel for iter1's binding work; reusable.
|
||||
|
||||
## What this leaves Phase 3 with
|
||||
|
||||
Phase 3 baseline measurements should:
|
||||
|
||||
1. Verify fresnel SSH is back, capture SDDM journal if regression fired.
|
||||
2. Apply Bug 1 fix as a scratch test on a throwaway branch, install, run mpv MPEG-2 again, capture the kernel-side EINVAL via strace.
|
||||
3. Capture the verbatim `VIDIOC_S_EXT_CTRLS` payload from the cross-validator anchor's strace dump (extract for one frame from `phase0_evidence/2026-05-07/cross_validator/mpeg2/ffmpeg.strace.*`).
|
||||
4. Capture VAAPI's `VAIQMatrixBufferMPEG2` payload from a real mpv-MPEG-2 decode via libva trace (LIBVA_TRACE), to confirm the matrix order assumption.
|
||||
|
||||
After Phase 3 closes, Phase 4 plan can lock the implementation diff against the captured contract.
|
||||
Reference in New Issue
Block a user