From 7cce2272682c35f9f6a8e6a7ba33f06a2ebacfa6 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 28 Jul 2026 18:40:08 +0200 Subject: [PATCH] fix(portals): do not spend a source's failover turn on mere selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setActiveSource` marked the source tried, but discovery calls it the moment the page opens and a pin or the picker can call it before anything plays. So opening a movie burned the route copy's turn: if a pinned alternative then failed, failover skipped a healthy untouched source — and with only one alternative, reported the options exhausted. Selection and attempt are now separate. `setActiveSource` selects; `markPlaying` also spends the turn, and only the three places that really start playback call it. `runFailover` additionally retires whatever is on screen before picking, so the failing source is spent however it got there — relying on the start paths alone would leave one hole per path, and the cost of missing it is a ping-pong between two sources. One existing spec asserted the old behaviour (a route copy burned by a switch it never played); it now plays first, so it still covers what it meant to — that the tried set survives a rediscovery. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 2 +- docs/architecture/vod-multi-source.md | 11 ++++++ .../vod-multi-source.controller.spec.ts | 39 +++++++++++++++---- .../vod-multi-source.controller.ts | 18 +++++++-- .../vod-multi-source-host-session.spec.ts | 20 ++++++++++ .../vod-multi-source-host.service.ts | 2 +- .../vod-details/vod-multi-source-session.ts | 13 ++++++- 7 files changed, 91 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c32b70dd6..98532d606 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -838,7 +838,7 @@ engine` (restart required) or - Switching = one `inlinePlayback.set({...next, startTime})`, never null-then-set, so the player and engine survive and re-seek. The carried position is read *before* the 15s persistence throttle, and `VodDetailsPlaybackService` uses a one-shot `resumeSettled` latch so a resuming engine's `timeupdate` at ~0 cannot overwrite the resume point. `handleInlineTimeUpdate` returns that verdict and the route feeds multi-source the requested `startTime` until the engine reaches it — one latch for both, or a switch during the initial seek would restart the film. Before anything plays there is no live position at all, so the controller is seeded from the persisted one (`seedResumeSeconds`, one-way: a live value always wins). Portal failures in the multi-source path log through the redacting `createLogger`/`redactSensitiveData` — an Xtream error message carries the stream URL, and that URL is built out of the username and password. - Pins are keyed portal-agnostically (`tmdb:{id}` else `title:{base}:{year}` else the yearless `title:{base}:`, `vod_source_pins` table); enrichment supplies the id and the year late, so a pin may sit under any poorer form — three key sets (`pinKeysFor`): `lookup` passes every alias most-trusted-first, `write` holds only keys naming exactly one film, and `loaded` records where the pin on screen was found — the yearless alias is readable but never written or deleted on spec, since it is shared by every remake, with the single exception of the row this session actually read. A pin is not decoration: the primary Play action starts from the pinned source (except when that button reads Stop — an active external session wins, or the control would launch a second player), and it outranks everything else in failover ranking. The row changes only after the write lands, so a refused pin is never shown as saved. Starting a pinned source loads THAT source's own playback position — progress is keyed by (playlist, stream), so the row the page loaded belongs to the route's copy. An external player launched for an alternative carries the OTHER playlist's ids, so `VodDetailsPlaybackBindings.activeSource` feeds one `ownsContent()` predicate used by BOTH the session matcher and the playback-position bridge — if they disagree, the page shows Stop for a session whose progress it throws away and a later switch rewinds hours. Two identity keys: `vodMultiSourceMovieKey` (title, year, tmdbId) makes TMDB enrichment re-trigger discovery and rebuild the pin keys, while `vodMultiSourceSessionKey` (`playlistId:contentId`) decides whether that rerun is a refresh or a new session — a refresh keeps the active source, its resolved facts, the tried set, the live position and any switch in flight; only a different film resets them. - Claims in the present tense (the "Playing from" caption and the source row's `Playing` badge) are gated on `VodDetailsRouteComponent.playbackLive`, never on `isActive` — discovery marks a source active before anything plays and it stays active after the player closes. Inline that means a `timeupdate` has arrived (`inlinePlayback()` is only the request to play); external it means the session is past `launching`. A merely selected row reads `Current`. -- Auto-failover is `Settings.vodAutoFailover`, **opt-in and off by default**, web engines only — the toggle is hidden in settings and in the sources menu on MPV, VLC and Embedded MPV, since only the built-in web players raise the playback diagnostic that triggers it (`reportsPlaybackFailures()`); it awaits a discovery still in flight before concluding there is nowhere to go (a stream can fail faster than SQLite answers) and re-checks the session afterwards, since the user can navigate during that wait; pinned Play takes the same guarded wait. Each source is tried at most once per session (`triedSourceIds` only grows), so it terminates structurally, and it continues past candidates that fail to resolve rather than stopping at the first one — `switchTo` reports whether it was unresolvable (keep going) or superseded (stop), since only the former marks the candidate tried. The switch is never silent: the toast names the new playlist (through `playlistDisplayLabel`, since a stored playlist name is routinely the pasted URL with credentials), offers Undo, and warns "dub may differ" only when both sides state an audio track as fact. +- Auto-failover is `Settings.vodAutoFailover`, **opt-in and off by default**, web engines only — the toggle is hidden in settings and in the sources menu on MPV, VLC and Embedded MPV, since only the built-in web players raise the playback diagnostic that triggers it (`reportsPlaybackFailures()`); it awaits a discovery still in flight before concluding there is nowhere to go (a stream can fail faster than SQLite answers) and re-checks the session afterwards, since the user can navigate during that wait; pinned Play takes the same guarded wait. Each source is tried at most once per session (`triedSourceIds` only grows), so it terminates structurally — but SELECTION is not an attempt: `setActiveSource` only selects, `markPlaying` spends the turn, and `runFailover` retires whatever is on screen before picking, so discovery selecting the route row (or a pin selecting an alternative) before anything plays cannot burn a healthy fallback; and it continues past candidates that fail to resolve rather than stopping at the first one — `switchTo` reports whether it was unresolvable (keep going) or superseded (stop), since only the former marks the candidate tried. The switch is never silent: the toast names the new playlist (through `playlistDisplayLabel`, since a stored playlist name is routinely the pasted URL with credentials), offers Undo, and warns "dub may differ" only when both sides state an audio track as fact. - HEAD probe reuses the main-process handler extracted to `apps/electron-backend/src/app/events/stream-probe.ts` (`STREAM_PROBE_URL`; `XTREAM_PROBE_URL` still delegates there for catchup), and carries the playlist's own `userAgent`/`referer`/`origin` (`StreamProbeHeaders`) — a panel that requires them answers 401/403 otherwise and a working source would be shown as dead. No ffprobe — the binary is not bundled. - See `docs/architecture/vod-multi-source.md` diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index 75d3a2ae5..fdaf2c1a7 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -338,6 +338,17 @@ Termination is structural: `triedSourceIds` only ever grows within a session, so an N-source movie fails over at most N−1 times and then shows the honest error screen. Returning to an earlier source by hand does not clear the set. +Selection is not an attempt. `setActiveSource` only selects; `markPlaying` also +spends the source's turn, and only the three places that really start playback +call it — a switch, the route's own Play/Resume, and restoring the playing row +after a rediscovery. Discovery selects the route's row the moment the page +opens, and a pin or the picker can select an alternative before anything plays; +counting those would let a later failure skip a healthy fallback, or call the +options exhausted with one untouched. `runFailover` then retires whatever is on +screen before picking, so the source that just failed is spent however it got +there — one hole per start path would be an infinite ping-pong between two +sources. + A source that cannot even be resolved is marked tried without becoming active, and failover **continues to the next candidate** rather than giving up — production calls `failover()` only once, on the original failure, so stopping at diff --git a/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.spec.ts b/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.spec.ts index 1a11673d5..e264891fd 100644 --- a/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.spec.ts +++ b/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.spec.ts @@ -47,11 +47,11 @@ describe('VodMultiSourceController', () => { describe('failover cannot loop', () => { it('never returns a source that was already tried', () => { const controller = controllerWith('a', 'b', 'c'); - controller.setActiveSource('a'); + controller.markPlaying('a'); const first = controller.pickFailoverTarget(); expect(first).not.toBeNull(); - controller.setActiveSource(first!.id); + controller.markPlaying(first!.id); const second = controller.pickFailoverTarget(); expect(second!.id).not.toBe(first!.id); @@ -60,7 +60,7 @@ describe('VodMultiSourceController', () => { it('exhausts after each source has been tried exactly once', () => { const controller = controllerWith('a', 'b', 'c'); - controller.setActiveSource('a'); + controller.markPlaying('a'); const visited = ['a']; for (let hop = 0; hop < 5; hop++) { @@ -70,7 +70,7 @@ describe('VodMultiSourceController', () => { } expect(visited).not.toContain(next.id); visited.push(next.id); - controller.setActiveSource(next.id); + controller.markPlaying(next.id); } // Three sources, three visits, then an honest dead end. @@ -81,13 +81,38 @@ describe('VodMultiSourceController', () => { it('stays exhausted even if the user returns to an earlier source', () => { const controller = controllerWith('a', 'b'); - controller.setActiveSource('a'); - controller.setActiveSource('b'); - controller.setActiveSource('a'); + controller.markPlaying('a'); + controller.markPlaying('b'); + controller.markPlaying('a'); // Returning by hand is allowed; automatic retrying is not. expect(controller.pickFailoverTarget()).toBeNull(); }); + + describe('selection is not an attempt', () => { + it('keeps the route source available after discovery selects it', () => { + // Discovery marks the route's row active the moment the page + // opens. Spending its turn there means a failure on some other + // source skips a healthy fallback. + const controller = controllerWith('a', 'b'); + controller.setActiveSource('a'); + + controller.markPlaying('b'); + + expect(controller.pickFailoverTarget()?.id).toBe('a'); + expect(controller.isExhausted()).toBe(false); + }); + + it('still exhausts once every source has actually played', () => { + const controller = controllerWith('a', 'b'); + controller.setActiveSource('a'); + + controller.markPlaying('b'); + controller.markPlaying('a'); + + expect(controller.isExhausted()).toBe(true); + }); + }); }); describe('failover ranking', () => { diff --git a/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.ts b/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.ts index 64e9e8c6d..85eec4d92 100644 --- a/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.ts +++ b/libs/portal/shared/data-access/src/lib/multi-source/vod-multi-source.controller.ts @@ -84,11 +84,23 @@ export class VodMultiSourceController { ); } + /** + * Select a source WITHOUT spending its failover turn. + * + * Discovery selects the route's row the moment the page opens, and a pin + * or the picker can select an alternative before anything plays. None of + * those is an attempt: counting them means a later failure finds the + * route source "tried" and skips a perfectly healthy fallback — or calls + * the options exhausted while one is untouched. + */ setActiveSource(sourceId: string | null) { this._activeSourceId.set(sourceId); - if (sourceId) { - this.tried.add(sourceId); - } + } + + /** Select a source AND spend its turn — playback is actually starting. */ + markPlaying(sourceId: string) { + this.markTried(sourceId); + this.setActiveSource(sourceId); } /** diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts index 482a3e7dc..f867a294e 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts @@ -136,8 +136,28 @@ describe('VodMultiSourceHostService — session lifecycle', () => { expect(discovery.discover).toHaveBeenCalledTimes(2); }); + it('does not burn a source the user only selected', async () => { + // One alternative, so the route copy is the ONLY fallback left and + // the outcome cannot depend on how candidates are ranked. + await loadMovie([ALT_TWO]); + + // Straight to the alternative from the picker: the route copy was + // selected by discovery but never played, so it is still a fallback. + await expect(service.play(ALT_TWO.id)).resolves.toBe(true); + expect(rowFor(CURRENT_A_ID)?.isTried).toBe(false); + + vodAutoFailover.set(true); + + // Burning it at discovery made this failure report the options + // exhausted while a healthy copy sat untouched. + await expect(service.failover()).resolves.not.toBeNull(); + expect(rowFor(CURRENT_A_ID)?.isActive).toBe(true); + }); + it('keeps the playing source when enrichment reloads the same movie', async () => { await loadMovie([ALT_TWO, ALT_THREE]); + // The route copy actually plays first, so its turn is genuinely spent. + service.markRouteSourceActive(); await expect(service.play(ALT_TWO.id)).resolves.toBe(true); service.reportPosition(2538); 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 89c01c09b..462c5b10b 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 @@ -358,7 +358,7 @@ export class VodMultiSourceHostService { markRouteSourceActive(): void { this.switchToken++; if (this.routeSourceId) { - this.controller.setActiveSource(this.routeSourceId); + this.controller.markPlaying(this.routeSourceId); this.publish(); } } diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts index 398c4d2f0..ecc62dd24 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-session.ts @@ -82,7 +82,7 @@ export function applyDiscoveredSources( : [...discovered, switchedTo]; controller.setSources([current, ...sources], matchKind); - controller.setActiveSource(switchedTo.id); + controller.markPlaying(switchedTo.id); } /** What `switchToSource` needs from the host, without reaching into it. */ @@ -140,7 +140,7 @@ export async function switchToSource( controller.updateSource(resolved.candidate); deps.setPreviousSource(previous?.id ?? null); - controller.setActiveSource(candidate.id); + controller.markPlaying(candidate.id); controller.setResumeSeconds(resumeSeconds); deps.startPlayback(resolved.playback); @@ -169,6 +169,15 @@ export async function runFailover( controller: VodMultiSourceController, switchTo: (candidate: VodSourceCandidate) => Promise ): Promise { + // Whatever is on screen just failed, so it is spent — whichever way it got + // there. Relying on the start paths to have marked it would leave one hole + // per path, and the cost of missing it is an infinite ping-pong between + // two sources. + const failing = controller.activeSourceId(); + if (failing) { + controller.markTried(failing); + } + for (;;) { const target = controller.pickFailoverTarget(); if (!target) {