marfrit/libva-v4l2-request-fourier
1
0
Fork 1
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

fresnel-fourier iter5b-β Phase 6 commit C: β refactor — OUTPUT lifecycle to CreateContext + CRIT-1 + CRIT-2

Browse Source 
Strip OUTPUT-side V4L2 device-format lifecycle out of
RequestCreateSurfaces2 entirely. Move S_FMT(OUTPUT), CAPTURE-format
probe, cap_pool_init, per-surface destination_* fill into
RequestCreateContext where config_id (and therefore the bound
VAProfile) is known via config_object->pixelformat (wired by
commit B). The α' multi-CreateSurfaces2-mid-stream failure mode
disappears because β has no in-CreateSurfaces2 teardown branch;
each context cycle does its own setup, DestroyContext handles
teardown.

Phase 5 v2 review amendments:
- CRIT-1: removed video_format==NULL early-return at context.c:64-66
  (would have rejected every first β CreateContext).
- CRIT-2: added request_pool_destroy() to DestroyContext before
  REQBUFS(0). Pre-β only surface.c's resolution-change branch
  called request_pool_destroy; β strips that, so DestroyContext
  becomes the sole per-session teardown site.
- IMP-1: probe CAPTURE format first to derive output_type from
  video_format->v4l2_mplane (eliminates the hardcoded mplane=true
  hack from the Phase 4 v2 plan).
- IMP-2: surface_reset_format_cache() deleted (function + declaration
  in surface.h + call in DestroyContext + last_output_{width,height}
  fields in request.h). All dead under β.

CreateSurfaces2 now ~50 LOC (was ~250). Pure surface ID allocation
+ per-surface lifecycle bookkeeping; no V4L2 device state touched.

Signed-off-by: claude-noether <claude-noether@reauktion.de>
This commit is contained in:
claude-noether
2026-05-12 14:41:35 +00:00
parent cc077a0c06
commit 7055b14f5e
4 changed files with 240 additions and 282 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/surface.c
+28 -228
View File
@@ -48,36 +48,19 @@
#include "video.h"
/*
* 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.
*
* 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.
* iter5b-β: the OUTPUT-side V4L2 device-format lifecycle moved out
* of this file. Pre-β CreateSurfaces2 owned the S_FMT(OUTPUT) +
* CAPTURE-format probe + cap_pool_init + per-surface destination_*
* fill; now that responsibility lives in context.c::RequestCreateContext
* where the bound config (and therefore the active VAProfile) is
* known via config_id. CreateSurfaces2 retains only surface object
* ID allocation and per-surface bookkeeping. The previous
* `surface_reset_format_cache` helper and `last_output_width/height`
* fields are deleted (β doesn't gate re-S_FMT on
* resolution — the lifecycle is CreateContext-centric and natural
* setup/teardown happens at each context cycle).
*/
void surface_reset_format_cache(struct request_data *driver_data)
{
driver_data->last_output_width = 0;
driver_data->last_output_height = 0;
}
/*
* Iter2 Fix 3 helpers — bind / unbind a cap_pool_slot to an
* object_surface. Called from BeginPicture (acquire+bind) and
@@ -141,190 +124,29 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
{
struct request_data *driver_data = context->pDriverData;
struct object_surface *surface_object;
struct video_format *video_format = NULL;
unsigned int destination_sizes[VIDEO_MAX_PLANES];
unsigned int destination_bytesperlines[VIDEO_MAX_PLANES];
unsigned int destination_planes_count;
unsigned int format_width, format_height;
unsigned int capture_type;
unsigned int i, j;
unsigned int i;
VASurfaceID id;
bool found;
int rc;
/*
* Set the OUTPUT format on (re)allocation when the resolution
* differs from the last set value. Without this, mpv's small
* probe surfaces (128x128) pin the CAPTURE format and the
* subsequent real-resolution surface ends up with wrong pitch
* in the export descriptor — causing Mesa to reject the
* DMA-BUF import. Detail in the LAST_OUTPUT_WIDTH/HEIGHT
* comment block at the top of this file.
* iter5b-β: only RT-format-level validation here. All V4L2
* device state (OUTPUT format, CAPTURE format probe,
* cap_pool_init, per-surface destination_* fill) is deferred
* to RequestCreateContext where the bound VAConfigID
* (and therefore the active VAProfile) is known. CreateSurfaces2
* has no config_id parameter; the VA-API contract is
* CreateConfig → CreateSurfaces → CreateContext, and we
* can't know the OUTPUT pixel format until CreateContext binds.
*
* 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.
* Surface objects allocated here hold only the requested
* width/height and per-surface lifecycle bookkeeping
* (current_slot, status, params, etc). The format-uniform
* destination_* fields are filled by CreateContext via
* surface_bind_format_uniform_fields(); the per-slot
* destination_* fields fill at BeginPicture via surface_bind_slot.
*/
unsigned int pixelformat = V4L2_PIX_FMT_H264_SLICE;
unsigned int output_type = v4l2_type_video_output(true);
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
* 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.
*
* Iter2 Fix 3 corollary: cap_pool owns the CAPTURE buffers'
* mmaps and slot states. Destroy it (which issues REQBUFS(0)
* on capture) before the format change so the next
* CreateSurfaces2 step can rebuild the pool at the new
* resolution. Without this, pool->initialized stays true,
* cap_pool_init below is skipped, and the slots' v4l2_index
* fields point to dead buffers from the prior resolution.
*/
if (driver_data->last_output_width != 0) {
if (driver_data->capture_pool.initialized)
cap_pool_destroy(&driver_data->capture_pool,
driver_data->video_fd,
v4l2_type_video_capture(true));
else
(void)v4l2_request_buffers(driver_data->video_fd,
v4l2_type_video_capture(true), 0);
/*
* iter7: tear down the OUTPUT pool too. Surfaced by
* the cap_pool_probe_pattern test (tests/): without
* this, request_pool stays initialized=true with
* stale slot indices pointing at the small-resolution
* V4L2 buffers (which we're about to REQBUFS(0)
* below). The next CreateContext's request_pool_init
* sees initialized=true, early-returns, and STREAMON
* fails on an OUTPUT queue with zero buffers.
*
* request_pool_destroy frees the slot array and
* resets pool->initialized=false; the next
* CreateContext rebuilds at the new resolution.
* Mirrors the cap_pool teardown above.
*/
if (driver_data->output_pool.initialized)
request_pool_destroy(&driver_data->output_pool);
(void)v4l2_request_buffers(driver_data->video_fd,
output_type, 0);
}
rc = v4l2_set_format(driver_data->video_fd, output_type, pixelformat,
width, height);
if (rc < 0)
return VA_STATUS_ERROR_OPERATION_FAILED;
driver_data->last_output_width = width;
driver_data->last_output_height = height;
}
if (format != VA_RT_FORMAT_YUV420)
return VA_STATUS_ERROR_UNSUPPORTED_RT_FORMAT;
if (!driver_data->video_format) {
found = v4l2_find_format(driver_data->video_fd,
V4L2_BUF_TYPE_VIDEO_CAPTURE,
V4L2_PIX_FMT_SUNXI_TILED_NV12);
if (found)
video_format = video_format_find(V4L2_PIX_FMT_SUNXI_TILED_NV12);
found = v4l2_find_format(driver_data->video_fd,
V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
V4L2_PIX_FMT_NV12);
if (found)
video_format = video_format_find(V4L2_PIX_FMT_NV12);
if (video_format == NULL)
return VA_STATUS_ERROR_OPERATION_FAILED;
driver_data->video_format = video_format;
capture_type = v4l2_type_video_capture(video_format->v4l2_mplane);
/*
* Do not VIDIOC_S_FMT on the CAPTURE queue. The hantro
* stateless decoder derives the CAPTURE format from the
* SPS attached to the OUTPUT request; explicitly setting
* it here can put the driver into an inconsistent state.
* GStreamer's v4l2slh264dec only G_FMTs CAPTURE (see
* gst-plugins-bad/sys/v4l2codecs/gstv4l2decoder.c::
* gst_v4l2_decoder_negotiate_src_format), and that
* variant produces correct decoded NV12 on the same
* hardware where this driver currently emits zeros.
*
* v4l2_get_format() below queries the driver's current
* state and gives us the bytesperline/sizes we need.
*/
} else {
video_format = driver_data->video_format;
capture_type = v4l2_type_video_capture(video_format->v4l2_mplane);
}
rc = v4l2_get_format(driver_data->video_fd, capture_type, &format_width,
&format_height, destination_bytesperlines,
destination_sizes, NULL);
if (rc < 0)
return VA_STATUS_ERROR_OPERATION_FAILED;
destination_planes_count = video_format->planes_count;
/*
* DEBUG INSTRUMENTATION (surface-export diagnosis 2026-05-04):
* dump what v4l2_get_format returned. Sonnet's Phase 5 review
* hypothesis #4 was that format_height might be 1080 (stream-
* signaled) vs 1088 (MB-aligned), causing UV offset to land
* 15360 bytes early. Earlier ftrace shows hantro returns
* height=1088 — but verify in-driver to be sure.
*/
/*
* Iter2 Fix 3: initialize the CAPTURE buffer pool on first call.
* Pool size = max(surfaces_count, MIN_CAP_POOL); the +headroom
* gives LRU recycling enough margin to never reuse a buffer
* within the consumer's compositor-hold window for typical
* playback patterns.
*
* If the pool already exists from a prior CreateSurfaces2 (e.g.
* mpv probe surfaces vs. real-resolution surfaces), it stays —
* but if the resolution changed (Fix 1's REQBUFS(0) on CAPTURE
* fired before this point), the pool was destroyed and we
* rebuild here.
*/
if (!driver_data->capture_pool.initialized) {
unsigned int pool_count = surfaces_count > MIN_CAP_POOL ?
surfaces_count : MIN_CAP_POOL;
rc = cap_pool_init(&driver_data->capture_pool,
driver_data->video_fd, capture_type,
pool_count, video_format->v4l2_buffers_count);
if (rc < 0)
return VA_STATUS_ERROR_ALLOCATION_FAILED;
}
/*
* Compute format-uniform destination_* values (sizes, offsets,
* bytesperlines, planes_count). These are the same for all
* surfaces of this format, set once per surface here, never
* changed by BeginPicture's slot acquisition.
*/
if (video_format->v4l2_buffers_count == 1) {
destination_sizes[0] = destination_bytesperlines[0] *
format_height;
for (j = 1; j < destination_planes_count; j++)
destination_sizes[j] = destination_sizes[0] / 2;
}
for (i = 0; i < surfaces_count; i++) {
id = object_heap_allocate(&driver_data->surface_heap);
surface_object = SURFACE(driver_data, id);
@@ -333,30 +155,8 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
surface_object->current_slot = NULL; /* iter2 Fix 3 */
surface_object->destination_index = 0; /* set on bind */
surface_object->destination_planes_count = destination_planes_count;
surface_object->destination_buffers_count =
video_format->v4l2_buffers_count;
if (video_format->v4l2_buffers_count == 1) {
for (j = 0; j < destination_planes_count; j++) {
surface_object->destination_offsets[j] =
j > 0 ? destination_sizes[j - 1] : 0;
surface_object->destination_sizes[j] =
destination_sizes[j];
surface_object->destination_bytesperlines[j] =
destination_bytesperlines[0];
}
} else if (video_format->v4l2_buffers_count == destination_planes_count) {
for (j = 0; j < destination_planes_count; j++) {
surface_object->destination_offsets[j] = 0;
surface_object->destination_sizes[j] =
destination_sizes[j];
surface_object->destination_bytesperlines[j] =
destination_bytesperlines[j];
}
} else {
return VA_STATUS_ERROR_ALLOCATION_FAILED;
}
surface_object->destination_planes_count = 0; /* set at CreateContext */
surface_object->destination_buffers_count = 0; /* set at CreateContext */
surface_object->status = VASurfaceReady;
surface_object->width = width;