# Iteration 3 — Phase 5 (sonnet review of Phase 4 deliverables)

Reviewer: Claude Sonnet 4.6, in-conversation subagent.
Date: 2026-05-04.
Inputs reviewed: `phase0_findings_iter3.md`, `phase2_iter3_situation.md`, `phase3_iter3_baseline.md`, `phase4_iter3_plan.md`, `firefox-fourier/0001-rdd-allow-stateless-v4l2-request-api.patch`, `firefox-fourier/PKGBUILD-overlay.md`. Reviewer additionally read the actual Mozilla source via fetch (not relying solely on Phase 2's quoted excerpts) and the `libva-v4l2-request-fourier/src/` nested fork.

## Verdict

**YELLOW** — proceed to Phase 6 with two named required fixes.

## Findings

### Y1 (BLOCKER for Phase 6) — string idiom mismatch in new function

The patch's `AddV4l2RequestApiDependencies` constructs the path as:

```cpp
nsAutoCString path("/dev/");
path.Append(dir_entry->d_name);
```

The existing Mozilla codebase in the same translation unit uses:

```cpp
nsCString path = "/dev/"_ns;
path += nsDependentCString(dir_entry->d_name);
```

`nsAutoCString` is a stack-buffered subclass of `nsCString` and the `(const char*)` constructor + `.Append()` exist, so the patch likely compiles, but it diverges from the file's own idiom and would be flagged by any Mozilla reviewer. Match the existing style.

**Fix applied to `firefox-fourier/0001-rdd-allow-stateless-v4l2-request-api.patch`** before build kickoff.

### Y2 (BLOCKER for Phase 7 capture, not Phase 6) — driver does not log `error_idx`

`v4l2_set_controls()` in libva-v4l2-request-fourier currently logs `"Unable to set control(s): %s"` on `VIDIOC_S_EXT_CTRLS` failure, but does not surface `controls.error_idx`. When `errno == EINVAL`, that field names exactly which control in the array was rejected. Without it, Phase 7 capture is no more diagnostic than iter2's existing log — Track A's plan to identify "which control fails on frame 11" cannot succeed.

**Fix:** in `v4l2_set_controls()` (or whichever wrapper actually calls `VIDIOC_S_EXT_CTRLS`), after the ioctl returns -1 with EINVAL, log `ext_controls.error_idx`, the offending control's `id` (with V4L2_CID_* symbolic name lookup), and its `size`/`value` content. One-line change. Apply at Phase 6 alongside the firefox-fourier build (driver build is independent and fast).

### Bonus (not Phase 4 induced; potential Track A fix candidate) — B-slice ref-list-1 copy-paste bug

In `libva-v4l2-request-fourier/src/h264.c`, the `h264_va_slice_to_v4l2()` function around line 663 has the B-slice ref-list-1 loop writing `slice->ref_pic_list0[i].fields = fields` instead of `slice->ref_pic_list1[i].fields = fields`. L1 entries `.fields` member is being written into L0 slot.

For bbb_1080p30 (mostly I+P frames in the BBB SFX intro segment), this bug may not fire. If frame 11 happens to be a B-frame in this stream, this could be the EINVAL cause — or could contribute to silent reference-list corruption with a downstream EINVAL signature.

**Disposition:** do NOT speculative-fix at Phase 6. We don't yet know whether frame 11 is a B-frame. Y2's `error_idx` logging will reveal whether the failing control is a SLICE_PARAMS field touching `ref_pic_list1` — if yes, the copy-paste fix becomes the obvious patch. Save the candidate fix for Phase 7's analysis stage.

### Minor — `--skipinteg` vs `--skipchecksums` in PKGBUILD overlay doc

The overlay doc references `makepkg -ef --skipinteg`. On modern Arch makepkg (7.1.0 inside the firefox-fourier container) the flag is `--skipchecksums`. Both work via `--skipinteg` aliasing in some pacman branches but `--skipchecksums` is canonical. Cosmetic; fix later.

### Phase 6 finding (overrides Sonnet) — `--enable-v4l2` is NOT a Mozilla 150 configure flag

Sonnet's review noted an `--enable-v4l2` mozconfig "verify present" gate. Empirical Phase 6 ground-truth (2026-05-05): Mozilla 150 has no `--enable-v4l2` flag at all. Adding it crashes configure with `mozbuild.configure.options.InvalidOptionError: Unknown option: --enable-v4l2`. The actual gate is in `toolkit/moz.configure:643-651`:

```python
@depends(target, toolkit_gtk)
def v4l2(target, toolkit_gtk):
    # V4L2 decode is only used in GTK/Linux and generally only appears on
    # embedded SOCs.
    if target.cpu in ("arm", "aarch64", "riscv64") and toolkit_gtk:
        return True

set_config("MOZ_ENABLE_V4L2", True, when=v4l2)
set_define("MOZ_ENABLE_V4L2", True, when=v4l2)
```

So MOZ_ENABLE_V4L2 is automatically set whenever the target is arm/aarch64/riscv64 and the toolkit is GTK. boltzmann's container is aarch64+GTK → MOZ_ENABLE_V4L2 is implicitly true; our patch's `#ifdef MOZ_ENABLE_V4L2` blocks compile in normally.

This is a tighter binding than --enable-v4l2 would have been: x86_64 desktop builds will NOT compile our patch. Acceptable for ohm; the ARM-only auto-enable in moz.configure also explains why upstream Mozilla doesn't ship `--enable-v4l2` as a user-facing option — it's a target-architecture decision, not a per-build choice. Filing-day implication: any upstream submission of this patch should not add a configure-flag toggle, but live inside the existing MOZ_ENABLE_V4L2 ifdef.

### Minor — mozconfig linker flag check at Phase 6 start

The upstream Arch PKGBUILD targets `arch=(x86_64)` and may not include aarch64-specific linker hints (`--enable-linker=lld` or equivalent). Low probability of build break given boltzmann's rust 1.95 + clang 22, but check `grep -E 'lld|linker' mozconfig` before kickoff. ALARM-style PKGBUILDs sometimes patch this; upstream Arch may not.

## Cap-filter and security review (NOT findings — green-lit)

The reviewer confirms:

- The `(CAPTURE_MPLANE & OUTPUT_MPLANE & STREAMING)` triple-AND for stateless decoders is the correct guard — camera-capture-only nodes lack OUTPUT_MPLANE; display-output-only nodes lack CAPTURE_MPLANE; the union with M2M arms is idempotent in `AddPath`.
- `/dev/media*` rdwr enumeration on the embedded ARM target is in the same security domain as `/dev/video*` already-rdwr — not a campaign-blocking attack-surface increase. For upstream Mozilla submission, a reviewer would prefer filtering by `MEDIA_IOC_DEVICE_INFO` + `MEDIA_ENT_F_PROC_VIDEO_DECODER`, but the campaign goal (verify on ohm) is well-served by the blunt enumeration. Note for an eventual Mozilla bug filing.
- Seccomp deferral is sound: ENETDOWN at `open()` time is broker-policy evidence; SIGSYS at ioctl time is unmistakable and different. Deferring `SandboxFilter.cpp` to Phase 7 empirical is correct.
- PKGBUILD pattern (rename + conflicts + provides) is valid and standard. `makepkg -e` semantics in the doc match makepkg actual behavior.

## Phase 5 → Phase 6 transition gates

- [x] Y1 patch fix applied (this Phase 5 close).
- [x] Y2 driver instrumentation applied (this Phase 5 close, in libva-v4l2-request-fourier).
- [ ] Phase 6 build kicked off in firefox-fourier container.
- [ ] Phase 6 first action: `grep -E 'lld|linker' mozconfig` after PKGBUILD fetch.
- [ ] Phase 7 includes the B-slice bug as a candidate Track A fix; trigger only if Y2's `error_idx` log names a `ref_pic_list1` field.