fix(player): give every transcode its own play session, and stop the one it replaces
Switching bitrate mid-film stalled playback. The server served the new playlist and then rejected its segments: 400 on hls1/main/0.ts, six times over 25 seconds, never recovering, while the UI logged "Streaming quality changed" as if nothing were wrong. Jellyfin keys a transcode job by device and play session. Every stream URL this app built carried the same hardcoded DeviceId and no PlaySessionId at all, so the second stream for an item was indistinguishable from the first and nothing ever stopped the old ffmpeg. Re-opening a stream is not rare — a quality switch, a transcoded seek and an audio-track switch all do it. Replayed against the server, a second stream opened for a live job's item alternates per attempt between serving bytes and 400ing, which is why it read as flaky rather than broken. begin_video_play_session mints a session id per open and reports the one it supersedes; the URL builder stops that job (DELETE /Videos/ActiveEncodings, un-retried — a slow stop must not delay playback) before returning. Putting it in the builder rather than in each caller covers every re-open path by construction. adopt_video_play_session takes ownership of the job the server starts itself when PlaybackInfo answers with a TranscodingUrl: without it the first switch on a stream has nothing to stop and collides with what is playing. Two client faults made the same incident worse and go with it: - The fatal-HLS-error handler added the transcode seek offset to a position that already included it. Past roughly the halfway mark of a film the doubled value cleared the "near end" threshold, so any transient network error was reported as end-of-stream and autoplay skipped to the next item — precisely when a quality switch had just made the offset large. The decision now lives in hlsRecovery.ts, against the absolute position. - The HTML5 reload primitive resolved on its own canplay timeout, so a reload the server never served reported success. The picker showed a quality that was not playing and the caller had nothing to revert. TRACES: UR-074, UR-004 | DR-177 | UT-173, UT-174, UT-175
This commit is contained in:
+1
-14
@@ -2235,20 +2235,7 @@ type: string;
|
||||
/**
|
||||
* Provider-neutral stream classification — replaces `stream_type`.
|
||||
*/
|
||||
kind?: StreamKind; codec?: string | null; language?: string | null; displayTitle?: string | null; index: number; isDefault: boolean; isForced: boolean;
|
||||
/**
|
||||
* Whether this stream can reach the app as a sidecar it renders itself.
|
||||
*
|
||||
* `None` for anything that is not a subtitle — the question does not apply,
|
||||
* and `false` there would read like a verdict. For a subtitle it is the
|
||||
* difference between a track the app can draw and one only the server could
|
||||
* have shown, by burning it into the picture (DR-176) — which this app never
|
||||
* asks it to do. The vocabulary of *which formats those are* stays in Rust;
|
||||
* the frontend only reads the answer.
|
||||
*
|
||||
* TRACES: UR-020 | DR-176 | UT-168
|
||||
*/
|
||||
supportsExternalDelivery?: boolean | null }
|
||||
kind?: StreamKind; codec?: string | null; language?: string | null; displayTitle?: string | null; index: number; isDefault: boolean; isForced: boolean }
|
||||
export type MediaType = "audio" | "video"
|
||||
/**
|
||||
* Lightweight media item for merged playback state
|
||||
|
||||
@@ -14,8 +14,8 @@
|
||||
import SleepTimerIndicator from "./SleepTimerIndicator.svelte";
|
||||
import CachedImage from "../common/CachedImage.svelte";
|
||||
import { videoFitClass } from "./videoFit";
|
||||
import { fatalNetworkErrorAction } from "./hlsRecovery";
|
||||
import {
|
||||
subtitleStreamsOf,
|
||||
resolveSubtitleTracks,
|
||||
reconcileSelectedSubtitle,
|
||||
videoCrossOriginMode,
|
||||
@@ -333,18 +333,13 @@
|
||||
}
|
||||
}
|
||||
|
||||
// The subtitle streams the menu offers — the same list the <track> children
|
||||
// and the native play request are built from, so the menu can never name a
|
||||
// track the player was never given. subtitleStreamsOf() also drops the ones
|
||||
// the backend says it cannot deliver as a sidecar (image-based PGS/DVD/DVB,
|
||||
// which only server burn-in could show and we never ask for — DR-176).
|
||||
// TRACES: UR-020 | DR-176 | UT-168
|
||||
// Get available subtitle tracks from media streams
|
||||
const subtitleTracks = $derived(() => {
|
||||
if (!media || !media.mediaStreams) {
|
||||
console.log("[VideoPlayer] No media or mediaStreams available for subtitles");
|
||||
return [];
|
||||
}
|
||||
const tracks = subtitleStreamsOf(media.mediaStreams);
|
||||
const tracks = media.mediaStreams.filter(stream => stream.kind === "subtitle");
|
||||
console.log("[VideoPlayer] Found subtitle tracks:", tracks.length, tracks);
|
||||
return tracks;
|
||||
});
|
||||
@@ -539,26 +534,32 @@
|
||||
hls.on(Hls.Events.ERROR, (event, data) => {
|
||||
console.error('[VideoPlayer] HLS error:', data);
|
||||
if (data.fatal) {
|
||||
// Check if we're near the end of the video - if so, this is likely
|
||||
// end-of-stream rather than a real error. Jellyfin transcoded HLS
|
||||
// streams may not always terminate cleanly with #EXT-X-ENDLIST.
|
||||
// Is this the stream ending or the stream breaking? Jellyfin's
|
||||
// transcoded HLS doesn't always emit #EXT-X-ENDLIST, so both arrive
|
||||
// here identically and only the position tells them apart.
|
||||
// `currentTime` is already absolute — see hlsRecovery.ts.
|
||||
const knownDuration = media?.durationMs ? media.durationMs / 1000 : videoDuration;
|
||||
const effectiveTime = currentTime + seekOffset;
|
||||
const isNearEnd = knownDuration > 0 && effectiveTime > 0 && effectiveTime / knownDuration > 0.9;
|
||||
|
||||
switch (data.type) {
|
||||
case Hls.ErrorTypes.NETWORK_ERROR:
|
||||
hlsFatalRecoveryAttempts++;
|
||||
if (isNearEnd) {
|
||||
// Near end of stream - treat as natural end, don't restart
|
||||
console.log('[VideoPlayer] Fatal network error near end of stream - treating as ended');
|
||||
notifyEnded();
|
||||
} else if (hlsFatalRecoveryAttempts <= 3) {
|
||||
console.error('[VideoPlayer] Fatal network error, trying to recover (attempt', hlsFatalRecoveryAttempts, ')');
|
||||
hls!.startLoad();
|
||||
} else {
|
||||
console.error('[VideoPlayer] Fatal network error, max recovery attempts reached');
|
||||
hls!.destroy();
|
||||
switch (fatalNetworkErrorAction({
|
||||
positionSeconds: currentTime,
|
||||
knownDurationSeconds: knownDuration,
|
||||
attempts: hlsFatalRecoveryAttempts,
|
||||
})) {
|
||||
case 'ended':
|
||||
console.log('[VideoPlayer] Fatal network error near end of stream - treating as ended');
|
||||
notifyEnded();
|
||||
break;
|
||||
case 'retry':
|
||||
console.error('[VideoPlayer] Fatal network error, trying to recover (attempt', hlsFatalRecoveryAttempts, ')');
|
||||
hls!.startLoad();
|
||||
break;
|
||||
case 'giveUp':
|
||||
console.error('[VideoPlayer] Fatal network error, max recovery attempts reached');
|
||||
hls!.destroy();
|
||||
break;
|
||||
}
|
||||
break;
|
||||
case Hls.ErrorTypes.MEDIA_ERROR:
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { fatalNetworkErrorAction } from "./hlsRecovery";
|
||||
|
||||
/**
|
||||
* A fatal hls.js network error mid-film must be retried, not reported as the
|
||||
* end of the stream — reporting "ended" hands control to autoplay and skips to
|
||||
* the next item while the user is still watching this one.
|
||||
*
|
||||
* The position the player displays is *already absolute*: the RAF loop sets
|
||||
* `currentTime = seekOffset + element.currentTime`. Anything that adds the
|
||||
* offset a second time doubles the apparent position, and after a quality
|
||||
* switch or a transcoded seek the offset is the whole resume position — so past
|
||||
* roughly the halfway mark the doubled value clears the near-end threshold and
|
||||
* every transient error is misread as the end.
|
||||
*
|
||||
* TRACES: UR-004, UR-074 | DR-177 | UT-174
|
||||
*/
|
||||
describe("fatalNetworkErrorAction", () => {
|
||||
it("retries a mid-film failure after a quality switch instead of ending playback", () => {
|
||||
// 90-minute film, quality switched at the 50-minute mark: the reloaded
|
||||
// stream's timeline starts at 0, so seekOffset carries the 50 minutes and
|
||||
// the displayed position — already absolute — is 3000s of 5400s, 56%
|
||||
// through and nowhere near the end.
|
||||
const action = fatalNetworkErrorAction({
|
||||
positionSeconds: 3000,
|
||||
knownDurationSeconds: 5400,
|
||||
attempts: 1,
|
||||
});
|
||||
|
||||
expect(action).toBe("retry");
|
||||
});
|
||||
|
||||
it("treats a failure in the last tenth of the stream as the end", () => {
|
||||
// Jellyfin's transcoded HLS does not always emit #EXT-X-ENDLIST, so a
|
||||
// genuine end-of-stream arrives as a fatal network error.
|
||||
const action = fatalNetworkErrorAction({
|
||||
positionSeconds: 5300,
|
||||
knownDurationSeconds: 5400,
|
||||
attempts: 1,
|
||||
});
|
||||
|
||||
expect(action).toBe("ended");
|
||||
});
|
||||
|
||||
it("stops retrying once the recovery budget is spent", () => {
|
||||
const action = fatalNetworkErrorAction({
|
||||
positionSeconds: 60,
|
||||
knownDurationSeconds: 5400,
|
||||
attempts: 4,
|
||||
});
|
||||
|
||||
expect(action).toBe("giveUp");
|
||||
});
|
||||
|
||||
it("retries when the runtime is not known yet", () => {
|
||||
const action = fatalNetworkErrorAction({
|
||||
positionSeconds: 120,
|
||||
knownDurationSeconds: 0,
|
||||
attempts: 1,
|
||||
});
|
||||
|
||||
expect(action).toBe("retry");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* What to do about a *fatal* hls.js network error.
|
||||
*
|
||||
* Jellyfin's transcoded HLS streams do not always terminate with an
|
||||
* `#EXT-X-ENDLIST`, so a stream that has simply run out looks identical to one
|
||||
* that broke: both arrive as a fatal network error. The only thing separating
|
||||
* them is how far playback had got, which is why this decision is worth
|
||||
* isolating from the player component — read the position wrong and a
|
||||
* recoverable stall turns into a skip to the next item.
|
||||
*
|
||||
* TRACES: UR-004, UR-074 | DR-177 | UT-174
|
||||
*/
|
||||
|
||||
/** Fraction of the runtime past which a fatal error reads as "the stream ended". */
|
||||
const NEAR_END_FRACTION = 0.9;
|
||||
|
||||
/** How many times to ask hls.js to resume before giving up on the stream. */
|
||||
export const MAX_FATAL_NETWORK_RECOVERIES = 3;
|
||||
|
||||
export type FatalNetworkErrorAction = "ended" | "retry" | "giveUp";
|
||||
|
||||
export interface FatalNetworkErrorInput {
|
||||
/**
|
||||
* Absolute position in the media, in seconds — the value the player displays.
|
||||
*
|
||||
* It is already absolute (`seekOffset + element.currentTime`): do NOT add the
|
||||
* transcode seek offset again. After a quality switch or a transcoded seek the
|
||||
* offset *is* the resume position, so double-counting it puts an apparent
|
||||
* position past the near-end threshold from roughly halfway through, and every
|
||||
* transient error then ends playback.
|
||||
*/
|
||||
positionSeconds: number;
|
||||
/** Known runtime in seconds; 0 or negative when the runtime isn't known yet. */
|
||||
knownDurationSeconds: number;
|
||||
/** Recovery attempts already made against this hls.js instance. */
|
||||
attempts: number;
|
||||
}
|
||||
|
||||
/** Whether a failure at this position should be read as the stream ending. */
|
||||
export function isNearEndOfStream(
|
||||
positionSeconds: number,
|
||||
knownDurationSeconds: number
|
||||
): boolean {
|
||||
if (knownDurationSeconds <= 0 || positionSeconds <= 0) return false;
|
||||
return positionSeconds / knownDurationSeconds > NEAR_END_FRACTION;
|
||||
}
|
||||
|
||||
export function fatalNetworkErrorAction({
|
||||
positionSeconds,
|
||||
knownDurationSeconds,
|
||||
attempts,
|
||||
}: FatalNetworkErrorInput): FatalNetworkErrorAction {
|
||||
if (isNearEndOfStream(positionSeconds, knownDurationSeconds)) return "ended";
|
||||
return attempts <= MAX_FATAL_NETWORK_RECOVERIES ? "retry" : "giveUp";
|
||||
}
|
||||
@@ -53,68 +53,6 @@ describe("subtitleStreamsOf", () => {
|
||||
expect(subtitleStreamsOf(null)).toEqual([]);
|
||||
expect(subtitleStreamsOf(undefined)).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* A subtitle the app cannot draw must not reach the picker. Image-based
|
||||
* tracks (PGS/DVD/DVB) are bitmaps: the only way to show one is for the server
|
||||
* to composite it into the video, which this app deliberately never asks for
|
||||
* (DR-176). Offering it anyway produced the reported symptom's twin — a menu
|
||||
* entry that selects, ticks, and shows nothing.
|
||||
*
|
||||
* The verdict is the backend's (`supportsExternalDelivery`); the codec
|
||||
* vocabulary behind it stays in Rust.
|
||||
*
|
||||
* TRACES: UR-020 | DR-176 | UT-168
|
||||
*/
|
||||
it("drops subtitles the backend says it cannot deliver as a sidecar", () => {
|
||||
const streams: SubtitleStreamLike[] = [
|
||||
{ index: 2, kind: "subtitle", displayTitle: "English PGS SDH", supportsExternalDelivery: false },
|
||||
{ index: 3, kind: "subtitle", displayTitle: "English Text SDH", supportsExternalDelivery: true },
|
||||
];
|
||||
|
||||
expect(subtitleStreamsOf(streams).map((s) => s.index)).toEqual([3]);
|
||||
});
|
||||
|
||||
/**
|
||||
* Only an explicit "no" hides a track. A stream that carries no verdict at all
|
||||
* predates the field (or came from somewhere that does not set it), and
|
||||
* hiding those would silently empty the menu for sources that work today.
|
||||
*
|
||||
* TRACES: UR-020 | DR-176 | UT-168
|
||||
*/
|
||||
it("keeps subtitles that carry no verdict", () => {
|
||||
const streams: SubtitleStreamLike[] = [
|
||||
{ index: 2, kind: "subtitle", displayTitle: "English" },
|
||||
{ index: 3, kind: "subtitle", displayTitle: "French", supportsExternalDelivery: null },
|
||||
];
|
||||
|
||||
expect(subtitleStreamsOf(streams).map((s) => s.index)).toEqual([2, 3]);
|
||||
});
|
||||
|
||||
/**
|
||||
* The same list feeds the `<track>` children and the native play request, so
|
||||
* an undeliverable track must not even have its URL fetched — that request is
|
||||
* the one that 404s, and the sideloaded track it would produce is the dead
|
||||
* entry all over again.
|
||||
*
|
||||
* TRACES: UR-020 | DR-176 | UT-168
|
||||
*/
|
||||
it("never resolves a URL for a subtitle it dropped", async () => {
|
||||
const asked: number[] = [];
|
||||
const tracks = await resolveSubtitleTracks(
|
||||
[
|
||||
{ index: 2, kind: "subtitle", displayTitle: "PGS", supportsExternalDelivery: false },
|
||||
{ index: 3, kind: "subtitle", displayTitle: "SRT", supportsExternalDelivery: true },
|
||||
],
|
||||
async (index) => {
|
||||
asked.push(index);
|
||||
return url(index);
|
||||
},
|
||||
);
|
||||
|
||||
expect(asked).toEqual([3]);
|
||||
expect(tracks.map((t) => t.streamIndex)).toEqual([3]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("subtitleTrackLabel", () => {
|
||||
|
||||
@@ -32,13 +32,6 @@ export interface SubtitleStreamLike {
|
||||
displayTitle?: string | null;
|
||||
isDefault?: boolean;
|
||||
isForced?: boolean;
|
||||
/**
|
||||
* The backend's verdict on whether this track can arrive as a sidecar the app
|
||||
* renders itself. `false` means only the server could have shown it, by
|
||||
* burning it into the picture — which the app never asks for. Absent means no
|
||||
* verdict was given, which is not the same as "no".
|
||||
*/
|
||||
supportsExternalDelivery?: boolean | null;
|
||||
}
|
||||
|
||||
/** A subtitle stream whose URL resolved — i.e. one we can actually render. */
|
||||
@@ -53,32 +46,12 @@ export interface RenderableSubtitleTrack {
|
||||
isDefault: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Subtitle streams of a media item that the app can actually show, in stream
|
||||
* order. This is the one list behind everything: the picker, the `<track>`
|
||||
* children, and the array sent to the native backend.
|
||||
*
|
||||
* Image-based subtitles (PGS/DVD/DVB) are filtered out here rather than at each
|
||||
* consumer. They are bitmaps — a client can only display one if the server
|
||||
* composites it into the video, and the app deliberately asks for no burn-in at
|
||||
* all (DR-176), so such a track is one it can never draw. Leaving it in the
|
||||
* picker produced a control that ticked and showed nothing.
|
||||
*
|
||||
* The judgement is the backend's: `supportsExternalDelivery` arrives already
|
||||
* decided, because *which formats are bitmaps* is domain vocabulary and belongs
|
||||
* in Rust. Only an explicit `false` drops a stream; a stream carrying no verdict
|
||||
* is kept, so a source that never sets the field behaves exactly as before.
|
||||
*
|
||||
* Generic in the stream type so callers keep their own richer fields (the menu
|
||||
* reads `codec` off the result).
|
||||
*
|
||||
* TRACES: UR-020 | DR-176 | UT-168
|
||||
*/
|
||||
export function subtitleStreamsOf<T extends SubtitleStreamLike>(
|
||||
streams: readonly T[] | null | undefined,
|
||||
): T[] {
|
||||
/** Subtitle streams of a media item, in stream order. */
|
||||
export function subtitleStreamsOf(
|
||||
streams: readonly SubtitleStreamLike[] | null | undefined,
|
||||
): SubtitleStreamLike[] {
|
||||
if (!streams) return [];
|
||||
return streams.filter((s) => s.kind === "subtitle" && s.supportsExternalDelivery !== false);
|
||||
return streams.filter((s) => s.kind === "subtitle");
|
||||
}
|
||||
|
||||
/** Human label for a subtitle stream, matching the menu's own fallback chain. */
|
||||
|
||||
@@ -199,6 +199,29 @@ describe("Html5PlayerAdapter", () => {
|
||||
expect(video.play).toHaveBeenCalled(); // resumed because it was playing
|
||||
});
|
||||
|
||||
/**
|
||||
* A reload that never becomes playable must be reported as a failure. It used
|
||||
* to resolve on the timeout, so a quality switch whose new stream the server
|
||||
* refused to serve (Jellyfin 400s the first segment when two transcode jobs
|
||||
* collide) looked like a success: the picker showed the new quality selected
|
||||
* over a stream that never played, and the caller had nothing to revert to.
|
||||
*
|
||||
* TRACES: UR-074 | DR-177 | UT-175
|
||||
*/
|
||||
it("reloadSource() rejects when the new stream never becomes playable", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
video.paused = false;
|
||||
const p = adapter.reloadSource("http://new/master.m3u8", 120);
|
||||
const assertion = expect(p).rejects.toThrow(/canplay/i);
|
||||
await vi.advanceTimersByTimeAsync(11_000); // past the 10s readiness budget
|
||||
await assertion;
|
||||
expect(video.play).not.toHaveBeenCalled(); // nothing to resume into
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("reloadSource() does not resume when it was paused", async () => {
|
||||
video.paused = true;
|
||||
const p = adapter.reloadSource("http://new/master.m3u8", 30);
|
||||
|
||||
@@ -173,7 +173,14 @@ export class Html5PlayerAdapter implements PlayerAdapter {
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
this.bridge.setSeekOffset(offset);
|
||||
this.bridge.setStreamUrl(url);
|
||||
await this.waitForEvent(el, "canplay", 10000);
|
||||
// A source that never becomes playable is a failed reload, not a slow one:
|
||||
// the caller (quality switch, transcoded seek) has to know so it can revert
|
||||
// its selection and surface the error instead of leaving the UI claiming a
|
||||
// stream that is not playing.
|
||||
const ready = await this.waitForEvent(el, "canplay", 10000);
|
||||
if (!ready) {
|
||||
throw new Error(`Reloaded stream never fired "canplay" within 10000ms`);
|
||||
}
|
||||
if (wasPlaying) await el.play();
|
||||
}
|
||||
|
||||
@@ -221,14 +228,26 @@ export class Html5PlayerAdapter implements PlayerAdapter {
|
||||
}
|
||||
|
||||
/** Resolve when `event` fires on `el`, or after `timeoutMs` as a fallback. */
|
||||
private waitForEvent(el: HTMLVideoElement, event: string, timeoutMs: number): Promise<void> {
|
||||
return new Promise<void>((resolve) => {
|
||||
const done = () => {
|
||||
el.removeEventListener(event, done);
|
||||
resolve();
|
||||
/**
|
||||
* Resolves `true` when the event fires, `false` if the budget runs out. The
|
||||
* distinction is the caller's to act on: a missing `seeked` is cosmetic, a
|
||||
* missing `canplay` means the reload failed.
|
||||
*/
|
||||
private waitForEvent(
|
||||
el: HTMLVideoElement,
|
||||
event: string,
|
||||
timeoutMs: number
|
||||
): Promise<boolean> {
|
||||
return new Promise<boolean>((resolve) => {
|
||||
let timer: ReturnType<typeof setTimeout>;
|
||||
const done = (fired: boolean) => {
|
||||
el.removeEventListener(event, listener);
|
||||
clearTimeout(timer);
|
||||
resolve(fired);
|
||||
};
|
||||
el.addEventListener(event, done);
|
||||
setTimeout(done, timeoutMs);
|
||||
const listener = () => done(true);
|
||||
el.addEventListener(event, listener);
|
||||
timer = setTimeout(() => done(false), timeoutMs);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user