From 671a0cb1ecc35bb3232c504eed99032ae445d41e Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 27 Sep 2026 12:56:23 +0200 Subject: [PATCH] fix(playback): confirm session-keyed history only by its own session A write deferred with a playback session key (M3U) is now confirmed only by that key. The app-wide MPV/VLC session confirmation carries just the URL, so opening the same stream externally from another playlist could still commit an abandoned attempt. An "Open in MPV/VLC" recovery launch is instead confirmed by the WebPlayerViewComponent that requested it, under its own session key, once the launch has opened. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/embedded-inline-playback.md | 16 ++++++---- .../video-player-recent-history.spec.ts | 16 ++++++++-- .../lib/playback-history-gate.service.spec.ts | 29 +++++++++++------ .../src/lib/playback-history-gate.service.ts | 15 +++++---- ...rnal-playback-recovery-coordinator.spec.ts | 32 +++++++++++++++++++ .../external-playback-recovery-coordinator.ts | 18 +++++++++-- .../web-player-view.component.ts | 12 +++++-- 7 files changed, 111 insertions(+), 27 deletions(-) diff --git a/docs/architecture/embedded-inline-playback.md b/docs/architecture/embedded-inline-playback.md index 3de409f16..6aac96903 100644 --- a/docs/architecture/embedded-inline-playback.md +++ b/docs/architecture/embedded-inline-playback.md @@ -1145,10 +1145,11 @@ reaches history. playlist when they defer, so navigating meanwhile cannot misfile it; an Xtream write confirmed after a playlist switch refreshes the store's recent list only if its playlist is still current. -- Matching: when both the write and the confirmation carry a session key, - only the session key is compared — the same URL in two playlists must not - let playback in one record a failed attempt in the other. Stream URLs are - the fallback when either side has none. +- Matching: a write deferred with a session key is confirmed only by that + same key — the same URL in two playlists must not let playback in one + (inline, or in MPV/VLC) record a failed attempt in the other. Writes + without one (portal resolvers, collection tabs) match any confirmation of + their stream URL. - `WebPlayerViewComponent` confirms its `playbackSessionKey`, `streamUrl` and `playback.streamUrl` once the owned engine's reported position has advanced by 2 seconds while playing (`PlaybackProgressConfirmation`). @@ -1161,8 +1162,11 @@ reaches history. when given). - MPV/VLC cannot report whether a live stream plays, so the Electron `ExternalPlaybackService` confirms a session's `streamUrl` once it is - `opened` or `playing`; a launch that ends in `error` is not recorded. M3U - keeps recording on selection when MPV/VLC is the configured player. + `opened` or `playing`; a launch that ends in `error` is not recorded. That + confirmation carries no session key, so an "Open in MPV/VLC" recovery + launch is also confirmed by the `WebPlayerViewComponent` that requested + it, under its own session key, once the launch has opened. M3U keeps + recording on selection when MPV/VLC is the configured player. - A confirmation commits every write matching it, once. Unconfirmed writes are bounded (oldest dropped) and simply never commit. The global live tab moves a confirmed row to the top of an open Recently Viewed list even if diff --git a/libs/playlist/m3u/feature-player/src/lib/video-player/video-player-recent-history.spec.ts b/libs/playlist/m3u/feature-player/src/lib/video-player/video-player-recent-history.spec.ts index d697236a1..02f2662c8 100644 --- a/libs/playlist/m3u/feature-player/src/lib/video-player/video-player-recent-history.spec.ts +++ b/libs/playlist/m3u/feature-player/src/lib/video-player/video-player-recent-history.spec.ts @@ -215,7 +215,10 @@ describe('VideoPlayerComponent — recently viewed history', () => { name: 'Second TV', }); - gate.confirm({ streamUrls: ['http://localhost/second.m3u8'] }); + gate.confirm({ + sessionKey: component.playbackSessionKey(), + streamUrls: ['http://localhost/second.m3u8'], + }); expect(playlistsServiceMock.addM3uRecentlyViewed).toHaveBeenCalledTimes( 1 @@ -226,7 +229,7 @@ describe('VideoPlayerComponent — recently viewed history', () => { ); }); - it('records a radio station once the audio player confirms its URL', () => { + it('records a radio station once the audio player confirms it', () => { const radio = { ...sampleChannel, radio: 'true' }; select(radio); @@ -234,7 +237,16 @@ describe('VideoPlayerComponent — recently viewed history', () => { playlistsServiceMock.addM3uRecentlyViewed ).not.toHaveBeenCalled(); + // The same URL opened elsewhere (MPV/VLC, another playlist) does not. gate.confirm({ streamUrls: [radio.url] }); + expect( + playlistsServiceMock.addM3uRecentlyViewed + ).not.toHaveBeenCalled(); + + gate.confirm({ + sessionKey: component.playbackSessionKey(), + streamUrls: [radio.url], + }); expect(playlistsServiceMock.addM3uRecentlyViewed).toHaveBeenCalledTimes( 1 diff --git a/libs/services/src/lib/playback-history-gate.service.spec.ts b/libs/services/src/lib/playback-history-gate.service.spec.ts index c599b2a86..4bcb84281 100644 --- a/libs/services/src/lib/playback-history-gate.service.spec.ts +++ b/libs/services/src/lib/playback-history-gate.service.spec.ts @@ -55,24 +55,35 @@ describe('PlaybackHistoryGate', () => { expect(playedInB).toHaveBeenCalledTimes(1); }); - it('falls back to the stream URL when either side has no session key', () => { - const portalWrite = jest.fn(); - const radioWrite = jest.fn(); - gate.defer({ streamUrls: ['http://portal/tmp'] }, portalWrite); + it('does not let a URL-only confirmation commit a write deferred with a session key', () => { + // Playlist A's attempt failed; the same URL then opens in MPV/VLC, + // whose app-wide session confirmation carries no session key. + const failedInA = jest.fn(); gate.defer( - { sessionKey: 'live:p1:radio', streamUrls: ['http://radio/1'] }, - radioWrite + { sessionKey: 'live:a:c1', streamUrls: ['http://shared/1'] }, + failedInA ); - // A player with a session key; a player (MPV/VLC session) without. + gate.confirm({ streamUrls: ['http://shared/1'] }); + + expect(failedInA).not.toHaveBeenCalled(); + }); + + it('matches writes without a session key by stream URL', () => { + const portalWrite = jest.fn(); + const externalWrite = jest.fn(); + gate.defer({ streamUrls: ['http://portal/tmp'] }, portalWrite); + gate.defer({ streamUrls: ['http://portal/vod'] }, externalWrite); + + // An inline player (with its own key) and an MPV/VLC session. gate.confirm({ sessionKey: 'live:portal:9', streamUrls: ['http://portal/tmp'], }); - gate.confirm({ streamUrls: ['http://radio/1'] }); + gate.confirm({ streamUrls: ['http://portal/vod'] }); expect(portalWrite).toHaveBeenCalledTimes(1); - expect(radioWrite).toHaveBeenCalledTimes(1); + expect(externalWrite).toHaveBeenCalledTimes(1); }); it('commits each confirmed write once', () => { diff --git a/libs/services/src/lib/playback-history-gate.service.ts b/libs/services/src/lib/playback-history-gate.service.ts index 0df771504..e053a3f27 100644 --- a/libs/services/src/lib/playback-history-gate.service.ts +++ b/libs/services/src/lib/playback-history-gate.service.ts @@ -4,10 +4,10 @@ import { Injectable } from '@angular/core'; export interface PlaybackHistoryTarget { /** * The playing host's `playbackSessionKey` (source and content scoped). - * When both sides carry one, it is the only thing that is compared. + * A write deferred with one is only confirmed by that same key. */ readonly sessionKey?: string | null; - /** Stream URLs; the fallback when either side has no session key. */ + /** Stream URLs; what a write without a session key is matched by. */ readonly streamUrls?: readonly (string | null | undefined)[]; } @@ -39,9 +39,12 @@ const MAX_PENDING_HISTORY_WRITES = 20; * launched. A stream that fails before that point never reaches history or * the dashboard hero. * - * A session key is the stronger correlation: the same stream URL can sit in - * two playlists, and playing it in one must not record a failed attempt in - * the other. Stream URLs only match when either side has no session key. + * A write deferred with a session key is only confirmed by the same key: the + * same stream URL can sit in two playlists, and playing it in one — inline + * or in MPV/VLC, whose app-wide confirmation knows only the URL — must not + * record a failed attempt in the other. Writers that cannot know the + * playing host's key (portal resolvers, collection tabs) defer by stream + * URL, which any confirmation of that URL matches. * Several writers may defer for the same playback; one confirmation commits * all of them. A write with nothing to match on cannot be confirmed and is * committed immediately, as before this gate existed. @@ -81,7 +84,7 @@ function matchesTarget( write: NormalizedTarget, confirmed: NormalizedTarget ): boolean { - if (write.sessionKey && confirmed.sessionKey) { + if (write.sessionKey) { return write.sessionKey === confirmed.sessionKey; } return [...write.streamUrls].some((url) => confirmed.streamUrls.has(url)); diff --git a/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.spec.ts b/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.spec.ts index ab005aa84..1593036e2 100644 --- a/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.spec.ts +++ b/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.spec.ts @@ -83,6 +83,38 @@ describe('ExternalPlaybackRecoveryCoordinator', () => { coordinator.destroy(); }); + it.each([ + ['an opened launch', session({ status: 'opened' }), 1], + ['a launch that failed', session({ status: 'error' }), 0], + ['a launch of another player', session({ player: 'vlc' }), 0], + ])( + 'reports %s to the view', + async (_label, launched: ExternalPlayerSession, calls: number) => { + const activeSession = signal(null); + const onLaunched = jest.fn(); + const coordinator = new ExternalPlaybackRecoveryCoordinator( + { + activeSession, + visibleSession: activeSession, + closeSession: jest.fn(), + dismissActiveSession: jest.fn(), + }, + onLaunched + ); + coordinator.syncSession('content-a'); + + coordinator.request('mpv', jest.fn(), (trackLaunch) => { + trackLaunch(Promise.resolve(launched)); + return true; + }); + await Promise.resolve(); + await Promise.resolve(); + + expect(onLaunched).toHaveBeenCalledTimes(calls); + coordinator.destroy(); + } + ); + it('closes a closable error before retrying another external target', async () => { const previous = session({ id: 'uncertain-process', diff --git a/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.ts b/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.ts index ad9ab6f14..53f1a7eee 100644 --- a/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.ts +++ b/libs/ui/playback/src/lib/web-player-view/external-playback-recovery-coordinator.ts @@ -21,8 +21,16 @@ export class ExternalPlaybackRecoveryCoordinator { readonly states = this.recovery.states; readonly pending = this.recovery.pending; + /** + * @param onLaunched runs once a launch this view requested has opened in + * MPV/VLC — still owned by the current intent — e.g. to record it as + * recently viewed under the view's own playback session. + */ constructor( - private readonly externalPlayback: PortalExternalPlayback | null + private readonly externalPlayback: PortalExternalPlayback | null, + private readonly onLaunched: ( + session: ExternalPlayerSession + ) => void = () => undefined ) {} observe(session: ExternalPlayerSession | null): void { @@ -93,7 +101,13 @@ export class ExternalPlaybackRecoveryCoordinator { void launch.then( (session) => { if (session) { - this.recovery.confirm(intent, session); + if ( + this.recovery.confirm(intent, session) && + (session.status === 'opened' || + session.status === 'playing') + ) { + this.onLaunched(session); + } } else { this.recovery.fail(intent); } diff --git a/libs/ui/playback/src/lib/web-player-view/web-player-view.component.ts b/libs/ui/playback/src/lib/web-player-view/web-player-view.component.ts index 597be7455..b9dc67db3 100644 --- a/libs/ui/playback/src/lib/web-player-view/web-player-view.component.ts +++ b/libs/ui/playback/src/lib/web-player-view/web-player-view.component.ts @@ -122,8 +122,16 @@ export class WebPlayerViewComponent implements OnDestroy { optional: true, }); private readonly recoverySession = new PlaybackRecoverySession(); + private readonly historyGate = inject(PlaybackHistoryGate); private readonly externalRecovery = new ExternalPlaybackRecoveryCoordinator( - this.externalPlayback + this.externalPlayback, + // "Open in MPV/VLC" after an inline failure is still this session's + // view; the app-wide session confirmation carries only the URL. + (session) => + this.historyGate.confirm({ + sessionKey: this.playbackSessionKey(), + streamUrls: [session.streamUrl], + }) ); private readonly applicationHandoff = new WebPlayerApplicationHandoffCoordinator( @@ -221,7 +229,7 @@ export class WebPlayerViewComponent implements OnDestroy { readonly playbackApplicationToken = this.applicationState.token; /** Commits deferred "recently viewed" writes once this stream plays. */ private readonly historyConfirmation = new PlaybackHistoryConfirmation({ - gate: inject(PlaybackHistoryGate), + gate: this.historyGate, target: () => ({ sessionKey: this.playbackSessionKey(), streamUrls: [this.streamUrl(), this.playback()?.streamUrl],