claude-noether
695588aaaf
Janet re-review verdict on v2: PROCEED with no architecture changes. Two minor plan-text amendments: A. cap_pool init for vpu981 — at backend init (NOT lazy). Three explicit init calls in VA_DRIVER_INIT_FUNC. Lazy init creates race window where concurrent VP8 (legacy hantro) and AV1 (vpu981) sessions can alias cap_pool state across the wrong device. B. Lock the F1 test vector: av1-1-b8-23-film_grain-50.ivf (AOM data set). Already used in Phase 0 kdirect; reuse via libva for byte- compare. Default 1080p encodes may be single-tile and never exercise F1; a named conformance vector locks the test surface. Cross-compile hazard documented: _Static_assert validates against COMPILE-TIME kernel headers. Sibling-campaign pattern is native arm64 build on ampere — no cross-compile in scope. Comment added near the assert for future Makefile changes. Phase 2 entry checklist locked. Plan stack: v1 → v2 (Q5 BLOCKER fix) → v3 amendment (Janet PROCEED + 2 fixes). Implementation starts with request.c third-device scaffolding (smallest atomic change), then small file edits, then NEW av1.h, then NEW av1.c (~700 LoC), then build + Phase 3 hardware test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
42 lines
3.1 KiB
Markdown
42 lines
3.1 KiB
Markdown
# Phase 1 plan v3 amendment — Janet PROCEED with 2 plan-text fixes
|
||
|
||
Date: 2026-05-17 09:55. Janet re-review verdict on v2: **PROCEED** with 2 minor plan-text amendments. No architecture changes.
|
||
|
||
## Amendment A: cap_pool init for vpu981 — at backend init, not lazy
|
||
|
||
v2 said "cap_pool extension needs the same pair-of-flags layout". Implicit: pool state for the vpu981 fd must be set up at backend init time alongside rkvdec + legacy hantro, NOT lazily on first AV1 RequestCreateConfig.
|
||
|
||
**Why**: lazy init creates a race window where a concurrent VP8 session (using legacy hantro fd) triggers cap_pool re-init on the legacy hantro device, while an AV1 session is mid-flight, can alias the pool state across the wrong device. Force init at backend open.
|
||
|
||
Concretely in `VA_DRIVER_INIT_FUNC`: after the three `find_decoder_device_*` calls succeed, immediately call `cap_pool_init(driver_data, video_fd_rkvdec)`, `cap_pool_init(driver_data, video_fd_hantro)`, `cap_pool_init(driver_data, video_fd_vpu981)` — three explicit init calls, one per device fd, before returning success to libva.
|
||
|
||
## Amendment B: lock the F1 test vector
|
||
|
||
v2 said "Phase 3 verification must include multi-tile 1080p+ AV1". Janet flagged that a default-encoded 1080p might be single-tile and never exercise F1.
|
||
|
||
**Specific test vector for F1**: `av1-1-b8-23-film_grain-50.ivf` from the AOM test data set — exercises film_grain (F3) AND tiling (F1) simultaneously. URL: `https://storage.googleapis.com/aom-test-data/av1-1-b8-23-film_grain-50.ivf`.
|
||
|
||
Phase 0 already used this for kdirect verification (10/10 frames bit-perfect). Phase 3 reruns it via libva path and byte-compares.
|
||
|
||
Additional multi-tile coverage: AOM test vector `av1-1-b8-22-svc-L2T1.ivf` (if exists) for layered/tiled stress, and any 1080p AV1 source encoded with `--tile-columns=2 --tile-rows=2`.
|
||
|
||
## Cross-compile build-time hazard (Janet flagged for documentation only)
|
||
|
||
The `_Static_assert(sizeof(struct v4l2_ctrl_av1_tile_group_entry) == 16, ...)` validates against kernel headers at COMPILE TIME. If the build moves cross-compile (e.g., builds on fresnel x86 against ampere arm64 kernel headers), the assert silently validates the wrong ABI.
|
||
|
||
**Mitigation**: builds happen on ampere (native arm64) per sibling-campaign pattern. No cross-compile in this campaign. Add a comment near the assert flagging the hazard for any future Makefile changes.
|
||
|
||
## Carry-forward verdict
|
||
|
||
Janet PROCEED v2 → PROCEED with these two text amendments → Phase 2 code can open.
|
||
|
||
## Phase 2 entry checklist
|
||
|
||
- [ ] Plan v2 + v3 amendments committed and pushed (this commit)
|
||
- [ ] Start with `request.c` scaffolding (vpu981 fd slots + capability probe) — smallest atomic change, easy to verify
|
||
- [ ] Then `codec.c` + `config.c` + `picture.c` + `surface.h` (small edits, no new files)
|
||
- [ ] Then NEW `av1.h` (~30 LoC, minimal interface)
|
||
- [ ] Then NEW `av1.c` (~700 LoC — biggest single file, mirror Kwiboo + apply F1/F2/F3 corrections from v2)
|
||
- [ ] Then `Makefile.am` + autoreconf + native build on ampere
|
||
- [ ] Then Phase 3 hardware test starting with the existing 208×208 + 352×288 vectors (sanity), then `av1-1-b8-23-film_grain-50.ivf` (multi-tile F1 + film_grain F3 stress)
|