diff --git a/backend/app/plex/stream.py b/backend/app/plex/stream.py index 224ff5a9..6ab7a95d 100644 --- a/backend/app/plex/stream.py +++ b/backend/app/plex/stream.py @@ -758,6 +758,36 @@ def current_session(key: str) -> HlsSession | None: return s +# A segment file, single-audio (`seg_12.ts`) or one rendition of a multi-audio session +# (`seg_12_0.ts`). ffmpeg writes them strictly in order, which is what makes `segment_may_arrive` +# exact rather than a heuristic. +_SEG_NAME_RE = re.compile(r"^seg_(\d+)(_\d+)?\.ts$") + + +def segment_may_arrive(directory: Path, filename: str) -> bool: + """Whether waiting for `filename` can possibly end in a file. + + ffmpeg produces segments sequentially, so a segment whose PREDECESSOR does not exist yet is not + the next one — it is somewhere beyond the generated region, and no amount of waiting will + conjure it. Saying so at once matters because the route that waits is a SYNC one: it holds a + thread off the ~40-slot pool, and a pooled DB connection with it, for the whole timeout. An + ordinary seek past the produced region was enough to park one for 20 s, and forty concurrent + requests for a name that will never exist parked the whole API (deep-review round 5). + + Anything that is not a segment (a playlist, a WebVTT track) is left to the timeout: those are + rewritten continuously and have no ordering to read.""" + m = _SEG_NAME_RE.match(filename) + if m is None: + return True + idx = int(m.group(1)) + if idx == 0: + return True + try: + return (directory / f"seg_{idx - 1}{m.group(2) or ''}.ts").exists() + except OSError: + return False + + def wait_for(path: Path, timeout: float = 20.0, proc: subprocess.Popen | None = None) -> bool: """Wait until a session file (playlist / segment) exists and is non-empty. Segments are produced ~faster than realtime, so a segment just ahead of playback appears quickly; a segment diff --git a/backend/app/routes/plex/playback.py b/backend/app/routes/plex/playback.py index 769bf8bd..5cd12387 100644 --- a/backend/app/routes/plex/playback.py +++ b/backend/app/routes/plex/playback.py @@ -656,6 +656,12 @@ def stream_hls_file( if s is None or s.id != session_id: raise HTTPException(status_code=404, detail="No active playback session") path = s.dir / filename + # A segment beyond the generated region can never arrive: ffmpeg writes them in order, so if the + # predecessor is not there this one is not next. Answering now instead of waiting it out is what + # keeps a seek past the produced region — or a deliberate flood of hopeless names — from parking + # threadpool workers and their DB connections (see `segment_may_arrive`). + if not plex_stream.segment_may_arrive(s.dir, filename): + raise HTTPException(status_code=404, detail="Not available yet") timeout = 30 if ext == ".m3u8" else 20 # `proc` so a session whose ffmpeg has already exited answers at once instead of polling out the # whole timeout: this is a SYNC route on the ~40-slot threadpool, holding a pooled DB connection diff --git a/backend/tests/test_plex_hls_file_route.py b/backend/tests/test_plex_hls_file_route.py index 8be286c7..17c3ab72 100644 --- a/backend/tests/test_plex_hls_file_route.py +++ b/backend/tests/test_plex_hls_file_route.py @@ -24,8 +24,11 @@ from app.plex import stream as plex_stream class _Proc: + def __init__(self, running: bool = False): + self.running = running + def poll(self): - return 0 # finished: `wait_for` may not poll out its whole timeout on a missing file + return None if self.running else 0 def terminate(self): pass @@ -35,10 +38,10 @@ class _Proc: class _Session: - def __init__(self, session_id: str, directory): + def __init__(self, session_id: str, directory, running: bool = False): self.id = session_id self.dir = directory - self.proc = _Proc() + self.proc = _Proc(running) self.last_access = time.time() @@ -75,6 +78,7 @@ def _client(db, monkeypatch): def test_a_segment_is_served_from_the_session_the_url_names(db, monkeypatch, tmp_path, sessions): d = tmp_path / "m1_0_0_+0.00" d.mkdir() + (d / "seg_11.ts").write_bytes(b"") # its predecessor, so seg_12 is the next one (d / "seg_12.ts").write_bytes(b"mine") sessions["m1"] = _Session("sid-a", d) client = _client(db, monkeypatch) @@ -93,6 +97,7 @@ def test_a_segment_of_a_replaced_session_is_a_404_not_the_new_sessions_bytes( next segment must fail honestly — the player reclaims at the playhead — not arrive as B's.""" theirs = tmp_path / "m1_1500_0_+0.00" theirs.mkdir() + (theirs / "seg_11.ts").write_bytes(b"") (theirs / "seg_12.ts").write_bytes(b"theirs") sessions["m1"] = _Session("sid-b", theirs) client = _client(db, monkeypatch) @@ -127,12 +132,15 @@ def test_a_playlist_with_no_live_session_is_a_404_not_a_silent_restart_at_zero( def test_a_segment_that_will_never_arrive_fails_at_once(db, monkeypatch, tmp_path, sessions): """`stream_hls_file` is a SYNC route: it runs on the ~40-slot threadpool and holds a pooled DB - connection while it waits. Waiting without the producer's handle polled out the whole 20 s even - when that ffmpeg had already exited and no segment could ever appear — one ordinary seek past the - produced region parked a worker and a connection for 20 s (deep-review round 5).""" + connection while it waits. It used to poll out the whole 20 s for a name that could never appear + — an ordinary seek past the produced region was enough to park a worker and a connection, and + forty hopeless names parked the API (deep-review round 5). ffmpeg writes segments in order, so a + segment whose predecessor is absent is not the next one, and that is knowable at once — even + while ffmpeg is still running, which is the case `proc` alone cannot shorten.""" d = tmp_path / "m1_0_0_+0.00" d.mkdir() - sessions["m1"] = _Session("sid-a", d) # its _Proc.poll() reports a finished ffmpeg + (d / "seg_0.ts").write_bytes(b"x") # the session is producing; seg_99999 is simply not next + sessions["m1"] = _Session("sid-a", d, running=True) client = _client(db, monkeypatch) started = time.monotonic() try: diff --git a/frontend/src/components/ui/dialogFocus.ts b/frontend/src/components/ui/dialogFocus.ts index 25626aa0..f52e5b3a 100644 --- a/frontend/src/components/ui/dialogFocus.ts +++ b/frontend/src/components/ui/dialogFocus.ts @@ -1,4 +1,4 @@ -import { useEffect, useState, type RefObject } from "react"; +import { useEffect, useRef, useState, type RefObject } from "react"; // The focus half of the dialog contract, in ONE place. `ui/Modal` owns it for every dialog that goes // through the shell; the onboarding wizard renders its own shell (a bare `ui/Overlay`, because it is @@ -54,19 +54,47 @@ export function useDialogChrome(scopeRef: RefObject): void { // later (in the effect) would capture a now-inside element that unmounts on close, stranding focus // on . useState initializer runs exactly once, on first render. const [restoreTo] = useState(() => document.activeElement as HTMLElement | null); + // What the dialog is focused ON, so a RE-mount can put the focus back where it was rather than on + // the first control. React 18's StrictMode runs every effect mount→cleanup→mount in development: + // the cleanup hands focus back to the trigger, and the second mount then found the focus outside + // the dialog and moved it to the first focusable — taking it off the `autoFocus` field a beat + // after the browser had put it there. Measured in Chrome: the command palette opened with the + // focus on its ✕ in dev and on its search field in prod, from the same code (round 5). + const focusedRef = useRef(null); + // Whether the dialog still holds the focus. Read at cleanup rather than computed there: by the + // time a passive cleanup runs on a real unmount the panel is already out of the document and the + // focus has fallen to , so "does the panel contain activeElement" answers false for both a + // close and a user who clicked away — and those two want opposite things. + const heldRef = useRef(true); useEffect(() => { const prev = document.body.style.overflow; document.body.style.overflow = "hidden"; // Initial focus: unless an autoFocus child already placed it inside (e.g. a rename input), move - // focus to the first control so the dialog opens focused, not stranded on the background. + // focus to where this dialog was focused before — or, the first time, to the first control, so + // the dialog opens focused rather than stranded on the background. const scope = scopeRef.current; if (scope && !scope.contains(document.activeElement)) { - (focusablesIn(scope)[0] ?? scope).focus(); + const back = focusedRef.current; + const target = + (back?.isConnected && scope.contains(back) ? back : null) ?? + focusablesIn(scope)[0] ?? + scope; + target.focus(); } + const onFocusIn = () => { + const inside = scopeRef.current?.contains(document.activeElement) ?? false; + heldRef.current = inside; + if (inside) focusedRef.current = document.activeElement as HTMLElement; + }; + onFocusIn(); + document.addEventListener("focusin", onFocusIn); return () => { + document.removeEventListener("focusin", onFocusIn); document.body.style.overflow = prev; - // Restore focus to the trigger, if it's still in the document and focusable. - if (restoreTo && restoreTo.isConnected) restoreTo.focus(); + // Restore focus to the trigger, if it's still in the document and focusable — and only if the + // dialog still held the focus. Once the user has moved it elsewhere, taking it back is the + // rudest thing a closing dialog can do. + if (heldRef.current && restoreTo && restoreTo.isConnected) restoreTo.focus(); }; }, [restoreTo, scopeRef]); }