Files
jellytau/src-tauri/src/player/legacy_player.rs
T
dtourolle fd8273824a
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 42s
🏗️ Build and Test JellyTau / Android Compile Check (push) Skipped
🏗️ Build and Test JellyTau / Supply Chain (push) Successful in 3m46s
Publish Documentation / Build & publish docs to gitea-pages (push) Successful in 5m53s
Traceability Validation / Check Requirement Traces (push) Successful in 16s
Build & Release / Run Tests (push) Successful in 15m15s
Build & Release / Build Linux (push) Successful in 21m14s
Build & Release / Build Windows (push) Successful in 15m53s
Build & Release / Build Android (push) Successful in 31m34s
Build & Release / Create Release (push) Successful in 53s
test(player): drive an engine that answers badly
Fair criticism: hardware time went into writing a checklist describing what the
tablet found, when it should have gone into making the suites able to find it.
A checklist decays and depends on someone following it. A test does not.

The gap was specific. Every engine the conformance suite drives reports sane
numbers, so it stayed green while a real one took the backend down. The old
PlayerBackend contract is a plain f64 — it never promised finite, never
promised positive, and nothing enforced it.

UT-223 adds the engine that was missing: a HostileBackend answering with
C.TIME_UNSET as seconds, NaN, both infinities, a negative and a zero. Reading a
snapshot must yield no duration and a zero position rather than panicking.
Against the adapter as originally written it fails with

    cannot convert float seconds to Duration: value is negative

which is the exact panic that produced a black screen on the tablet — now
reproduced in 0.00s on a laptop instead of by backgrounding an app.

UT-224 pins the other hardware-only finding: stopping clears an active
background-audio handoff, flag and base offset both. That was verified by
listening to a device, which is not a test.

Both were confirmed to fail against the pre-fix code before being kept.

The verification plan now says to prefer moving cases out of it and into tests,
and that what remains should be what genuinely needs eyes, ears or a display —
not what merely has not been automated yet.

793 Rust tests.
2026-08-23 10:54:49 +02:00

259 lines
9.0 KiB
Rust

//! 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
use std::time::Duration;
use super::backend::{PlayerBackend, PlayerError};
use super::media_player::{
duration_from_secs, Capabilities, MediaPlayer, OpenRequest, Phase, PlaybackSnapshot,
};
use super::state::PlayerState;
pub struct LegacyPlayer<B: PlayerBackend> {
inner: B,
/// Declared at construction: this wrapper is generic over engines with very
/// different abilities, and only the composition root knows which one it
/// just built. Guessing here would reintroduce exactly the inference DR-238
/// removed.
capabilities: Capabilities,
/// 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, capabilities: Capabilities) -> Self {
Self {
inner,
capabilities,
has_item: false,
}
}
}
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(self.inner.position()).unwrap_or(Duration::ZERO),
duration: self.inner.duration().and_then(duration_from_secs),
seekable: true,
volume: self.inner.volume(),
muted: false,
rate: 1.0,
audio_track: None,
subtitle_track: None,
}
}
fn set_audio_settings(
&mut self,
settings: &crate::settings::AudioSettings,
) -> Result<(), PlayerError> {
self.inner.set_audio_settings(settings)
}
fn audio_settings(&self) -> crate::settings::AudioSettings {
self.inner.audio_settings()
}
fn capabilities(&self) -> Capabilities {
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<f64> {
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<i32>) -> Result<(), PlayerError> {
Ok(())
}
}
fn hostile(duration: f64, position: f64) -> LegacyPlayer<HostileBackend> {
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));
}
}