diff --git a/docs/native-player-verification.md b/docs/native-player-verification.md index 4f0e9cf0..1620e5c9 100644 --- a/docs/native-player-verification.md +++ b/docs/native-player-verification.md @@ -25,8 +25,19 @@ release mechanics. This covers whether the player is fit to release at all. a wrong capability for ExoPlayer (DR-246 follow-up) and a `Duration` panic (DR-252). Both were invisible to the test suites. -The suites verify engines that behave. **The manual passes exist to catch -engines that do not.** +The suites originally verified only engines that *behave*, which is why both +regressions passed them. That gap is now partly closed in code rather than in +this document: `UT-223` drives a deliberately hostile engine — `C.TIME_UNSET`, +NaN, infinities, negatives — through the adapter, and fails with the exact +panic that produced a black screen on a tablet. `UT-224` pins the handoff +clearing that was previously verified by listening to a device. + +**Prefer moving cases out of this file and into tests.** Anything here that +could fail automatically should; a checklist depends on someone remembering to +follow it, and the two defects it was written for cost hardware time that would +have been better spent making the suites realistic. What is left below is what +genuinely needs eyes, ears, or a display — not what merely has not been +automated yet. ## 1. Automated gates diff --git a/docs/requirements.md b/docs/requirements.md index d6d2ca3f..560759f7 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -760,6 +760,8 @@ Internal architecture, components, and application logic. | 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 | | UT-221 | An engine that cannot report a duration does not erase the one the item carries: with the queue holding a 1800s item and the engine answering nothing usable, the controller still reports 1800s | DR-251 | Done | | UT-222 | The values that killed the backend are rejected rather than converted: `C.TIME_UNSET` as seconds, negatives, zero, NaN and both infinities all yield no duration, while a real runtime survives | DR-252 | Done | +| UT-223 | The adapter survives an engine that answers badly. A `HostileBackend` reports `C.TIME_UNSET` as seconds, NaN, both infinities, a negative and a zero; reading a snapshot yields no duration and a zero position rather than panicking, and a well-behaved engine still round-trips. The conformance suite could not have caught this — it only ever drives engines that report sane numbers, which is why it stayed green while a real one took the backend down | DR-252 | Done | +| UT-224 | Stopping clears an active background-audio handoff, both the flag and the base offset, so a later position read cannot be interpreted against a handoff that no longer exists. Previously verified only by listening to a device | DR-250 | Done | ### Integration Tests diff --git a/src-tauri/src/player/legacy_player.rs b/src-tauri/src/player/legacy_player.rs index 03675497..d5a7dd4a 100644 --- a/src-tauri/src/player/legacy_player.rs +++ b/src-tauri/src/player/legacy_player.rs @@ -152,3 +152,107 @@ impl MediaPlayer for LegacyPlayer { self.capabilities } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::player::media::MediaItem; + use crate::settings::AudioSettings; + + /// A backend that answers badly, on purpose. + /// + /// Every engine the conformance suite drives reports sane numbers, which is + /// why it passed while a real one did not: ExoPlayer returns + /// `C.TIME_UNSET` — `Long::MIN_VALUE`, about -9.2e15 seconds — for any + /// stream whose length it does not know, and the adapter converted that + /// straight into a `Duration` and panicked the whole backend. + /// + /// The old `PlayerBackend` contract is a plain `f64`. It never promised + /// finite, never promised positive, and nothing enforced it. So this is the + /// engine the suites were missing. + struct HostileBackend { + duration: f64, + position: f64, + } + + impl PlayerBackend for HostileBackend { + fn load(&mut self, _media: &MediaItem) -> Result<(), PlayerError> { + Ok(()) + } + fn play(&mut self) -> Result<(), PlayerError> { + Ok(()) + } + fn pause(&mut self) -> Result<(), PlayerError> { + Ok(()) + } + fn stop(&mut self) -> Result<(), PlayerError> { + Ok(()) + } + fn seek(&mut self, _position: f64) -> Result<(), PlayerError> { + Ok(()) + } + fn set_volume(&mut self, _volume: f32) -> Result<(), PlayerError> { + Ok(()) + } + fn position(&self) -> f64 { + self.position + } + fn duration(&self) -> Option { + Some(self.duration) + } + fn state(&self) -> PlayerState { + PlayerState::Idle + } + fn volume(&self) -> f32 { + 1.0 + } + fn set_audio_settings(&mut self, _s: &AudioSettings) -> Result<(), PlayerError> { + Ok(()) + } + fn audio_settings(&self) -> AudioSettings { + AudioSettings::default() + } + fn set_audio_track(&mut self, _i: i32) -> Result<(), PlayerError> { + Ok(()) + } + fn set_subtitle_track(&mut self, _i: Option) -> Result<(), PlayerError> { + Ok(()) + } + } + + fn hostile(duration: f64, position: f64) -> LegacyPlayer { + LegacyPlayer::new( + HostileBackend { duration, position }, + crate::player::media_player::Capabilities::mpv(), + ) + } + + /// Reading an engine that answers badly must not take the process down. + /// + /// This is DR-252 as a test. It fails — by panicking — against the adapter + /// as originally written, which is the property the conformance suite could + /// not have: it only ever drove engines that behave. + /// + /// TRACES: UR-005 | DR-252 | UT-223 + #[test] + fn test_snapshot_survives_an_engine_that_answers_badly() { + // The exact value ExoPlayer reports for an unknown length. + let s = hostile(-9_223_372_036_854_776.0, 0.0).snapshot(); + assert_eq!(s.duration, None, "a negative duration is not a duration"); + + for bad in [f64::NAN, f64::NEG_INFINITY, f64::INFINITY, -1.0, 0.0] { + let s = hostile(bad, bad).snapshot(); + assert_eq!(s.duration, None, "{bad} should not become a duration"); + assert_eq!( + s.position, + Duration::ZERO, + "{bad} should not become a position" + ); + } + + // And a well-behaved engine still works. + let s = hostile(6997.024, 540.0).snapshot(); + assert_eq!(s.duration, Some(Duration::from_secs_f64(6997.024))); + assert_eq!(s.position, Duration::from_secs_f64(540.0)); + } +} diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs index c630ce19..d9a7ff60 100644 --- a/src-tauri/src/player/mod.rs +++ b/src-tauri/src/player/mod.rs @@ -2313,6 +2313,45 @@ impl Default for PlayerController { #[cfg(test)] mod tests { + /// Stopping clears a background-audio handoff. + /// + /// This was verified by listening to a tablet, which is not a test. The + /// handoff swaps which renderer owns playback, and the swap is bookkeeping: + /// leaving the base offset and the active flag behind after a stop lets a + /// later position read be interpreted against a handoff that no longer + /// exists, and left the film playing on as an audio track in the mini + /// player. + /// + /// TRACES: UR-040, UR-005 | DR-250 | UT-224 + #[test] + fn test_stop_clears_an_active_background_audio_handoff() { + let controller = PlayerController::default(); + let item = MediaItem::sample("item-1", "https://example.invalid/a.mp4"); + { + let queue_arc = controller.queue(); + let mut queue = queue_arc.lock_safe(); + queue.set_queue(vec![item], 0); + } + + controller.enter_background_audio(557.5); + assert!( + controller.is_background_audio_active(), + "precondition: the handoff is active" + ); + + controller.stop().expect("stop failed"); + + assert!( + !controller.is_background_audio_active(), + "a stop must not leave a handoff behind for the next position read" + ); + assert_eq!( + *controller.background_audio_base.lock_safe(), + 0.0, + "the handoff base must be cleared with it" + ); + } + /// A duration the engine does not know must fall back to the one the item /// carries, and zero must count as "does not know". ///