From 32f8de5c91e20ae32f9d232fbc18d1c7b5972096 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Tue, 4 Aug 2026 19:39:14 +0200 Subject: [PATCH] fix(player): survive a flaky stream, and don't read 0:00 at EOF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two MPV-side fixes for the same failure story — a wifi blip during playback. The demuxer gave up the moment a read failed and MPV raised EndFile(ERROR), so a momentary outage killed the track outright. Enabling ffmpeg's reconnect options handles the common case entirely below our level, so most outages never reach the recovery path at all. Set non-fatally: their availability varies with the libmpv/ffmpeg build, and losing resilience is not a reason to refuse to play anything. Separately, `time-pos` and `duration` are live properties of the *loaded* file: at EOF MPV unloads it and both stop resolving. Reading them straight through returned 0.0/unknown at exactly the moment end-of-file handling needed to know where playback had reached, so the player appeared to rewind to 0:00 as a track ended. `ObservedTime` records the last reading seen while media was loaded and the accessors fall back to it. --- src-tauri/src/player/mpv_backend.rs | 88 +++++++++++++++++++++-- src-tauri/src/player/stream_end.rs | 106 ++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/player/mpv_backend.rs b/src-tauri/src/player/mpv_backend.rs index 7d929116..466ed796 100644 --- a/src-tauri/src/player/mpv_backend.rs +++ b/src-tauri/src/player/mpv_backend.rs @@ -2,6 +2,7 @@ use super::backend::{PlayerBackend, PlayerError}; use super::events::{PlayerEventEmitter, PlayerStatusEvent}; use super::media::{MediaItem, MediaSource}; use super::state::PlayerState; +use super::stream_end::ObservedTime; use crate::playback_reporting::{EventThrottler, PlaybackOperation, PlaybackReporter}; use crate::settings::{AudioSettings, VolumeLevel, EQ_BANDS}; use crate::utils::conversions::{seconds_to_ticks, volume_to_percent}; @@ -26,6 +27,13 @@ pub struct MpvBackend { playback_reporter: Arc>>, position_throttler: Arc, last_seek_time: Arc, + /// Last position/duration seen while a file was loaded. + /// + /// `time-pos` and `duration` are live properties of the *loaded* file: at + /// EOF MPV unloads it and both stop resolving, so reading them straight + /// through reported 0.0 / unknown exactly when end-of-file handling needed to + /// know where playback reached. See [`ObservedTime`]. + observed: Arc>, } struct InternalState { @@ -139,6 +147,31 @@ impl MpvBackend { message: format!("Failed to set initial volume: {:?}", e), })?; + // Survive a flaky connection instead of dying on it. Without these, + // ffmpeg's HTTP demuxer gives up the moment a read fails and MPV raises + // EndFile(ERROR) — a blip on wifi kills the track outright. Reconnecting + // in the demuxer handles the common case entirely below our level, so + // most outages never reach the recovery in `player_recover_stream`. + // + // Non-fatal: these are ffmpeg-side options whose availability varies with + // the libmpv/ffmpeg build, and losing resilience is not a reason to + // refuse to play anything (graceful backend init, CLAUDE.md). + mpv.set_property( + "stream-lavf-o", + "reconnect=1,reconnect_streamed=1,reconnect_on_network_error=1,reconnect_delay_max=5", + ) + .unwrap_or_else(|e| { + warn!( + "[MpvBackend] Could not enable stream reconnection: {:?} — \ + playback will not survive network interruptions", + e + ); + }); + mpv.set_property("network-timeout", 15i64) + .unwrap_or_else(|e| { + warn!("[MpvBackend] Could not set network timeout: {:?}", e); + }); + let state = Arc::new(Mutex::new(InternalState { current_media: None, volume: 1.0, @@ -152,6 +185,7 @@ impl MpvBackend { playback_reporter, position_throttler, last_seek_time: Arc::new(AtomicU64::new(0)), + observed: Arc::new(Mutex::new(ObservedTime::default())), }; // Start event loop in background thread @@ -250,8 +284,22 @@ impl MpvBackend { debug!("[MpvBackend] Player quitting, NOT emitting PlaybackEnded"); // Don't emit - player is shutting down } else if reason == MPV_END_FILE_REASON_ERROR { - warn!("[MpvBackend] Track ended with error, NOT emitting PlaybackEnded"); - // Don't emit - we should handle errors separately + // NOT PlaybackEnded — the track did not finish, so + // autoplay must not advance. It is an error, and it + // has to be *said*: emitting nothing here left + // playback halted with the UI still showing + // "playing" and no way back. Marked recoverable so + // the frontend echoes it into player_recover_stream, + // which re-opens the stream where it stopped — + // MPV's own reconnect handles shorter blips before + // they ever get this far. + warn!("[MpvBackend] Track ended with an error — reporting as recoverable"); + if let Some(emitter) = &event_emitter { + emitter.emit(PlayerStatusEvent::Error { + message: "Playback stream failed".to_string(), + recoverable: true, + }); + } } else { debug!("[MpvBackend] Unknown end file reason {}, NOT emitting PlaybackEnded", reason); } @@ -283,6 +331,7 @@ impl MpvBackend { let reporter_for_position = reporter.clone(); let throttler_for_position = throttler.clone(); let last_seek_time_for_position = self.last_seek_time.clone(); + let observed_for_position = self.observed.clone(); std::thread::spawn(move || { loop { @@ -294,6 +343,13 @@ impl MpvBackend { mpv_for_position.get_property::("time-pos"), mpv_for_position.get_property::("duration"), ) { + // Remember it: both properties belong to the *loaded* file and + // stop resolving the instant MPV unloads it at EOF, which is + // exactly when end-of-file handling asks where playback got to. + // Recorded before the post-seek skip below so a track that ends + // right after a seek still reports the seek target, not zero. + observed_for_position.lock_safe().record(pos, dur); + // Check if we recently seeked - skip position updates briefly after seeks // to avoid "jumping to zero" visual glitches while MPV is seeking let now = SystemTime::now() @@ -404,6 +460,9 @@ impl PlayerBackend for MpvBackend { let mut state = self.state.lock_safe(); state.current_media = Some(media.clone()); } + // A different file: the previous one's timestamp must not survive as this + // one's "last observed" position. + self.observed.lock_safe().reset(); // Load the media file self.mpv @@ -469,6 +528,10 @@ impl PlayerBackend for MpvBackend { message: format!("Failed to seek: {:?}", e), })?; + // The poll thread suppresses updates for 150ms after a seek, so without + // this a file ending inside that window would report the pre-seek time. + self.observed.lock_safe().record_position(position); + Ok(()) } @@ -491,15 +554,26 @@ impl PlayerBackend for MpvBackend { Ok(()) } + /// Current position — the live `time-pos`, or the last one observed while a + /// file was loaded. + /// + /// The fallback is the point: `time-pos` is a property of the *loaded* file, + /// so at EOF it stops resolving and a bare `unwrap_or(0.0)` reported 0:00 at + /// exactly the moment end-of-file handling asks where playback reached. + /// + /// TRACES: UR-005 | DR-130 | UT-118 fn position(&self) -> f64 { - self.mpv.get_property::("time-pos").unwrap_or(0.0) + let live = self.mpv.get_property::("time-pos").ok(); + self.observed.lock_safe().position_or_last(live) } + /// Total duration — live, or the last one observed. Unloaded at EOF for the + /// same reason as `position`. + /// + /// TRACES: UR-005 | DR-130 | UT-118 fn duration(&self) -> Option { - self.mpv - .get_property::("duration") - .ok() - .filter(|d| *d > 0.0) + let live = self.mpv.get_property::("duration").ok(); + self.observed.lock_safe().duration_or_last(live) } fn state(&self) -> PlayerState { diff --git a/src-tauri/src/player/stream_end.rs b/src-tauri/src/player/stream_end.rs index fad5e6b9..40f83446 100644 --- a/src-tauri/src/player/stream_end.rs +++ b/src-tauri/src/player/stream_end.rs @@ -102,6 +102,54 @@ pub fn with_start_time(url: &str, position_seconds: f64) -> String { format!("{}?{}", base, parts.join("&")) } +/// The last playback time actually observed while media was loaded. +/// +/// Some backends expose position and duration as **live** properties of the +/// loaded file — MPV's `time-pos` and `duration` stop resolving the moment it +/// unloads the file at EOF. Reading them straight through means that at exactly +/// the moment end-of-file handling wants to know where playback got to, the +/// answer is `0.0` / unknown: the player appears to rewind to 0:00 as it ends. +/// +/// The polling thread records here, and the accessors fall back to it, so an EOF +/// reads as the last timestamp rather than as zero. +#[derive(Debug, Default, Clone, Copy)] +pub struct ObservedTime { + position: f64, + duration: Option, +} + +impl ObservedTime { + /// Record a live reading. Non-positive durations are treated as unknown — + /// that is how a backend reports "not established yet", not a real zero. + pub fn record(&mut self, position: f64, duration: f64) { + self.position = position.max(0.0); + if duration > 0.0 { + self.duration = Some(duration); + } + } + + /// Record a position alone, e.g. straight after a seek, before the next poll. + pub fn record_position(&mut self, position: f64) { + self.position = position.max(0.0); + } + + /// Forget everything — a different file is loading, and the previous one's + /// timestamp must not leak into it. + pub fn reset(&mut self) { + *self = Self::default(); + } + + /// The live reading if there is one, else the last observed value. + pub fn position_or_last(&self, live: Option) -> f64 { + live.filter(|p| *p >= 0.0).unwrap_or(self.position) + } + + /// The live reading if there is one, else the last observed value. + pub fn duration_or_last(&self, live: Option) -> Option { + live.filter(|d| *d > 0.0).or(self.duration) + } +} + /// Budget for consecutive resume attempts that make no progress. /// /// Held by the player controller across ends of the *same* stream. Any position @@ -225,6 +273,64 @@ mod tests { ); } + /// The bug: MPV unloads the file at EOF, so `time-pos` stops resolving and a + /// straight read reports 0.0 — the position collapses to zero at precisely + /// the moment end-of-file handling needs to know where playback reached. + #[test] + fn test_eof_reads_as_the_last_observed_timestamp() { + let mut observed = ObservedTime::default(); + observed.record(178.0, 180.0); + + // The file is gone: both live properties fail. + assert_eq!(observed.position_or_last(None), 178.0); + assert_eq!(observed.duration_or_last(None), Some(180.0)); + } + + #[test] + fn test_live_readings_win_while_the_file_is_loaded() { + let mut observed = ObservedTime::default(); + observed.record(178.0, 180.0); + + assert_eq!(observed.position_or_last(Some(12.0)), 12.0); + assert_eq!(observed.duration_or_last(Some(240.0)), Some(240.0)); + } + + #[test] + fn test_unestablished_duration_is_not_recorded_as_zero() { + let mut observed = ObservedTime::default(); + // A backend reports 0.0 for "duration not known yet", not a real zero. + observed.record(5.0, 0.0); + assert_eq!(observed.duration_or_last(None), None); + assert_eq!(observed.position_or_last(None), 5.0); + + observed.record(6.0, 180.0); + assert_eq!(observed.duration_or_last(Some(0.0)), Some(180.0)); + } + + #[test] + fn test_reset_stops_the_previous_file_leaking_into_the_next() { + let mut observed = ObservedTime::default(); + observed.record(178.0, 180.0); + observed.reset(); + + assert_eq!(observed.position_or_last(None), 0.0); + assert_eq!(observed.duration_or_last(None), None); + } + + #[test] + fn test_seek_updates_the_last_position_before_the_next_poll() { + let mut observed = ObservedTime::default(); + observed.record(10.0, 180.0); + observed.record_position(120.0); + + assert_eq!(observed.position_or_last(None), 120.0); + assert_eq!( + observed.duration_or_last(None), + Some(180.0), + "seeking does not change how long the file is" + ); + } + #[test] fn test_resume_tracker_bounds_stalled_retries() { let mut tracker = ResumeTracker::default();