From 4b2288fa9ace9f2b5683af3d5c4f9e37c1a9a63f Mon Sep 17 00:00:00 2001 From: claude-noether Date: Tue, 12 May 2026 09:23:31 +0000 Subject: [PATCH] =?UTF-8?q?fresnel-fourier=20iter5b=20Phase=206=20commit?= =?UTF-8?q?=20C:=20surface.c=20=E2=80=94=20profile-derived=20OUTPUT=20pixe?= =?UTF-8?q?l=20format?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hardcoded `V4L2_PIX_FMT_H264_SLICE` at surface.c:173 with a profile-derived lookup via find_sole_active_pixelformat(). The helper walks the config_heap; with one active config (universal across mpv, ffmpeg, Firefox, Chromium) it returns the cached pixelformat populated at CreateConfig in commit B. Falls back to the pre-iter5b H264_SLICE for the pathological "zero or multiple configs" case (probe surfaces before CreateConfig; multi-config-then-surfaces). Extend the existing resolution-change gate to also fire on pixelformat (codec) change. The teardown branch handles both cases identically — REQBUFS(0) on both queues before re-S_FMT. The kernel behavior pre-iter5b on RK3399: - hantro: hantro_find_format(H264_SLICE) returns NULL on the RK3399 decoder block (no H.264 support); hantro_try_fmt silently substitutes the first format in rk3399_vpu_dec_fmts = MPEG2_SLICE → codec_mode = MPEG2_DECODER. VP8 bitstream dispatched to MPEG2 ops → all-zero CAPTURE. MPEG-2 worked by accident (bitstream matched the substituted codec_mode). - rkvdec: format/control mismatch; decoder silently drops the request → all-zero CAPTURE. Same bug class as iter4 commit `692eaa0` (h264_start_code unconditional set). Both fixes thread the active VAProfile into codec-specific kernel state. Signed-off-by: claude-noether --- src/surface.c | 89 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/src/surface.c b/src/surface.c index 936f9a3..42adfa4 100644 --- a/src/surface.c +++ b/src/surface.c @@ -42,6 +42,8 @@ #include #include +#include "codec.h" +#include "config.h" #include "media.h" #include "utils.h" #include "v4l2.h" @@ -132,6 +134,46 @@ void surface_unbind_slot(struct request_data *driver_data, surface_object->current_slot = NULL; } +/* + * iter5b: walk config_heap looking for exactly one active config; return + * its cached pixelformat FOURCC. Returns 0 if zero or multiple configs + * exist (caller falls back to a safe default). + * + * The VA-API contract is CreateConfig(profile) → CreateSurfaces2 → CreateContext + * (binding both). CreateSurfaces2's signature has no config_id parameter, + * so the surface allocation can't know its profile directly. But every + * real consumer (mpv, ffmpeg, Firefox, Chromium) keeps exactly one config + * alive between CreateConfig and CreateSurfaces2; we exploit that to set + * the right V4L2 OUTPUT format here. The pathological "multiple configs + * alive simultaneously" case is not used by any in-scope consumer and + * falls back to the pre-iter5b hardcoded H264_SLICE. + */ +static unsigned int find_sole_active_pixelformat(struct request_data *driver_data) +{ + struct object_config *config_object; + unsigned int found = 0; + int seen = 0; + int iter; + + config_object = (struct object_config *) + object_heap_first(&driver_data->config_heap, &iter); + while (config_object != NULL) { + if (seen == 0) + found = config_object->pixelformat; + seen++; + config_object = (struct object_config *) + object_heap_next(&driver_data->config_heap, &iter); + } + + if (seen == 1) + return found; + + if (seen > 1) + request_log("CreateSurfaces2: %d active configs, profile-pick ambiguous; falling back to default OUTPUT format\n", + seen); + return 0; +} + VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format, unsigned int width, unsigned int height, VASurfaceID *surfaces_ids, @@ -154,13 +196,24 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format, /* * 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 + * or codec 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: derive the OUTPUT pixel format from the single active + * config's profile instead of the pre-iter5b hardcoded + * V4L2_PIX_FMT_H264_SLICE. Without this fix, HEVC / VP9 / VP8 + * surfaces ended up with H264_SLICE on the OUTPUT queue, which + * hantro silently substituted with the default MPEG2_DECODER + * codec_mode (producing all-zero VP8 output) and rkvdec silently + * dropped the decode entirely (producing all-zero HEVC + VP9 + * output). Same bug class as iter4's unconditional + * h264_start_code — codec-specific kernel state set without + * profile gating. See phase0_iter5_loopback.md commit cd34ec1. + * * 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 @@ -170,20 +223,33 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format, * + 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 = find_sole_active_pixelformat(driver_data); unsigned int output_type = v4l2_type_video_output(true); + if (pixelformat == 0) { + /* + * Pre-CreateConfig probe surfaces (mpv pattern) or the + * pathological multiple-configs-alive case. Fall back to + * pre-iter5b behavior; the gate below will re-S_FMT once + * the real config exists and pixelformat changes. + */ + pixelformat = V4L2_PIX_FMT_H264_SLICE; + } + if (driver_data->last_output_width != width || - driver_data->last_output_height != height) { + driver_data->last_output_height != height || + driver_data->last_output_pixelformat != pixelformat) { /* * 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. + * resolution or codec, 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 + * configuration 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; iter5b extends the gate predicate to also fire on + * pixelformat change (codec switch). * * Iter2 Fix 3 corollary: cap_pool owns the CAPTURE buffers' * mmaps and slot states. Destroy it (which issues REQBUFS(0) @@ -229,6 +295,7 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format, driver_data->last_output_width = width; driver_data->last_output_height = height; + driver_data->last_output_pixelformat = pixelformat; } if (format != VA_RT_FORMAT_YUV420)