diff --git a/src-tauri/src/commands/player/mod.rs b/src-tauri/src/commands/player/mod.rs index bc2ab566..15f41e2b 100644 --- a/src-tauri/src/commands/player/mod.rs +++ b/src-tauri/src/commands/player/mod.rs @@ -454,6 +454,49 @@ pub(super) fn background_audio_source( } } +/// How a background-audio handoff must start playback, given where its audio +/// actually begins. +/// +/// TRACES: UR-040, UR-071 | DR-180 | UT-181 +pub(super) struct BackgroundAudioPlan { + /// The position the stream's own zero corresponds to, recorded as the + /// handoff base so later readings can be shifted back to the episode's + /// timeline. + pub base_seconds: f64, + /// Where to seek after loading, if the source does not already start there. + pub seek_to: Option, +} + +/// Decide the base and the seek for a handoff at `position_seconds`. +/// +/// The two sources start in different places. An audio-only **stream** is built +/// with `StartTimeTicks`, so the server makes the handoff point that stream's +/// zero: the base is the handoff position, and seeking would skip *past* the +/// content by that much again. A downloaded **file** has no such parameter and +/// begins at the episode's own zero, so it needs the opposite — no base, and a +/// real seek. Treating a file like a stream is why backgrounding a downloaded +/// episode restarted it from 0:00 while the lockscreen showed the right time. +/// +/// TRACES: UR-040, UR-071 | DR-180 | UT-181 +pub(super) fn background_audio_plan( + is_local_file: bool, + position_seconds: f64, +) -> BackgroundAudioPlan { + let position = position_seconds.max(0.0); + + if is_local_file { + BackgroundAudioPlan { + base_seconds: 0.0, + seek_to: (position > 0.0).then_some(position), + } + } else { + BackgroundAudioPlan { + base_seconds: position, + seek_to: None, + } + } +} + /// Resolve the on-disk file backing a completed download, if there is one. /// /// A `downloads` row is not proof of a file: it can outlive the bytes (manual @@ -710,6 +753,9 @@ pub async fn player_enter_background_audio( item.id ); } + // A downloaded file starts at the episode's zero; a stream starts at the + // handoff point. Only one of them has a base, and only the other needs a seek. + let plan = background_audio_plan(local_path.is_some(), position_seconds); let source = background_audio_source(local_path, item.stream_url, &item.id); // Build an AUDIO media item pointing at the audio-only stream. We do not use @@ -753,21 +799,28 @@ pub async fn player_enter_background_audio( // Same base offset drives the lockscreen scrubber: ExoPlayer reports position // relative to the stream's StartTimeTicks zero, but the metadata duration is // absolute, so shift the reported position back to absolute for the scrubber. - let _ = crate::player::set_lockscreen_position_offset(position_seconds.max(0.0)); + let _ = crate::player::set_lockscreen_position_offset(plan.base_seconds); let controller = player.0.lock().await; - // Remember where the video was: the audio stream's zero == this position - // (the URL was built with StartTimeTicks=position_seconds), so on exit we add - // this base to the native player's relative position to get the absolute one. - // The controller owns it so a backend-driven advance to the next episode - // clears it along with the stream it described. - controller.enter_background_audio(position_seconds); + // Remember where the video was: for a stream the audio's zero == this + // position (the URL was built with StartTimeTicks=position_seconds), so on + // exit we add this base to the native player's relative position to get the + // absolute one. The controller owns it so a backend-driven advance to the + // next episode clears it along with the stream it described. + controller.enter_background_audio(plan.base_seconds); controller .play_item(media_item) .map_err(|e| e.to_string())?; - // NOTE: do NOT seek here. The audio-only URL already starts at the handoff - // position via StartTimeTicks; the stream's timeline begins at 0 == that - // point, so an extra seek(position_seconds) would jump PAST the content. + // Seek ONLY a local file. The audio-only URL already starts at the handoff + // position via StartTimeTicks — its timeline begins at 0 == that point — so + // seeking a stream would jump PAST the content by the handoff position again. + if let Some(seek_to) = plan.seek_to { + info!( + "player_enter_background_audio: seeking the downloaded file to {:.1}s", + seek_to + ); + controller.seek(seek_to).map_err(|e| e.to_string())?; + } controller.emit_queue_changed(); if let Some(emitter) = controller.event_emitter() { @@ -801,7 +854,14 @@ pub async fn player_exit_background_audio( // moment it matters most. Capturing into a `let` before stop() is also the // lock discipline from CLAUDE.md: never hold work across a re-entrant call. // (DR-159) - let absolute = controller.position(); + // + // `absolute_position` rather than `position`, because a tick that has not + // landed *yet* is the same hazard from the other side: returning to the + // foreground while the audio-only transcode is still opening read 0.0, and + // the video reloaded at StartTimeTicks=0 — the episode restarting from the + // beginning. Flooring at the handoff base cannot overshoot: the stream is + // physically incapable of being behind its own starting point. (DR-178) + let absolute = controller.absolute_position(); // Now safe to tear the handoff down, native side first. let _ = crate::player::set_lockscreen_position_offset(0.0); @@ -1846,7 +1906,11 @@ pub(super) fn get_player_status(controller: &PlayerController) -> PlayerStatus { PlayerStatus { state: controller.state(), - position: controller.position(), + // The position on the item's timeline, whichever of the three paths is + // rendering it — the native backend answers for only one of them, and + // reads 0 for webview video and for a handoff that has not ticked yet. + // TRACES: UR-005 | DR-178 + position: controller.absolute_position(), duration: controller.duration(), volume: controller.volume(), muted: controller.muted(), @@ -2838,6 +2902,49 @@ mod tests { } } + /// The two sources start in different places, so the handoff cannot treat + /// them alike. + /// + /// An audio-only *stream* is built with `StartTimeTicks`, so the server makes + /// the handoff point that stream's zero: the base is the handoff position and + /// seeking would jump past the content. A *downloaded file* has no such + /// parameter — it starts at the episode's own zero — so basing it at the + /// handoff position claims 18 minutes of audio that is about to play from the + /// beginning. That is the downloaded-episode version of "it restarts when the + /// screen sleeps", and it needs the opposite treatment: no base, and a seek. + /// + /// TRACES: UR-040, UR-071 | DR-180 | UT-181 + #[test] + fn test_background_audio_plan_seeks_a_file_and_bases_a_stream() { + use super::background_audio_plan; + + let local = background_audio_plan(true, 1104.0); + assert_eq!(local.base_seconds, 0.0); + assert_eq!(local.seek_to, Some(1104.0)); + + let streamed = background_audio_plan(false, 1104.0); + assert_eq!(streamed.base_seconds, 1104.0); + assert_eq!( + streamed.seek_to, None, + "the URL already starts at the handoff point; seeking again skips past it" + ); + } + + /// Handing off at the very start has nothing to seek to and nothing to base: + /// both sources are already where they need to be. + /// + /// TRACES: UR-040, UR-071 | DR-180 | UT-181 + #[test] + fn test_background_audio_plan_at_the_start_neither_seeks_nor_bases() { + use super::background_audio_plan; + + for local in [true, false] { + let plan = background_audio_plan(local, 0.0); + assert_eq!(plan.base_seconds, 0.0); + assert_eq!(plan.seek_to, None); + } + } + /// A downloaded item must resolve to its file, and a `downloads` row whose /// file has gone must resolve to `None` so the caller falls back to /// streaming instead of handing the player a path that cannot be opened. diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs index 2f1ed0ef..5c1497df 100644 --- a/src-tauri/src/player/mod.rs +++ b/src-tauri/src/player/mod.rs @@ -55,6 +55,75 @@ pub use android::{ set_remote_volume_handler, MediaCommandHandler, RemoteVolumeHandler, }; +/// Where the player's playback reports go. +/// +/// The controller's side of reporting is "send this, don't make me wait": a slow +/// or failing sync must never stall playback, so every send is fire-and-forget. +/// Production wires this to [`PlaybackReporter`] (local DB, server sync, offline +/// queueing); tests capture the operations instead of standing up a database and +/// an HTTP client, which is what let the missing reports below be written as +/// failing tests rather than found on a device. +/// +/// TRACES: UR-025 | DR-179 +pub trait PlaybackReportSink: Send + Sync { + /// Deliver `operation`. Must not block the caller. + fn send(&self, operation: PlaybackOperation); +} + +/// The production sink: hands each operation to the `PlaybackReporter`. +/// +/// Reports originate on whatever thread playback ended or ticked on — including +/// JNI callbacks with no Tokio runtime attached — so the spawn falls back to a +/// throwaway runtime on its own thread rather than assuming one is current. +struct ReporterSink { + reporter: Arc>>, +} + +impl PlaybackReportSink for ReporterSink { + fn send(&self, operation: PlaybackOperation) { + let reporter = self.reporter.clone(); + let task = async move { + let guard = reporter.lock().await; + let Some(reporter) = guard.as_ref() else { + warn!("[PlayerController] PlaybackReporter not initialized; dropping report"); + return; + }; + // `report` decides local-vs-server and queues for sync itself. + if let Err(e) = reporter.report(operation, true).await { + log::error!("[PlayerController] Failed to report playback: {}", e); + } + }; + + if let Ok(handle) = tokio::runtime::Handle::try_current() { + handle.spawn(task); + } else { + std::thread::spawn(move || match tokio::runtime::Runtime::new() { + Ok(rt) => rt.block_on(task), + Err(e) => log::error!( + "[PlayerController] No runtime available to report playback: {}", + e + ), + }); + } + } +} + +/// The position to report when a stream ends naturally. +/// +/// The item's runtime when we know it, because the point of the report is to say +/// the episode *finished* and Jellyfin decides that by percentage — the last +/// position actually observed can be seconds short, and on a handoff whose ticks +/// stopped early it can be nowhere near the end. Without a runtime the best +/// available answer is where playback got to. +/// +/// TRACES: UR-025, UR-040 | DR-179 | UT-179 +fn completion_report_position(runtime: Option, last_position: f64) -> f64 { + match runtime { + Some(runtime) if runtime > 0.0 => runtime, + _ => last_position.max(0.0), + } +} + /// Seconds added per attempt before retrying a stream that failed with an error. /// /// Attempt 1 waits this long, attempt 2 twice as long, and so on — a spread that @@ -127,6 +196,7 @@ use crate::playback_reporting::{ }; use crate::repository::MediaRepository; use crate::settings::AudioSettings; +use crate::utils::conversions::seconds_to_ticks; /// Central player controller that coordinates playback pub struct PlayerController { @@ -153,9 +223,12 @@ pub struct PlayerController { // Playback reporting (dual sync: local DB + server) playback_reporter: Arc>>, - // Event throttler to prevent spam from position updates - // Will be used when position update hooks are added to backends - #[allow(dead_code)] + // Where playback reports go. Swappable so tests can assert on what the + // player tells Jellyfin. See `PlaybackReportSink`. + reports: Arc>>, + + // Bounds progress reports to one per item per 30s. Position ticks arrive + // four times a second; the server needs a resume point, not a firehose. position_throttler: Arc, // End reason tracking for autoplay decision making @@ -209,6 +282,19 @@ pub struct PlayerController { // competing intents to take opposing actions. `None` means no webview media // is active and the native backend is authoritative. See DR-097. html5_playing: Arc>>, + + // Last position/duration reported by webview-rendered media. + // + // On the webview path the `