From 5b5162dd1e77bf812422f5963c7c503ef42906de Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sun, 23 Aug 2026 09:02:30 +0200 Subject: [PATCH] fix(player): closing the player stops every renderer, not the believed one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DR-250. The user's diagnosis, and a better fix than modelling the handoff more carefully: a close that stops only what we believe is playing is fragile by construction. A close that stops everything is correct whatever the bookkeeping thinks. Two places it did not. The teardown's stop was gated on `didStartNativePlayback && !didStopBackendEarly` — flags describing what *this component* started. A background-audio handoff swaps the renderer underneath them, so after one they describe a player that is no longer making sound and the stop was skipped entirely. The audio stream kept running and the mini player adopted it, which is exactly why a movie reappeared as an audio track. It is unconditional now; `playerStop` is idempotent, so the cost of calling it when nothing plays is a no-op round trip, against the alternative of silently leaving audio running. And `PlayerController::stop` never cleared the handoff. Leaving the base offset and the active flag behind lets a later position read be interpreted against a handoff that no longer exists. Stopping now clears both. `didStartNativePlayback` had no remaining reader and is deleted rather than silenced — dead bookkeeping about which renderer was in charge is precisely the frontend playback state this contract is meant to remove, and the eslint ratchet caught it going one over. The underlying unconfirmed state swap is still there and still worth fixing — it is written up in media-player-controller.md. This makes the symptom impossible while that lands. 789 Rust tests, 1088 frontend, lint back at 158, clippy clean both ways. --- docs/requirements.md | 1 + src-tauri/src/player/mod.rs | 22 ++++++++++++++ src/lib/components/player/VideoPlayer.svelte | 32 ++++++++++++-------- 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/docs/requirements.md b/docs/requirements.md index 6f0c1ddf..0c5f7b92 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -443,6 +443,7 @@ Internal architecture, components, and application logic. | DR-245 | `PlayerController` holds a `MediaPlayer` rather than a `PlayerBackend`, and every engine reaches it through that one contract — `LegacyPlayer` carries the not-yet-ported ones across unchanged, so the port swaps a seam rather than four implementations. Loading an item is now a single `open` carrying its start position, and the controller maps the engine's `Phase` back onto `PlayerState` using the queue, so nothing outside changes. `LegacyPlayer` drives the old `PlayerBackend` through the `MediaPlayer` contract, so engines not yet ported keep working during the migration and the two designs can be compared on one engine and one file. It reproduces the old load-then-play-then-seek sequence faithfully rather than a fixed-up version, because making it pass would defeat its purpose | Player | UR-081 | Done | | DR-246 | The seek strategy turns on an ability the engine declares, not on the container the stream arrives in. `Capabilities::seeks_transcoded_in_place` is stated by each engine — true for hls.js, which seeks within the VOD playlist it was handed; false for mpv, which cannot make the server transcode from a new offset — and the command asks the engine currently rendering instead of inferring from `is_hls` and `use_html5`. The item's transport is no longer read at the seek site at all. Re-negotiating a stream needs the repository, which sits above the engine, so the engine states the capability and the caller acts on it rather than the engine owning the whole decision | Player | UR-040, UR-081 | Done | | DR-247 | ExoPlayer can be told where to start. `JellyTauPlayer.load(url, mediaId)` had no way to express a start position, so every caller loaded and then seeked; the position is now handed to ExoPlayer with the media item via `setMediaItem(item, startPositionMs)`, and the two-argument form delegates to it. Running the conformance cases on a device also settled which half of DR-241 was engine-specific: ExoPlayer already queues a seek issued before `prepare()` completes, so it never had the lost-seek defect mpv did — only the missing vocabulary for a start position | Player | UR-081, UR-005 | Done | +| DR-250 | Stopping means nothing is playing, from any renderer — not "whatever we believe owns playback has been asked to stop". A background-audio handoff swaps which renderer that is, and the swap is bookkeeping that can be mid-flight: `exit_background_audio` marks the webview element the player again the moment it is called, while the element has not reloaded. The teardown's stop was gated on flags describing what the component started, so after a handoff it described a player that was no longer making sound and the stop was skipped — the audio stream kept running and the mini player adopted it, which is why a movie reappeared as an audio track. The stop is now unconditional (it is idempotent) and clears the handoff base and flag, so a later position read cannot be interpreted against a handoff that no longer exists | Player | UR-040, UR-005 | Done | | DR-198 | The webview runs under a real Content-Security-Policy, and the asset protocol is scoped to the one directory it still serves. `csp` was `null`, which disables CSP entirely: any script that reached the web layer — through a future `{@html}`, a dependency, or a devtools paste — would have inherited the whole IPC surface, and with it the user's session. `script-src 'self'` (Tauri injects a nonce for SvelteKit's inline bootstrap script at build time, so no `'unsafe-inline'` is needed) plus `object-src`/`frame-src 'none'` and `base-uri 'self'` is the part that is genuinely restrictive. `img-src`/`media-src`/`connect-src` cannot be: the Jellyfin origin is typed in by the user at run time and is commonly plain `http` on a LAN, so they allow `http:`/`https:` — a wide grant for *data*, but one that still bars `file:`, `filesystem:` and scripting schemes, and leaves `script-src` untouched. `style-src` keeps `'unsafe-inline'` because Svelte compiles `style="…"` attributes (including `app.html`'s `display: contents` wrapper) into markup; this is safe only while no `