Plan subagent with model: sonnet, open-consultation. Raw verbatim
review text — operator transcribes to DokuWiki.
Substantive findings beyond the iteration 2 plan as written:
ARCHITECTURE BLOCKERS (must address before Fix 3 implementation):
1. The 3-state slot machine (FREE/IN_DECODE/EXPORTED) is incomplete.
Need a 4th state DECODED for the window between SyncSurface DQBUF
and either ExportSurfaceHandle (vaapi path) or DeriveImage
(vaapi-copy path). Without it, vaapi-copy races Fix 3 — a slot
in FREE state can be claimed by BeginPicture for a new frame
while DeriveImage is still copying from its mmap.
2. pthread_mutex_t required around cap_pool slot state. VAAPI is
re-entrant for multi-threaded consumers; mpv vaapi may call
EndPicture/SyncSurface from decoder thread and ExportSurfaceHandle
from display thread.
3. The patch-0011 sentinel write at picture.c:365-373 currently uses
surface_object->destination_map[0]. After pool refactor, must
reference slot->map[0] instead.
ADDITIONAL FINDINGS (addressed in Fix 1):
4. The resolution-change path at surface.c:121-123 only does REQBUFS(0)
on OUTPUT, not CAPTURE. Hantro derives CAPTURE format from OUTPUT,
so leftover CAPTURE buffers from prior resolution would also block
the implicit format change. Pre-existing bug; Fix 3 pool refactor
would expose more frequently. Folded into Fix 1 implementation.
5. WSI fix should be conditional on pitch alignment, not universal
MOD_INVALID — preserves LINEAR semantics for already-aligned
1920-wide content.
DEFERRED to iteration 3:
- LAST_OUTPUT_WIDTH/HEIGHT thread safety (static globals; multi-context race)
- Full Option B (V4L2_MEMORY_DMABUF + userspace allocation)
- Multi-context concurrent use
Sonnet endorsed Option A (more buffers + LRU recycling) over Option B
for iteration 2's hardening scope. MIN_CAP_POOL=24 conservatively sized
for single-stream playback (verified against Firefox VideoFramePool
source for hold-window estimates).
Fix 1 + Fix 2 already implemented per revised architecture and pushed
to fork (06beef6, e64bb08). Fix 3 implementation pending ohm
availability for verification.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
