fix(player): survive a flaky stream, and don't read 0:00 at EOF
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.
This commit is contained in:
@@ -2,6 +2,7 @@ use super::backend::{PlayerBackend, PlayerError};
|
|||||||
use super::events::{PlayerEventEmitter, PlayerStatusEvent};
|
use super::events::{PlayerEventEmitter, PlayerStatusEvent};
|
||||||
use super::media::{MediaItem, MediaSource};
|
use super::media::{MediaItem, MediaSource};
|
||||||
use super::state::PlayerState;
|
use super::state::PlayerState;
|
||||||
|
use super::stream_end::ObservedTime;
|
||||||
use crate::playback_reporting::{EventThrottler, PlaybackOperation, PlaybackReporter};
|
use crate::playback_reporting::{EventThrottler, PlaybackOperation, PlaybackReporter};
|
||||||
use crate::settings::{AudioSettings, VolumeLevel, EQ_BANDS};
|
use crate::settings::{AudioSettings, VolumeLevel, EQ_BANDS};
|
||||||
use crate::utils::conversions::{seconds_to_ticks, volume_to_percent};
|
use crate::utils::conversions::{seconds_to_ticks, volume_to_percent};
|
||||||
@@ -26,6 +27,13 @@ pub struct MpvBackend {
|
|||||||
playback_reporter: Arc<TokioMutex<Option<PlaybackReporter>>>,
|
playback_reporter: Arc<TokioMutex<Option<PlaybackReporter>>>,
|
||||||
position_throttler: Arc<EventThrottler>,
|
position_throttler: Arc<EventThrottler>,
|
||||||
last_seek_time: Arc<AtomicU64>,
|
last_seek_time: Arc<AtomicU64>,
|
||||||
|
/// 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<Mutex<ObservedTime>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
struct InternalState {
|
struct InternalState {
|
||||||
@@ -139,6 +147,31 @@ impl MpvBackend {
|
|||||||
message: format!("Failed to set initial volume: {:?}", e),
|
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 {
|
let state = Arc::new(Mutex::new(InternalState {
|
||||||
current_media: None,
|
current_media: None,
|
||||||
volume: 1.0,
|
volume: 1.0,
|
||||||
@@ -152,6 +185,7 @@ impl MpvBackend {
|
|||||||
playback_reporter,
|
playback_reporter,
|
||||||
position_throttler,
|
position_throttler,
|
||||||
last_seek_time: Arc::new(AtomicU64::new(0)),
|
last_seek_time: Arc::new(AtomicU64::new(0)),
|
||||||
|
observed: Arc::new(Mutex::new(ObservedTime::default())),
|
||||||
};
|
};
|
||||||
|
|
||||||
// Start event loop in background thread
|
// Start event loop in background thread
|
||||||
@@ -250,8 +284,22 @@ impl MpvBackend {
|
|||||||
debug!("[MpvBackend] Player quitting, NOT emitting PlaybackEnded");
|
debug!("[MpvBackend] Player quitting, NOT emitting PlaybackEnded");
|
||||||
// Don't emit - player is shutting down
|
// Don't emit - player is shutting down
|
||||||
} else if reason == MPV_END_FILE_REASON_ERROR {
|
} else if reason == MPV_END_FILE_REASON_ERROR {
|
||||||
warn!("[MpvBackend] Track ended with error, NOT emitting PlaybackEnded");
|
// NOT PlaybackEnded — the track did not finish, so
|
||||||
// Don't emit - we should handle errors separately
|
// 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 {
|
} else {
|
||||||
debug!("[MpvBackend] Unknown end file reason {}, NOT emitting PlaybackEnded", reason);
|
debug!("[MpvBackend] Unknown end file reason {}, NOT emitting PlaybackEnded", reason);
|
||||||
}
|
}
|
||||||
@@ -283,6 +331,7 @@ impl MpvBackend {
|
|||||||
let reporter_for_position = reporter.clone();
|
let reporter_for_position = reporter.clone();
|
||||||
let throttler_for_position = throttler.clone();
|
let throttler_for_position = throttler.clone();
|
||||||
let last_seek_time_for_position = self.last_seek_time.clone();
|
let last_seek_time_for_position = self.last_seek_time.clone();
|
||||||
|
let observed_for_position = self.observed.clone();
|
||||||
|
|
||||||
std::thread::spawn(move || {
|
std::thread::spawn(move || {
|
||||||
loop {
|
loop {
|
||||||
@@ -294,6 +343,13 @@ impl MpvBackend {
|
|||||||
mpv_for_position.get_property::<f64>("time-pos"),
|
mpv_for_position.get_property::<f64>("time-pos"),
|
||||||
mpv_for_position.get_property::<f64>("duration"),
|
mpv_for_position.get_property::<f64>("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
|
// Check if we recently seeked - skip position updates briefly after seeks
|
||||||
// to avoid "jumping to zero" visual glitches while MPV is seeking
|
// to avoid "jumping to zero" visual glitches while MPV is seeking
|
||||||
let now = SystemTime::now()
|
let now = SystemTime::now()
|
||||||
@@ -404,6 +460,9 @@ impl PlayerBackend for MpvBackend {
|
|||||||
let mut state = self.state.lock_safe();
|
let mut state = self.state.lock_safe();
|
||||||
state.current_media = Some(media.clone());
|
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
|
// Load the media file
|
||||||
self.mpv
|
self.mpv
|
||||||
@@ -469,6 +528,10 @@ impl PlayerBackend for MpvBackend {
|
|||||||
message: format!("Failed to seek: {:?}", e),
|
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(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -491,15 +554,26 @@ impl PlayerBackend for MpvBackend {
|
|||||||
Ok(())
|
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 {
|
fn position(&self) -> f64 {
|
||||||
self.mpv.get_property::<f64>("time-pos").unwrap_or(0.0)
|
let live = self.mpv.get_property::<f64>("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<f64> {
|
fn duration(&self) -> Option<f64> {
|
||||||
self.mpv
|
let live = self.mpv.get_property::<f64>("duration").ok();
|
||||||
.get_property::<f64>("duration")
|
self.observed.lock_safe().duration_or_last(live)
|
||||||
.ok()
|
|
||||||
.filter(|d| *d > 0.0)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn state(&self) -> PlayerState {
|
fn state(&self) -> PlayerState {
|
||||||
|
|||||||
@@ -102,6 +102,54 @@ pub fn with_start_time(url: &str, position_seconds: f64) -> String {
|
|||||||
format!("{}?{}", base, parts.join("&"))
|
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<f64>,
|
||||||
|
}
|
||||||
|
|
||||||
|
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>) -> 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<f64>) -> Option<f64> {
|
||||||
|
live.filter(|d| *d > 0.0).or(self.duration)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Budget for consecutive resume attempts that make no progress.
|
/// Budget for consecutive resume attempts that make no progress.
|
||||||
///
|
///
|
||||||
/// Held by the player controller across ends of the *same* stream. Any position
|
/// 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]
|
#[test]
|
||||||
fn test_resume_tracker_bounds_stalled_retries() {
|
fn test_resume_tracker_bounds_stalled_retries() {
|
||||||
let mut tracker = ResumeTracker::default();
|
let mut tracker = ResumeTracker::default();
|
||||||
|
|||||||
Reference in New Issue
Block a user