fix(playback): wait for mpv's reply to every reused-instance IPC command (#1876)

Every reused-instance MPV IPC command now carries a request_id and waits, within the existing 2 s bound, for mpv's matching reply; an error or missing reply falls back to a fresh launch like a socket failure, and the racing post-loadfile seek is gone. MPV launches run one at a time through a launch queue so overlapping launches never act on the same child; Stop on a queued launch delegates to the in-flight launch's own closer, a failed exit stays retryable, and the dock keeps the newest launch. Position polling and output-error reporting moved out of mpv-session.service.ts, which leaves the max-lines baseline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5.5 authored and GitHub committed 2026-10-11 22:42:43 +02:00
1 parent 94570bc387
commit 4e9795ac54
21 files changed
+2920 -394

No files matched your search

+62 -2
View File
@@ -1029,6 +1029,61 @@ is also idempotent: it returns the closed snapshot without invoking the saved
closer, and a later process error cannot revive it as a visible failure. Reused
MPV commands use the socket captured for their exact child, so a delayed close
cannot send `quit` to a replacement process through a newer global socket.
Every reused MPV command carries a `request_id` and waits, within the same 2 s
IPC bound, for mpv's matching newline-delimited reply; event lines and replies
to other requests on that connection are skipped. An error reply or a missing
reply fails the handoff exactly like a socket error: the reused child is torn
down and a fresh launch follows. `loadfile` marks the content as possibly
changed and moves ownership of the child to the attempted session from the
moment it is written, whatever mpv replies, and stops the previous session's
position poll there, including a read already awaiting mpv, so an exit or a
stale Stop for the previous session during
the reply wait settles the right session and no poll can save the new stream's
position under the old content; a child that exited while its reply was in
flight is returned as the session its exit handler settled, never reported as
opened and never replaced by a fresh launch under that terminal session.
MPV launches run one at a time. A reused launch waits for mpv's replies and a
failed one for its teardown and fallback spawn, so a launch that arrives
meanwhile, such as a quick channel switch, starts only once the previous one
has settled and acts on the player that one left behind; two launches never
act on the same child. Each launch begins its session at once, so the newest
launch is the active session; the renderer lets a `launching` update take the
dock only when that launch did not start before the current one, by the
session's `launchOrder` (which separates launches in the same millisecond) or
else its `startedAt`, so an older launch still settling cannot pull the dock
back. Only the newest queued launch
runs: an older
one that comes up while a newer one waits behind it settles closed without
reaching mpv or taking the dock. Stop on a launch still waiting in the queue
cancels it and every launch queued before it; on the newest queued launch it
also closes, through their own closers, the in-flight launch and the session
owning the stored player at that moment (read from the per-child mapping, which
a failed attempt hands back to the previous owner during its teardown), so
their exact-child, retry and settle rules apply. Because launches run one at a
time, nothing else acts on a reused child while an attempt is in flight, so
that attempt's own Stop always reaches its child until the attempt settles,
whatever the per-child mapping names (the previous owner before `loadfile` or
during a failed attempt's teardown, possibly closed already); after it settles,
ownership decides as before, except that a Stop of that attempt which could
not confirm the exit keeps the right to retry that exact child. These rules apply
and nothing starts playing after Stop. Those targets are fixed at the first
Stop, so a retried Stop never reaches a player started later, and a late Stop
for an older queued launch cancels only that launch. The cancelled launch
settles only once its own Stop has: if the exit cannot be confirmed it stays a
closable error, so Stop can be retried. App shutdown cancels the whole queue.
A queued launch that finds the teardown gate closed, because the launch before
it could not confirm its child's exit, restores the session that still owns
that child, preferring the last launch that started (with reuse off its
detached child is not stored anywhere else); if that is not possible, it stays
a closable error so Stop can still tear the child down. Stopping the stored player, when reuse is disabled or
the app quits, cancels an in-flight reuse attempt: it dispatches no further
commands, never falls back to a fresh reusable player, also after waiting for
a failed child's exit, and settles its own session as closed once the child's
exit is confirmed, or as a closable error when it is not. No `seek` follows
`loadfile`: mpv rejects a seek before the file is loaded, so the resume offset
travels as the per-file `start` option. The `loadfile` reply only acknowledges
the command; a stream that later fails to open is reported through the reused
child's output and exit, not through the reply.
If Stop is observed before a pending MPV content command or VLC enqueue command
is dispatched, that command is skipped. A source handoff also fails closed
while a live session has no closer (`canClose: false`); renderer Dismiss is not
@@ -1142,8 +1197,13 @@ External MPV/VLC integration is split across focused main-process modules:
session tracking, trace logging, renderer notifications, playback-position
forwarding, and user-facing start errors.
- `apps/electron-backend/src/app/events/mpv-session.service.ts` owns fresh MPV
launches and progress polling; `mpv-reusable-process.ts` owns the tracked
child, captured socket, remapping, retryable close, and reuse handoff.
launches; `mpv-position-polling.ts` owns progress polling over the IPC socket
and `mpv-output-errors.ts` the child's stdout/stderr error reporting;
`mpv-reusable-process.ts` owns the tracked
child, captured socket, remapping, retryable close, and reuse handoff;
`mpv-ipc-command.ts` owns the bounded request/reply IPC command the reuse
handoff and its quit use; `mpv-launch-queue.ts` runs MPV launches one at a
time and owns queued-launch cancellation and Stop.
- `apps/electron-backend/src/app/events/vlc-session.service.ts` owns fresh VLC
launches and progress polling; `vlc-reusable-process.ts` owns the tracked
child, RC-port remapping, retryable close, and reuse handoff, while
+1 -1
View File
@@ -210,7 +210,7 @@ user who leaves that screen open pays them indefinitely, minimized included.
| Source | What it does | Period | Cost per firing | Justified | Evidence | Follow-up |
| --- | --- | --- | --- | --- | --- | --- |
| [mpv-session.service.ts:140–141](../../apps/electron-backend/src/app/events/mpv-session.service.ts) | External MPV position poll | 2 s delay, then 5 s | Two IPC-socket round-trips and one `playback-position-update` | Yes while playing. The early-exit leak found here (an unstored start-delay handle leaving an orphaned 5 s poll) is **resolved by #1720**. Still open: in reuse mode the poll keeps querying an idle MPV (no IPC while `time-pos` is null). | Static | Early-exit leak: done (#1720). Reuse mode: needs an idle signal from MPV before the poll can stop; not worth its own thread at two local socket calls per 5 s. |
| [mpv-position-polling.ts](../../apps/electron-backend/src/app/events/mpv-position-polling.ts) | External MPV position poll | 2 s delay, then 5 s | Two IPC-socket round-trips and one `playback-position-update` | Yes while playing. The early-exit leak found here (an unstored start-delay handle leaving an orphaned 5 s poll) is **resolved by #1720**. Still open: in reuse mode the poll keeps querying an idle MPV (no IPC while `time-pos` is null). | Static | Early-exit leak: done (#1720). Reuse mode: needs an idle signal from MPV before the poll can stop; not worth its own thread at two local socket calls per 5 s. |
| [vlc-session.service.ts:81–82](../../apps/electron-backend/src/app/events/vlc-session.service.ts) | External VLC position poll | 1.5 s delay, then 2 s | Up to three localhost TCP connections, then IPC | Yes while playing. The early-exit leak (orphaned 2 s poll) is **resolved by #1720**. | Static | Done (#1720). |
| [embedded-mpv-native.service.ts:1068](../../apps/electron-backend/src/app/services/embedded-mpv-native.service.ts) | Embedded MPV session snapshot poll | 500 ms while a session exists | Native snapshot, diff, and IPC only on change | Yes during playback. It is cleared when the last session closes. | Static | None |
| [embedded-mpv-session-controller.ts:197](../../libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts) | Renderer-side bounds poll for a native-view embedded MPV session: compares `getBoundingClientRect()` with the last synced bounds | 500 ms while a native-view session is open (skipped for frame-copy) | One layout read outside the Angular zone; on drift a rAF and a `setEmbeddedMpvBounds` IPC | Yes during playback: a position-only layout shift would otherwise leave the native view misplaced | Static | Cover with the throttling thread: it kept its rate while minimized under `backgroundThrottling: false`. |