iter5 sweep follow-up: remove additional DEBUG sites flagged by Phase 5 review
Phase 5 sonnet review caught four DEBUG sites the first sweep pass
missed (the vaapi-copy + --vo=null stress test didn't exercise the
ExportSurfaceHandle path, so per-frame ExportSurfaceHandle dumps went
undetected).
Removed:
- surface.c::CreateSurfaces2 format-dump (per-CreateSurfaces2 noise,
labeled DEBUG INSTRUMENTATION (surface-export diagnosis 2026-05-04))
- surface.c::ExportSurfaceHandle full-descriptor dump (per-frame for
consumers using DMA-BUF, also labeled DEBUG)
- surface.c::QuerySurfaceStatus -> status= line (per-call noise)
- h264.c V4L2 readback block (~67 lines): static bool readback_warned
+ the per-frame VIDIOC_G_EXT_CTRLS attempt + the readback success
log + the "V4L2 readback unavailable" fallback announcement. With
the iter4 fixes landed, the readback EACCES is no longer load-bearing
to investigate — drop the block + the per-process global state.
Removing the readback block also resolves Phase 5 finding C2: the
static bool readback_warned was new mutable process-global state
introduced post-Track-E, inconsistent with that track's intent.
Net: -107 lines from src/{h264,surface}.c.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
-67
@@ -988,73 +988,6 @@ int h264_set_controls(struct request_data *driver_data,
|
||||
if (rc < 0)
|
||||
return VA_STATUS_ERROR_OPERATION_FAILED;
|
||||
|
||||
/*
|
||||
* DEBUG INSTRUMENTATION (3F observability — added 2026-05-04):
|
||||
* VIDIOC_G_EXT_CTRLS readback on the request fd to confirm the
|
||||
* V4L2 layer accepted our writes verbatim. Cross-validates the
|
||||
* patch-0015 POC sentinel strip (top_field_order_cnt should be
|
||||
* the stripped value, not 65536), the slice-header parser
|
||||
* outputs (idr_pic_id, dec_ref_pic_marking_bit_size,
|
||||
* pic_order_cnt_bit_size), and the SCALING_MATRIX_PRESENT flag.
|
||||
*
|
||||
* Read into separate stack-allocated targets to avoid any
|
||||
* aliasing with the SET-side controls; the ioctl will overwrite
|
||||
* the targets with what's currently stored in the request.
|
||||
*/
|
||||
{
|
||||
struct v4l2_ctrl_h264_decode_params dec_rb = { 0 };
|
||||
struct v4l2_ctrl_h264_pps pps_rb = { 0 };
|
||||
struct v4l2_ext_control rb_controls[2] = { 0 };
|
||||
int rb_rc;
|
||||
|
||||
rb_controls[0].id = V4L2_CID_STATELESS_H264_DECODE_PARAMS;
|
||||
rb_controls[0].p_h264_decode_params = &dec_rb;
|
||||
rb_controls[0].size = sizeof(dec_rb);
|
||||
|
||||
rb_controls[1].id = V4L2_CID_STATELESS_H264_PPS;
|
||||
rb_controls[1].p_h264_pps = &pps_rb;
|
||||
rb_controls[1].size = sizeof(pps_rb);
|
||||
|
||||
static bool readback_warned = false;
|
||||
|
||||
rb_rc = v4l2_get_controls(driver_data->video_fd,
|
||||
surface->request_fd,
|
||||
rb_controls, 2);
|
||||
if (rb_rc == 0) {
|
||||
request_log("V4L2 readback: dec.idr_pic_id=%u "
|
||||
"dec.poc_lsb=%u dec.refmark_bits=%u "
|
||||
"dec.poc_bits=%u dec.top_foc=%d "
|
||||
"dec.bot_foc=%d dec.frame_num=%u "
|
||||
"pps.flags=0x%llx (SMP=%d) "
|
||||
"pps.refidx_l0=%u pps.refidx_l1=%u\n",
|
||||
dec_rb.idr_pic_id, dec_rb.pic_order_cnt_lsb,
|
||||
dec_rb.dec_ref_pic_marking_bit_size,
|
||||
dec_rb.pic_order_cnt_bit_size,
|
||||
dec_rb.top_field_order_cnt,
|
||||
dec_rb.bottom_field_order_cnt,
|
||||
dec_rb.frame_num,
|
||||
(unsigned long long)pps_rb.flags,
|
||||
!!(pps_rb.flags & V4L2_H264_PPS_FLAG_SCALING_MATRIX_PRESENT),
|
||||
pps_rb.num_ref_idx_l0_default_active_minus1,
|
||||
pps_rb.num_ref_idx_l1_default_active_minus1);
|
||||
} else if (!readback_warned) {
|
||||
/*
|
||||
* Rate-limit: log once per process. Linux 6.19.x
|
||||
* hantro+v4l2 returns EACCES when reading compound
|
||||
* H.264 controls from a request fd that is in
|
||||
* QUEUEING state. Not actionable from userspace;
|
||||
* symptomatic of a kernel-side permission check
|
||||
* not yet investigated. Decode itself is unaffected
|
||||
* — the SET-side write succeeded; we just can't
|
||||
* verify it via readback from this rig.
|
||||
*/
|
||||
request_log("V4L2 readback unavailable on this "
|
||||
"rig (EACCES from VIDIOC_G_EXT_CTRLS on "
|
||||
"request_fd) — not retrying\n");
|
||||
readback_warned = true;
|
||||
}
|
||||
}
|
||||
|
||||
dpb_insert(context, &surface->params.h264.picture.CurrPic, output);
|
||||
|
||||
return VA_STATUS_SUCCESS;
|
||||
|
||||
@@ -272,15 +272,6 @@ VAStatus RequestCreateSurfaces2(VADriverContextP context, unsigned int format,
|
||||
* 15360 bytes early. Earlier ftrace shows hantro returns
|
||||
* height=1088 — but verify in-driver to be sure.
|
||||
*/
|
||||
request_log("CreateSurfaces2: surf_width=%u surf_height=%u "
|
||||
"fmt_width=%u fmt_height=%u bytesperline[0]=%u "
|
||||
"sizes[0]=%u sizes[1]=%u planes_count=%u "
|
||||
"v4l2_buffers_count=%u\n",
|
||||
width, height, format_width, format_height,
|
||||
destination_bytesperlines[0],
|
||||
destination_sizes[0], destination_sizes[1],
|
||||
destination_planes_count, video_format->v4l2_buffers_count);
|
||||
|
||||
/*
|
||||
* Iter2 Fix 3: initialize the CAPTURE buffer pool on first call.
|
||||
* Pool size = max(surfaces_count, MIN_CAP_POOL); the +headroom
|
||||
@@ -618,10 +609,6 @@ VAStatus RequestQuerySurfaceStatus(VADriverContextP context,
|
||||
|
||||
*status = surface_object->status;
|
||||
|
||||
request_log(" -> status=%d (Ready=%d Rendering=%d Displaying=%d)\n",
|
||||
*status, VASurfaceReady, VASurfaceRendering,
|
||||
VASurfaceDisplaying);
|
||||
|
||||
return VA_STATUS_SUCCESS;
|
||||
}
|
||||
|
||||
@@ -816,33 +803,6 @@ VAStatus RequestExportSurfaceHandle(VADriverContextP context,
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* DEBUG INSTRUMENTATION (surface-export diagnosis 2026-05-04):
|
||||
* dump the full descriptor so we can compare against what mpv
|
||||
* reports importing via --msg-level=vd=v --msg-level=vo=v.
|
||||
* Phase 5 review identified DMA-BUF surface export as the
|
||||
* likely root cause of the solid-blue render in mpv vaapi mode.
|
||||
*/
|
||||
request_log("ExportSurfaceHandle: surf=%u fd[0]=%d fourcc=0x%x "
|
||||
"w=%u h=%u num_objects=%u num_layers=%u "
|
||||
"obj[0].size=%u drm_fmt=0x%x drm_mod=0x%llx num_planes=%u "
|
||||
"p[0].off=%u pitch=%u p[1].off=%u pitch=%u\n",
|
||||
surface_id,
|
||||
export_fds_count > 0 ? export_fds[0] : -1,
|
||||
surface_descriptor->fourcc,
|
||||
surface_descriptor->width,
|
||||
surface_descriptor->height,
|
||||
surface_descriptor->num_objects,
|
||||
surface_descriptor->num_layers,
|
||||
surface_descriptor->objects[0].size,
|
||||
surface_descriptor->layers[0].drm_format,
|
||||
(unsigned long long)surface_descriptor->objects[0].drm_format_modifier,
|
||||
surface_descriptor->layers[0].num_planes,
|
||||
surface_descriptor->layers[0].offset[0],
|
||||
surface_descriptor->layers[0].pitch[0],
|
||||
planes_count > 1 ? surface_descriptor->layers[0].offset[1] : 0,
|
||||
planes_count > 1 ? surface_descriptor->layers[0].pitch[1] : 0);
|
||||
|
||||
status = VA_STATUS_SUCCESS;
|
||||
goto complete;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user