From 6fc2b201aeca58cbeea0b312f4c103c3bdd9d227 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 29 Sep 2026 19:39:51 +0200 Subject: [PATCH] fix(playback): report software scaler results and scope smoke ring diagnostics The software-renderer fallback now reports which scaler options mpv accepted and which it rejected (with mpv's error), instead of always printing success. The smoke's black-canvas diagnostics read only the failed session's rings (-g), not every impv-fc ring in /dev/shm. Addresses Greptile review feedback. Co-Authored-By: Claude Opus 5.5 --- .../embedded-mpv-frame-copy-packaged-diagnostics.ts | 13 +++++++++---- .../native/helper/mpv_frame_helper.cpp | 13 ++++++++++--- docs/development/electron-debugging.md | 2 +- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged-diagnostics.ts b/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged-diagnostics.ts index 50a909b04..f276dec33 100644 --- a/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged-diagnostics.ts +++ b/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged-diagnostics.ts @@ -20,7 +20,6 @@ import { */ const SHM_DIRECTORY = '/dev/shm'; -const SHM_PREFIX = 'impv-fc-'; const FRAME_SHM_MAGIC = 0x564d5046; const FRAME_SHM_RING_SLOTS = 3; const HEADER_BYTES = 56 + FRAME_SHM_RING_SLOTS * 16; @@ -103,10 +102,16 @@ export function describeFrameRing( }; } -export function describeFrameRings(): FrameRingDiagnostics[] | string { +/** + * The helper names its rings `/-g`, so the prefix + * keeps stale or concurrent sessions out of the report. + */ +export function describeFrameRings( + sessionId: string +): FrameRingDiagnostics[] | string { try { return readdirSync(SHM_DIRECTORY) - .filter((entry) => entry.startsWith(SHM_PREFIX)) + .filter((entry) => entry.startsWith(`${sessionId}-g`)) .map((entry) => describeFrameRing( entry, @@ -146,7 +151,7 @@ export async function expectRenderedFrame( videoHeight: session.videoHeight, stats: session.stats, }, - frameRings: describeFrameRings(), + frameRings: describeFrameRings(sessionId), }; const body = JSON.stringify(diagnostics, null, 2); console.log(`[frame-copy smoke diagnostics] ${body}`); diff --git a/apps/electron-backend/native/helper/mpv_frame_helper.cpp b/apps/electron-backend/native/helper/mpv_frame_helper.cpp index b69df2346..f2cfbf762 100644 --- a/apps/electron-backend/native/helper/mpv_frame_helper.cpp +++ b/apps/electron-backend/native/helper/mpv_frame_helper.cpp @@ -803,12 +803,19 @@ void applySoftwareRendererOptions(const std::set& sessionKeys) { {"dscale", "bilinear"}, {"sigmoid-upscaling", "no"}, }; + std::string applied; + std::string failed; for (const auto& [key, value] : kOptions) { - if (sessionKeys.count(key) == 0) { - mpv_set_property_string(g_state.mpv, key, value); + if (sessionKeys.count(key) != 0) continue; + const int result = mpv_set_property_string(g_state.mpv, key, value); + std::string& list = result < 0 ? failed : applied; + list += (list.empty() ? "" : ", ") + std::string(key) + "=" + value; + if (result < 0) { + list += std::string(" (") + mpv_error_string(result) + ")"; } } - std::fprintf(stderr, "software renderer: using bilinear scalers\n"); + std::fprintf(stderr, "software renderer: applied [%s]; failed [%s]\n", + applied.c_str(), failed.c_str()); } struct HelperArgs { diff --git a/docs/development/electron-debugging.md b/docs/development/electron-debugging.md index dc1a36f17..f4c594095 100644 --- a/docs/development/electron-debugging.md +++ b/docs/development/electron-debugging.md @@ -93,7 +93,7 @@ The Linux portable build uploads `packaged-frame-copy-smoke` reports and traces even when the smoke fails. Check the paused-frame screenshot and trace before classifying a zero rendered-frame signal as an infrastructure flake. A zero signal also attaches `frame-copy-diagnostics` (the session's stream stats, -including mpv's drop counter, and each helper ring read from `/dev/shm`) and +including mpv's drop counter, and its helper rings read from `/dev/shm`) and `mpv-log`, the session's verbose mpv log. A `latestSeq` of 0 means the helper never published; a `latestFrameSignal` of 0 means mpv rendered black; a visible ring frame behind a black canvas points at the preload pump.