diff --git a/backend/app/plex/stream.py b/backend/app/plex/stream.py index da08c438..393bea58 100644 --- a/backend/app/plex/stream.py +++ b/backend/app/plex/stream.py @@ -840,12 +840,13 @@ def start_session( ) respawned = _spawn(False, False, replace_only=s) if respawned is None: - # Superseded/culled mid-fallback — yield whoever is registered now (a newer seek-restart), or - # our own now-dead session if the key was cap-culled. Never None here: the POST /session route - # turns a None return into a "Media file not found on disk" 404 — dishonest for a file that - # exists. A (dead) session instead degrades to a retryable 404 ("No active playback session" / - # "Not available yet") the client re-requests. - return current_session(user_id, key) or s + # Superseded/culled mid-fallback — answer with our own now-dead session. Never the newer one + # registered meanwhile: this answer goes to the request that LOST, whose player drops it as + # stale and stops the session it names (`stop_session`), which stopped the playback that won. + # Never None either: the POST /session route turns a None return into a "Media file not found + # on disk" 404 — dishonest for a file that exists. A dead session instead degrades to a + # retryable 404 ("No active playback session" / "Not available yet") the client re-requests. + return s s = respawned ready = wait_for(_seg(0), timeout=25.0, proc=s.proc) if ready: diff --git a/backend/tests/test_plex_stream.py b/backend/tests/test_plex_stream.py index bb0e85c1..baffdf28 100644 --- a/backend/tests/test_plex_stream.py +++ b/backend/tests/test_plex_stream.py @@ -695,7 +695,9 @@ class TestTranscodeStartSession: # F2/race: start_session waits on seg_0 OUTSIDE _lock. If a concurrent same-key start (a rapid # seek-restart) or _enforce_cap replaces _sessions[key] while we wait, our proc dying must NOT # trigger the SW fallback — that would tear down the NEWER session and breach the transcode cap. - # The identity guard must detect it's been superseded and yield the currently-registered session. + # The identity guard must detect it's been superseded. It answers with its OWN (dead) session, + # never the newer one: the answer goes to the request that lost, whose player drops it as stale + # and stops the session it names — naming the newer one stopped the playback that won. monkeypatch.setattr( stream, "_select_encode", lambda db, item, transcode, src: (True, False) ) @@ -708,9 +710,12 @@ class TestTranscodeStartSession: newer = _Newer() + captured = {} + def wait_then_supersede(path, timeout=20.0, proc=None): # Simulate the concurrent replacement happening during our out-of-lock wait, then report the # (now-killed) HW proc produced nothing. + captured["s"] = stream._sessions[(7, "12345")] stream._sessions[(7, "12345")] = newer return False @@ -720,7 +725,7 @@ class TestTranscodeStartSession: ) # No SW respawn — only the initial HW _spawn ran; the newer session was left intact. assert len(_CapturingProc.calls) == 1 - assert s is newer # yielded the currently-registered session, did not clobber it + assert s is captured["s"] # its own session, not the newer one assert stream._sessions[(7, "12345")] is newer def test_a_cap_culled_hw_session_does_not_respawn_and_is_not_reported_missing( diff --git a/frontend/src/lib/hlsEngine.test.ts b/frontend/src/lib/hlsEngine.test.ts index b272acd4..4f292643 100644 --- a/frontend/src/lib/hlsEngine.test.ts +++ b/frontend/src/lib/hlsEngine.test.ts @@ -515,3 +515,56 @@ it("tells React only when the snapshot really changed", async () => { expect(seen.mock.calls.length).toBe(after); await asks[0]!.resolve(direct("A")); }); + +describe("an item the viewer comes back to while its request is still out", () => { + it("waits for that request instead of sending a second one beside it (A → B → A)", async () => { + // The in-flight request was tracked in one slot: B's request took it, and the way back to A + // sent A2 next to the A1 still out — two spawns for one item, and the loser's answer could + // name the winner's session for the stop. + const { engine, asks } = setup(); + engine.start("A", 0); + engine.leaveItem(); + engine.start("B", 0); + engine.leaveItem(); + engine.start("A", 0); + expect(asks.map((a) => a.id)).toEqual(["A", "B"]); + await asks[0]!.resolve(hls("A", 0, "s1")); + expect(asks.map((a) => a.id)).toEqual(["A", "B", "A"]); + }); +}); + +describe("a session that was replaced before its manifest parsed", () => { + it("neither marks the player ready nor plays the outgoing media", async () => { + const { engine, asks, hlses, media } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + engine.seek("A", 900); // nothing seekable yet: a restart + const plays = media.plays; + hlses[0]!.handlers.onManifestParsed(); // the replaced instance parses late + expect(engine.getSnapshot().ready).toBe(false); + expect(media.plays).toBe(plays); + }); +}); + +describe("a pause given after the answer, before the media can play", () => { + it("stands: neither the manifest nor the first canplay plays it again", async () => { + const { engine, asks, hlses, media } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + engine.requestPlay(false); + hlses[0]!.handlers.onManifestParsed(); + expect(media.paused).toBe(true); + media.fire("canplay"); + expect(media.paused).toBe(true); + }); + + it("stands after the manifest played it too", async () => { + const { engine, asks, hlses, media } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + hlses[0]!.handlers.onManifestParsed(); + engine.requestPlay(false); + media.fire("canplay"); + expect(media.paused).toBe(true); + }); +}); diff --git a/frontend/src/lib/hlsEngine.ts b/frontend/src/lib/hlsEngine.ts index 8703b9a4..b4c26c43 100644 --- a/frontend/src/lib/hlsEngine.ts +++ b/frontend/src/lib/hlsEngine.ts @@ -164,7 +164,9 @@ export class HlsEngine { private attachedId: string | null = null; // The attached session's play intent, which stands until it is ready (see `wantsPlay`). private attachedIntent: Intent | null = null; - private inFlight: Request | null = null; + // The request out for each item. Per item, not one slot: another item's request took the slot, and + // the way back to the first item then sent a second request beside the one still out for it. + private readonly inFlight = new Map(); private queued: Request | null = null; // How often a session the server answered 404 for may be reclaimed (see `onHlsError`). private reclaims = { n: 0, at: 0 }; @@ -275,13 +277,13 @@ export class HlsEngine { // stale "Preparing stream…" for a session not known yet — set honestly from the answer. this.set({ ready: false, error: null, transcode: false }); const req: Request = { id, startAt, intent, gen }; - if (this.inFlight && this.inFlight.id === id) this.queued = req; + if (this.inFlight.has(id)) this.queued = req; else this.send(req); } private send(req: Request): void { this.queued = null; - this.inFlight = req; + this.inFlight.set(req.id, req); void this.deps.requestSession(req.id, req.startAt, this.deps.params()).then( (sess) => this.answered(req, sess), (e: unknown) => this.failed(req, e), @@ -295,9 +297,9 @@ export class HlsEngine { /** The in-flight request settled: the queued one goes now, unless it is stale too (the viewer * left the item, or the player closed). */ private settledInFlight(req: Request): void { - if (this.inFlight === req) this.inFlight = null; + if (this.inFlight.get(req.id) === req) this.inFlight.delete(req.id); const next = this.queued; - if (!next || this.inFlight) return; + if (!next || this.inFlight.has(next.id)) return; this.queued = null; if (this.alive && next.gen === this.gen) this.send(next); } @@ -345,7 +347,7 @@ export class HlsEngine { // Belt and braces: the play() on MANIFEST_PARSED / loadedmetadata can be rejected when the media // is not ready yet (slow first segments over a proxy), leaving the video paused after a seek the // viewer was watching through. Retried once the element can play; the intent is read when it - // fires, since the viewer may have asked for play or pause meanwhile. + // fires, since the viewer may have asked for play or pause meanwhile (`requestPlay` keeps it). this.listen(media, req, "canplay", () => { if (intent.play && media.paused) media.play().catch(() => {}); }); @@ -397,9 +399,14 @@ export class HlsEngine { private hlsHandlers(req: Request, media: HTMLMediaElement): HlsHandlers { return { + // Only the current session's: an instance replaced before its manifest parsed (a restart asked + // for right after an attach) marked the player ready while the next session was still being + // asked for — so that one then attached past `wantsPlay`'s not-ready guard — and played the + // outgoing media. onManifestParsed: () => { + if (!this.current(req)) return; this.set({ ready: true }); - if (this.alive && req.intent.play) media.play().catch(() => {}); + if (req.intent.play) media.play().catch(() => {}); }, // The selected audio is restored once hls.js has the renditions: on MANIFEST_PARSED the list // was not populated yet, the choice was dropped, and the default track played under the @@ -509,6 +516,9 @@ export class HlsEngine { if (!want && !media.paused) media.pause(); return true; } + // The attached session's intent follows: its manifest and first canplay still play by it, and a + // pause given before they fired was played over. + if (this.attachedIntent) this.attachedIntent.play = want; if (want) media.play().catch(() => {}); else media.pause(); return false; diff --git a/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx b/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx index 5a9d9c39..e5103edd 100644 --- a/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx +++ b/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx @@ -402,3 +402,40 @@ describe("usePlexTracks — a whose own VTT fetch failed", () => { expect(harness.current!.subNotice).toBeNull(); }); }); + +describe("usePlexTracks — the debounced A/V-offset restart", () => { + afterEach(() => vi.useRealTimers()); + + it("is dropped when the item changes before it fires — it would restart the previous item", () => { + vi.useFakeTimers(); + const restart = vi.fn(); + const qc = client(); + const { result, rerender } = renderHook( + ({ id }: { id: string }) => + usePlexTracks({ + id, + detail: detail(), + sessionK: 0, + subOffset: 0, + subPos: 88, + restartSession: (at) => restart(id, at), + setAudioTrack: () => false, + patchPrefs: () => {}, + videoRef: createRef(), + audioRef: { current: 0 } as RefObject, + aoffRef: { current: 0 }, + playhead: () => 600, + }), + { + wrapper: ({ children }: { children: ReactNode }) => ( + {children} + ), + initialProps: { id: RK }, + }, + ); + act(() => result.current.changeAudioOffset(0.2)); + rerender({ id: "next-episode" }); + act(() => void vi.advanceTimersByTime(1000)); + expect(restart).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/plexPlayerHooks.ts b/frontend/src/lib/plexPlayerHooks.ts index 1a99c5ee..0383d6e7 100644 --- a/frontend/src/lib/plexPlayerHooks.ts +++ b/frontend/src/lib/plexPlayerHooks.ts @@ -723,6 +723,16 @@ export function usePlexTracks(opts: { }, [patchPrefs, restartSession, aoffRef, playhead], ); + // A restart still waiting belongs to the item it was asked on: fired after a move to the next one, + // it restarted the PREVIOUS item, whose stream then played under the new title. The new item's + // session reads the offset itself (`aoffRef`), so nothing is lost by dropping it. + useEffect( + () => () => { + if (aoffTimer.current) window.clearTimeout(aoffTimer.current); + aoffTimer.current = null; + }, + [id], + ); // --- is the selected track actually extractable yet? ------------------------------------------- // Pulling one subtitle track out of a big MKV means ffmpeg streams the whole container: measured on