From e8d1cf2108aef6cfb86dc8ef754b0756bf60fb81 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sat, 13 Jun 2026 11:41:17 +0200 Subject: [PATCH] fix: move linux mpv polling off main thread --- .../native/src/embedded_mpv_wid_common.h | 146 ++++++++++++++---- .../embedded-mpv-native-source.spec.ts | 67 +++++++- docs/architecture/embedded-mpv-native.md | 2 +- 3 files changed, 180 insertions(+), 35 deletions(-) diff --git a/apps/electron-backend/native/src/embedded_mpv_wid_common.h b/apps/electron-backend/native/src/embedded_mpv_wid_common.h index 1a8419292..99bee138c 100644 --- a/apps/electron-backend/native/src/embedded_mpv_wid_common.h +++ b/apps/electron-backend/native/src/embedded_mpv_wid_common.h @@ -146,6 +146,9 @@ struct Session { std::atomic gNextSessionId{1}; std::atomic gNextAsyncRequestId{1}; +#ifdef __linux__ +std::atomic gNextLinuxIpcSocketId{1}; +#endif std::mutex gSessionsMutex; std::unordered_map> gSessions; @@ -339,6 +342,11 @@ Bounds readBounds(const Napi::Object& object) } #ifdef __linux__ +struct LinuxMpvProcess { + pid_t processId = -1; + std::string ipcSocketPath; +}; + bool isExecutableAvailable(const char* executableName) { const char* pathValue = std::getenv("PATH"); @@ -403,7 +411,7 @@ std::vector buildLinuxMpvEnvironment() void updateLinuxProcessState(const std::shared_ptr& session) { - if (!session) { + if (!session || !session->running.load()) { return; } @@ -421,8 +429,14 @@ void updateLinuxProcessState(const std::shared_ptr& session) if (result != processId) { return; } + if (!session->running.load()) { + return; + } std::lock_guard lock(session->mutex); + if (session->mpvProcessId != processId) { + return; + } session->mpvProcessId = -1; session->snapshot.status = WIFEXITED(status) && WEXITSTATUS(status) == 0 @@ -433,46 +447,65 @@ void updateLinuxProcessState(const std::shared_ptr& session) } } -void terminateLinuxMpvProcess(const std::shared_ptr& session) +void unlinkLinuxIpcSocket(const std::string& ipcSocketPath) { + if (!ipcSocketPath.empty()) { + unlink(ipcSocketPath.c_str()); + } +} + +LinuxMpvProcess takeLinuxMpvProcess(const std::shared_ptr& session) +{ + LinuxMpvProcess process; if (!session) { - return; + return process; } - pid_t processId = -1; - std::string ipcSocketPath; { std::lock_guard lock(session->mutex); - processId = session->mpvProcessId; + process.processId = session->mpvProcessId; session->mpvProcessId = -1; - ipcSocketPath = session->mpvIpcSocketPath; + process.ipcSocketPath = session->mpvIpcSocketPath; session->mpvIpcSocketPath.clear(); } - if (processId <= 0) { - if (!ipcSocketPath.empty()) { - unlink(ipcSocketPath.c_str()); - } + + return process; +} + +void waitForLinuxMpvProcessExit(LinuxMpvProcess process) +{ + if (process.processId <= 0) { + unlinkLinuxIpcSocket(process.ipcSocketPath); return; } - kill(processId, SIGTERM); for (int attempt = 0; attempt < 10; attempt += 1) { int status = 0; - const pid_t result = waitpid(processId, &status, WNOHANG); - if (result == processId || result == -1) { - if (!ipcSocketPath.empty()) { - unlink(ipcSocketPath.c_str()); - } + const pid_t result = waitpid(process.processId, &status, WNOHANG); + if (result == process.processId || result == -1) { + unlinkLinuxIpcSocket(process.ipcSocketPath); return; } std::this_thread::sleep_for(std::chrono::milliseconds(50)); } - kill(processId, SIGKILL); - waitpid(processId, nullptr, 0); - if (!ipcSocketPath.empty()) { - unlink(ipcSocketPath.c_str()); + kill(process.processId, SIGKILL); + waitpid(process.processId, nullptr, 0); + unlinkLinuxIpcSocket(process.ipcSocketPath); +} + +void terminateLinuxMpvProcessAsync(const std::shared_ptr& session) +{ + auto process = takeLinuxMpvProcess(session); + if (process.processId <= 0) { + unlinkLinuxIpcSocket(process.ipcSocketPath); + return; } + + kill(process.processId, SIGTERM); + std::thread([process = std::move(process)]() mutable { + waitForLinuxMpvProcessExit(std::move(process)); + }).detach(); } void appendLinuxMpvOption( @@ -574,7 +607,8 @@ std::string buildLinuxIpcSocketPath(const std::shared_ptr& session) return "/tmp/iptvnator-embedded-mpv-" + std::to_string(static_cast(getpid())) + - "-" + safeSessionId + ".sock"; + "-" + safeSessionId + + "-" + std::to_string(gNextLinuxIpcSocketId.fetch_add(1)) + ".sock"; } bool writeAll(int fileDescriptor, const std::string& payload) @@ -795,6 +829,9 @@ bool sendLinuxMpvCommand( void refreshLinuxMpvSnapshot(const std::shared_ptr& session) { updateLinuxProcessState(session); + if (!session || !session->running.load()) { + return; + } std::string socketPath; pid_t processId = -1; @@ -814,6 +851,13 @@ void refreshLinuxMpvSnapshot(const std::shared_ptr& session) const auto path = queryLinuxMpvString(socketPath, "path"); std::lock_guard lock(session->mutex); + if ( + !session->running.load() || + session->mpvProcessId != processId || + session->mpvIpcSocketPath != socketPath + ) { + return; + } if (position) { session->snapshot.positionSeconds = std::max(0.0, *position); } @@ -833,6 +877,37 @@ void refreshLinuxMpvSnapshot(const std::shared_ptr& session) } } +void runLinuxProcessPollLoop(std::shared_ptr session) +{ + while (session && session->running.load()) { + refreshLinuxMpvSnapshot(session); + for (int tick = 0; tick < 10 && session->running.load(); tick += 1) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + } +} + +bool startLinuxProcessPolling(const std::shared_ptr& session) +{ + if (!session) { + return false; + } + + bool expected = false; + if (!session->running.compare_exchange_strong(expected, true)) { + return true; + } + + try { + session->eventThread = std::thread(runLinuxProcessPollLoop, session); + } catch (...) { + session->running.store(false); + return false; + } + + return true; +} + pid_t spawnLinuxMpvProcess( const std::vector& arguments, const std::vector& environment) @@ -884,7 +959,7 @@ void loadLinuxProcessPlayback( const std::string& referer, double startTime) { - terminateLinuxMpvProcess(session); + terminateLinuxMpvProcessAsync(session); const auto ipcSocketPath = buildLinuxIpcSocketPath(session); unlink(ipcSocketPath.c_str()); const auto arguments = buildLinuxMpvArguments( @@ -924,6 +999,13 @@ void loadLinuxProcessPlayback( session->snapshot.recordingStartedAt.clear(); session->snapshot.recordingError.clear(); } + if (!startLinuxProcessPolling(session)) { + terminateLinuxMpvProcessAsync(session); + throw Napi::Error::New( + env, + "Failed to start embedded MPV snapshot polling." + ); + } } #endif @@ -1311,7 +1393,19 @@ void destroySession(const std::shared_ptr& session) } #ifdef __linux__ - terminateLinuxMpvProcess(session); + session->running.store(false); + terminateLinuxMpvProcessAsync(session); + if (session->eventThread.joinable()) { + session->eventThread.detach(); + } + session->host.destroy(); + { + std::lock_guard lock(session->mutex); + session->snapshot.status = SessionStatus::Closed; + session->snapshot.recordingActive = false; + session->snapshot.recordingStartedAt.clear(); + } + return; #endif session->running.store(false); if (session->handle) { @@ -2005,10 +2099,6 @@ Napi::Value GetSessionSnapshot(const Napi::CallbackInfo& info) return env.Null(); } -#ifdef __linux__ - refreshLinuxMpvSnapshot(session); -#endif - SessionSnapshot snapshot; { std::lock_guard lock(session->mutex); diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native-source.spec.ts b/apps/electron-backend/src/app/services/embedded-mpv-native-source.spec.ts index f043188f3..c6015c61c 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native-source.spec.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native-source.spec.ts @@ -41,22 +41,30 @@ describe('Embedded MPV native source recording invariants', () => { ); function functionBody(name: string): string { - const start = nativeSource.indexOf(`Napi::Value ${name}(`); + return sourceFunctionBody(nativeSource, `Napi::Value ${name}(`, name); + } + + function sourceFunctionBody( + source: string, + signature: string, + name: string + ): string { + const start = source.indexOf(signature); expect(start).toBeGreaterThanOrEqual(0); - const bodyStart = nativeSource.indexOf('{', start); + const bodyStart = source.indexOf('{', start); expect(bodyStart).toBeGreaterThanOrEqual(0); let depth = 0; - for (let index = bodyStart; index < nativeSource.length; index += 1) { - if (nativeSource[index] === '{') { + for (let index = bodyStart; index < source.length; index += 1) { + if (source[index] === '{') { depth += 1; } - if (nativeSource[index] === '}') { + if (source[index] === '}') { depth -= 1; } if (depth === 0) { - return nativeSource.slice(bodyStart, index + 1); + return source.slice(bodyStart, index + 1); } } @@ -346,6 +354,53 @@ describe('Embedded MPV native source recording invariants', () => { ); }); + it('keeps Linux MPV snapshot IPC off the NAPI snapshot read path', () => { + const getSnapshotBody = sourceFunctionBody( + widCommonSource, + 'Napi::Value GetSessionSnapshot(', + 'GetSessionSnapshot' + ); + expect(getSnapshotBody).not.toContain('refreshLinuxMpvSnapshot'); + expect(widCommonSource).toContain('void runLinuxProcessPollLoop'); + expect(widCommonSource).toContain('refreshLinuxMpvSnapshot(session);'); + expect(widCommonSource).toContain('startLinuxProcessPolling(session)'); + expect(widCommonSource).toContain( + 'session->eventThread = std::thread(runLinuxProcessPollLoop, session);' + ); + }); + + it('terminates Linux MPV processes away from the NAPI teardown path', () => { + const destroyBody = sourceFunctionBody( + widCommonSource, + 'void destroySession(', + 'destroySession' + ); + expect(destroyBody).toContain( + 'terminateLinuxMpvProcessAsync(session);' + ); + expect(destroyBody).toContain('session->eventThread.detach();'); + expect(destroyBody).not.toContain('std::this_thread::sleep_for'); + expect(destroyBody).not.toContain('SIGKILL'); + + const asyncTerminationBody = sourceFunctionBody( + widCommonSource, + 'void terminateLinuxMpvProcessAsync(', + 'terminateLinuxMpvProcessAsync' + ); + expect(asyncTerminationBody).toContain( + 'kill(process.processId, SIGTERM);' + ); + expect(asyncTerminationBody).toContain('std::thread('); + expect(asyncTerminationBody).toContain('waitForLinuxMpvProcessExit'); + }); + + it('uses generation-unique Linux MPV IPC socket paths', () => { + expect(widCommonSource).toContain('gNextLinuxIpcSocketId'); + expect(widCommonSource).toContain( + 'std::to_string(gNextLinuxIpcSocketId.fetch_add(1))' + ); + }); + it('keeps dynamic libmpv symbol declarations compatible with distro headers', () => { expect(widCommonSource).not.toContain('MPV_CPLUGIN_DYNAMIC_SYM'); expect(widCommonSource).toContain('#ifdef IPTVNATOR_DYNAMIC_LIBMPV'); diff --git a/docs/architecture/embedded-mpv-native.md b/docs/architecture/embedded-mpv-native.md index 0a4c04fbe..b34c10f24 100644 --- a/docs/architecture/embedded-mpv-native.md +++ b/docs/architecture/embedded-mpv-native.md @@ -28,7 +28,7 @@ The build directory contains files such as `Makefile`, `binding.Makefile`, `conf The embedded player renders MPV frames into an app-owned native video surface. macOS uses the libmpv render API in an `NSOpenGLView` because the mpv `wid` path produced a black video surface inside Electron. Windows loads `libmpv` through the native Node addon and uses mpv's `wid` option against an IPTVnator-owned child `HWND`. Linux creates an IPTVnator-owned X11/Xwayland child `Window` and starts an out-of-process `mpv --wid=` instance for that child window. -On Linux, `embedded_mpv.node` must not link directly to `libmpv` or load libmpv in-process. Electron loads its own `libffmpeg` and Chromium graphics stack; in-process libmpv can resolve FFmpeg/GL symbols against incompatible Electron symbols, while isolated dynamic-loader namespaces introduce thread/runtime ownership problems. The Linux addon therefore owns only the X11 child-window embedding, process lifecycle, and a private MPV JSON IPC socket. It starts `mpv --wid= --input-ipc-server=`, polls `time-pos`, `duration`, `volume`, and `pause`, and forwards pause/seek/volume/audio-track commands through that socket. A healthy Linux build lists X11/Xext as addon dependencies, but `ldd apps/electron-backend/native/build/Release/embedded_mpv.node` must not list `libmpv`. Runtime support also requires an `mpv` executable on `PATH`. +On Linux, `embedded_mpv.node` must not link directly to `libmpv` or load libmpv in-process. Electron loads its own `libffmpeg` and Chromium graphics stack; in-process libmpv can resolve FFmpeg/GL symbols against incompatible Electron symbols, while isolated dynamic-loader namespaces introduce thread/runtime ownership problems. The Linux addon therefore owns only the X11 child-window embedding, process lifecycle, and a private MPV JSON IPC socket. It starts `mpv --wid= --input-ipc-server=`, polls `time-pos`, `duration`, `volume`, and `pause`, and forwards pause/seek/volume/audio-track commands through that socket. The Linux MPV JSON IPC polling runs on an addon-owned background thread; `getSessionSnapshot()` returns the last cached snapshot and must not perform socket round trips on Electron's main thread. Linux MPV process teardown sends `SIGTERM` on the caller path, then waits and escalates to `SIGKILL` on a detached cleanup thread. A healthy Linux build lists X11/Xext as addon dependencies, but `ldd apps/electron-backend/native/build/Release/embedded_mpv.node` must not list `libmpv`. Runtime support also requires an `mpv` executable on `PATH`. Linux native Wayland embedding is not implemented. When Electron is started on Xwayland, the Linux backend also starts the child MPV process with `WAYLAND_DISPLAY` removed, `XDG_SESSION_TYPE=x11`, `--vo=gpu,x11`, and `--gpu-context=x11egl`. This prevents MPV from choosing a Wayland VO in a Wayland desktop session, which would ignore the X11 `--wid` target and open a separate top-level MPV window.