From 42f718568e6ffe63cad2fc3f62107daec2104472 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 5 May 2026 16:31:25 +0200 Subject: [PATCH] fix(embedded-mpv): tighten effect signal-tracking to prevent latent re-run bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit followup to the support-loop and volume-restart fixes. Two more effects pulled in transitive signal deps that would have caused the same class of regression the next time the surrounding helpers grew. 1. Component session-fan-out effect: the body called scheduleControlsHide(), which reads isPlaying/menus.anyOpen/statusLabel/ controlsVisible. Those became tracked deps of the effect, so opening a popover, pausing, or hovering re-ran the whole body — re-emitting timeUpdate. If a parent ever wires timeUpdate back into playback.startTime as a "resume where I left off" feature, this would have been the next stream-restart bug. Wrap the side-effect block in untracked() so the effect only listens to session changes. 2. Controller stalled-tracker effect: tracked the full session signal even though only status was needed. The session payload updates ~2 Hz during playback (positionSeconds advances), making the effect re-run constantly to call a no-op. Add a sessionStatus computed and track that instead — fires only on real status transitions. No behavior change for current users; both fixes are preventative. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: f957cd9849e0 --- .../embedded-mpv-player.component.ts | 16 +++++++++++----- .../embedded-mpv-session-controller.ts | 10 +++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-player.component.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-player.component.ts index e9001a79b..3c907b858 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-player.component.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-player.component.ts @@ -295,12 +295,18 @@ export class EmbeddedMpvPlayerComponent implements OnDestroy { if (!session) { return; } - this.volume.set(session.volume); - this.timeUpdate.emit({ - currentTime: session.positionSeconds, - duration: session.durationSeconds ?? 0, + // Side effects must not pull in transitive signal deps via + // scheduleControlsHide — otherwise opening a popover, pausing, or + // hovering would re-run this body and re-emit timeUpdate (which + // could feed back into playback inputs and restart the stream). + untracked(() => { + this.volume.set(session.volume); + this.timeUpdate.emit({ + currentTime: session.positionSeconds, + duration: session.durationSeconds ?? 0, + }); + this.scheduleControlsHide(); }); - this.scheduleControlsHide(); }); } diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts index 9a9757099..0038f45c5 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts @@ -1,6 +1,7 @@ import { DestroyRef, Injectable, + computed, effect, inject, signal, @@ -26,6 +27,10 @@ export class EmbeddedMpvSessionController { readonly stalled = signal(false); readonly retryToken = signal(0); + private readonly sessionStatus = computed( + () => this.session()?.status ?? null + ); + private readonly destroyRef = inject(DestroyRef); private readonly unsubscribeSessionUpdate?: () => void; @@ -54,8 +59,11 @@ export class EmbeddedMpvSessionController { }); } + // Track the narrowest possible signal — status only — so this effect + // does not re-run on every position-poll snapshot (~2 Hz during play) + // even though handleStalledTracking would be a no-op for those. effect(() => { - const status = this.session()?.status ?? null; + const status = this.sessionStatus(); untracked(() => this.handleStalledTracking(status)); });