From 7e23da46e240ca8dea9faa148944ba7040a11891 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sun, 23 Aug 2026 10:13:43 +0200 Subject: [PATCH] fix(player): a junk duration from an engine must not panic the backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DR-252, and a regression I introduced in DR-245. `Duration::from_secs_f64` panics on a negative or non-finite value. The old PlayerBackend contract passed durations around as a bare Option and never promised otherwise, so junk flowed through harmlessly. LegacyPlayer converts that value to a Duration on the way into the MediaPlayer contract, which turned it into a hard panic. ExoPlayer reports C.TIME_UNSET — Long::MIN_VALUE, about -9.2e15 seconds — for any stream whose length it does not know. That is every background-audio handoff: /Audio/{id}/universal is a chunked, length-less transcode. So the panic fired exactly when the handoff started, killed the Rust backend mid-swap, and left a black screen with no controls. Caught on the device, in the user's own repro sequence: enable background audio, background the app, come back. Not by any suite — the conformance cases run against engines that report sane numbers, and nothing was asking what happens when one does not. One guard on the contract now, used by every engine crossing into it, rather than each adapter deciding for itself. mpv had the same unguarded conversion for its duration property and would have hit it the moment libmpv reported something odd. UT-222 pins the values: TIME_UNSET as seconds, negatives, zero, NaN and both infinities yield no duration; a real runtime survives. 791 Rust tests, mpv conformance still 9/9, clippy clean both ways. --- docs/requirements.md | 2 ++ src-tauri/src/player/legacy_player.rs | 8 +++-- src-tauri/src/player/media_player.rs | 46 +++++++++++++++++++++++++++ src-tauri/src/player/mpv_player.rs | 6 ++-- 4 files changed, 57 insertions(+), 5 deletions(-) diff --git a/docs/requirements.md b/docs/requirements.md index 23cc2f8e..d6d2ca3f 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -445,6 +445,7 @@ Internal architecture, components, and application logic. | 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-251 | A duration of zero is treated as "the engine does not know yet", and falls back to the runtime the item already carries. ExoPlayer reports `C.TIME_UNSET` until it resolves one and `JellyTauPlayer.getDuration()` maps that to `0.0`, so the engine answered `Some(0.0)` rather than `None` — which satisfied every "unknown duration" fallback and left the seek bar with no scale. It presented as scrubbing being broken rather than as a duration that never arrived, and the catalog had the runtime the whole time | Player | UR-005, UR-040 | Done | +| DR-252 | Seconds reported by an engine are converted to a `Duration` only when finite and positive. `Duration::from_secs_f64` panics on a negative or non-finite value and no engine promises otherwise: ExoPlayer reports `C.TIME_UNSET` (`Long::MIN_VALUE`, about -9.2e15) for a stream whose length it does not know, which is every background-audio handoff — `/Audio/{id}/universal` is a chunked, length-less transcode. Held as a float that junk was harmless; converted to a `Duration` by the `MediaPlayer` adapter it became a panic that killed the backend mid-handoff and left a black screen with no controls. One guard on the contract, used by every engine crossing into it | Player | 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 `