iter5 Track E: move LAST_OUTPUT_WIDTH/HEIGHT from process-global to per-driver-data
Sonnet review 7.3 / 9.6 from iter1 + carried iter2/3/4 substrate. Two libva driver_data instances in the same process (e.g. Firefox playing two tabs at different resolutions, or Firefox + mpv via the same dlopened backend) would race on the static cache. Move to struct request_data.last_output_width/height. The V4L2 device fd is already per-driver_data, so this is the correct binding unit (one fd, one current OUTPUT format). Verified: two concurrent mpv processes (2s stagger) both decode 300 frames cleanly with no cross-corruption. Same-instant init still hits kernel-level fd contention on /dev/video1 (hantro is a single-instance device); cross-process serialization is out of scope for a libva backend. Resolves the surface_reset_format_cache() callsite: now takes driver_data parameter (was zero-arg). Also drops the 'rc' unused-variable warning in v4l2_ioctl_controls that the iter5 sweep left behind. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -263,7 +263,7 @@ VAStatus RequestDestroyContext(VADriverContextP context, VAContextID context_id)
|
||||
* cached "already 1920x1088"); the exported descriptor encoded
|
||||
* wrong pitch/offset.
|
||||
*/
|
||||
surface_reset_format_cache();
|
||||
surface_reset_format_cache(driver_data);
|
||||
|
||||
return VA_STATUS_SUCCESS;
|
||||
}
|
||||
|
||||
@@ -80,6 +80,25 @@ struct request_data {
|
||||
* visible stutter on mpv vaapi --vo=gpu.
|
||||
*/
|
||||
struct cap_pool capture_pool;
|
||||
|
||||
/*
|
||||
* Per-driver-data cache of the OUTPUT format we've set on the
|
||||
* V4L2 device (iter5 Track E: was process-global static
|
||||
* LAST_OUTPUT_WIDTH/HEIGHT, which would race when two libva
|
||||
* driver_data instances share a process — e.g. Firefox playing
|
||||
* one tab while Chromium plays another, or two mpv processes
|
||||
* via the same dlopened backend). Kept per-driver_data because
|
||||
* the V4L2 device fd is per-driver_data; one fd, one current
|
||||
* format. Process-global was always wrong, just didn't surface
|
||||
* until iter5's audit.
|
||||
*
|
||||
* See surface.c::CreateSurfaces2 for the consumer pattern (mpv
|
||||
* probes with small surfaces then re-allocates at real
|
||||
* resolution; we re-set the OUTPUT format whenever this pair
|
||||
* changes).
|
||||
*/
|
||||
unsigned int last_output_width;
|
||||
unsigned int last_output_height;
|
||||
};
|
||||
|
||||
VAStatus VA_DRIVER_INIT_FUNC(VADriverContextP context);
|
||||
|
||||
+29
-25
@@ -48,31 +48,34 @@
|
||||
#include "video.h"
|
||||
|
||||
/*
|
||||
* 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.
|
||||
* Per-driver-data cache of the OUTPUT format we've set on the V4L2
|
||||
* device. iter5 Track E: was process-global static
|
||||
* LAST_OUTPUT_WIDTH/HEIGHT before this commit. See request.h
|
||||
* struct request_data.last_output_width/height for the rationale.
|
||||
*
|
||||
* 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.
|
||||
* 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) per driver_data 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;
|
||||
|
||||
void surface_reset_format_cache(void)
|
||||
void surface_reset_format_cache(struct request_data *driver_data)
|
||||
{
|
||||
LAST_OUTPUT_WIDTH = 0;
|
||||
LAST_OUTPUT_HEIGHT = 0;
|
||||
driver_data->last_output_width = 0;
|
||||
driver_data->last_output_height = 0;
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -170,7 +173,8 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
|
||||
unsigned int pixelformat = V4L2_PIX_FMT_H264_SLICE;
|
||||
unsigned int output_type = v4l2_type_video_output(true);
|
||||
|
||||
if (LAST_OUTPUT_WIDTH != width || LAST_OUTPUT_HEIGHT != height) {
|
||||
if (driver_data->last_output_width != width ||
|
||||
driver_data->last_output_height != height) {
|
||||
/*
|
||||
* If we've previously allocated buffers at a different
|
||||
* resolution, tear them down on BOTH queues before re-setting
|
||||
@@ -189,7 +193,7 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
|
||||
* cap_pool_init below is skipped, and the slots' v4l2_index
|
||||
* fields point to dead buffers from the prior resolution.
|
||||
*/
|
||||
if (LAST_OUTPUT_WIDTH != 0) {
|
||||
if (driver_data->last_output_width != 0) {
|
||||
if (driver_data->capture_pool.initialized)
|
||||
cap_pool_destroy(&driver_data->capture_pool,
|
||||
driver_data->video_fd,
|
||||
@@ -206,8 +210,8 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
|
||||
if (rc < 0)
|
||||
return VA_STATUS_ERROR_OPERATION_FAILED;
|
||||
|
||||
LAST_OUTPUT_WIDTH = width;
|
||||
LAST_OUTPUT_HEIGHT = height;
|
||||
driver_data->last_output_width = width;
|
||||
driver_data->last_output_height = height;
|
||||
}
|
||||
|
||||
if (format != VA_RT_FORMAT_YUV420)
|
||||
|
||||
+3
-5
@@ -162,12 +162,10 @@ VAStatus RequestExportSurfaceHandle(VADriverContextP context,
|
||||
* 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.
|
||||
* Iter5 Track E: cache lives per-driver_data (request_data.last_output_*),
|
||||
* resolving the Sonnet review 7.3 / 9.6 multi-context race.
|
||||
*/
|
||||
void surface_reset_format_cache(void);
|
||||
void surface_reset_format_cache(struct request_data *driver_data);
|
||||
|
||||
/*
|
||||
* Iter2 Fix 3: bind / unbind a CAPTURE-pool slot to an object_surface.
|
||||
|
||||
@@ -433,7 +433,6 @@ static int v4l2_ioctl_controls(int video_fd, int request_fd, unsigned long ioc,
|
||||
unsigned int num_controls)
|
||||
{
|
||||
struct v4l2_ext_controls controls;
|
||||
int rc;
|
||||
|
||||
memset(&controls, 0, sizeof(controls));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user