surface: re-set OUTPUT format on resolution change
The static SET_FORMAT_OF_OUTPUT_ONCE flag pinned the OUTPUT format to the first call's dimensions, which for mpv's probe pattern means 128x128. Subsequent CreateSurfaces2 for the real 1920x1088 resolution would then read CAPTURE format from the kernel (which derives from the OUTPUT format) and get 128x128 sizes back — leading to a VADRMPRIMESurfaceDescriptor with width=1920 height=1088 but pitch=128 offset=16384. Mesa's WSI rejected this as 'pitch too small,' and the mpv vaapi --vo=gpu render landed on a solid blue frame. Same root cause for Firefox 150's SW fallback after frame 0. Replace SET_FORMAT_OF_OUTPUT_ONCE with LAST_OUTPUT_WIDTH/HEIGHT tracking. When dimensions change, call REQBUFS(0) on OUTPUT to drop any stale buffers (S_FMT is rejected by V4L2 while buffers exist), then re-S_FMT at the new resolution. The kernel will derive the new CAPTURE format from this OUTPUT format on the next CreateBufs + G_FMT cycle. Caveat (TODO for next iteration): for consumers that legitimately stream multiple resolutions in sequence (mid-stream resolution change via V4L2_EVENT_SOURCE_CHANGE), the current approach still requires CreateSurfaces2 to be called, which mpv does on probe. A proper context-level redesign would handle SOURCE_CHANGE inline with STREAMOFF + REQBUFS(0) + new S_FMT. Diagnosis and root cause: surfaced by 2026-05-04 Phase 5 sonnet review (finding 7.3) as a 'latent bug to document.' Today's instrumentation captured it as the active bug — the ExportSurfaceHandle dump showed pitch=128 for 1920x1088 surfaces right before MESA reported 'WSI pitch too small' and dropped to software. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+53
-18
@@ -47,7 +47,27 @@
|
|||||||
#include "v4l2.h"
|
#include "v4l2.h"
|
||||||
#include "video.h"
|
#include "video.h"
|
||||||
|
|
||||||
bool SET_FORMAT_OF_OUTPUT_ONCE = false;
|
/*
|
||||||
|
* Per-process cache of the OUTPUT format we've set. The previous
|
||||||
|
* SET_FORMAT_OF_OUTPUT_ONCE pattern was a latent bug (Sonnet Phase 5
|
||||||
|
* review finding 7.3): mpv probes with small surfaces (e.g. 128x128)
|
||||||
|
* before requesting the real resolution (e.g. 1920x1088). The
|
||||||
|
* once-only set kept the OUTPUT — and consequently the kernel-derived
|
||||||
|
* CAPTURE — format pinned to the probe size. Subsequent
|
||||||
|
* v4l2_get_format on CAPTURE then returned the small format, the
|
||||||
|
* VADRMPRIMESurfaceDescriptor was filled with width=1920 height=1088
|
||||||
|
* but pitch=128 offset=16384, and Mesa rejected the import with
|
||||||
|
* "WSI pitch too small." That manifested as the solid-blue render in
|
||||||
|
* mpv vaapi mode and the SW fallback in Firefox after frame 0.
|
||||||
|
*
|
||||||
|
* Fix: track (width, height) and re-set the OUTPUT format whenever
|
||||||
|
* the resolution changes. Re-setting requires REQBUFS(0) on both
|
||||||
|
* queues first because S_FMT after CREATE_BUFS is rejected by V4L2;
|
||||||
|
* we tear down and let the next allocation cycle recreate buffers
|
||||||
|
* at the new resolution.
|
||||||
|
*/
|
||||||
|
static unsigned int LAST_OUTPUT_WIDTH = 0;
|
||||||
|
static unsigned int LAST_OUTPUT_HEIGHT = 0;
|
||||||
|
|
||||||
VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
|
VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
|
||||||
unsigned int width, unsigned int height,
|
unsigned int width, unsigned int height,
|
||||||
@@ -71,31 +91,46 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
|
|||||||
bool found;
|
bool found;
|
||||||
int rc;
|
int rc;
|
||||||
|
|
||||||
//////////// HACK: this portion of the code should get cleaned up.
|
/*
|
||||||
|
* Set the OUTPUT format on (re)allocation when the resolution
|
||||||
// v4l2_set_format needs to be called BEFORE we create any buffers
|
* differs from the last set value. Without this, mpv's small
|
||||||
//
|
* probe surfaces (128x128) pin the CAPTURE format and the
|
||||||
// we originally did this for the output stream in context.c, but
|
* subsequent real-resolution surface ends up with wrong pitch
|
||||||
// RequestCreateSurfaces2 gets called multiple times before RequestCreateContext
|
* in the export descriptor — causing Mesa to reject the
|
||||||
// to allocate & map buffers. this doesn't seem to work in recent kernel versions.
|
* DMA-BUF import. Detail in the LAST_OUTPUT_WIDTH/HEIGHT
|
||||||
//
|
* comment block at the top of this file.
|
||||||
// we declare SET_FORMAT_OF_OUTPUT_ONCE to ensure v4l2_set_format only gets called once
|
*
|
||||||
// (in the first RequestCreateSurfaces2 call BEFORE any buffers are created later on)
|
* TODO: this is still not a clean architecture — v4l2_set_format
|
||||||
|
* after CREATE_BUFS requires REQBUFS(0) first (kernel returns
|
||||||
|
* EBUSY otherwise). For mpv's pattern (probe with small, then
|
||||||
|
* allocate big) the small probe surfaces have not been streamed
|
||||||
|
* yet, so REQBUFS(0) on them works. For consumers that legitimately
|
||||||
|
* stream multiple resolutions in sequence, we'd need to STREAMOFF
|
||||||
|
* + REQBUFS(0) + new S_FMT + new CREATE_BUFS — that's a context-
|
||||||
|
* level redesign for the next iteration.
|
||||||
|
*/
|
||||||
unsigned int pixelformat = V4L2_PIX_FMT_H264_SLICE;
|
unsigned int pixelformat = V4L2_PIX_FMT_H264_SLICE;
|
||||||
unsigned int output_type = v4l2_type_video_output(true);
|
unsigned int output_type = v4l2_type_video_output(true);
|
||||||
|
|
||||||
if (!SET_FORMAT_OF_OUTPUT_ONCE) {
|
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 (LAST_OUTPUT_WIDTH != 0)
|
||||||
|
(void)v4l2_request_buffers(driver_data->video_fd,
|
||||||
|
output_type, 0);
|
||||||
|
|
||||||
rc = v4l2_set_format(driver_data->video_fd, output_type, pixelformat,
|
rc = v4l2_set_format(driver_data->video_fd, output_type, pixelformat,
|
||||||
width, height);
|
width, height);
|
||||||
if (rc < 0) {
|
if (rc < 0)
|
||||||
return VA_STATUS_ERROR_OPERATION_FAILED;
|
return VA_STATUS_ERROR_OPERATION_FAILED;
|
||||||
}
|
|
||||||
|
|
||||||
SET_FORMAT_OF_OUTPUT_ONCE = true;
|
LAST_OUTPUT_WIDTH = width;
|
||||||
|
LAST_OUTPUT_HEIGHT = height;
|
||||||
}
|
}
|
||||||
|
|
||||||
/////////// ENDHACK
|
|
||||||
|
|
||||||
if (format != VA_RT_FORMAT_YUV420)
|
if (format != VA_RT_FORMAT_YUV420)
|
||||||
return VA_STATUS_ERROR_UNSUPPORTED_RT_FORMAT;
|
return VA_STATUS_ERROR_UNSUPPORTED_RT_FORMAT;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user