From 3f6638bf9609348605d196941054616f3b751594 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 3 Nov 2026 21:10:51 +0100 Subject: [PATCH] fix(plex): a seek during a quiet reclaim is no longer undone A quiet request leaves `live` alone, so a seek inside the old session's range went native and the answer then jumped back to where it asked from. `seek` now also goes by `pending`; the server-away retry had the same hole. Comments that still named 404 for a lost session say 410, and the hls.js retry comment now tells a finished playlist from a growing one. --- backend/app/plex/stream.py | 8 +++--- backend/tests/test_hls_reaper.py | 2 +- backend/tests/test_plex_hls_file_route.py | 2 +- frontend/src/lib/hlsEngine.test.ts | 27 ++++++++++++++++++- frontend/src/lib/hlsEngine.ts | 19 +++++++------ .../modules/plex/PlexPlayer.hls.dom.test.tsx | 15 +++++------ 6 files changed, 50 insertions(+), 23 deletions(-) diff --git a/backend/app/plex/stream.py b/backend/app/plex/stream.py index e491773b..8e25bbdb 100644 --- a/backend/app/plex/stream.py +++ b/backend/app/plex/stream.py @@ -125,7 +125,7 @@ class HlsSession: # replaces the viewer's registered session for the item — and the old player's next request # for `seg_12.ts`, resolved by (viewer, rating_key) alone, was answered 200 with the NEW session's twelfth segment, produced from a different offset # under `-copyts`. Wrong bytes, not an error, so the client's reclaim never fired on it. - # A request naming a session that is no longer registered is a 404, + # A request naming a session that is no longer registered is a 410, # which the player recovers from by restarting at the playhead. self.id = secrets.token_urlsafe(9) self.key = key # the item's rating_key (what the logs name) @@ -729,7 +729,7 @@ def _enforce_caps( whole segmented copy of the film under `_HLS_ROOT`, written faster than realtime. Counted only by the transcode cap, N distinct items were N ffmpegs and N film-sized scratch directories. - Somebody's playback stops at each drop. The route turns the missing session into a retryable 404 + Somebody's playback stops at each drop. The route turns the missing session into a 410 and the player restarts at the playhead, so from the server nothing failed — and "my film keeps rebuffering" has no other explanation anywhere, hence the line.""" @@ -918,7 +918,7 @@ def start_session( # 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. + # 410 ("session_gone") on its URLs, and the client asks again. return s s = respawned ready = wait_for(_seg(0), timeout=25.0, proc=s.proc) @@ -1200,7 +1200,7 @@ def reap_idle(finished_idle_s: float = _FINISHED_IDLE_S) -> int: _sessions.pop(reg, None) doomed.append(_detach(s)) reaped.append((s.key, "finished_idle" if done else "idle", int(idle))) - # The viewer of a session wrongly taken sees only a 404 later, so this line is the one record of why. + # The viewer of a session wrongly taken sees only a 410 later, so this line is the one record of why. for key, reason, idle_s in reaped: log.info( "an idle session was reaped", diff --git a/backend/tests/test_hls_reaper.py b/backend/tests/test_hls_reaper.py index c4b14368..8560ab8d 100644 --- a/backend/tests/test_hls_reaper.py +++ b/backend/tests/test_hls_reaper.py @@ -138,7 +138,7 @@ def test_the_periodic_pass_still_reaps_what_nobody_has_touched_for_the_idle_limi def test_every_reap_says_which_rule_took_the_session(tmp_path, sessions, caplog): - """A session that vanishes leaves the viewer a 404 and nothing else: with no line here, a live + """A session that vanishes leaves the viewer a 410 and nothing else: with no line here, a live stall could only ever be blamed on the reaper by inference.""" sessions["1"] = _Session( tmp_path, "gone_done", finished=True, idle_s=stream._SESSION_IDLE_S - 5 diff --git a/backend/tests/test_plex_hls_file_route.py b/backend/tests/test_plex_hls_file_route.py index c60ff471..24e8ee9f 100644 --- a/backend/tests/test_plex_hls_file_route.py +++ b/backend/tests/test_plex_hls_file_route.py @@ -5,7 +5,7 @@ seek-restart — or a second viewer of the same item — replaces the registered viewer's next request for `seg_12.ts`, resolved by rating_key, was answered 200 with the NEW session's twelfth segment: bytes produced from a different `-ss` offset under `-copyts`, i.e. a timeline the client is not on. Wrong content with a success status is the one failure a player cannot -notice, and the client's reclaim-on-404 could never fire on it. +notice, and the client's reclaim could never fire on it. No `db` fixture: the route reads nothing from the database (its own `Depends(get_db)` went with the removed auto-start, and `current_user` is overridden wholesale), so these must not sit in the DB diff --git a/frontend/src/lib/hlsEngine.test.ts b/frontend/src/lib/hlsEngine.test.ts index 25b3feb2..38c54da3 100644 --- a/frontend/src/lib/hlsEngine.test.ts +++ b/frontend/src/lib/hlsEngine.test.ts @@ -374,7 +374,7 @@ describe("a failed session request", () => { }); }); -describe("a session the server no longer has (404)", () => { +describe("something that never arrives (a fatal 404)", () => { it("is reclaimed where the viewer is", async () => { const { engine, asks, hlses, state } = setup(); engine.start("A", 600); @@ -482,6 +482,17 @@ describe("a session the server says is gone (410)", () => { expect(engine.getSnapshot().error).toBe("generic"); }); + it("keeps a seek made while it is out, inside what the old session still reports", async () => { + const { engine, asks, hlses, media } = await playing(); + media.seekEnd = 800; + hlses[0]!.gone(); + media.currentTime = 42; + engine.seek("A", 1250); + expect(media.currentTime).toBe(42); // the outgoing media stays where it is + await asks[1]!.resolve(hls("A", 1299)); + expect(asks.map((a) => a.startAt)).toEqual([600, 1299, 1250]); + }); + it("leaves a segment that is merely not there yet to hls.js's retries", async () => { const { engine, asks, hlses } = await playing(); hlses[0]!.retrying(404); @@ -493,6 +504,20 @@ describe("a session the server says is gone (410)", () => { describe("a server that is restarting (a deploy)", () => { // The gateway answers 502 while the app restarts, and nothing at all may answer (status 0): the // stream is not broken, the server is away. The buffer keeps playing meanwhile. + it("keeps a seek made while the retry is out", async () => { + const { engine, asks, hlses, state, fire, media } = setup(); + engine.start("A", 600); + await asks[0]!.resolve(hls("A", 600)); + hlses[0]!.handlers.onManifestParsed(); + media.seekEnd = 800; + state.position = 1299; + hlses[0]!.lost(502); + expect(fire()).toBe(true); + engine.seek("A", 1250); + await asks[1]!.resolve(hls("A", 1299)); + expect(asks.map((a) => a.startAt)).toEqual([600, 1299, 1250]); + }); + it.each([502, 503, 504, 0, null])( "is waited out on %s: no error, and the stream comes back where the viewer is", async (status) => { diff --git a/frontend/src/lib/hlsEngine.ts b/frontend/src/lib/hlsEngine.ts index 99f14a70..fc764040 100644 --- a/frontend/src/lib/hlsEngine.ts +++ b/frontend/src/lib/hlsEngine.ts @@ -285,7 +285,7 @@ export class HlsEngine { /** (Re)start `id` at `startAt` seconds. Another item's request goes out at once; one for the item * already being asked for waits for that answer (see the header). `quiet`: the attached stream is - * still playing from its buffer (the server went away under it), so it stays the player's until + * still playing from its buffer (the server went away or lost its session), so it stays the player's until * the answer replaces it — no "Loading…" over a running picture, its clock still true. */ start(id: string, startAt: number, play = true, quiet = false): void { if (!this.deps.media()) return; @@ -467,12 +467,14 @@ export class HlsEngine { // Except a session the server no longer has (410): gone, not broken. Either the server reaped it // or restarted, or it was superseded before it ever played (two requests for one item: the // server's LAST spawn wins), and the reclaim is the asking again that answer wants. It does not - // wait for hls.js to give up: with one video variant it has nothing to switch to, so it retries - // even a 4xx, six times over about half a minute (measured: seven requests, then the reclaim), and - // in a throttled page once a minute (four minutes). Quiet, since the buffer is still playing. + // wait for hls.js to give up. On a finished playlist, with one video variant and nothing to + // switch to, it retries even a 4xx, six times over about half a minute (measured: seven requests, + // then the reclaim), and in a throttled page once a minute (four minutes). On a growing one it + // takes the segment for a gap and moves on, so the error is never fatal. Either way the first + // answer is reported. Quiet, since the buffer is still playing. // - // And except a fatal 404: a segment still not there after those retries, so a transcode has fallen - // behind, and asking again is all that is left. + // And except a fatal 404: hls.js gave up on something the server still does not have, and asking + // again is all that is left. // // Both are bounded by the budget, not by whether the session got as far as playing: that belongs to // ONE session, and every restart builds a fresh one, so it could never bound a loop. A transcode @@ -565,9 +567,10 @@ export class HlsEngine { const rel = T - this.sessionStart.current; // Native only when the attached media IS the current session. While one is still being asked // for, `sessionStart` and `seekable` describe the OLD one: a native seek there left the pending - // restart current, and its answer then jumped to the earlier target. + // restart current, and its answer then jumped to the earlier target. A quiet request leaves + // `live` alone, so `pending` is what says one is out. const end = media.seekable.length ? media.seekable.end(0) : 0; - if (this.live && rel >= 0 && rel <= end + 0.5) { + if (this.live && !this.pending && rel >= 0 && rel <= end + 0.5) { media.currentTime = rel; // keeps playing on its own if (play) media.play().catch(() => {}); } else { diff --git a/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx b/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx index 40ac9c05..69167485 100644 --- a/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx +++ b/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx @@ -134,13 +134,13 @@ const lost404 = { fatal: true, type: Hls.ErrorTypes.NETWORK_ERROR, details: "fragLoadError", - response: { code: 404, text: "No active playback session" }, + response: { code: 404, text: "Not available yet" }, }; -describe("an HLS session the server no longer has", () => { +describe("an HLS segment that never arrives (a fatal 404)", () => { it("is restarted where the viewer is, instead of ending playback", async () => { - // The server reaps a session nobody fetches from (a long pause; a deploy). hls.js never retries - // a 4xx, so the next segment's 404 used to end playback with "couldn't play". + // Once hls.js has given up on it, the 404 used to end playback with "couldn't play". A session + // the server no longer has answers 410 instead: `hlsEngine.test.ts` holds that case. const session = mount(); await waitFor(() => expect(hlsInstances).toHaveLength(1)); act(() => hlsInstances[0]!.emit(Hls.Events.MANIFEST_PARSED)); @@ -155,10 +155,9 @@ describe("an HLS session the server no longer has", () => { it("gives up and says so when the reclaimed session keeps 404ing", async () => { // `parsed` cannot bound this: it lives on ONE hls instance and every restart builds a fresh one, - // so a session that parses its manifest and then 404s re-arms the guard every cycle. The server - // also answers 404 for "that segment is not there YET" (a transcode falling behind realtime), - // which is indistinguishable on the wire — so the player respawned ffmpeg forever behind a - // spinner that never became an error. + // so a session that parses its manifest and then 404s re-arms the guard every cycle. A transcode + // falling behind realtime answers 404 for "that segment is not there YET" every time, so the + // player respawned ffmpeg forever behind a spinner that never became an error. const session = mount(); const v = () => document.querySelector("video") as Media; // The first session plus the two it is allowed to reclaim; the third 404 has nothing left.