From bc052804d9e7f786a4b95b7f679f93553ddf8f9e Mon Sep 17 00:00:00 2001 From: 4gray Date: Sat, 3 Oct 2026 17:26:59 +0200 Subject: [PATCH] fix(portals): the movie menu's MPV/VLC launch honours the pinned copy The menu resolved the route's copy while reading the resume flag of the copy the primary button acts on, so with a pin it could launch a different source from Play and Start over and apply the wrong resume decision. The launch is host-owned now: a pinned copy plays through the pinned-source orchestration with the forced player and its own resume point, otherwise the route copy starts from its own position. Co-Authored-By: Claude Fable 5.1 --- .../vod-details-menu.service.spec.ts | 9 ++++ .../vod-details/vod-details-menu.service.ts | 12 ++--- .../vod-details-playback.service.spec.ts | 5 +- .../vod-details-playback.service.ts | 31 ++++------- .../vod-details-route-playback.spec.ts | 34 +++++++++++++ .../vod-details-route.component.ts | 51 ++++++++++++++++--- .../vod-multi-source-host-pin.spec.ts | 21 ++++++++ .../vod-multi-source-host-races.spec.ts | 3 +- .../vod-multi-source-host.service.spec.ts | 4 +- .../vod-multi-source-host.service.ts | 28 ++++++---- 10 files changed, 149 insertions(+), 49 deletions(-) diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.spec.ts index f3b3a4b55..dcc6168aa 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.spec.ts @@ -22,6 +22,7 @@ describe('VodDetailsMenuService', () => { const playbackStartPending = signal(false); const isExternalLaunchPending = signal(false); const hasPlaybackPosition = signal(true); + const openExternal = jest.fn().mockResolvedValue(undefined); let service: VodDetailsMenuService; beforeEach(() => { @@ -75,6 +76,7 @@ describe('VodDetailsMenuService', () => { vodId: signal(7), category: signal(null), restart: jest.fn(), + openExternal, }); }); @@ -90,6 +92,13 @@ describe('VodDetailsMenuService', () => { expect(row(VOD_MENU_ACTION.StartOver)?.disabled).toBeFalsy(); }); + it('hands the external launch to the host with the configured player', async () => { + // The host honours a pinned copy and its resume point; the menu + // must not resolve the route copy on its own. + await service.run(VOD_MENU_ACTION.ExternalPlayer); + expect(openExternal).toHaveBeenCalledWith('mpv'); + }); + it('holds both rows while a start resolves or a launch awaits the player', () => { // Another start would be refused while one is in flight: an enabled // row would close the menu and do nothing. diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.ts index 32a6906ee..94ecd5098 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-menu.service.ts @@ -34,6 +34,8 @@ interface VodDetailsMenuBindings { readonly category: Signal | null>; /** Start over honours a pinned copy, so the host owns it. */ readonly restart: () => Promise; + /** The MPV/VLC launch honours the pinned copy and its resume point too. */ + readonly openExternal: (player: ExternalPlayerName) => Promise; } /** @@ -142,15 +144,7 @@ export class VodDetailsMenuService { const item = this.bindings()?.item() ?? null; switch (actionId) { case VOD_MENU_ACTION.ExternalPlayer: - await this.playback - .openInExternalPlayer( - item, - this.msUi.hasPlaybackPosition(), - this.externalPlayer() - ) - .catch((error) => - this.logger.warn('External launch failed', error) - ); + await this.bindings()?.openExternal(this.externalPlayer()); return; case VOD_MENU_ACTION.CopyUrl: await this.copyStreamUrl(item); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts index 44b11ff66..d60edcd25 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.spec.ts @@ -172,7 +172,7 @@ describe('VodDetailsPlaybackService — external session ownership', () => { container_extension: 'mkv', }, } as never; - const launch = service.openInExternalPlayer(movie, false, 'vlc'); + const launch = service.playVod(movie, 'vlc'); expect(launch).not.toBeNull(); await launch; @@ -205,7 +205,7 @@ describe('VodDetailsPlaybackService — external session ownership', () => { closeSession.mockClear(); openExternalPlayback.mockClear(); - await service.openInExternalPlayer( + await service.playVod( { info: {}, movie_data: { @@ -214,7 +214,6 @@ describe('VodDetailsPlaybackService — external session ownership', () => { container_extension: 'mkv', }, } as never, - false, 'mpv' ); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts index a2becf9fa..063f032ed 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-playback.service.ts @@ -241,14 +241,21 @@ export class VodDetailsPlaybackService { }; } - async playVod(vodItem: XtreamVodDetails | null): Promise { + /** `player` forces MPV/VLC (the "…" menu); the start is route-owned either way. */ + async playVod( + vodItem: XtreamVodDetails | null, + player?: ExternalPlayerName + ): Promise { const playback = this.buildVodPlayback(vodItem, false); - return playback ? await this.startPlayback(playback) : false; + return playback ? await this.startPlayback(playback, player) : false; } - async resumeVod(vodItem: XtreamVodDetails | null): Promise { + async resumeVod( + vodItem: XtreamVodDetails | null, + player?: ExternalPlayerName + ): Promise { const playback = this.buildVodPlayback(vodItem, true); - return playback ? await this.startPlayback(playback) : false; + return playback ? await this.startPlayback(playback, player) : false; } onPrimaryAction(vodItem: XtreamVodDetails | null): void { @@ -308,22 +315,6 @@ export class VodDetailsPlaybackService { ); } - /** - * The "…" menu's explicit MPV/VLC launch: a regular route-owned start - * with the player forced, so the view is recorded and a running - * session is replaced rather than doubled. - */ - openInExternalPlayer( - vodItem: XtreamVodDetails | null, - resume: boolean, - player: ExternalPlayerName - ): Promise { - const playback = this.buildVodPlayback(vodItem, resume); - return playback - ? this.startPlayback(playback, player) - : Promise.resolve(false); - } - /** * Hands the playback to MPV or VLC regardless of the configured player * (the inline player's fallback), owned and settled like a regular diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts index 6ff66dd1c..2126817e1 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts @@ -727,6 +727,40 @@ describe('VodDetailsRouteComponent — playback actions', () => { ).resolves.toBe(2538); }); + it('launches the pinned copy, from its own resume point, in the forced player', async () => { + currentPlaylist.set({ id: 'playlist-1' }); + const component = fixture.componentInstance; + withActiveSource('playlist-1', 650020); + Object.defineProperty(component['msUi'], 'primaryIsPinnedCopy', { + configurable: true, + value: () => true, + }); + const pinnedPlay = jest + .spyOn(component.multiSource, 'playPinnedSource') + .mockResolvedValue('played'); + const routePlay = jest.spyOn(component, 'playVod'); + + await component.openInExternalPlayer( + { + movie_data: { + stream_id: 650020, + name: 'Example', + container_extension: 'mp4', + }, + } as never, + 'mpv' + ); + + // The menu's launch and the primary button must agree on the copy + // and on where it resumes: the route copy's own position says + // nothing about the pinned one. + expect(pinnedPlay).toHaveBeenCalledWith( + component['msUi'].resumeSecondsFor, + 'mpv' + ); + expect(routePlay).not.toHaveBeenCalled(); + }); + it('restarts the pinned copy, not the route copy', async () => { currentPlaylist.set({ id: 'playlist-1' }); const component = fixture.componentInstance; diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts index 5d1e2f0c8..ff8c228d2 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts @@ -65,6 +65,7 @@ import { XtreamVodInfo, XtreamVodStream, youtubeEmbedUrl, + type ExternalPlayerName, type PlaybackPositionData, type VodSourceCandidate, type VodSourceDescriptor, @@ -454,10 +455,11 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { this.multiSource.bind({ // Route every switch through the same inline-vs-external fork a // normal Play uses, so the two paths cannot drift apart. - startPlayback: async (playback, isCurrent) => { + startPlayback: async (playback, isCurrent, player) => { const started = await this.playback.startResolvedPlayback( playback, - isCurrent + isCurrent, + player ); if (started) { // A switch mounts a DIFFERENT stream in the same host, so @@ -511,6 +513,8 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { vodId: this.selectedVodId, category: this.selectedCategory, restart: () => this.restartVod(this.playableVodItem()), + openExternal: (player) => + this.openInExternalPlayer(this.playableVodItem(), player), }); registerContentMetadataBackfill({ @@ -606,9 +610,12 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { this.xtreamStore.setSelectedItem(null); } - async playVod(vodItem: XtreamVodDetails | null): Promise { + async playVod( + vodItem: XtreamVodDetails | null, + player?: ExternalPlayerName + ): Promise { this.multiSource.supersedePendingSwitch(); - const started = await this.playback.playVod(vodItem); + const started = await this.playback.playVod(vodItem, player); if (!started) { return false; } @@ -646,9 +653,12 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { await this.playVod(vodItem); } - async resumeVod(vodItem: XtreamVodDetails | null): Promise { + async resumeVod( + vodItem: XtreamVodDetails | null, + player?: ExternalPlayerName + ): Promise { this.multiSource.supersedePendingSwitch(); - const started = await this.playback.resumeVod(vodItem); + const started = await this.playback.resumeVod(vodItem, player); if (!started) { return false; } @@ -689,6 +699,35 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { await this.playFromProviderSource(vodItem); } + /** + * The "…" menu's MPV/VLC launch: the copy the primary button acts on, + * from where it would resume. A pinned copy outranks the route's, as it + * does for Play and Restart, so the launch and the button never disagree + * about the source or the position. + */ + async openInExternalPlayer( + vodItem: XtreamVodDetails | null, + player: ExternalPlayerName + ): Promise { + if (this.isExternalLaunchPending()) { + return; + } + if (this.msUi.primaryIsPinnedCopy()) { + const outcome = await this.multiSource.playPinnedSource( + this.msUi.resumeSecondsFor, + player + ); + if (outcome !== 'unavailable') { + return; + } + } + if (this.playback.hasPlaybackPosition()) { + await this.resumeVod(vodItem, player); + return; + } + await this.playVod(vodItem, player); + } + async playFromProviderSource( vodItem: XtreamVodDetails | null ): Promise { diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts index d6b0c1f83..d629311b5 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-pin.spec.ts @@ -48,6 +48,27 @@ describe('VodMultiSourceHostService — pinning', () => { expect(rowFor(ALT_TWO.id)?.isActive).toBe(true); }); + it('launches the pinned copy in the forced player', async () => { + pins.get.mockResolvedValue({ + matchKey: 'title:the matrix:1999', + playlistId: ALT_TWO.playlistId, + contentId: ALT_TWO.contentId, + portalType: 'xtream', + }); + await loadMovie([ALT_TWO]); + + // The "…" menu's MPV/VLC launch honours the pin like Play does, so + // the two never start different copies of the film. + await expect(service.playPinnedSource(undefined, 'vlc')).resolves.toBe( + 'played' + ); + expect(startPlayback).toHaveBeenCalledWith( + expect.anything(), + expect.any(Function), + 'vlc' + ); + }); + it('resumes the pinned source from the stored position', async () => { pins.get.mockResolvedValue({ matchKey: 'title:the matrix:1999', diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts index 11140efca..529904ffb 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts @@ -138,7 +138,8 @@ describe('VodMultiSourceHostService — stale resolutions', () => { expect.objectContaining({ streamUrl: expect.stringContaining(String(ALT_THREE.contentId)), }), - expect.any(Function) + expect.any(Function), + undefined ); }); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.spec.ts index 9226b9865..e80072523 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.spec.ts @@ -175,9 +175,11 @@ describe('VodMultiSourceHostService', () => { { startTime: 2538 } ); expect(startPlayback).toHaveBeenCalledTimes(1); + // No forced player: the host picks inline or external itself. expect(startPlayback).toHaveBeenCalledWith( expect.objectContaining({ startTime: 2538 }), - expect.any(Function) + expect.any(Function), + undefined ); }); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts index c787cd0c9..ab914a8a7 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts @@ -50,6 +50,7 @@ import { type PinKeySets, } from './vod-multi-source-pin'; import { + type ExternalPlayerName, type ResolvedPortalPlayback, type VodSourceCandidate, type VodSourceDescriptor, @@ -66,12 +67,14 @@ import { export interface VodMultiSourceBindings { /** - * Applies a playback — inline swap or external launch, host's choice. - * False leaves the controller on its current source. + * Applies a playback — inline swap or external launch, host's choice, + * unless `player` forces MPV/VLC. False leaves the controller on its + * current source. */ startPlayback: ( playback: ResolvedPortalPlayback, - isCurrent: () => boolean + isCurrent: () => boolean, + player?: ExternalPlayerName ) => Promise; /** The movie on screen, or null while its identity is not yet knowable. */ movie: Signal; @@ -382,7 +385,8 @@ export class VodMultiSourceHostService { * charge; a superseded attempt must NOT fall through that way. */ playPinnedSource( - resumeFor?: (source: VodSourceCandidate) => Promise + resumeFor?: (source: VodSourceCandidate) => Promise, + player?: ExternalPlayerName ): Promise { const session = this.sessionToken; // Claim a switch generation up front. The discovery wait and the @@ -396,7 +400,7 @@ export class VodMultiSourceHostService { pinnedSourceId: () => this.pendingPinnedSourceId(), resumeFor, isCurrent: () => this.isCurrentSwitch(session, attempt), - play: (sourceId) => this.runPlay(sourceId), + play: (sourceId) => this.runPlay(sourceId, player), }); } @@ -406,7 +410,10 @@ export class VodMultiSourceHostService { } /** As `play`, but keeping the distinction the pinned path needs. */ - private async runPlay(sourceId: string): Promise { + private async runPlay( + sourceId: string, + player?: ExternalPlayerName + ): Promise { const candidate = this.controller.findSource(sourceId); if (!candidate || !this.bindings) { return 'unavailable'; @@ -414,7 +421,7 @@ export class VodMultiSourceHostService { this._busySourceId.set(sourceId); try { - return toPinnedOutcome(await this.switchTo(candidate)); + return toPinnedOutcome(await this.switchTo(candidate, player)); } finally { // Only while this attempt still owns the spinner, or a slower // pick would clear the row that is still resolving. @@ -559,7 +566,10 @@ export class VodMultiSourceHostService { this.controller.seedResumeSeconds(seconds); } - private switchTo(candidate: VodSourceCandidate): Promise { + private switchTo( + candidate: VodSourceCandidate, + player?: ExternalPlayerName + ): Promise { const bindings = this.bindings; if (!bindings || bindings.playbackStartBlocked()) { return Promise.resolve('superseded'); @@ -573,7 +583,7 @@ export class VodMultiSourceHostService { resolve: (target, options) => this.resolver.resolve(target, options), startPlayback: (playback, isCurrent) => - bindings.startPlayback(playback, isCurrent), + bindings.startPlayback(playback, isCurrent, player), isCurrent: () => this.isCurrentSwitch(session, attempt), setPreviousSource: (id) => this._previousSourceId.set(id), setNotice: (notice) => this._lastSwitch.set(notice),