From 90931b47506cddbc30b1f5cc2ab433bc5a2420ad Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 7 Nov 2026 21:58:36 +0100 Subject: [PATCH] fix(plex): a playlist that never answers is asked for again; the slow line reads this stream A playlist timeout was resumed in the same session, but hls.js reloads no manifest on startLoad: no request, no error, a spinner for good. It is now asked for again like a server away. An absence also ended only on a fragment, which native HLS never reports, and a viewer trying again after the error started with the spent window; both now start a fresh one. The slow-connection meter was reset only when hls.js was built, so a direct answer showed the previous stream's numbers, and a replaced session's late fragment still counted; its rates now use the stats overlay's units. --- frontend/src/lib/format.test.ts | 9 --- frontend/src/lib/format.ts | 14 ---- frontend/src/lib/hlsEngine.test.ts | 74 +++++++++++++++++++ frontend/src/lib/hlsEngine.ts | 18 +++-- .../modules/plex/PlexPlayer.hls.dom.test.tsx | 36 +++++++++ frontend/src/modules/plex/PlexPlayer.tsx | 15 ++-- 6 files changed, 133 insertions(+), 33 deletions(-) diff --git a/frontend/src/lib/format.test.ts b/frontend/src/lib/format.test.ts index 5d075edd..096bb752 100644 --- a/frontend/src/lib/format.test.ts +++ b/frontend/src/lib/format.test.ts @@ -6,7 +6,6 @@ import { formatCompact, formatCountOrDash, formatDecimal, - formatMbps, formatMoney, formatDate, formatDateTime, @@ -285,11 +284,3 @@ describe("numbers in the UI language", () => { expect(formatMoney(160_000_000, "hu")).not.toBe(formatMoney(160_000_000, "en")); }); }); - -describe("formatMbps", () => { - it("writes a rate in the browser's own unit, a decimal only under 10", () => { - expect(formatMbps(19_600_000, "en")).toBe("20 Mb/s"); - expect(formatMbps(4_130_000, "en")).toBe("4.1 Mb/s"); - expect(formatMbps(4_130_000, "hu")).toBe("4,1 Mb/mp"); - }); -}); diff --git a/frontend/src/lib/format.ts b/frontend/src/lib/format.ts index 519e9bf8..9fe40ca6 100644 --- a/frontend/src/lib/format.ts +++ b/frontend/src/lib/format.ts @@ -133,20 +133,6 @@ export function formatUnit( ); } -/** A rate in megabits per second, in the browser's own short form ("20 Mb/s" / "20 Mb/mp"): one - * decimal under 10, whole above. */ -export function formatMbps(bitsPerSecond: number, lang: string): string { - const mbps = bitsPerSecond / 1_000_000; - const digits = mbps < 10 ? 1 : 0; - return numberFormat(lang, { - style: "unit", - unit: "megabit-per-second", - unitDisplay: "short", - maximumFractionDigits: digits, - minimumFractionDigits: digits, - }).format(mbps); -} - /** Compact human runtime ("1 hr 30 min", "45 min", "45 sec"; HU "1 ó 30 p") from seconds. Formats * BOTH a single item's length (Plex movies/episodes) and an aggregate total (playlist playtime, on * the YouTube and the Plex side alike) — it is the one renderer for a playlist total, so a total diff --git a/frontend/src/lib/hlsEngine.test.ts b/frontend/src/lib/hlsEngine.test.ts index 88f52781..3e936069 100644 --- a/frontend/src/lib/hlsEngine.test.ts +++ b/frontend/src/lib/hlsEngine.test.ts @@ -718,6 +718,80 @@ describe("a link too slow for the stream", () => { expect(hlses[0]!.resumes).toBe(1); expect(asks).toHaveLength(1); }); + + it("asks for the session again when its playlist never answered", async () => { + // Loading cannot resume what never parsed: hls.js reloads no manifest on startLoad, so a resume + // here sent nothing, raised nothing, and left the spinner up for good. + const { engine, asks, hlses, fire } = setup(); + engine.start("A", 300); + await asks[0]!.resolve(hls("A", 300)); + hlses[0]!.timedOut("manifestLoadTimeOut"); + expect(fire()).toBe(true); + expect(asks).toHaveLength(2); + expect(asks[1]!.startAt).toBe(300); + await asks[1]!.resolve(hls("A", 300)); + hlses[1]!.handlers.onManifestParsed(); + expect(engine.getSnapshot()).toMatchObject({ ready: true, error: null }); + }); + + it("says it cannot play once the playlist has not answered for the whole window", async () => { + const { engine, asks, hlses, fire, advance } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + hlses[0]!.timedOut("manifestLoadTimeOut"); + advance(AWAY_WINDOW_MS + 1); + expect(fire()).toBe(true); + await asks[1]!.resolve(hls("A")); + hlses[1]!.timedOut("manifestLoadTimeOut"); + expect(engine.getSnapshot().error).not.toBeNull(); + }); +}); + +describe("what a fragment reports", () => { + it("is passed on from the session asked for only, never from one it replaced", async () => { + const seen: number[] = []; + const { engine, asks, hlses } = setup({ onFragLoaded: (f) => void seen.push(f.bytes) }); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + hlses[0]!.arrived(); + engine.seek("A", 2000); + await asks[1]!.resolve(hls("A", 2000)); + hlses[0]!.arrived(); // the replaced session's last fragment lands late + expect(seen).toEqual([1_000_000]); + hlses[1]!.arrived(); + expect(seen).toEqual([1_000_000, 1_000_000]); + }); +}); + +describe("an absence when no hls.js plays the stream", () => { + it("ends with an answer the browser plays itself (native HLS)", async () => { + // Without MSE an HLS answer plays natively and no fragment is ever reported, so the answer is + // the only sign the server is back: a later absence gets its own window. + const { engine, asks, advance, fire } = setup({ createHls: () => null }); + engine.start("A", 0); + await asks[0]!.reject({ status: 503 }); + expect(fire()).toBe(true); + await asks[1]!.resolve(hls("A")); + advance(AWAY_WINDOW_MS + 60_000); + engine.seek("A", 900); + await asks[2]!.reject({ status: 503 }); + expect(engine.getSnapshot().error).toBeNull(); + expect(fire()).toBe(true); + }); + + it("starts afresh when the viewer tries again after the error was shown", async () => { + const { engine, asks, advance, fire } = setup(); + engine.start("A", 0); + await asks[0]!.reject({ status: 503 }); + advance(AWAY_WINDOW_MS + 1); + expect(fire()).toBe(true); + await asks[1]!.reject({ status: 503 }); + expect(engine.getSnapshot().error).toBe("generic"); + engine.start("A", 0); // the viewer presses play again + await asks[2]!.reject({ status: 503 }); + expect(engine.getSnapshot().error).toBeNull(); + expect(fire()).toBe(true); + }); }); describe("play intent while a session is asked for", () => { diff --git a/frontend/src/lib/hlsEngine.ts b/frontend/src/lib/hlsEngine.ts index 7fa75875..89b91a53 100644 --- a/frontend/src/lib/hlsEngine.ts +++ b/frontend/src/lib/hlsEngine.ts @@ -318,6 +318,9 @@ export class HlsEngine { if (quiet) { this.set({ error: null }); } else { + // The viewer trying again after the error was shown: a new attempt, with a whole window of its + // own — the spent one turned its first failure straight into the error again. + if (this.snapshot.error) this.away = null; this.live = false; this.deps.onLoadStart?.(); // Clear the prior session's transcode flag up front, so the cold-start overlay cannot show a @@ -377,9 +380,6 @@ export class HlsEngine { } const media = this.deps.media(); if (!media) return; - // A direct stream reports no fragments: its answer is the only sign the server is back. An HLS - // absence ends when a fragment arrives (see `AWAY_WINDOW_MS`). - if (sess.mode !== "hls") this.away = null; const { intent, startAt, id } = req; // The stream that kept playing is replaced from here: the new one loads like any other. if (req.quiet) this.set({ ready: false }); @@ -412,6 +412,10 @@ export class HlsEngine { // Named for the stop on the way out whoever plays it: hls.js, or Safari's native HLS below. if (sess.mode === "hls" && sess.id) this.attachedSession = { id, sessionId: sess.id }; const hls = sess.mode === "hls" ? this.deps.createHls(this.hlsHandlers(req, media)) : null; + // What hls.js does not play reports no fragments — a direct file, or HLS the browser plays + // natively (no MSE) — so its answer is the only sign the server is back. Under hls.js an + // absence ends when a fragment arrives (see `AWAY_WINDOW_MS`). + if (!hls) this.away = null; if (hls) { this.hls = hls; hls.load(`${sess.url}?t=${Math.floor(startAt)}`, media); @@ -478,7 +482,9 @@ export class HlsEngine { } }, onFragLoaded: (frag) => { - if (this.current(req)) this.away = null; // media gets through: whatever was away is back + // A superseded session's late fragment is not this stream's measurement. + if (!this.current(req)) return; + this.away = null; // media gets through: whatever was away is back this.deps.onFragLoaded?.(frag); }, onError: (e) => this.onHlsError(req, media, e), @@ -513,7 +519,9 @@ export class HlsEngine { return; } if (!e.fatal) return; - if (e.network && e.timeout) { + // A playlist that never answered is not resumed: hls.js reloads no manifest on startLoad, so a + // resume sent nothing and the spinner stayed. It is asked for again, as a server away is. + if (e.network && e.timeout && e.details !== "manifestLoadTimeOut") { this.slowLink(); return; } diff --git a/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx b/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx index 69167485..215097d4 100644 --- a/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx +++ b/frontend/src/modules/plex/PlexPlayer.hls.dom.test.tsx @@ -279,3 +279,39 @@ describe("an HLS segment that never arrives (a fatal 404)", () => { expect(session).toHaveBeenCalledTimes(3); }); }); + +describe("the slow-connection line", () => { + it("measures only the stream being asked for, not the one before it", async () => { + // A restart that a direct file answers builds no hls.js, so a meter reset only there kept the + // slow HLS session's numbers on screen under the next stream. + vi.useFakeTimers({ + shouldAdvanceTime: true, + toFake: ["Date", "setTimeout", "clearTimeout", "setInterval", "clearInterval"], + }); + try { + const session = mount(); + session + .mockResolvedValueOnce({ mode: "hls", url: "/hls/A/index.m3u8", start_s: 600 } as never) + .mockResolvedValueOnce({ mode: "direct", url: "/media/A", start_s: 0 } as never); + await waitFor(() => expect(hlsInstances).toHaveLength(1)); + // 6 MB of a 6 s fragment (8 Mbit/s) that took 12 s to arrive (4 Mbit/s). + act(() => + hlsInstances[0]!.emit(Hls.Events.FRAG_LOADED, { + frag: { + type: "main", + duration: 6, + stats: { total: 6_000_000, loading: { start: 0, first: 10, end: 12_000 } }, + }, + }), + ); + act(() => void vi.advanceTimersByTime(5_100)); + expect(screen.getByText("plex.player.slowLink")).toBeTruthy(); + act(() => hlsInstances[0]!.emit(Hls.Events.ERROR, lost404)); // restarted: answered direct + await waitFor(() => expect(session).toHaveBeenCalledTimes(2)); + act(() => void vi.advanceTimersByTime(2_100)); + expect(screen.queryByText("plex.player.slowLink")).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/frontend/src/modules/plex/PlexPlayer.tsx b/frontend/src/modules/plex/PlexPlayer.tsx index 1d24338e..49781c69 100644 --- a/frontend/src/modules/plex/PlexPlayer.tsx +++ b/frontend/src/modules/plex/PlexPlayer.tsx @@ -53,7 +53,7 @@ import ClipPanel, { ClipCropFrame, type ClipCrop } from "../../features/player/C import { markInAt, markOutAt, openAt, type ClipRange } from "../../lib/clipRange"; import { PlexPlayerSettings } from "./PlexPlayerSettings"; import { PlexStatsOverlay } from "./PlexStatsOverlay"; -import { createFragMeter } from "../../lib/plexStats"; +import { createFragMeter, fmtBitrate } from "../../lib/plexStats"; import { episodeLine } from "../../lib/plexDetail"; import { HlsEngine } from "../../lib/hlsEngine"; import { createHlsPort } from "../../lib/hlsPort"; @@ -103,7 +103,7 @@ import { MenuCheck } from "../../components/ui/MenuCheck"; import { PopoverItem, PopoverSurface } from "../../components/ui/AnchoredPopover"; import { closeThenAct, useAnchoredPopover } from "../../components/ui/anchoredPopoverHooks"; import { Slider } from "../../components/ui/NativeControls"; -import { formatMbps, formatUnit } from "../../lib/format"; +import { formatUnit } from "../../lib/format"; import { trackLabel } from "../../lib/vocab"; import { msg } from "../../lib/notifications"; @@ -260,7 +260,12 @@ export default function PlexPlayer({ // `req` rethrows fetch's own TypeError when nothing answered at all: that is a status of 0. statusOf: (e) => e instanceof HttpError ? e.status : e instanceof TypeError ? 0 : undefined, - onLoadStart: () => setBuffering(false), + // Every stream asked for (another item, a seek, a restart) is measured from nothing: a + // direct file answering it builds no hls.js, and must not show the previous one's numbers. + onLoadStart: () => { + setBuffering(false); + fragMeterRef.current.reset(); + }, onFragLoaded: (frag) => fragMeterRef.current.add(frag), // Fatal ones are what the viewer saw fail; the recoverable ones hls.js retries by itself only // matter to a detailed diagnosis. @@ -1346,8 +1351,8 @@ export default function PlexPlayer({ {t("plex.player.slowLink")} {t("plex.player.slowLinkRates", { - need: formatMbps(slowRates.need, i18n.language), - got: formatMbps(slowRates.got, i18n.language), + need: fmtBitrate(slowRates.need, i18n.language), + got: fmtBitrate(slowRates.got, i18n.language), })}