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)