From 3b91922ccab686487679b5dbd5e44499ba21bf0a Mon Sep 17 00:00:00 2001
From: Duncan Tourolle <duncan@tourolle.paris>
Date: Sat, 22 Aug 2026 21:17:50 +0200
Subject: [PATCH] feat(player): the MediaPlayer contract
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

DR-242. Intent, not device operations.

`open` carries the start position, so no caller sequences load-then-seek and
none can race an engine's asynchronous load — the engine is the only layer
that knows when its pipeline can accept a position, and it absorbs that
internally by deferring or re-opening.

`seek` states a destination and nothing else. Whether that is an in-place seek
or a re-opened stream is the engine's business: hls.js seeks within a VOD
playlist, mpv's HLS demuxer cannot make a server transcode from a new offset.
Callers stop guessing on behalf of engines they do not own.

`snapshot` is one coherent read rather than a dozen getters, because reading
position and duration separately is how a player reported <position> / 0.0
when a file unloaded between the two calls.

`Phase::Opening` names the state the previous design could not express, and is
the direct cause of DR-241: a seek arriving with nothing loaded had no phase
to be queued against, so it was discarded.

`Capabilities` exists so callers adapt without naming engines. If a caller
ever branches on which engine it holds, this struct is missing something —
engine identity leaking into callers is the coupling DR-238 came from.

Nothing consumes it yet; PlayerController is ported in DR-245. Carries an
explicit allow(dead_code) tied to that step rather than being hidden behind
cfg(test), because it is production code being built in shippable pieces.
---
 docs/requirements.md                 |  11 ++
 src-tauri/src/player/media_player.rs | 206 +++++++++++++++++++++++++++
 2 files changed, 217 insertions(+)
 create mode 100644 src-tauri/src/player/media_player.rs

diff --git a/docs/requirements.md b/docs/requirements.md
index c994dc87..96d76375 100644
--- a/docs/requirements.md
+++ b/docs/requirements.md
@@ -90,6 +90,7 @@ For a narrative overview of the system design, see
 | UR-078 | JellyTau keeps a record of what it did, and can hand it over. The app forgot everything the moment it exited: the backend logged to stdout only — which a user launching from a desktop icon never sees, and which on Android is not logcat, so the Rust half was invisible on the platform carrying the hardest bugs. A crash left nothing at all. Logs are now written to a size-capped rotating file, a panic is recorded before the process dies, the frontend's messages land in the same timeline as the backend's, and Settings exports the lot as one file to attach to a bug report. Nothing is transmitted anywhere — the user attaches it themselves, which is also what keeps this from being telemetry. Access tokens and passwords never reach the file | Medium | Done |
 | UR-079 | The app decides *what stream to play* and says so. Playing a video used to mean asking the server to re-encode it, always — a decision made nowhere, written down nowhere, and re-derived downstream by whoever needed it: the player worked out whether it had been handed a playlist by looking for `.m3u8` in the URL. So a viewer paid for a transcode of a file their device could have played untouched, and the app could not tell them which it was. Now one negotiation produces one self-describing answer — direct play, remux, or transcode; over a playlist, a plain HTTP file, or a local one — and every renderer consumes that same answer instead of guessing from a string. On Android, where the player decodes almost everything the library holds, this stops around 85% of plays from starting a transcode nobody needed | Medium | Done |
 | UR-080 | Video on the desktop plays as itself. The picture was drawn by a webview `<video>` element, which decodes little beyond h264 — so the app told the server it could accept only h264, and the server re-encoded almost everything before sending it. That was never a statement about the machine: the same machine already runs mpv for audio, which decodes essentially the whole library. Measured against a real library, 93% of desktop playback was a transcode nobody needed, against 15% on Android where a real decoder does the work. mpv now draws the picture, the app claims what it can genuinely decode, and video is sent as it was stored wherever that is possible — sparing the server the work, the network the bitrate, and the picture a generation of re-encoding | Medium | Proposed |
+| UR-081 | Playback behaves the same whichever engine renders it | High | In Progress |
 | UR-074 | Video streaming can be held to a **bandwidth budget the viewer sets**, rather than spent at whatever rate the server would otherwise send. A ceiling chosen once — from the source's own bitrate down to a rung that still plays on a poor connection — governs every video the app opens, live TV included, and survives a restart, so a metered connection is not quietly drained by the next thing played. A single video can be moved to a different ceiling from the player, resuming where it was, without disturbing that default | Medium | Done |
 
 ---
@@ -432,6 +433,12 @@ Internal architecture, components, and application logic.
 | DR-235 | The webview video path is deleted, not merely bypassed. Staged, because a path cannot be removed while a shipped platform still needs it: Linux moves to mpv first, Windows follows, and only then do `hls.js`, `html5Adapter.ts`, `videoLoaderFor` and the `<video>` element go. The staging is the point — a Linux-only version would leave the fork alive permanently, taking video from three renderers to four and giving every seek strategy, track switch and lifecycle bug one more place to be got right. Android keeps ExoPlayer and keeps the webview as its documented opt-out; the background-audio `<audio>` path is untouched. With no HTML5 fallback left, a failed mpv init emits `backend-init-failed` and surfaces a real error rather than silently degrading to the transcode this work exists to stop paying for | Playback | UR-080 | Proposed |
 | DR-236 | Hardware-decode policy is decided from what mpv reports it **selected** (`hwdec-current`), never from what it was asked for. The spike established that hardware decode works through the render API at all — the load-bearing result, since it means direct play is not bought with software decoding — but also that `auto` reached for the discrete GPU in copy-back mode on a hybrid Intel+NVIDIA laptop, the least efficient hardware path, and that `vaapi` fell back to software silently because the libva driver was absent. So zero-copy VA-API on the integrated GPU is preferred where the driver is present, `auto` is a fallback rather than the default, and a missing driver is detected and logged rather than mistaken for a compositing limit | Playback | UR-080 | Proposed |
 | DR-237 | Windows reaches the same mpv path, reusing everything except the surface. The surface is genuinely different code — a native child window beneath a transparent WebView2, not GTK — but the render context, lifetime discipline, frame pacing, device profile and hwdec policy are shared, which is why none of them may be guarded on `cfg!(target_os = "linux")`. The cost is mostly build, not video: `libmpv` is currently a Linux-only dependency while Windows is cross-compiled from Linux via `x86_64-pc-windows-msvc` + `cargo-xwin`, so a Windows libmpv must reach that cross-build and its DLL must ship in the NSIS bundle, carrying the LGPL obligations DR-216 already records — dynamic linkage, licence text shipped alongside. Windows gains a native audio decoder as a side effect, which is what the long-blocked Windows audio work wants and cannot otherwise have | Playback | UR-080 | Proposed |
+| DR-238 | A transcoded seek re-negotiates the stream on every renderer, not just the webview. Jellyfin produces a transcode *from* `StartTimeTicks`, so where a seek lands is a property of the request rather than of the stream in hand. `determine_video_seek_strategy` treated `is_hls` as a proxy for "seekable in place", which held only because hls.js was always the HLS renderer — it seeks within the VOD playlist it is handed and lets the server catch up. mpv's HLS demuxer cannot make the server transcode from a new offset, so with native video on, every transcoded seek became a backend seek that silently did nothing and presented as "resume does not work". The rule is now written on `needs_transcoding` with hls.js as the stated exception; all four webview cells are unchanged | Player | UR-040 | Done |
+| DR-239 | Properties the mpv event loop handles are registered with `observe_property`. libmpv delivers `PropertyChange` only for observed properties, so a `match` arm for an unobserved one is unreachable code that reads as implemented — the handler is right there. `pause` was handled and never observed, so `StateChanged` was never emitted on pause or resume and the play/pause control never moved. It stayed invisible while Linux video played in the webview, because the `<video>` element's own DOM events drove that control; native video made the UI depend on the event that never came | Player | UR-005 | Done |
+| DR-240 | Fullscreen moves whatever actually owns the pixels. `requestFullscreen()` fullscreens the *document*, which sufficed while every renderer lived inside it — the HTML5 `<video>` element is part of the document, so WebKit scaled it and the OS window's real size never mattered. A native surface is drawn behind the webview at **window** size, so a document-only fullscreen expands the page and leaves the picture where it was; on WebKitGTK the result is a maximised window with decorations still holding a strip of the screen, which reads as "fullscreen is broken" rather than as a windowing problem. Android needed the same rule for the system bars (DR-157); this is its desktop half | Player | UR-066 | Done |
+| DR-241 | A seek issued before MPV has a file to seek in is honoured, not dropped. `loadfile` returns as soon as the command is queued, so `time-pos` — a live property of the *loaded* file — does not resolve yet and setting it fails. The two callers that always hit that window are the ones a viewer notices: resume, and a transcoded seek, both of which re-open the stream and then ask for a position. The failed seek was discarded and the stream played from zero, which reads as "resume is broken" and "I cannot skip". The position is now held and applied by the `FileLoaded` handler; a seek that lands normally clears any deferred one, so the newer intent wins | Player | UR-040, UR-005 | Done |
+| DR-242 | The player contract expresses intent, not device operations. `MediaPlayer::open` carries the start position, so no caller sequences load-then-seek and none can race an engine's asynchronous load; `seek` states a destination and leaves in-place-vs-re-open to the engine, which is the only layer that knows its own transport; `snapshot` is one coherent read; and `Phase::Opening` names the window a seek used to be lost in. Replaces `PlayerBackend`, which abstracted a device and required each of the three engines to re-derive the same rules | Player | UR-081 | In Progress |
+| DR-243 | Every engine passes one conformance suite, and a `FakePlayer` implements the contract deterministically. The suite is written before the second engine so it cannot encode whatever the first happened to do, and it drives readiness through a harness rather than sleeping. `FakePlayer` models the one behaviour that matters — opening is not instantaneous — so the load/seek race can be expressed on purpose, and lets the controller, queue, autoplay and session logic be tested with no engine at all | Player | UR-081 | In Progress |
 | 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 `<style>` element survives into `index.html`, since a nonce there would make Tauri's injection outrank — and therefore void — `'unsafe-inline'`. `worker-src blob:` and `media-src blob:` are hls.js: it demuxes in a worker built from a blob and attaches MSE through `URL.createObjectURL`. `asset:` and `http://asset.localhost` are the same protocol under the two naming schemes `convertFileSrc` emits (custom scheme on Linux/macOS, `http` host on Windows/Android); `ipc:`/`http://ipc.localhost` is the invoke transport, which would otherwise be blocked by `connect-src`. A run-time CSP naming the server origin exactly was rejected: Tauri computes the header from immutable config when it serves the HTML, so it would mean rebuilding config and reloading the webview on every server change, for a policy the user can already point anywhere. The asset-protocol scope narrows from `$APPDATA/**` to `$APPDATA/thumbnails/**` — since DR-137 moved downloaded media to the loopback server, `imageCache` is the only `convertFileSrc` caller left, so the database and the encrypted-token fallback file no longer sit inside the grant | Security | UR-012, UR-071 | Done |
 
 ---
@@ -740,6 +747,10 @@ Internal architecture, components, and application logic.
 | UT-214 | The loader comes from the transport, never the URL. hls.js is attached for `hls` when available and the element's own loader when not; progressive and local files load directly; the element's `src` is emptied only when hls.js drives it. The two cases that fail against a substring check, and the reason the field exists: a `progressive` stream whose URL contains `.m3u8` is *not* given an HLS loader, and an `hls` stream whose URL contains no `.m3u8` *is*. Both failed against the pre-DR-225 implementation before the fix landed | DR-224 | Done |
 | UT-215 | Waiting for the repository rather than racing it: it resolves immediately when the session is already restored, resolves when the session arrives later (the race the player page lost on mount), still rejects when there genuinely is no session, unsubscribes once settled so a later store change cannot re-settle it, and leaves no armed timer to reject an already-resolved promise | DR-013 | Done |
 | UT-216 | The native-video opt-in is read from one place and only explicit truthy values enable it: absent, empty, `0`, `no`, `false` and anything unrecognised all mean off, because a half-set variable that half-enabled the renderer would configure mpv for video with nothing drawing it — audio over a black rectangle | DR-231 | Done |
+| UT-217 | A transcoded HLS stream on the native backend re-negotiates rather than seeking in place, while the same stream under hls.js still seeks in place — the cell that native video made reachable for the first time | DR-238 | Done |
+| UT-218 | Every property name matched by the mpv event loop also appears in an `observe_property` call, asserted against the source because the registration cannot be observed at runtime without a live mpv | DR-239 | Done |
+| UT-219 | A fullscreen toggle moves the document only when an in-document `<video>` renders, and moves the OS window as well when a native surface does | DR-240 | Done |
+| UT-220 | The conformance suite: opening at a position starts there and never at zero, a seek issued while opening is honoured and overrides the start it overtook, pause and play are observable, close is silent and idempotent, and an open cancelled by close never begins playing | DR-242, DR-243 | In Progress |
 
 ### Integration Tests
 
diff --git a/src-tauri/src/player/media_player.rs b/src-tauri/src/player/media_player.rs
new file mode 100644
index 00000000..767955d3
--- /dev/null
+++ b/src-tauri/src/player/media_player.rs
@@ -0,0 +1,206 @@
+//! The `MediaPlayer` contract: one API, interchangeable engines.
+//!
+//! See docs/specs/media-player-controller.md.
+//!
+//! This replaces [`PlayerBackend`](super::backend::PlayerBackend), which
+//! abstracts a *device* — `load`, then `seek` — rather than an *intent*. That
+//! distinction is not academic; it produced four shipped defects in one day:
+//!
+//! * A start position was not expressible, so every caller sequenced
+//!   `load()` + `seek()` itself and each raced the engine's asynchronous load
+//!   independently. Resume worked through one caller and silently failed through
+//!   another (DR-241).
+//! * Whether a stream could be seeked in place was decided *above* the engines,
+//!   by a truth table in a command handler, for engines it does not own (DR-238).
+//! * Nothing in the contract obliged an engine to report its own state, so a
+//!   handler for mpv's `pause` property sat unreachable and the play/pause
+//!   control never moved (DR-239).
+//!
+//! The contract below is written so each of those is a compile-time or
+//! conformance-time failure rather than a runtime surprise.
+//!
+//! TRACES: UR-081 | DR-242
+
+// Scaffolding: nothing consumes this contract until `PlayerController` is
+// ported to it (DR-245). Kept out of `cfg(test)` deliberately — it is production
+// code being built in shippable steps, not a test fixture. Remove this allow
+// when the controller talks to `MediaPlayer`.
+#![allow(dead_code)]
+
+use std::time::Duration;
+
+use super::backend::PlayerError;
+use super::media::MediaItem;
+use crate::repository::stream_selection::StreamSelection;
+
+/// What an engine is doing right now.
+///
+/// `Opening` is the state the previous design could not express, and is the
+/// direct cause of DR-241: a seek that arrived while the engine had nothing
+/// loaded had no phase to be queued against, so it was simply discarded and
+/// playback began at zero.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum Phase {
+    /// Nothing loaded. `close()` must reach this, and must be silent here.
+    Idle,
+    /// An `open` is in flight. Position is not yet meaningful; a `seek` arriving
+    /// now must be honoured once the engine reaches `Ready`, never dropped.
+    Opening,
+    /// Loaded and able to play, but not advancing.
+    Ready,
+    Playing,
+    Paused,
+    /// Reached the end of the item by itself. Distinct from `Idle`, because
+    /// autoplay cares which one happened.
+    Ended,
+    Failed(String),
+}
+
+impl Phase {
+    /// Whether the engine currently holds an item.
+    pub fn has_media(&self) -> bool {
+        !matches!(self, Phase::Idle | Phase::Failed(_))
+    }
+
+    /// Whether playback is advancing.
+    pub fn is_active(&self) -> bool {
+        matches!(self, Phase::Playing)
+    }
+}
+
+/// Everything the UI consumes, read as one coherent value.
+///
+/// Deliberately a single snapshot rather than a dozen getters: reading position
+/// and duration through separate calls is how a paused player reported
+/// `<position> / 0.0` when a file unloaded between them.
+#[derive(Debug, Clone)]
+pub struct PlaybackSnapshot {
+    pub phase: Phase,
+    pub position: Duration,
+    /// `None` while unknown — a live stream, or an item still opening.
+    pub duration: Option<Duration>,
+    /// Whether `seek` can be expected to land. False for live edges.
+    pub seekable: bool,
+    /// 0.0 – 1.0.
+    pub volume: f32,
+    pub muted: bool,
+    pub rate: f64,
+    pub audio_track: Option<i32>,
+    pub subtitle_track: Option<i32>,
+}
+
+impl Default for PlaybackSnapshot {
+    fn default() -> Self {
+        Self {
+            phase: Phase::Idle,
+            position: Duration::ZERO,
+            duration: None,
+            seekable: false,
+            volume: 1.0,
+            muted: false,
+            rate: 1.0,
+            audio_track: None,
+            subtitle_track: None,
+        }
+    }
+}
+
+/// What an engine can do, so callers adapt without naming engines.
+///
+/// If a caller ever branches on *which* engine it holds, this struct is missing
+/// something — add it here rather than sniffing. Engine identity leaking into
+/// callers is the coupling DR-238 came from.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct Capabilities {
+    /// The engine renders pictures, not only sound.
+    pub video: bool,
+    /// Audio settings (EQ, normalisation, gapless) are honoured.
+    pub audio_settings: bool,
+    /// Subtitle tracks can be selected without re-opening.
+    pub subtitle_switching: bool,
+    /// Audio tracks can be selected without re-opening.
+    pub audio_track_switching: bool,
+}
+
+/// A request to present an item.
+///
+/// `start` is the reason this type exists. Carrying it here — rather than
+/// leaving callers to `seek` after `open` — is what closes the load/seek race,
+/// because the engine is the only layer that knows when its pipeline can accept
+/// a position.
+#[derive(Debug, Clone)]
+pub struct OpenRequest {
+    pub media: MediaItem,
+    pub selection: StreamSelection,
+    /// Where to begin. `Duration::ZERO` means the start of the item.
+    pub start: Duration,
+    pub audio_track: Option<i32>,
+    pub subtitle_track: Option<i32>,
+    /// Begin playing as soon as the engine is able.
+    pub autoplay: bool,
+}
+
+impl OpenRequest {
+    /// Open at the beginning, playing.
+    pub fn new(media: MediaItem, selection: StreamSelection) -> Self {
+        Self {
+            media,
+            selection,
+            start: Duration::ZERO,
+            audio_track: None,
+            subtitle_track: None,
+            autoplay: true,
+        }
+    }
+
+    pub fn starting_at(mut self, start: Duration) -> Self {
+        self.start = start;
+        self
+    }
+}
+
+/// Anything that can present media.
+///
+/// Implementations: `MpvPlayer` (Linux/Windows), `ExoPlayerPlayer` (Android),
+/// `WebviewPlayer` (HTML5 element), and `FakePlayer` for tests. Every one of
+/// them must pass [`super::conformance`].
+pub trait MediaPlayer: Send {
+    /// Present `req.selection`, beginning at `req.start`.
+    ///
+    /// One operation, deliberately. An engine that cannot start at an offset
+    /// natively absorbs that internally — by deferring until loaded, or by
+    /// re-opening — because it is the only layer that knows when it can.
+    /// Callers must never follow `open` with a `seek` to achieve a start
+    /// position; that is the bug this signature exists to prevent.
+    fn open(&mut self, req: OpenRequest) -> Result<(), PlayerError>;
+
+    fn play(&mut self) -> Result<(), PlayerError>;
+    fn pause(&mut self) -> Result<(), PlayerError>;
+
+    /// Stop and release the current item.
+    ///
+    /// Must be **idempotent** and must leave the engine **silent**. "Stopped"
+    /// and "producing no audio" were not the same thing in the previous design,
+    /// and the gap between them is audible.
+    fn close(&mut self) -> Result<(), PlayerError>;
+
+    /// Seek to an absolute position on the item's own timeline.
+    ///
+    /// Whether that is an in-place seek or a re-open of the stream is the
+    /// engine's business: hls.js seeks within a VOD playlist, mpv's HLS demuxer
+    /// cannot make a server transcode from a new offset. Callers state the
+    /// destination and nothing else.
+    fn seek(&mut self, to: Duration) -> Result<(), PlayerError>;
+
+    fn set_volume(&mut self, volume: f32) -> Result<(), PlayerError>;
+    fn set_muted(&mut self, muted: bool) -> Result<(), PlayerError>;
+    fn set_rate(&mut self, rate: f64) -> Result<(), PlayerError>;
+
+    fn select_audio_track(&mut self, index: Option<i32>) -> Result<(), PlayerError>;
+    fn select_subtitle_track(&mut self, index: Option<i32>) -> Result<(), PlayerError>;
+
+    /// One coherent read of the engine's state.
+    fn snapshot(&self) -> PlaybackSnapshot;
+
+    fn capabilities(&self) -> Capabilities;
+}