diff --git a/src-tauri/src/player/conformance.rs b/src-tauri/src/player/conformance.rs new file mode 100644 index 00000000..9ae1c8a3 --- /dev/null +++ b/src-tauri/src/player/conformance.rs @@ -0,0 +1,277 @@ +//! The conformance suite every [`MediaPlayer`] must pass. +//! +//! One set of behaviours, run against every engine: `FakePlayer` and `MpvPlayer` +//! in `cargo test`, `ExoPlayerPlayer` instrumented on a device, `WebviewPlayer` +//! in vitest. A new engine is finished when it passes this. +//! +//! Written *before* the second engine on purpose. A suite written afterwards +//! encodes whatever the first engine happened to do, which is how three separate +//! playback implementations drifted apart in the first place. +//! +//! Each case names the defect it exists to prevent. Two of them — +//! [`opens_at_a_start_position`] and [`seek_while_opening_is_honoured`] — fail +//! against the pre-migration mpv path, which is what makes them a reproduction +//! of DR-241 rather than a restatement of it. +//! +//! Engines differ in *when* an open completes, so the suite drives that through +//! a [`Harness`] rather than sleeping: the fake completes on demand, mpv waits +//! for its `FileLoaded` event, ExoPlayer for `STATE_READY`. +//! +//! Available to `cargo test` and, behind the `conformance` feature, to the +//! `player-conformance` binary — so an engine that cannot run in-process +//! (ExoPlayer on a device) is driven by exactly the same cases rather than by a +//! second, drifting checklist. +//! +//! TRACES: UR-081 | DR-243 | UT-220 + +use std::time::Duration; + +use super::media_player::{MediaPlayer, OpenRequest, Phase}; + +/// How the suite drives one engine. +pub trait Harness { + type Player: MediaPlayer; + + fn player(&mut self) -> &mut Self::Player; + + /// A request this engine can actually open, at `start`. + fn request(&self, start: Duration) -> OpenRequest; + + /// Block until an in-flight `open` has finished (or failed). + /// + /// The fake completes on demand; a real engine waits for its own readiness + /// event. Never a sleep — a timing-dependent suite is worse than none. + fn settle(&mut self); + + /// Whether the engine is producing audio. Engines that cannot answer may + /// return `None`, which skips the silence assertions rather than passing + /// them vacuously. + fn audible(&mut self) -> Option; + + /// How far a landed position may differ from the one asked for. Keyframe + /// granularity makes exactness the wrong bar for a real decoder. + fn seek_tolerance(&self) -> Duration { + Duration::from_secs(5) + } +} + +fn assert_near(actual: Duration, expected: Duration, tolerance: Duration, what: &str) { + let delta = actual.abs_diff(expected); + assert!( + delta <= tolerance, + "{what}: expected ~{expected:?}, got {actual:?} (tolerance {tolerance:?})" + ); +} + +/// Opening at zero reaches a usable state and starts near the beginning. +pub fn opens_from_the_beginning(h: &mut H) { + let req = h.request(Duration::ZERO); + h.player().open(req).expect("open failed"); + h.settle(); + + let s = h.player().snapshot(); + assert!( + matches!(s.phase, Phase::Playing | Phase::Ready), + "after open the engine should hold media, phase was {:?}", + s.phase + ); + assert_near( + s.position, + Duration::ZERO, + h.seek_tolerance(), + "start of item", + ); +} + +/// **DR-241.** Opening at a position starts *there*, not at zero. +/// +/// The whole reason `OpenRequest` carries `start`. Under the previous contract a +/// caller had to `load()` then `seek()`, and because `loadfile` is asynchronous +/// the seek was issued against a player with nothing loaded, failed, and was +/// discarded — so resume and transcoded skip both played from the beginning. +pub fn opens_at_a_start_position(h: &mut H) { + let start = Duration::from_secs(600); + let req = h.request(start); + h.player().open(req).expect("open failed"); + h.settle(); + + let s = h.player().snapshot(); + assert_ne!( + s.position, + Duration::ZERO, + "opened at {start:?} but playback began at zero - the start position was dropped" + ); + assert_near(s.position, start, h.seek_tolerance(), "start position"); +} + +/// **DR-241.** A seek issued while opening is honoured, not lost. +/// +/// The engine owns this window; no caller can avoid it, because a caller cannot +/// see when the pipeline becomes ready. +pub fn seek_while_opening_is_honoured(h: &mut H) { + let target = Duration::from_secs(300); + let req = h.request(Duration::ZERO); + h.player().open(req).expect("open failed"); + + // Deliberately before settle(): this is the race, expressed on purpose. + h.player().seek(target).expect("seek during open failed"); + h.settle(); + + let s = h.player().snapshot(); + assert_near( + s.position, + target, + h.seek_tolerance(), + "seek issued while opening", + ); +} + +/// A later intent wins: the seek replaces the start position it overtook. +pub fn seek_while_opening_overrides_start(h: &mut H) { + let start = Duration::from_secs(600); + let target = Duration::from_secs(120); + let req = h.request(start); + h.player().open(req).expect("open failed"); + h.player().seek(target).expect("seek during open failed"); + h.settle(); + + assert_near( + h.player().snapshot().position, + target, + h.seek_tolerance(), + "seek should override the start position it overtook", + ); +} + +/// Seeking a settled item lands where asked. +pub fn seeks_after_open(h: &mut H) { + let req = h.request(Duration::ZERO); + h.player().open(req).expect("open failed"); + h.settle(); + + let target = Duration::from_secs(420); + h.player().seek(target).expect("seek failed"); + + assert_near( + h.player().snapshot().position, + target, + h.seek_tolerance(), + "seek after open", + ); +} + +/// **DR-239.** Pause and play are reflected in the engine's own state. +/// +/// An engine that changes nothing observable is indistinguishable from one that +/// ignored the call — which is exactly how a handler for mpv's `pause` property +/// sat unreachable while the UI waited for an event that never came. +pub fn pause_and_play_are_observable(h: &mut H) { + let req = h.request(Duration::ZERO); + h.player().open(req).expect("open failed"); + h.settle(); + + h.player().pause().expect("pause failed"); + assert_eq!( + h.player().snapshot().phase, + Phase::Paused, + "pause must be visible in the snapshot" + ); + if let Some(audible) = h.audible() { + assert!(!audible, "a paused engine must be silent"); + } + + h.player().play().expect("play failed"); + assert_eq!( + h.player().snapshot().phase, + Phase::Playing, + "play must be visible in the snapshot" + ); +} + +/// `close()` reaches Idle, is silent, and can be called twice. +pub fn close_is_silent_and_idempotent(h: &mut H) { + let req = h.request(Duration::ZERO); + h.player().open(req).expect("open failed"); + h.settle(); + + h.player().close().expect("close failed"); + assert_eq!(h.player().snapshot().phase, Phase::Idle); + if let Some(audible) = h.audible() { + assert!(!audible, "a closed engine must be silent"); + } + + h.player().close().expect("close must be idempotent"); + assert_eq!(h.player().snapshot().phase, Phase::Idle); +} + +/// Closing during an open must not let playback start afterwards. +/// +/// The shape of the "audio keeps playing after leaving the player" report: an +/// open still in flight completed after the stop, and nothing was left to tell +/// it not to. +pub fn close_during_open_never_plays(h: &mut H) { + let req = h.request(Duration::ZERO); + h.player().open(req).expect("open failed"); + h.player().close().expect("close during open failed"); + h.settle(); + + let s = h.player().snapshot(); + assert!( + !s.phase.is_active(), + "an open cancelled by close must not start playing, phase was {:?}", + s.phase + ); + if let Some(audible) = h.audible() { + assert!(!audible, "an engine closed during open must be silent"); + } +} + +/// Volume, mute and rate round-trip through the snapshot. +pub fn transport_settings_round_trip(h: &mut H) { + let req = h.request(Duration::ZERO); + h.player().open(req).expect("open failed"); + h.settle(); + + h.player().set_volume(0.25).expect("set_volume failed"); + h.player().set_muted(true).expect("set_muted failed"); + h.player().set_rate(1.5).expect("set_rate failed"); + + let s = h.player().snapshot(); + assert!((s.volume - 0.25).abs() < 0.01, "volume did not round-trip"); + assert!(s.muted, "mute did not round-trip"); + assert!((s.rate - 1.5).abs() < 0.01, "rate did not round-trip"); +} + +/// Run every case against one engine. +/// +/// Each case gets a fresh harness, because a suite whose cases depend on each +/// other's leftovers is one that hides state bugs instead of finding them. +#[macro_export] +macro_rules! media_player_conformance { + ($name:ident, $make:expr) => { + mod $name { + use super::*; + use $crate::player::conformance as c; + + macro_rules! case { + ($case:ident) => { + #[test] + fn $case() { + let mut h = $make; + c::$case(&mut h); + } + }; + } + + case!(opens_from_the_beginning); + case!(opens_at_a_start_position); + case!(seek_while_opening_is_honoured); + case!(seek_while_opening_overrides_start); + case!(seeks_after_open); + case!(pause_and_play_are_observable); + case!(close_is_silent_and_idempotent); + case!(close_during_open_never_plays); + case!(transport_settings_round_trip); + } + }; +} diff --git a/src-tauri/src/player/fake_player.rs b/src-tauri/src/player/fake_player.rs new file mode 100644 index 00000000..27aafa35 --- /dev/null +++ b/src-tauri/src/player/fake_player.rs @@ -0,0 +1,230 @@ +//! A deterministic in-memory [`MediaPlayer`], for tests. +//! +//! Two jobs: +//! +//! 1. Give the conformance suite something that is correct by construction, so a +//! failure there means the *suite* is wrong rather than an engine. +//! 2. Let everything above the engine — controller, queue, autoplay, sleep +//! timer, session — be tested with no mpv, no device and no network. Most of +//! that logic is currently only reachable through a real engine, which is why +//! so little of it is covered. +//! +//! It models the one behaviour that matters most: **opening is not +//! instantaneous**. `open()` lands in [`Phase::Opening`] and stays there until +//! [`FakePlayer::complete_open`] is called, so a test can put a `seek` into that +//! window on purpose. That is the window DR-241 lived in. +//! +//! TRACES: UR-081 | DR-243 + +// `tick` and `fail_open` are for tests not yet written — the controller-level +// ones DR-245 unlocks. Remove this allow once those exist. +#![allow(dead_code)] + +use std::time::Duration; + +use super::backend::PlayerError; +use super::media_player::{Capabilities, MediaPlayer, OpenRequest, Phase, PlaybackSnapshot}; + +#[derive(Debug, Clone, PartialEq)] +pub enum FakeEvent { + Opened { url: String, start: Duration }, + Played, + Paused, + Closed, + Sought(Duration), +} + +pub struct FakePlayer { + snapshot: PlaybackSnapshot, + /// Set while `Opening`; applied when the open completes. + pending_start: Duration, + /// A seek that arrived while opening. Honoured on completion, never dropped. + deferred_seek: Option, + autoplay: bool, + duration: Duration, + /// Every call, in order — so tests can assert what an engine was *asked* to + /// do, not only where it ended up. + pub log: Vec, + /// Whether audio is being produced. `close()` must clear it; the bug that + /// motivated all this had a "stopped" player that was still audible. + pub audible: bool, + pub capabilities: Capabilities, +} + +impl Default for FakePlayer { + fn default() -> Self { + Self::new() + } +} + +impl FakePlayer { + pub fn new() -> Self { + Self { + snapshot: PlaybackSnapshot::default(), + pending_start: Duration::ZERO, + deferred_seek: None, + autoplay: true, + duration: Duration::from_secs(3600), + log: Vec::new(), + audible: false, + capabilities: Capabilities { + video: true, + audio_settings: true, + subtitle_switching: true, + audio_track_switching: true, + }, + } + } + + /// The item this fake will report once opened. + pub fn with_duration(mut self, duration: Duration) -> Self { + self.duration = duration; + self + } + + /// Finish an in-flight `open`, as a real engine's "file loaded" would. + /// + /// Applies the requested start position, then any seek that arrived while + /// opening — the later intent wins. + pub fn complete_open(&mut self) { + if self.snapshot.phase != Phase::Opening { + return; + } + self.snapshot.duration = Some(self.duration); + self.snapshot.seekable = true; + self.snapshot.position = self.deferred_seek.take().unwrap_or(self.pending_start); + if self.autoplay { + self.snapshot.phase = Phase::Playing; + self.audible = true; + } else { + self.snapshot.phase = Phase::Ready; + } + } + + /// Advance playback, for tests that care about time passing. + pub fn tick(&mut self, by: Duration) { + if self.snapshot.phase.is_active() { + self.snapshot.position = (self.snapshot.position + by).min(self.duration); + if self.snapshot.position >= self.duration { + self.snapshot.phase = Phase::Ended; + self.audible = false; + } + } + } + + pub fn fail_open(&mut self, why: &str) { + self.snapshot.phase = Phase::Failed(why.to_string()); + self.audible = false; + } +} + +impl MediaPlayer for FakePlayer { + fn open(&mut self, req: OpenRequest) -> Result<(), PlayerError> { + self.log.push(FakeEvent::Opened { + url: req.selection.url.clone(), + start: req.start, + }); + self.snapshot = PlaybackSnapshot { + phase: Phase::Opening, + volume: self.snapshot.volume, + muted: self.snapshot.muted, + rate: self.snapshot.rate, + audio_track: req.audio_track, + subtitle_track: req.subtitle_track, + ..PlaybackSnapshot::default() + }; + self.pending_start = req.start; + self.deferred_seek = None; + self.autoplay = req.autoplay; + self.audible = false; + Ok(()) + } + + fn play(&mut self) -> Result<(), PlayerError> { + self.log.push(FakeEvent::Played); + if self.snapshot.phase.has_media() { + if self.snapshot.phase == Phase::Opening { + self.autoplay = true; + } else { + self.snapshot.phase = Phase::Playing; + self.audible = true; + } + } + Ok(()) + } + + fn pause(&mut self) -> Result<(), PlayerError> { + self.log.push(FakeEvent::Paused); + if self.snapshot.phase == Phase::Opening { + self.autoplay = false; + } else if self.snapshot.phase.has_media() { + self.snapshot.phase = Phase::Paused; + self.audible = false; + } + Ok(()) + } + + fn close(&mut self) -> Result<(), PlayerError> { + self.log.push(FakeEvent::Closed); + self.snapshot = PlaybackSnapshot { + volume: self.snapshot.volume, + muted: self.snapshot.muted, + rate: self.snapshot.rate, + ..PlaybackSnapshot::default() + }; + self.pending_start = Duration::ZERO; + self.deferred_seek = None; + // An open that was still in flight must not come back to life. + self.autoplay = false; + self.audible = false; + Ok(()) + } + + fn seek(&mut self, to: Duration) -> Result<(), PlayerError> { + self.log.push(FakeEvent::Sought(to)); + match self.snapshot.phase { + // The window DR-241 lived in: hold it, do not discard it. + Phase::Opening => self.deferred_seek = Some(to), + Phase::Idle | Phase::Failed(_) => { + return Err(PlayerError { + message: "seek with nothing open".to_string(), + }) + } + _ => self.snapshot.position = to.min(self.duration), + } + Ok(()) + } + + fn set_volume(&mut self, volume: f32) -> Result<(), PlayerError> { + self.snapshot.volume = volume.clamp(0.0, 1.0); + Ok(()) + } + + fn set_muted(&mut self, muted: bool) -> Result<(), PlayerError> { + self.snapshot.muted = muted; + Ok(()) + } + + fn set_rate(&mut self, rate: f64) -> Result<(), PlayerError> { + self.snapshot.rate = rate; + Ok(()) + } + + fn select_audio_track(&mut self, index: Option) -> Result<(), PlayerError> { + self.snapshot.audio_track = index; + Ok(()) + } + + fn select_subtitle_track(&mut self, index: Option) -> Result<(), PlayerError> { + self.snapshot.subtitle_track = index; + Ok(()) + } + + fn snapshot(&self) -> PlaybackSnapshot { + self.snapshot.clone() + } + + fn capabilities(&self) -> Capabilities { + self.capabilities + } +} diff --git a/src-tauri/src/player/fake_player_conformance.rs b/src-tauri/src/player/fake_player_conformance.rs new file mode 100644 index 00000000..64c2f10c --- /dev/null +++ b/src-tauri/src/player/fake_player_conformance.rs @@ -0,0 +1,56 @@ +//! `FakePlayer` runs the conformance suite. +//! +//! It is correct by construction, so a failure here means the *suite* is wrong, +//! not an engine. That is what makes it safe to trust the same cases when they +//! fail against a real one. +//! +//! TRACES: UR-081 | DR-243 | UT-220 + +use std::time::Duration; + +use super::conformance::Harness; +use super::fake_player::FakePlayer; +use super::media::MediaItem; +use super::media_player::OpenRequest; +use crate::repository::stream_selection::StreamSelection; + +struct FakeHarness { + player: FakePlayer, +} + +impl FakeHarness { + fn new() -> Self { + Self { + player: FakePlayer::new().with_duration(Duration::from_secs(7200)), + } + } +} + +impl Harness for FakeHarness { + type Player = FakePlayer; + + fn player(&mut self) -> &mut FakePlayer { + &mut self.player + } + + fn request(&self, start: Duration) -> OpenRequest { + let selection = StreamSelection::local_file("http://example.invalid/stream.mp4"); + let media = MediaItem::sample("fake-item", &selection.url); + OpenRequest::new(media, selection).starting_at(start) + } + + fn settle(&mut self) { + self.player.complete_open(); + } + + fn audible(&mut self) -> Option { + Some(self.player.audible) + } + + /// Exact: the fake has no keyframes to round to, so any drift is a bug. + fn seek_tolerance(&self) -> Duration { + Duration::ZERO + } +} + +crate::media_player_conformance!(fake, FakeHarness::new()); diff --git a/src-tauri/src/player/media.rs b/src-tauri/src/player/media.rs index 2cba4c47..781cb073 100644 --- a/src-tauri/src/player/media.rs +++ b/src-tauri/src/player/media.rs @@ -198,6 +198,48 @@ impl MediaItem { } } +impl MediaItem { + /// A minimal item for tests. + /// + /// The struct has twenty-odd fields, almost none of which any given test + /// cares about, and repeating the literal per test is how a new field ends + /// up added in thirty places. Set what matters on the result. + /// + /// TRACES: UR-081 | DR-243 + #[cfg(any(test, feature = "conformance"))] + pub fn sample(id: &str, url: &str) -> Self { + Self { + transport: None, + id: id.to_string(), + title: id.to_string(), + name: None, + artist: None, + album: None, + album_name: None, + album_id: None, + artist_items: None, + artists: None, + primary_image_tag: None, + image_id: None, + item_type: None, + playlist_id: None, + duration: None, + artwork_url: None, + media_type: MediaType::Video, + source: MediaSource::DirectUrl { + url: url.to_string(), + }, + video_codec: None, + needs_transcoding: false, + video_width: None, + video_height: None, + subtitles: vec![], + series_id: None, + server_id: None, + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs index a81e30fb..12fb99a2 100644 --- a/src-tauri/src/player/mod.rs +++ b/src-tauri/src/player/mod.rs @@ -5,8 +5,17 @@ pub mod autoplay; pub mod backend; pub mod background_policy; +#[cfg(any(test, feature = "conformance"))] +pub mod conformance; pub mod events; +#[cfg(any(test, feature = "conformance"))] +pub mod fake_player; +#[cfg(test)] +mod fake_player_conformance; pub mod media; +pub mod media_player; +#[cfg(target_os = "linux")] +pub mod mpv_player; pub mod queue; pub mod seek; pub mod session;