From 06beef624814d19b9cbe7a1b0b3defffb637d0d3 Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Mon, 4 May 2026 19:11:03 +0000 Subject: [PATCH] iter2 Fix 1: invalidate format cache on DestroyContext + REQBUFS(0) on CAPTURE in resolution-change path Fix 1 of iteration 2 per phase4_iter2_plan.md. Adds surface_reset_format_cache() exposed from src/surface.h. Called from RequestDestroyContext after the dual REQBUFS(0). Without this, multi-video Firefox sessions on mozilla.org corrupted the next session's CAPTURE format query: the kernel reset to defaults but our LAST_OUTPUT_WIDTH/HEIGHT cache still said 'already 1920x1088,' so the next G_FMT returned 48x48 and the exported descriptor encoded wrong pitch/offset. Also adds REQBUFS(0) on CAPTURE in the resolution-change path of RequestCreateSurfaces2 (Sonnet Phase 5 review iter2 9.1). The existing code only did REQBUFS(0) on OUTPUT before re-S_FMTting; hantro derives CAPTURE format from OUTPUT format, so leftover CAPTURE buffers from the prior resolution would also block the implicit format change. Pre-existing bug surfaced by Sonnet's audit; Fix 3 pool refactor would have exposed it more often. Limitation noted in surface.h docblock: the LAST_OUTPUT_WIDTH/ HEIGHT cache is a static process-global, so concurrent multi- context use still races (Sonnet 7.3 / 9.6). Iteration 2 only addresses sequential sessions. Multi-context safety is iteration 3+. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/context.c | 12 ++++++++++++ src/surface.c | 22 ++++++++++++++++++---- src/surface.h | 20 ++++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/context.c b/src/context.c index d959d1b..ad5870b 100644 --- a/src/context.c +++ b/src/context.c @@ -248,5 +248,17 @@ VAStatus RequestDestroyContext(VADriverContextP context, VAContextID context_id) if (rc < 0) return VA_STATUS_ERROR_OPERATION_FAILED; + /* + * Iteration 2 Fix 1: the kernel CAPTURE format state is no longer + * guaranteed after the dual REQBUFS(0). Invalidate the + * LAST_OUTPUT_WIDTH/HEIGHT cache so the next CreateSurfaces2 will + * unconditionally re-S_FMT on OUTPUT. Without this, multi-video + * Firefox sessions on mozilla.org corrupted the next session's + * CAPTURE format query (kernel returned 48x48 instead of the + * cached "already 1920x1088"); the exported descriptor encoded + * wrong pitch/offset. + */ + surface_reset_format_cache(); + return VA_STATUS_SUCCESS; } diff --git a/src/surface.c b/src/surface.c index 58ca9cb..abcabe3 100644 --- a/src/surface.c +++ b/src/surface.c @@ -69,6 +69,12 @@ static unsigned int LAST_OUTPUT_WIDTH = 0; static unsigned int LAST_OUTPUT_HEIGHT = 0; +void surface_reset_format_cache(void) +{ + LAST_OUTPUT_WIDTH = 0; + LAST_OUTPUT_HEIGHT = 0; +} + VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format, unsigned int width, unsigned int height, VASurfaceID *surfaces_ids, @@ -114,13 +120,21 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format, if (LAST_OUTPUT_WIDTH != width || LAST_OUTPUT_HEIGHT != height) { /* - * If we've previously allocated OUTPUT buffers at a different - * resolution, tear them down before re-setting the format — - * S_FMT is rejected by V4L2 while buffers exist. + * If we've previously allocated buffers at a different + * resolution, tear them down on BOTH queues before re-setting + * the OUTPUT format. S_FMT is rejected by V4L2 while buffers + * exist; hantro derives CAPTURE format from OUTPUT format, so + * leftover CAPTURE buffers from the prior resolution would + * also block the implicit format change. Sonnet Phase 5 + * review (iter2 9.1) flagged this as a missing REQBUFS(0) + * gap on the CAPTURE side of the resolution-change path. */ - if (LAST_OUTPUT_WIDTH != 0) + if (LAST_OUTPUT_WIDTH != 0) { (void)v4l2_request_buffers(driver_data->video_fd, output_type, 0); + (void)v4l2_request_buffers(driver_data->video_fd, + v4l2_type_video_capture(true), 0); + } rc = v4l2_set_format(driver_data->video_fd, output_type, pixelformat, width, height); diff --git a/src/surface.h b/src/surface.h index e673fc7..5e2bef3 100644 --- a/src/surface.h +++ b/src/surface.h @@ -126,4 +126,24 @@ VAStatus RequestExportSurfaceHandle(VADriverContextP context, VASurfaceID surface_id, uint32_t mem_type, uint32_t flags, void *descriptor); +/* + * Iteration 2 Fix 1: invalidate the LAST_OUTPUT_WIDTH/HEIGHT cache used + * by RequestCreateSurfaces2 to skip redundant v4l2_set_format calls. + * + * Must be called when the kernel's CAPTURE format state is no longer + * guaranteed to match what we last set on OUTPUT — at minimum, on + * RequestDestroyContext after REQBUFS(0). Without this, Firefox + * playing a multi-video page (mozilla.org with 864-wide intro + * videos at varying resolutions) corrupts the next session's CAPTURE + * format query: the cache says "already 1920x1088" while the kernel + * has reset to defaults, our subsequent G_FMT returns 48x48, and the + * exported descriptor encodes wrong pitch/offset. + * + * Limitation (Sonnet Phase 5 review 7.3 / 9.6): the cache is a + * static process-global, so concurrent multi-context use still races. + * Iteration 2 only addresses sequential sessions. Multi-context + * safety is iteration 3+ scope. + */ +void surface_reset_format_cache(void); + #endif