diff --git a/docs/requirements.md b/docs/requirements.md index d7ef42a3..8cfd5063 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -248,6 +248,7 @@ Internal architecture, components, and application logic. | DR-091 | Search result groups split TV into separate Shows and Episodes groups and add a People group (default order: Shows → Episodes → Movies → Songs → Albums → Artists → People); a stored `tvShows` order from before the split expands in place to shows+episodes so an upgrading user keeps their arrangement | UI | UR-060 | Done | | DR-092 | Video tap gestures resolve in `tapGestures.ts` (pure, unit-tested) rather than inline in `VideoPlayer.svelte`: `registerTap` returns `pending` for a first tap — the component defers `togglePlayPause` behind a `DOUBLE_TAP_WINDOW_MS` (300 ms) timer that a second tap cancels — or `seek` (+30 s right / −10 s left) for a second tap inside the window; a consumed second tap resets the state so a third tap starts fresh, and a swipe cancels the pending tap. The compatibility `click` the browser synthesizes after a touch tap is filtered in `handleVideoClick` so it cannot bypass the deferral. `resolveSeekTarget` converts the delta to the absolute position the facade requires, clamped per DR-095 and chained off a still-in-flight `pendingSeekTarget` so back-to-back skips accumulate instead of all resolving against a not-yet-updated position | UI | UR-061 | Done | | DR-094 | Frontend boundary tripwire (`scripts/check-frontend-boundary.sh`) detects Jellyfin item-type array literals **anywhere** in `src/` rather than only inline at an `includeItemTypes:` query site, so a category→type mapping cannot evade the check by being assigned to a named const (the evasion that let the `scoped-search` leak pass CI); requires two adjacent type literals so single-type presentation and `item.type ===` inspection stay legal, and caps the allowlist to force taxonomy into Rust instead of accumulating exceptions | Tooling | - | Done | +| DR-096 | `Html5PlayerAdapter.play()` is resilient to stall recovery: an in-flight attempt is memoised so concurrent callers (UI plus hls.js gap-controller recovery) share one `element.play()` instead of stacking calls, and an `AbortError` ("play() request was interrupted by a call to pause()") is logged at debug rather than pushed to `host.onError`. The browser raises it whenever a pending play promise is superseded by a pause/seek/source change, which hls.js does routinely while nudging past a stall — reporting it surfaced a player error roughly once per second for the whole stall and left the UI stuck showing paused | Player | UR-005 | Done | | DR-095 | Seek targets clamp strictly *inside* the media (`clampSeekTarget`, `END_SEEK_MARGIN_SECONDS` = 6 s ≈ one HLS segment) instead of to the exact `duration`. Landing on the duration makes hls.js request the segment whose start time lies past the end of the media (e.g. a 6330.324 s item → segment 1055 starting at 6336.33 s), which Jellyfin never produces; the fetch times out and hls.js' gap-controller stalls at the last buffered position, presenting as "unpausing or skipping bounces straight back to paused". Applied on both seek paths — the relative-skip `resolveSeekTarget` and the seek-bar drag, whose range input `max` is the duration itself — and floored at 0 so media shorter than the margin still seeks to the start | UI | UR-061 | Done | | DR-093 | Traceability coverage gate derives its requirement denominators from `requirements.md` at run time rather than hardcoded literals: `countDefinedRequirements` counts an ID only where it leads a markdown table row (ignoring the "Traces To" column and prose) and deduplicates IDs listed both in the definition tables and in the §3 traceability matrix; `computeCoverage` reports the *intersection* of traced and defined IDs so an ID traced in code but absent from `requirements.md` is surfaced as `orphaned` instead of inflating the ratio past 100%. UT/IT test identifiers are excluded as a separate taxonomy. CI and `bun run traces:coverage` share this computation and fail on both a sub-threshold and an impossible >100% result | Tooling | - | Done | diff --git a/scripts/extract-traces.test.ts b/scripts/extract-traces.test.ts index 4f730485..f65f7bf0 100644 --- a/scripts/extract-traces.test.ts +++ b/scripts/extract-traces.test.ts @@ -175,8 +175,8 @@ describe("live requirements.md", () => { expect(defined.UR).toBe(61); expect(defined.IR).toBe(29); - expect(defined.DR).toBe(92); + expect(defined.DR).toBe(93); expect(defined.JA).toBe(32); - expect(defined.total).toBe(214); + expect(defined.total).toBe(215); }); }); diff --git a/src/lib/player/adapters/html5Adapter.test.ts b/src/lib/player/adapters/html5Adapter.test.ts index 8ce933a7..423b507b 100644 --- a/src/lib/player/adapters/html5Adapter.test.ts +++ b/src/lib/player/adapters/html5Adapter.test.ts @@ -95,6 +95,63 @@ describe("Html5PlayerAdapter", () => { expect(video.play).toHaveBeenCalledTimes(1); }); + // A stalling HLS stream makes hls.js' gap-controller nudge the element, which + // aborts an in-flight play(). That AbortError is transient — the element is + // still trying to play — so it must not be surfaced as a player error, or the + // UI reports failure ~once a second for the whole stall. + it("play() does not report an interrupted-by-pause AbortError as an error", async () => { + const abort = new DOMException( + "The play() request was interrupted by a call to pause().", + "AbortError" + ); + video.play = vi.fn(async () => { + throw abort; + }); + + await adapter.play(); + + expect(host.onError).not.toHaveBeenCalled(); + }); + + it("play() still reports a genuine failure", async () => { + video.play = vi.fn(async () => { + throw new DOMException("no supported source", "NotSupportedError"); + }); + + await adapter.play(); + + expect(host.onError).toHaveBeenCalledTimes(1); + expect(String((host.onError as any).mock.calls[0][0])).toContain("play() failed"); + }); + + it("play() coalesces concurrent attempts into one element.play() call", async () => { + // During a stall the UI and recovery paths can both ask to play. Stacking + // element.play() calls is what generates the AbortError storm. + let resolvePlay: () => void = () => {}; + video.play = vi.fn( + () => + new Promise((r) => { + resolvePlay = () => { + video.paused = false; + r(); + }; + }) + ); + + const first = adapter.play(); + const second = adapter.play(); + resolvePlay(); + await Promise.all([first, second]); + + expect(video.play).toHaveBeenCalledTimes(1); + }); + + it("play() works again after a previous attempt settled", async () => { + await adapter.play(); + await adapter.play(); + expect(video.play).toHaveBeenCalledTimes(2); + }); + it("pause() calls element.pause()", async () => { video.paused = false; await adapter.pause(); diff --git a/src/lib/player/adapters/html5Adapter.ts b/src/lib/player/adapters/html5Adapter.ts index 590cf93a..3965ba78 100644 --- a/src/lib/player/adapters/html5Adapter.ts +++ b/src/lib/player/adapters/html5Adapter.ts @@ -16,7 +16,7 @@ * intents flowing through the PlayerAdapter interface while preserving the * hard-won element behavior verbatim. * - * TRACES: UR-003, UR-005, UR-020, UR-021 | DR-001, DR-023, DR-024, DR-028 + * TRACES: UR-003, UR-005, UR-020, UR-021 | DR-001, DR-023, DR-024, DR-028, DR-096 */ import type { AdapterHost, PlayerAdapter, PlayerLoadOptions } from "./types"; @@ -41,10 +41,24 @@ export interface Html5ElementBridge { getMediaSourceId(): string | null; } +/** + * True for the `AbortError` the browser raises when a pending `play()` promise is + * cancelled by a `pause()` (or a source/seek change). It signals "that specific + * play attempt was superseded", not "playback failed" — hls.js' stall recovery + * produces it routinely, so it must not reach the player's error channel. + */ +function isPlayInterruptedError(err: unknown): boolean { + if (!err || typeof err !== "object") return false; + const { name, message } = err as { name?: string; message?: string }; + return name === "AbortError" || (message ?? "").includes("interrupted"); +} + export class Html5PlayerAdapter implements PlayerAdapter { readonly kind = "html5" as const; private attachedElement: HTMLVideoElement | null = null; + /** In-flight play() attempt, so concurrent callers share one element.play(). */ + private pendingPlay: Promise | null = null; private host: AdapterHost; private bridge: Html5ElementBridge; @@ -81,12 +95,31 @@ export class Html5PlayerAdapter implements PlayerAdapter { async play(): Promise { const el = this.element; if (!el) return; - try { - await el.play(); - // handlePlay on the element reports "playing"; no double-report here. - } catch (err) { - this.host.onError(`play() failed: ${err}`); - } + // Coalesce concurrent attempts. While an HLS stream stalls, the UI and the + // gap-controller recovery path can both ask to play; stacking element.play() + // calls is what turns one stall into an AbortError storm. + if (this.pendingPlay) return this.pendingPlay; + + this.pendingPlay = (async () => { + try { + await el.play(); + // handlePlay on the element reports "playing"; no double-report here. + } catch (err) { + // A play() aborted by a pause() is transient, not a failure: hls.js + // nudges the element to recover from a stall, which cancels the pending + // play promise while the element keeps trying. Surfacing it would report + // an error roughly once a second for the duration of the stall. + if (isPlayInterruptedError(err)) { + console.debug("[Html5PlayerAdapter] play() interrupted by pause (stall recovery)"); + } else { + this.host.onError(`play() failed: ${err}`); + } + } finally { + this.pendingPlay = null; + } + })(); + + return this.pendingPlay; } async pause(): Promise {