From 29f16ece13b4c00451d05f641d9be71363752b3b Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Wed, 20 May 2026 20:35:06 +0200 Subject: [PATCH] kernel: bind request controls to p_cur before reading them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit device_run was reading ctrl->p_cur.p_h264_* directly, but v4l2-m2m's request scheduler does NOT auto-bind the in-flight media_request's control values to the ctrl handler's p_cur slots — drivers have to call v4l2_ctrl_request_setup() explicitly. cedrus / rkvdec / hantro all do this in their device_run; daedalus didn't. Result: daedalus_collect_h264_meta() read stale or default values (whatever the prior request had left in p_cur, or v4l2_ctrl_new_custom initial state if no prior request had completed) instead of the S_EXT_CTRLS V4L2_CTRL_WHICH_REQUEST_VAL values libva-v4l2-request- fourier had just sent for THIS frame. The mismatch was a smoking gun on higgs after libva PR #9 / packages PR #52 landed an instrumentation log at h264_set_controls entry: libva boundary (sent to kernel): VAProfile=13 seq_fields=0x00032051 pic_fields=0x00000500 num_ref_frames=1 daedalus daemon (read from kernel p_cur): prof=100 level=41 ref_frames=0 flags=0x10 pps_flags=0x0 After calling v4l2_ctrl_request_setup() at the top of device_run: daedalus daemon (read from kernel p_cur): prof=66 level=11 ref_frames=1 poc_type=2 flags=0x50 pps_flags=0x88 — matches what libva sent, matches the bitstream's actual SPS. End-to-end test on higgs with libva-v4l2-request-fourier 1.0.0+r382 +gc1bb444 (after-fix-3-and-fix-4-instrumentation) + this kernel patch: $ LIBVA_DRIVER_NAME=v4l2_request ffmpeg -hwaccel vaapi \ -hwaccel_device /dev/dri/renderD128 -i h264_test.mp4 \ -frames:v 1 -f null - ... rc=0 daemon journal: zero "error while decoding MB" lines, zero "reference frames exceeds max" lines. Per-frame fnv1a hashes differ (0xf1c515aa, 0x16e915e8, 0x16bd16cc, ...) instead of the constant 0x6a6a05c5 "give-up-and-zero" hash from before — libavcodec is actually decoding real pixel content from each P-frame. Pair note: the daemon side already calls v4l2_ctrl_request_complete in daedalus_complete_resp_frame (line 834) — this commit pairs the setup half with that completion half. The daemon side change (decoder.c) is a small log-level promotion: the per-frame "h264 SPS/PPS prepended ..." trace went from log_debug to log_info so the journal shows what's being shipped into libavcodec without needing a daemon rebuild with --debug. Matches the libva- side h264_set_controls instrumentation that landed in libva PR #9. Closes part of issue libva-v4l2-request-fourier#8 — the SPS/PPS field-value gap. Profile/level still come from libva's session- derived hardcoded values (h264_profile_to_idc + h264_derive_level_ idc) which is sufficient for libavcodec to accept the synthesised NAL unit; a true stream-parsed profile/level would need SPS-NAL parsing in libva — separate operator-design call. Co-Authored-By: Claude Opus 4.7 --- daemon/src/decoder.c | 17 +++++++++++++++-- kernel/daedalus_v4l2_main.c | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/daemon/src/decoder.c b/daemon/src/decoder.c index 9d85ee8..e91eb35 100644 --- a/daemon/src/decoder.c +++ b/daemon/src/decoder.c @@ -416,8 +416,21 @@ int daedalus_decoder_run_request(struct daedalus_decoder *dec, dec->pkt->data = assembled; dec->pkt->size = (int) assembled_len; - log_debug("decoder: h264 prepended SPS=%zuB PPS=%zuB slice=%uB", - sps_len, pps_len, req->bitstream_len); + log_info("decoder: h264 SPS prof=%u level=%u ref_frames=%u w_mbs=%u h_units=%u poc_type=%u flags=0x%x", + h264_meta->sps.profile_idc, + h264_meta->sps.level_idc, + h264_meta->sps.max_num_ref_frames, + h264_meta->sps.pic_width_in_mbs_minus1, + h264_meta->sps.pic_height_in_map_units_minus1, + h264_meta->sps.pic_order_cnt_type, + h264_meta->sps.flags); + log_info("decoder: h264 PPS spsid=%u ppsid=%u qp-26=%d flags=0x%x", + h264_meta->pps.seq_parameter_set_id, + h264_meta->pps.pic_parameter_set_id, + h264_meta->pps.pic_init_qp_minus26, + h264_meta->pps.flags); + log_info("decoder: h264 prepended SPS=%zuB PPS=%zuB slice=%uB", + sps_len, pps_len, req->bitstream_len); } else { /* * VP9/AV1: bitstream is self-contained per frame, point the diff --git a/kernel/daedalus_v4l2_main.c b/kernel/daedalus_v4l2_main.c index ffc616b..b493a4b 100644 --- a/kernel/daedalus_v4l2_main.c +++ b/kernel/daedalus_v4l2_main.c @@ -677,6 +677,28 @@ static void daedalus_device_run(void *priv) goto fail_buf_error; } + /* + * Bind the in-flight media_request's stateless control values to + * the ctrl_handler's p_cur slots so daedalus_collect_h264_meta() + * sees this request's SPS/PPS/scaling_matrix/decode_params — not + * the previous request's stale values (or driver defaults from + * v4l2_ctrl_new_custom when no prior request has run yet). + * + * Without this, p_cur reads back zero/default for everything the + * userspace driver set via S_EXT_CTRLS with + * V4L2_CTRL_WHICH_REQUEST_VAL — caught by libva-v4l2-request- + * fourier's libva-boundary instrumentation (issue #8) showing + * num_ref_frames=1 sent vs. ref_frames=0 read. Pair with the + * v4l2_ctrl_request_complete call already present in the + * completion path (daedalus_complete_resp_frame). + * + * cedrus / rkvdec / hantro all call this from device_run; the + * m2m core does NOT do it automatically. + */ + if (src_buf->vb2_buf.req_obj.req) + v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req, + &ctx->hdl); + { u32 cid = daedalus_fourcc_to_codec_id(ctx->src_fmt.pixelformat); size_t meta_len = 0;