diff --git a/backend/app/config.py b/backend/app/config.py index bd916a90..62acd26e 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -265,7 +265,7 @@ class Settings(BaseSettings): # --- Transcode (playable="transcode") re-encode tuning (E6 S1) --- # Hardware-encoder policy: "auto" = VAAPI (h264_vaapi) when the iGPU render node is present AND the # source codec is hardware-decodable, else libx264 software; "on" = force VAAPI wherever the render - # node exists (skip the decodability gate); "off" = always software. See plex/stream.py `_want_hw`. + # node exists (skip the decodability gate); "off" = always software. See plex/stream.py `_select_encode`. plex_transcode_hw: str = "auto" # Software-fallback libx264 tuning (IGNORED on the VAAPI path). `preset` trades CPU for size; `crf` # trades quality for size (lower = better + bigger). `max_height` caps the SOFTWARE transcode's output diff --git a/backend/app/plex/stream.py b/backend/app/plex/stream.py index 4685938c..f404fac0 100644 --- a/backend/app/plex/stream.py +++ b/backend/app/plex/stream.py @@ -145,7 +145,7 @@ _HLS_TAIL = [ # --- Transcode (playable="transcode") video re-encode ----------------------------------------- # The remux path stream-copies the video (`-c:v copy`); a non-h264 source can't be copied and must be -# re-encoded to H.264. Two encoders, chosen per-session by `_want_hw`: +# re-encoded to H.264. The encoder + tonemap are chosen per-session by `_select_encode`: # * VAAPI hardware (`h264_vaapi`) — the prod default once the iGPU is passed in. A full GPU pipeline # (decode + scale + encode all on-device) offloads the CPU almost entirely, so several concurrent # 1080p transcodes are fine and the API stays responsive. @@ -382,8 +382,8 @@ def _select_encode(db: DbSession, item: PlexItem, transcode: bool, src: Path) -> def _want_hw(db: DbSession, item: PlexItem, transcode: bool, src: Path) -> bool: - """The HW gate on its own (the `hw` half of `_select_encode`). Kept as a thin wrapper for the - callers/tests that only care whether a session runs on the GPU at all.""" + """The HW gate on its own (the `hw` half of `_select_encode`). A readability shim for the tests that + assert only whether a session runs on the GPU (production decides via `_select_encode` directly).""" return _select_encode(db, item, transcode, src)[0] @@ -587,7 +587,15 @@ def start_session( # multi-audio: the video rendition is variant v0 → seg_{i}_0.ts; single-audio → seg_{i}.ts return directory / (f"seg_{i}_0.ts" if is_multi else f"seg_{i}.ts") - def _spawn(hw: bool, tonemap: bool = False) -> HlsSession: + def _spawn( + hw: bool, tonemap: bool = False, *, replace_only: "HlsSession | None" = None + ) -> "HlsSession | None": + # `replace_only` makes the supersede identity-aware for the runtime fallback: the whole + # check-and-replace happens under ONE _lock, so a newer same-key session (a concurrent + # seek-restart) installed between the fallback's decision and here is never clobbered. When the + # registered session is no longer `replace_only`, we abort and return None (the caller yields to + # whoever is registered now). The INITIAL spawn passes replace_only=None and always supersedes — + # that is the seek-restart's intended last-writer-wins. # Software-fallback encode tuning — read only on the SW transcode path (the VAAPI path and # remux ignore preset/crf/max_height, and remux must not touch the DB for them). preset, crf, max_height = "veryfast", 20, 0 @@ -600,6 +608,8 @@ def start_session( # platform without fork (the API always runs in a Linux container in prod) it's simply omitted. preexec = _sw_child_preexec if (transcode and not hw and os.name == "posix") else None with _lock: + if replace_only is not None and _sessions.get(key) is not replace_only: + return None # superseded/culled while we waited out of lock — don't clobber the newer one old = _sessions.pop(key, None) if old is not None: _kill(old) @@ -647,22 +657,21 @@ def start_session( if not ready and s.hw: # We waited on _seg(0) OUTSIDE _lock, so `s.proc` dying is ambiguous: it may have failed on its # own (VAProfile/init/tonemap refusal), OR a concurrent same-key _spawn (a rapid seek-restart) or - # _enforce_cap may have killed it to install/keep a newer session. Only fall back if THIS session - # is still the registered one — otherwise respawning would tear down the newer session and (via - # _spawn's supersede + _enforce_cap) breach the transcode cap. If superseded, yield to whoever is - # registered now. (A tiny window remains between this check and _spawn's own lock; that just makes - # the fallback lose to an even-newer seek, which is the correct seek-restart outcome anyway.) - with _lock: - superseded = _sessions.get(key) is not s - if superseded: - return current_session(key) - # Runtime HW→SW fallback: respawn on libx264 (no HW, no tonemap) so a failed GPU session never - # strands playback; the software path CPU-decodes and downconverts every source. s.hw is now - # False, so the route honestly reports "CPU" and the player's Preparing-stream state stays correct. + # _enforce_cap may have killed it to install/keep a newer session. Respawn on libx264 (no HW, no + # tonemap) so a genuinely-failed GPU session never strands playback — but pass `replace_only=s` so + # the respawn is a no-op if we've been superseded: the identity check and the pop+replace share + # ONE lock in _spawn, so we can never tear down the newer session. s.hw is now False, so the route + # honestly reports "CPU" and the player's Preparing-stream state stays correct. log.warning( "plex transcode: HW session produced no segments — falling back to software key=%s", key ) - s = _spawn(False, False) + respawned = _spawn(False, False, replace_only=s) + if respawned is None: + # Superseded/culled mid-fallback — yield the session registered now (a newer seek-restart), or + # our own now-dead session if the key was cap-culled (its segment requests degrade to the + # route's retryable "not available yet", never a dishonest "file not found" from a None). + return current_session(key) or s + s = respawned ready = wait_for(_seg(0), timeout=25.0, proc=s.proc) if ready: # For a seek (start_s>0) learn the REAL start offset K from the first segment's absolute PTS, diff --git a/backend/tests/test_plex_stream.py b/backend/tests/test_plex_stream.py index fb300923..39be017f 100644 --- a/backend/tests/test_plex_stream.py +++ b/backend/tests/test_plex_stream.py @@ -590,9 +590,9 @@ class TestTranscodeStartSession: (cmd, *_), kwargs = _CapturingProc.calls[0] assert _venc(cmd) == "h264_vaapi" assert "libx264" not in cmd - assert ( - "-vf" not in cmd or cmd[cmd.index("-vf") + 1] == "scale_vaapi=format=nv12" - ) # no SW cap + # Plain-HW path unconditionally emits the nv12 downconvert (a disjunction on "-vf not in cmd" + # would silently pass if the filter were dropped — assert it is actually present). + assert cmd[cmd.index("-vf") + 1] == "scale_vaapi=format=nv12" # no SW cap assert kwargs.get("preexec_fn") is None # GPU encode is cheap — never niced def test_hdr_tonemap_transcode_threads_the_opencl_graph_and_flags_the_session( @@ -704,6 +704,41 @@ class TestTranscodeStartSession: assert s is newer # yielded the currently-registered session, did not clobber it assert stream._sessions["12345"] is newer + def test_a_cap_culled_hw_session_does_not_respawn_and_is_not_reported_missing( + self, capturing_env, monkeypatch + ): + # F2/race, the cull sub-case: while we wait on seg_0 out of lock, cross-key cap pressure lets + # _enforce_cap evict THIS session's key entirely (no replacement). The fallback must still not + # respawn (that would re-breach the cap the cull just enforced), and must NOT return None — a None + # is turned by the route into a dishonest "file not found" 404 for a file that exists. It yields + # our own (now-dead) session instead, so the client's segment requests degrade to the retryable + # "not available yet" path. + monkeypatch.setattr( + stream, "_select_encode", lambda db, item, transcode, src: (True, False) + ) + captured = {} + + def wait_then_cull(path, timeout=20.0, proc=None): + captured["s"] = stream._sessions[ + "12345" + ] # the HW session the initial _spawn just registered + del stream._sessions[ + "12345" + ] # simulate _enforce_cap dropping the LRU under cap pressure + return False + + monkeypatch.setattr(stream, "wait_for", wait_then_cull) + s = stream.start_session( + db=None, item=_FakeItem(codec_video="hevc"), start_s=0.0, transcode=True + ) + assert ( + len(_CapturingProc.calls) == 1 + ) # no SW respawn — the cull is respected, cap not re-breached + assert ( + s is captured["s"] + ) # our own session yielded, NOT None (which the route would 404 as missing) + assert "12345" not in stream._sessions # the culled key stays gone + def test_remux_reads_no_transcode_knobs_and_stream_copies(self, capturing_env, monkeypatch): # A remux start must stream-copy the video and spawn no niced child (the transcode knobs and # the preexec nice are gated behind `transcode=True`). @@ -790,9 +825,21 @@ class TestOpenclPresent: assert stream._opencl_present() is False def test_false_and_no_raise_when_the_dir_is_missing(self, tmp_path, monkeypatch): + # A missing dir: Path.glob yields nothing (no raise) → False. monkeypatch.setattr(stream, "_OPENCL_ICD_DIR", tmp_path / "nope") assert stream._opencl_present() is False + def test_false_when_glob_raises_oserror(self, monkeypatch): + # The `except OSError` guard proper: an unreadable ICD dir (permission denied) makes glob RAISE, + # not return empty. That must be swallowed to False, never propagated up through _select_encode + # into start_session (which would crash HDR routing). + class _BoomDir: + def glob(self, pattern): + raise OSError("permission denied") + + monkeypatch.setattr(stream, "_OPENCL_ICD_DIR", _BoomDir()) + assert stream._opencl_present() is False + class TestWaitForProcFastFail: """wait_for(proc=…) must stop as soon as the ffmpeg proc exits (a VAAPI init refusal dies in ~1s