From 893c9d62a6833e588353ee25ae0bf84dd989f8e0 Mon Sep 17 00:00:00 2001 From: 4gray Date: Mon, 28 Sep 2026 10:58:58 +0200 Subject: [PATCH] fix(playback): stop random black frames on software GL in frame-copy mpv's LUT scalers (the default lanczos/spline family) build their weight texture from an uninitialized talloc_array: mp_compute_lut fills only kernel->size of every stride floats, so each row's padding is heap garbage (reinit_scaler in video/out/gpu/video.c, mpv 0.41 and master). The LUT is an rgba16f texture sampled with linear filtering. When the padding holds NaN, or a value large enough to become Inf as half-float, Mesa's llvmpipe carries it through the zero-weight neighbour texel, the weights become NaN and the frame clamps to pure black. Whether it happens depends on heap contents, so the packaged smoke's paused frame was black in most attempts on the CI runner. Evidence from CI (8 repeats each, no retries): baseline 2/8 passed; LUT-free bilinear scalers 8/8; gpu-dumb-mode 8/8; LP_NUM_THREADS=1 6/8. A synchronous glReadPixels right after mpv_render_context_render() showed the black frame already in the helper's FBO. The readback, the shared-memory ring and the preload pump were correct. On a CPU rasterizer the helper now selects mpv's LUT-free bilinear scalers and disables sigmoid upscaling. Software GL cannot afford the LUT scalers anyway. A session option for the same key still wins. Co-Authored-By: Claude Opus 5.5 --- ...layback-frame-copy-software-black-video.md | 6 ++++ .../native/helper/frame_helper_gl.h | 31 ++++++++++-------- .../native/helper/frame_helper_render.h | 10 ++++-- .../native/helper/mpv_frame_helper.cpp | 32 ++++++++++++++++++- docs/architecture/embedded-mpv-native.md | 7 +++- docs/development/electron-debugging.md | 7 +++- 6 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 .changes/playback-frame-copy-software-black-video.md diff --git a/.changes/playback-frame-copy-software-black-video.md b/.changes/playback-frame-copy-software-black-video.md new file mode 100644 index 000000000..f2404fb0d --- /dev/null +++ b/.changes/playback-frame-copy-software-black-video.md @@ -0,0 +1,6 @@ +--- +type: fix +area: playback +--- + +The experimental Embedded MPV frame-copy engine no longer shows a black picture at random on computers that render graphics in software (for example without GPU drivers or in a virtual machine). diff --git a/apps/electron-backend/native/helper/frame_helper_gl.h b/apps/electron-backend/native/helper/frame_helper_gl.h index a0eec7098..2651d5c7f 100644 --- a/apps/electron-backend/native/helper/frame_helper_gl.h +++ b/apps/electron-backend/native/helper/frame_helper_gl.h @@ -28,6 +28,8 @@ */ #pragma once +#include +#include #include #if defined(__APPLE__) @@ -126,6 +128,20 @@ namespace frame_helper { /* Matches mpv_opengl_init_params.get_proc_address. */ using GlGetProcAddressFn = void* (*)(void* ctx, const char* name); +/* GL_RENDERER strings of CPU rasterizers (Mesa llvmpipe/softpipe/swrast). */ +inline bool isSoftwareGlRenderer(const std::string& renderer) { + std::string normalized = renderer; + std::transform(normalized.begin(), normalized.end(), normalized.begin(), + [](unsigned char value) { + return static_cast(std::tolower(value)); + }); + for (const char* marker : {"llvmpipe", "softpipe", "swrast", + "software rasterizer", "lavapipe"}) { + if (normalized.find(marker) != std::string::npos) return true; + } + return false; +} + #if defined(__APPLE__) inline void* glDlsymGetProcAddress(void* ctx, const char* name) { @@ -382,19 +398,6 @@ private: candidate.gbmFd >= 0 || candidate.current; } - static bool isSoftwareRenderer(const std::string& renderer) { - std::string normalized = renderer; - std::transform(normalized.begin(), normalized.end(), normalized.begin(), - [](unsigned char value) { - return static_cast(std::tolower(value)); - }); - for (const char* marker : {"llvmpipe", "softpipe", "swrast", - "software rasterizer", "lavapipe"}) { - if (normalized.find(marker) != std::string::npos) return true; - } - return false; - } - static bool openGbmDevice(Candidate& candidate) { for (int node = 128; node <= 131; node++) { char devicePath[32]; @@ -508,7 +511,7 @@ private: const GLubyte* renderer = glGetString(GL_RENDERER); if (!renderer) return fail("GL_RENDERER unavailable"); candidate.renderer = reinterpret_cast(renderer); - candidate.softwareRenderer = isSoftwareRenderer(candidate.renderer); + candidate.softwareRenderer = isSoftwareGlRenderer(candidate.renderer); if (eglMakeCurrent(candidate.display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) != EGL_TRUE) { diff --git a/apps/electron-backend/native/helper/frame_helper_render.h b/apps/electron-backend/native/helper/frame_helper_render.h index 1f0dcaf32..a24268573 100644 --- a/apps/electron-backend/native/helper/frame_helper_render.h +++ b/apps/electron-backend/native/helper/frame_helper_render.h @@ -144,6 +144,9 @@ public: std::function onGenerationChanged; + /* Called from the render thread before the mpv render context exists + * when the bound context is a CPU rasterizer (see setupGl()). */ + std::function onSoftwareRenderer; bool start(mpv_handle* mpv, const std::string& shmBaseName, int width, int height, std::string& errorOut); @@ -205,8 +208,11 @@ inline bool RenderPipeline::setupGl(std::string& errorOut) { * software tiers when a later hardware-backed EGL candidate was usable. */ const GLubyte* renderer = glGetString(GL_RENDERER); if (renderer) { - std::fprintf(stderr, "gl renderer: %s\n", - reinterpret_cast(renderer)); + const char* rendererName = reinterpret_cast(renderer); + std::fprintf(stderr, "gl renderer: %s\n", rendererName); + if (onSoftwareRenderer && isSoftwareGlRenderer(rendererName)) { + onSoftwareRenderer(); + } } if (!rebuildTargets(width_, height_)) { diff --git a/apps/electron-backend/native/helper/mpv_frame_helper.cpp b/apps/electron-backend/native/helper/mpv_frame_helper.cpp index e7fab3032..b69df2346 100644 --- a/apps/electron-backend/native/helper/mpv_frame_helper.cpp +++ b/apps/electron-backend/native/helper/mpv_frame_helper.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -788,6 +789,28 @@ void onRenderUpdate(void* pipeline) { static_cast(pipeline)->notifyUpdate(); } +/* mpv's LUT scalers (the default lanczos/spline family) upload their weight + * texture with uninitialized row padding (reinit_scaler in + * video/out/gpu/video.c, still present in mpv 0.41 and master). Mesa's CPU + * rasterizers carry NaN/Inf from that padding through zero-weight linear + * filtering, so a session randomly renders pure black depending on heap + * contents. Software GL gets mpv's LUT-free bilinear scalers instead, which + * is also what it can afford; a session option for the same key wins. */ +void applySoftwareRendererOptions(const std::set& sessionKeys) { + static const std::pair kOptions[] = { + {"scale", "bilinear"}, + {"cscale", "bilinear"}, + {"dscale", "bilinear"}, + {"sigmoid-upscaling", "no"}, + }; + for (const auto& [key, value] : kOptions) { + if (sessionKeys.count(key) == 0) { + mpv_set_property_string(g_state.mpv, key, value); + } + } + std::fprintf(stderr, "software renderer: using bilinear scalers\n"); +} + struct HelperArgs { std::string shmBase = "/impv"; std::string hwdec = "auto"; @@ -987,6 +1010,7 @@ int main(int argc, char** argv) { mpv_set_option_string(g_state.mpv, "audio-delay", args.audioDelay.c_str()); } + std::set sessionOptionKeys; if (args.mpvOptionsOnStdin) { /* `mpv-options\to000=\to001=...` in the regular command * encoding; zero-padded keys keep the application order (network @@ -1022,7 +1046,9 @@ int main(int argc, char** argv) { const std::string value = option.substr(separator + 1); const int optionResult = mpv_set_option_string( g_state.mpv, key.c_str(), value.c_str()); - if (optionResult < 0) { + if (optionResult >= 0) { + sessionOptionKeys.insert(key); + } else { emitLine(JsonWriter() .str("event", "log") .str("level", "warn") @@ -1095,6 +1121,10 @@ int main(int argc, char** argv) { .finish()); }; + g_state.pipeline.onSoftwareRenderer = [sessionOptionKeys] { + applySoftwareRendererOptions(sessionOptionKeys); + }; + g_state.viewportWidth = args.width; g_state.viewportHeight = args.height; diff --git a/docs/architecture/embedded-mpv-native.md b/docs/architecture/embedded-mpv-native.md index c8541e829..69f46a5d3 100644 --- a/docs/architecture/embedded-mpv-native.md +++ b/docs/architecture/embedded-mpv-native.md @@ -443,7 +443,12 @@ Trade-offs and constraints: string to stderr. If an early tier selects Mesa software rendering (for example, while a proprietary NVIDIA driver is reachable through the default display or GBM), it probes the remaining tiers and uses software only when - no hardware-backed context works. Windows (any arch with a helper, in + no hardware-backed context works. On a software renderer the helper sets + `scale`, `cscale` and `dscale` to `bilinear` and `sigmoid-upscaling=no` + unless a session option sets the same key: mpv's LUT scalers upload their + weight texture with uninitialized row padding (`reinit_scaler`, mpv 0.41), + and llvmpipe carries NaN/Inf from it through zero-weight filtering, so + sessions otherwise render pure black at random. Windows (any arch with a helper, in practice x64) is ported: WGL renders offscreen against a hidden window, the shm ring is a session-local named file mapping, and the reader addon compiles as C++ there (MSVC has no C11 ``). The helper diff --git a/docs/development/electron-debugging.md b/docs/development/electron-debugging.md index 1d88fc503..dc1a36f17 100644 --- a/docs/development/electron-debugging.md +++ b/docs/development/electron-debugging.md @@ -91,7 +91,12 @@ exit. The retained handle still provides the actual exit code and signal. 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. +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 +`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. The Snap and Flatpak `--embedded-mpv-runtime-probe` launches in `build-and-make.yaml` run under GNU `timeout --verbose -k 10 300` with `ELECTRON_ENABLE_LOGGING=1`, so a main process that throws before app ready