fix(player): a stale session answer can no longer stop the playback that won
- The engine tracks the request out per item: A, then B, then back to A sent a second request for A beside the one still out. - A hardware transcode superseded during its fallback answered with the NEWER session; the losing request's player then stopped it as stale. It now answers with its own (dead) session. - A replaced hls.js instance's late manifest no longer marks the player ready or plays the outgoing media. - A pause given after the answer stands: the manifest and the first canplay read the live intent. - The debounced A/V-offset restart is dropped on an item change instead of restarting the previous item under the next one's title.
This commit is contained in:
@@ -840,12 +840,13 @@ def start_session(
|
|||||||
)
|
)
|
||||||
respawned = _spawn(False, False, replace_only=s)
|
respawned = _spawn(False, False, replace_only=s)
|
||||||
if respawned is None:
|
if respawned is None:
|
||||||
# Superseded/culled mid-fallback — yield whoever is registered now (a newer seek-restart), or
|
# Superseded/culled mid-fallback — answer with our own now-dead session. Never the newer one
|
||||||
# our own now-dead session if the key was cap-culled. Never None here: the POST /session route
|
# registered meanwhile: this answer goes to the request that LOST, whose player drops it as
|
||||||
# turns a None return into a "Media file not found on disk" 404 — dishonest for a file that
|
# stale and stops the session it names (`stop_session`), which stopped the playback that won.
|
||||||
# exists. A (dead) session instead degrades to a retryable 404 ("No active playback session" /
|
# Never None either: the POST /session route turns a None return into a "Media file not found
|
||||||
# "Not available yet") the client re-requests.
|
# on disk" 404 — dishonest for a file that exists. A dead session instead degrades to a
|
||||||
return current_session(user_id, key) or s
|
# retryable 404 ("No active playback session" / "Not available yet") the client re-requests.
|
||||||
|
return s
|
||||||
s = respawned
|
s = respawned
|
||||||
ready = wait_for(_seg(0), timeout=25.0, proc=s.proc)
|
ready = wait_for(_seg(0), timeout=25.0, proc=s.proc)
|
||||||
if ready:
|
if ready:
|
||||||
|
|||||||
@@ -695,7 +695,9 @@ class TestTranscodeStartSession:
|
|||||||
# F2/race: start_session waits on seg_0 OUTSIDE _lock. If a concurrent same-key start (a rapid
|
# 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
|
# 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.
|
# 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(
|
monkeypatch.setattr(
|
||||||
stream, "_select_encode", lambda db, item, transcode, src: (True, False)
|
stream, "_select_encode", lambda db, item, transcode, src: (True, False)
|
||||||
)
|
)
|
||||||
@@ -708,9 +710,12 @@ class TestTranscodeStartSession:
|
|||||||
|
|
||||||
newer = _Newer()
|
newer = _Newer()
|
||||||
|
|
||||||
|
captured = {}
|
||||||
|
|
||||||
def wait_then_supersede(path, timeout=20.0, proc=None):
|
def wait_then_supersede(path, timeout=20.0, proc=None):
|
||||||
# Simulate the concurrent replacement happening during our out-of-lock wait, then report the
|
# Simulate the concurrent replacement happening during our out-of-lock wait, then report the
|
||||||
# (now-killed) HW proc produced nothing.
|
# (now-killed) HW proc produced nothing.
|
||||||
|
captured["s"] = stream._sessions[(7, "12345")]
|
||||||
stream._sessions[(7, "12345")] = newer
|
stream._sessions[(7, "12345")] = newer
|
||||||
return False
|
return False
|
||||||
|
|
||||||
@@ -720,7 +725,7 @@ class TestTranscodeStartSession:
|
|||||||
)
|
)
|
||||||
# No SW respawn — only the initial HW _spawn ran; the newer session was left intact.
|
# No SW respawn — only the initial HW _spawn ran; the newer session was left intact.
|
||||||
assert len(_CapturingProc.calls) == 1
|
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
|
assert stream._sessions[(7, "12345")] is newer
|
||||||
|
|
||||||
def test_a_cap_culled_hw_session_does_not_respawn_and_is_not_reported_missing(
|
def test_a_cap_culled_hw_session_does_not_respawn_and_is_not_reported_missing(
|
||||||
|
|||||||
@@ -515,3 +515,56 @@ it("tells React only when the snapshot really changed", async () => {
|
|||||||
expect(seen.mock.calls.length).toBe(after);
|
expect(seen.mock.calls.length).toBe(after);
|
||||||
await asks[0]!.resolve(direct("A"));
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -164,7 +164,9 @@ export class HlsEngine {
|
|||||||
private attachedId: string | null = null;
|
private attachedId: string | null = null;
|
||||||
// The attached session's play intent, which stands until it is ready (see `wantsPlay`).
|
// The attached session's play intent, which stands until it is ready (see `wantsPlay`).
|
||||||
private attachedIntent: Intent | null = null;
|
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<string, Request>();
|
||||||
private queued: Request | null = null;
|
private queued: Request | null = null;
|
||||||
// How often a session the server answered 404 for may be reclaimed (see `onHlsError`).
|
// How often a session the server answered 404 for may be reclaimed (see `onHlsError`).
|
||||||
private reclaims = { n: 0, at: 0 };
|
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.
|
// stale "Preparing stream…" for a session not known yet — set honestly from the answer.
|
||||||
this.set({ ready: false, error: null, transcode: false });
|
this.set({ ready: false, error: null, transcode: false });
|
||||||
const req: Request = { id, startAt, intent, gen };
|
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);
|
else this.send(req);
|
||||||
}
|
}
|
||||||
|
|
||||||
private send(req: Request): void {
|
private send(req: Request): void {
|
||||||
this.queued = null;
|
this.queued = null;
|
||||||
this.inFlight = req;
|
this.inFlight.set(req.id, req);
|
||||||
void this.deps.requestSession(req.id, req.startAt, this.deps.params()).then(
|
void this.deps.requestSession(req.id, req.startAt, this.deps.params()).then(
|
||||||
(sess) => this.answered(req, sess),
|
(sess) => this.answered(req, sess),
|
||||||
(e: unknown) => this.failed(req, e),
|
(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
|
/** The in-flight request settled: the queued one goes now, unless it is stale too (the viewer
|
||||||
* left the item, or the player closed). */
|
* left the item, or the player closed). */
|
||||||
private settledInFlight(req: Request): void {
|
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;
|
const next = this.queued;
|
||||||
if (!next || this.inFlight) return;
|
if (!next || this.inFlight.has(next.id)) return;
|
||||||
this.queued = null;
|
this.queued = null;
|
||||||
if (this.alive && next.gen === this.gen) this.send(next);
|
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
|
// 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
|
// 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
|
// 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", () => {
|
this.listen(media, req, "canplay", () => {
|
||||||
if (intent.play && media.paused) media.play().catch(() => {});
|
if (intent.play && media.paused) media.play().catch(() => {});
|
||||||
});
|
});
|
||||||
@@ -397,9 +399,14 @@ export class HlsEngine {
|
|||||||
|
|
||||||
private hlsHandlers(req: Request, media: HTMLMediaElement): HlsHandlers {
|
private hlsHandlers(req: Request, media: HTMLMediaElement): HlsHandlers {
|
||||||
return {
|
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: () => {
|
onManifestParsed: () => {
|
||||||
|
if (!this.current(req)) return;
|
||||||
this.set({ ready: true });
|
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
|
// 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
|
// 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();
|
if (!want && !media.paused) media.pause();
|
||||||
return true;
|
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(() => {});
|
if (want) media.play().catch(() => {});
|
||||||
else media.pause();
|
else media.pause();
|
||||||
return false;
|
return false;
|
||||||
|
|||||||
@@ -402,3 +402,40 @@ describe("usePlexTracks — a <track> whose own VTT fetch failed", () => {
|
|||||||
expect(harness.current!.subNotice).toBeNull();
|
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<HTMLVideoElement>(),
|
||||||
|
audioRef: { current: 0 } as RefObject<number | null>,
|
||||||
|
aoffRef: { current: 0 },
|
||||||
|
playhead: () => 600,
|
||||||
|
}),
|
||||||
|
{
|
||||||
|
wrapper: ({ children }: { children: ReactNode }) => (
|
||||||
|
<QueryClientProvider client={qc}>{children}</QueryClientProvider>
|
||||||
|
),
|
||||||
|
initialProps: { id: RK },
|
||||||
|
},
|
||||||
|
);
|
||||||
|
act(() => result.current.changeAudioOffset(0.2));
|
||||||
|
rerender({ id: "next-episode" });
|
||||||
|
act(() => void vi.advanceTimersByTime(1000));
|
||||||
|
expect(restart).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -723,6 +723,16 @@ export function usePlexTracks(opts: {
|
|||||||
},
|
},
|
||||||
[patchPrefs, restartSession, aoffRef, playhead],
|
[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? -------------------------------------------
|
// --- is the selected track actually extractable yet? -------------------------------------------
|
||||||
// Pulling one subtitle track out of a big MKV means ffmpeg streams the whole container: measured on
|
// Pulling one subtitle track out of a big MKV means ffmpeg streams the whole container: measured on
|
||||||
|
|||||||
Reference in New Issue
Block a user