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

DR-245, first half. `LegacyPlayer` implements `MediaPlayer` over the existing
`PlayerBackend`, so engines not yet ported — ExoPlayer, the webview element,
the null backend — keep working while `PlayerController` moves across. Without
it the port would have to land all four engines at once.

It also makes the two designs comparable on one engine and one file. `open`
reproduces the old sequence faithfully: load, play, then seek for a start
position, with the seek's failure ignored exactly as callers used to ignore
it. Making it pass would defeat the point.

Running both engines over the same media is more informative than expected:

  MpvPlayer     9/9
  LegacyPlayer  8/9 - transport_settings_round_trip fails

Two things fall out of that. The start-position case now passes on *both*,
because DR-241 was fixed inside MpvBackend rather than only in the new engine
— so the suite confirms that fix independently, on a path it was not written
against. And the one genuine failure is a capability gap rather than a bug:
the old trait has no mute and no playback rate, so `LegacyPlayer` reports them
unsupported instead of folding mute into volume and losing the user's level.

That is the abstraction earning its keep on the first run: a missing
capability that was previously invisible is now a named, failing case.

The runner takes an engine argument:

    player-conformance <media-file> [mpv|legacy]
---
 docs/requirements.md                    |   2 +
 src-tauri/src/bin/player_conformance.rs |  26 ++++-
 src-tauri/src/conformance_runner.rs     |  80 +++++++++----
 src-tauri/src/player/legacy_player.rs   | 146 ++++++++++++++++++++++++
 src-tauri/src/player/mod.rs             |   2 +
 5 files changed, 232 insertions(+), 24 deletions(-)
 create mode 100644 src-tauri/src/player/legacy_player.rs

diff --git a/docs/requirements.md b/docs/requirements.md
index 96d76375..d4469f0f 100644
--- a/docs/requirements.md
+++ b/docs/requirements.md
@@ -439,6 +439,8 @@ Internal architecture, components, and application logic.
 | 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-244 | `MpvPlayer` implements `MediaPlayer` over libmpv, applying the start position at load time via mpv's own `start` option rather than seeking after an asynchronous `loadfile`, and holding a seek that arrives during `Opening` until the file loads. A standalone `player-conformance` binary runs the suite against it with audio and video routed to null, so a wrapper is verifiable without building or launching the app | Player | UR-081, UR-040 | Done |
+| DR-245 | `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 | 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 |
 
 ---
diff --git a/src-tauri/src/bin/player_conformance.rs b/src-tauri/src/bin/player_conformance.rs
index 0f83eef9..9435747c 100644
--- a/src-tauri/src/bin/player_conformance.rs
+++ b/src-tauri/src/bin/player_conformance.rs
@@ -2,16 +2,34 @@
 //! access to the player internals — one exported function rather than a public
 //! module tree.
 //!
-//! TRACES: UR-081 | DR-244
+//!     player-conformance <media-file> [mpv|legacy]
+//!
+//! `legacy` drives the old `PlayerBackend` through the same cases, so the
+//! difference between the two designs is demonstrated on one engine and one
+//! file rather than argued.
+//!
+//! TRACES: UR-081 | DR-244, DR-245
 
 use std::process::ExitCode;
 
+use jellytau_lib::conformance_runner::{run_engine, Engine};
+
 fn main() -> ExitCode {
-    let Some(url) = std::env::args().nth(1) else {
-        eprintln!("usage: player-conformance <media-file-or-url>");
+    let mut args = std::env::args().skip(1);
+    let Some(url) = args.next() else {
+        eprintln!("usage: player-conformance <media-file-or-url> [mpv|legacy]");
         return ExitCode::from(2);
     };
-    if jellytau_lib::conformance_runner::run(&url) == 0 {
+    let engine = match args.next().as_deref() {
+        None | Some("mpv") => Engine::Mpv,
+        Some("legacy") => Engine::Legacy,
+        Some(other) => {
+            eprintln!("unknown engine {other:?} - expected mpv or legacy");
+            return ExitCode::from(2);
+        }
+    };
+
+    if run_engine(&url, engine) == 0 {
         ExitCode::SUCCESS
     } else {
         ExitCode::FAILURE
diff --git a/src-tauri/src/conformance_runner.rs b/src-tauri/src/conformance_runner.rs
index f3c7d055..9161ed7f 100644
--- a/src-tauri/src/conformance_runner.rs
+++ b/src-tauri/src/conformance_runner.rs
@@ -15,20 +15,22 @@
 use std::time::{Duration, Instant};
 
 use crate::player::conformance::Harness;
+use crate::player::legacy_player::LegacyPlayer;
 use crate::player::media::MediaItem;
 use crate::player::media_player::{MediaPlayer, OpenRequest, Phase};
+use crate::player::mpv_backend::MpvBackend;
 use crate::player::mpv_player::{MpvPlayer, Output};
 use crate::repository::stream_selection::StreamSelection;
 
-struct MpvHarness {
-    player: MpvPlayer,
+struct EngineHarness<P: MediaPlayer> {
+    player: P,
     url: String,
 }
 
-impl Harness for MpvHarness {
-    type Player = MpvPlayer;
+impl<P: MediaPlayer> Harness for EngineHarness<P> {
+    type Player = P;
 
-    fn player(&mut self) -> &mut MpvPlayer {
+    fn player(&mut self) -> &mut P {
         &mut self.player
     }
 
@@ -69,12 +71,12 @@ impl Harness for MpvHarness {
 }
 
 macro_rules! run {
-    ($failed:ident, $url:expr, $case:path) => {{
+    ($failed:ident, $url:expr, $make:expr, $case:path) => {{
         let name = stringify!($case).rsplit("::").next().unwrap();
         print!("  {name:.<52}");
         let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
-            let mut h = MpvHarness {
-                player: MpvPlayer::new(Output::Null).expect("could not create mpv"),
+            let mut h = EngineHarness {
+                player: $make,
                 url: $url.to_string(),
             };
             $case(&mut h);
@@ -91,22 +93,55 @@ macro_rules! run {
     }};
 }
 
-/// Run every conformance case against `MpvPlayer`. Returns the failure count.
-pub fn run(url: &str) -> u32 {
-    println!("MediaPlayer conformance - MpvPlayer");
+/// Which engine to interrogate.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Engine {
+    /// The `MediaPlayer` implementation.
+    Mpv,
+    /// The old `PlayerBackend`, driven through `LegacyPlayer`.
+    ///
+    /// Present so the difference between the two designs can be *demonstrated*
+    /// on the same engine and the same media, rather than argued.
+    Legacy,
+}
+
+/// Run every conformance case against `engine`. Returns the failure count.
+pub fn run_engine(url: &str, engine: Engine) -> u32 {
+    println!("MediaPlayer conformance - {engine:?}");
     println!("media: {url}\n");
 
     let mut failed = 0u32;
     use crate::player::conformance as c;
-    run!(failed, url, c::opens_from_the_beginning);
-    run!(failed, url, c::opens_at_a_start_position);
-    run!(failed, url, c::seek_while_opening_is_honoured);
-    run!(failed, url, c::seek_while_opening_overrides_start);
-    run!(failed, url, c::seeks_after_open);
-    run!(failed, url, c::pause_and_play_are_observable);
-    run!(failed, url, c::close_is_silent_and_idempotent);
-    run!(failed, url, c::close_during_open_never_plays);
-    run!(failed, url, c::transport_settings_round_trip);
+
+    macro_rules! all_cases {
+        ($make:expr) => {
+            run!(failed, url, $make, c::opens_from_the_beginning);
+            run!(failed, url, $make, c::opens_at_a_start_position);
+            run!(failed, url, $make, c::seek_while_opening_is_honoured);
+            run!(failed, url, $make, c::seek_while_opening_overrides_start);
+            run!(failed, url, $make, c::seeks_after_open);
+            run!(failed, url, $make, c::pause_and_play_are_observable);
+            run!(failed, url, $make, c::close_is_silent_and_idempotent);
+            run!(failed, url, $make, c::close_during_open_never_plays);
+            run!(failed, url, $make, c::transport_settings_round_trip);
+        };
+    }
+
+    match engine {
+        Engine::Mpv => {
+            all_cases!(MpvPlayer::new(Output::Null).expect("could not create mpv"));
+        }
+        Engine::Legacy => {
+            all_cases!(LegacyPlayer::new(
+                MpvBackend::new(
+                    None,
+                    std::sync::Arc::new(tokio::sync::Mutex::new(None)),
+                    std::sync::Arc::new(crate::playback_reporting::throttle::EventThrottler::new()),
+                )
+                .expect("could not create the legacy backend")
+            ));
+        }
+    }
 
     if failed == 0 {
         println!("\nall cases passed");
@@ -115,3 +150,8 @@ pub fn run(url: &str) -> u32 {
     }
     failed
 }
+
+/// Default entry point: the new engine.
+pub fn run(url: &str) -> u32 {
+    run_engine(url, Engine::Mpv)
+}
diff --git a/src-tauri/src/player/legacy_player.rs b/src-tauri/src/player/legacy_player.rs
new file mode 100644
index 00000000..0e306deb
--- /dev/null
+++ b/src-tauri/src/player/legacy_player.rs
@@ -0,0 +1,146 @@
+//! A [`MediaPlayer`] over the old [`PlayerBackend`] trait.
+//!
+//! Two purposes.
+//!
+//! **Migration.** Engines not yet ported — ExoPlayer, the webview element, the
+//! null backend — keep working while `PlayerController` moves onto the new
+//! contract (DR-245). Without this the port would have to land all four engines
+//! at once.
+//!
+//! **Evidence.** It reproduces exactly what every caller used to do: `load`,
+//! then `play`, then `seek` for a start position. Running the conformance suite
+//! against it therefore shows the old path failing the cases the new one passes,
+//! on the same engine and the same media — which is the difference between
+//! asserting that a design was wrong and demonstrating it.
+//!
+//! It is deliberately a faithful reproduction, not a fixed-up one. Making it
+//! pass would defeat the point.
+//!
+//! TRACES: UR-081 | DR-245
+
+#![allow(dead_code)] // Consumed when PlayerController is ported (DR-245).
+
+use std::time::Duration;
+
+use super::backend::{PlayerBackend, PlayerError};
+use super::media_player::{Capabilities, MediaPlayer, OpenRequest, Phase, PlaybackSnapshot};
+use super::state::PlayerState;
+
+pub struct LegacyPlayer<B: PlayerBackend> {
+    inner: B,
+    /// The old trait has no notion of "opening", so this is the best the wrapper
+    /// can do: it knows an item was handed over, not whether the engine is ready
+    /// for one. That gap is the whole problem.
+    has_item: bool,
+}
+
+impl<B: PlayerBackend> LegacyPlayer<B> {
+    pub fn new(inner: B) -> Self {
+        Self {
+            inner,
+            has_item: false,
+        }
+    }
+
+    pub fn inner_mut(&mut self) -> &mut B {
+        &mut self.inner
+    }
+}
+
+impl<B: PlayerBackend + Send> MediaPlayer for LegacyPlayer<B> {
+    /// Load, play, then seek — the sequence every caller used to write.
+    ///
+    /// The seek is issued immediately, because a caller has no way to know when
+    /// the engine becomes ready. On an engine whose load is asynchronous it
+    /// fails and is discarded, and playback begins at zero: DR-241, reproduced.
+    fn open(&mut self, req: OpenRequest) -> Result<(), PlayerError> {
+        self.inner.load(&req.media)?;
+        self.has_item = true;
+        if req.autoplay {
+            self.inner.play()?;
+        }
+        if !req.start.is_zero() {
+            // Faithfully ignoring the failure, exactly as the old callers did.
+            let _ = self.inner.seek(req.start.as_secs_f64());
+        }
+        Ok(())
+    }
+
+    fn play(&mut self) -> Result<(), PlayerError> {
+        self.inner.play()
+    }
+
+    fn pause(&mut self) -> Result<(), PlayerError> {
+        self.inner.pause()
+    }
+
+    fn close(&mut self) -> Result<(), PlayerError> {
+        self.has_item = false;
+        self.inner.stop()
+    }
+
+    fn seek(&mut self, to: Duration) -> Result<(), PlayerError> {
+        self.inner.seek(to.as_secs_f64())
+    }
+
+    fn set_volume(&mut self, volume: f32) -> Result<(), PlayerError> {
+        self.inner.set_volume(volume)
+    }
+
+    /// The old trait has no mute. Folding it into volume would lose the user's
+    /// level, so this reports unsupported rather than pretending.
+    fn set_muted(&mut self, _muted: bool) -> Result<(), PlayerError> {
+        Err(PlayerError {
+            message: "mute is not supported by this backend".to_string(),
+        })
+    }
+
+    fn set_rate(&mut self, _rate: f64) -> Result<(), PlayerError> {
+        Err(PlayerError {
+            message: "playback rate is not supported by this backend".to_string(),
+        })
+    }
+
+    fn select_audio_track(&mut self, index: Option<i32>) -> Result<(), PlayerError> {
+        self.inner.set_audio_track(index.unwrap_or(-1))
+    }
+
+    fn select_subtitle_track(&mut self, index: Option<i32>) -> Result<(), PlayerError> {
+        self.inner.set_subtitle_track(index)
+    }
+
+    fn snapshot(&self) -> PlaybackSnapshot {
+        let phase = match self.inner.state() {
+            _ if !self.has_item => Phase::Idle,
+            PlayerState::Playing { .. } => Phase::Playing,
+            PlayerState::Paused { .. } => Phase::Paused,
+            PlayerState::Idle => Phase::Idle,
+            PlayerState::Error { error, .. } => Phase::Failed(error),
+            // `Loading` is the closest the old trait comes to an opening state,
+            // but it is set once the engine has accepted the item rather than
+            // while it is still accepting it — which is precisely the window it
+            // cannot describe.
+            PlayerState::Loading { .. } | PlayerState::Seeking { .. } => Phase::Ready,
+        };
+        PlaybackSnapshot {
+            phase,
+            position: Duration::from_secs_f64(self.inner.position().max(0.0)),
+            duration: self.inner.duration().map(Duration::from_secs_f64),
+            seekable: true,
+            volume: self.inner.volume(),
+            muted: false,
+            rate: 1.0,
+            audio_track: None,
+            subtitle_track: None,
+        }
+    }
+
+    fn capabilities(&self) -> Capabilities {
+        Capabilities {
+            video: false,
+            audio_settings: true,
+            subtitle_switching: true,
+            audio_track_switching: true,
+        }
+    }
+}
diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs
index 12fb99a2..f2b687aa 100644
--- a/src-tauri/src/player/mod.rs
+++ b/src-tauri/src/player/mod.rs
@@ -12,6 +12,8 @@ pub mod events;
 pub mod fake_player;
 #[cfg(test)]
 mod fake_player_conformance;
+#[cfg(any(test, feature = "conformance"))]
+pub mod legacy_player;
 pub mod media;
 pub mod media_player;
 #[cfg(target_os = "linux")]