From 30dc3ba7f69510fa70f9097c9fda404c4afbf8a9 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Tue, 4 Aug 2026 20:15:28 +0200 Subject: [PATCH] fix(player): recover a failed stream on Linux instead of stopping (DR-130) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A recoverable player error meant "playback is over": the frontend's error handler stopped the player unconditionally, so a wifi blip killed the track. Android already decides in its JNI callback, but MpvBackend is constructed before PlayerController exists, so its event thread has no controller to ask. So MPV reports the failure and the frontend echoes it into the new player_recover_stream command — the same shape as PlaybackEnded -> player_on_playback_ended, keeping the decision in Rust. The command re-opens the stream where it stopped, with the existing attempt budget and backoff, and returns whether it handled it; only a false answer falls through to the old stop path. Android now reports the errors it has already declined as *unrecoverable*, so the echo never asks the same question twice. TRACES: UR-004, UR-040 | DR-130 | UT-117 --- docs/requirements.md | 4 +- scripts/extract-traces.test.ts | 4 +- src-tauri/src/commands/player/timers.rs | 47 ++++++++++++ src-tauri/src/lib.rs | 2 + src-tauri/src/player/android/mod.rs | 7 +- src-tauri/src/player/mod.rs | 10 ++- src-tauri/src/player/mpv_backend.rs | 4 +- src/lib/api/bindings.ts | 23 ++++++ .../services/playerEvents.regression.test.ts | 73 +++++++++++++++++++ src/lib/services/playerEvents.ts | 23 ++++++ 10 files changed, 186 insertions(+), 11 deletions(-) diff --git a/docs/requirements.md b/docs/requirements.md index 36d98d92..ae7b1a8e 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -298,6 +298,7 @@ Internal architecture, components, and application logic. | DR-127 | A cache entry *is* a download with a shorter life: same `downloads` row and same file handling, distinguished by `download_source = 'auto'` plus an expiry, so there is one storage model rather than a cache and a download library that can disagree. Temporary rows are reclaimed on whichever comes first — the life limit elapsing, or eviction under space pressure (DR-126). Permanent (`'user'`) rows have no expiry. A temporary row can be promoted to permanent by the user choosing to keep it, which only clears the expiry and flips the source; the bytes never move | Storage | UR-071 | Done | | DR-128 | Audio-only playback of *downloaded* media reads the local file rather than fetching an audio-only stream. No transcode is involved or wanted: the Linux backend already runs MPV with `video: no`, so handing it the downloaded video file decodes the audio track and ignores the video, and ExoPlayer disables its video renderer equivalently. Transcoding to a separate audio artifact would cost CPU and battery, need an encoder the project does not ship, and produce a second file to keep in step — for no gain over simply not decoding the video | Playback | UR-071 | Done | | DR-129 | A stream that stops delivering is recovered, not treated as terminal. Two failure shapes, because the streams differ. (a) *Phantom end* — the background audio-only handoff uses a progressive mp3 transcode over plain HTTP, chunked and therefore length-less, so a dropped connection reaches the player as end-of-input and ExoPlayer reports `STATE_ENDED` indistinguishably from the real end. The item's runtime is the only thing that can tell them apart: an end reported more than a tolerance short of it (comparing the *absolute* position — handoff base plus the player's relative position) is a truncation. Left unhandled, playback parked in `STATE_ENDED` and the next play intent from the lockscreen, notification or a Bluetooth reconnect seeks an ended player to position 0 — the user-visible "the episode randomly restarted". (b) *Recoverable error* — music (`/Audio/{id}/stream?Static=true`) and video (`/Videos/{id}/master.m3u8`) declare their length, so the player detects the truncation itself and raises an error; the frontend's handler stopped playback outright, turning a hiccup into silence. Both resume the current item **in place** (never via `play_item`, which would replace the queue with a single item and lose the album), the error path after a per-attempt backoff. Seekable streams are re-prepared at the URL they already have and seeked; the length-less transcode, which cannot be seeked, has `StartTimeTicks` rewritten into its existing URL so the user's audio-track selection survives and recovery needs no network round-trip. Only `Remote` sources qualify — a local file cannot fail from the network. A shared budget of consecutive attempts at the same position, refilled whenever playback progresses, stops an unreachable server from looping | Playback | UR-040, UR-004 | Done | +| DR-130 | A backend's position and duration must survive the end of the file they describe. MPV exposes `time-pos`/`duration` as properties of the *loaded* file, so at EOF it unloads and both stop resolving — the accessors reported `0.0`/unknown at exactly the moment end-of-file handling asks where playback reached, and any position-versus-runtime check would have read every natural end as a truncation. The poll thread records the last reading and the accessors fall back to it. Linux resilience is layered on the same principle that the stream, not the player, is what failed: MPV is configured with ffmpeg reconnection (`stream-lavf-o`, `network-timeout`) so ordinary blips never surface, and `EndFile(ERROR)` — previously a bare log, which left playback halted while the UI still showed "playing" — is emitted as a *recoverable* error. Because MpvBackend is constructed before `PlayerController` exists, it cannot decide in-process like the Android JNI callback: the frontend echoes the error into `player_recover_stream`, which keeps the decision in Rust (the same shape as `PlaybackEnded` → `player_on_playback_ended`). Android reports errors it has already declined as *unrecoverable*, so the echo never asks twice | Playback | UR-004, UR-040 | Done | | DR-093 | Traceability coverage gate derives its requirement denominators from `requirements.md` at run time rather than hardcoded literals: `countDefinedRequirements` counts an ID only where it leads a markdown table row (ignoring the "Traces To" column and prose) and deduplicates IDs listed both in the definition tables and in the §3 traceability matrix; `computeCoverage` reports the *intersection* of traced and defined IDs so an ID traced in code but absent from `requirements.md` is surfaced as `orphaned` instead of inflating the ratio past 100%. UT/IT test identifiers are excluded as a separate taxonomy. CI and `bun run traces:coverage` share this computation and fail on both a sub-threshold and an impossible >100% result | Tooling | - | Done | --- @@ -347,7 +348,7 @@ Internal architecture, components, and application logic. | UR-037 | IR-010 | DR-042 | | UR-038 | IR-010 | DR-043 | | UR-039 | - | DR-045, DR-046 | -| UR-040 | IR-025 | DR-051, DR-052, DR-129 | +| UR-040 | IR-025 | DR-051, DR-052, DR-129, DR-130 | | UR-041 | IR-026 | DR-053 | | UR-042 | IR-009, IR-014 | DR-054 | | UR-043 | IR-027 | DR-055 | @@ -504,6 +505,7 @@ Internal architecture, components, and application logic. | UT-120 | Expiry reclaim takes only expired temporary entries: derived from `completed_at`+TTL, honouring an `expires_at` override, never a user download, and disabled by a zero TTL | DR-127 | Done | | UT-108 | LRU eviction reclaims only `'auto'` downloads and never a user's own, even when the user's is the oldest | DR-126 | Done | | UT-117 | A background audio-only stream cut short resumes where it died instead of ending the episode; a real end still advances; the absolute position is compared against the runtime; retries at a stuck position give up. A recoverable error resumes music and video too, with growing backoff, leaving the rest of the queue intact and the seekable stream's URL untouched; local and DirectUrl sources are excluded | DR-129 | Done | +| UT-121 | An EOF reads as the last observed timestamp, not zero: live readings win while the file is loaded, a not-yet-established duration is not recorded as a real zero, a seek updates the position before the next poll, and loading a new file clears the previous one's | DR-130 | Done | ### Integration Tests diff --git a/scripts/extract-traces.test.ts b/scripts/extract-traces.test.ts index b3701785..f70733e3 100644 --- a/scripts/extract-traces.test.ts +++ b/scripts/extract-traces.test.ts @@ -175,8 +175,8 @@ describe("live requirements.md", () => { expect(defined.UR).toBe(71); expect(defined.IR).toBe(32); - expect(defined.DR).toBe(126); + expect(defined.DR).toBe(127); expect(defined.JA).toBe(34); - expect(defined.total).toBe(263); + expect(defined.total).toBe(264); }); }); diff --git a/src-tauri/src/commands/player/timers.rs b/src-tauri/src/commands/player/timers.rs index 5bef84c4..013c3b29 100644 --- a/src-tauri/src/commands/player/timers.rs +++ b/src-tauri/src/commands/player/timers.rs @@ -279,6 +279,53 @@ pub async fn player_on_playback_ended( Ok(()) } +/// Try to recover playback after a **recoverable** player error, reporting +/// whether it was handled. +/// +/// The frontend's error handler stops the player, which is right for a real +/// failure and wrong for a network blip — it turned every hiccup into "playback +/// died". This is the echo path for backends that cannot decide in-process: +/// MpvBackend is constructed before `PlayerController` exists ([`lib.rs`]), so +/// its event thread has no controller to ask. It emits the error, the frontend +/// echoes it here, and the decision stays in Rust — the same shape as +/// `PlaybackEnded` → `player_on_playback_ended`. +/// +/// Returns `true` when the stream was re-opened and the caller must NOT stop the +/// player; `false` when the error is real and should be surfaced as before. +/// Android decides inside its JNI callback and only emits errors it has already +/// declined to recover, so this reports `false` for those without a second +/// opinion — the shared attempt budget is spent by then either way. +/// +/// TRACES: UR-004, UR-040 | DR-130 | UT-117 +#[tauri::command] +#[specta::specta] +pub async fn player_recover_stream(player: State<'_, PlayerStateWrapper>) -> Result { + let (position, delay_secs) = { + let controller = player.0.lock().await; + match controller.recoverable_error_resume() { + Some(resume) => resume, + None => return Ok(false), + } + }; + + log::warn!( + "[Recovery] Stream failed — re-opening at {:.1}s in {}s", + position, + delay_secs + ); + // Give a brief outage time to clear; retrying instantly just burns the budget. + tokio::time::sleep(std::time::Duration::from_secs(delay_secs)).await; + + let controller = player.0.lock().await; + match controller.resume_stream_at(position).await { + Ok(()) => Ok(true), + Err(e) => { + log::error!("[Recovery] Failed to re-open stream: {}", e); + Ok(false) + } + } +} + // ===== HTML5 video state-report commands ===== // // On platforms where video renders in the webview (Linux WebKitGTK HTML5 diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index f9b4ff6a..f04cc601 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -142,6 +142,7 @@ use commands::{ player_play_tracks, player_preload_upcoming, player_previous, + player_recover_stream, player_remove_from_queue, player_report_media_loaded, player_report_position, @@ -698,6 +699,7 @@ fn specta_builder() -> Builder { player_cancel_autoplay_countdown, player_play_next_episode, player_on_playback_ended, + player_recover_stream, player_report_state, player_report_position, player_report_media_loaded, diff --git a/src-tauri/src/player/android/mod.rs b/src-tauri/src/player/android/mod.rs index fd023a42..bacbbfa3 100644 --- a/src-tauri/src/player/android/mod.rs +++ b/src-tauri/src/player/android/mod.rs @@ -1007,10 +1007,13 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO tauri::async_runtime::spawn(async move { let resume = controller.lock().await.recoverable_error_resume(); let Some((position, delay_secs)) = resume else { + // Declined here, so report it as NOT recoverable: the frontend + // would otherwise echo it into player_recover_stream and ask + // the same question a second time. if let Some(emitter) = EVENT_EMITTER.get() { emitter.emit(PlayerStatusEvent::Error { message: message_str, - recoverable: true, + recoverable: false, }); } return; @@ -1032,7 +1035,7 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO if let Some(emitter) = EVENT_EMITTER.get() { emitter.emit(PlayerStatusEvent::Error { message: message_str, - recoverable: true, + recoverable: false, }); } } diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs index bf68952f..72c4e870 100644 --- a/src-tauri/src/player/mod.rs +++ b/src-tauri/src/player/mod.rs @@ -1390,11 +1390,13 @@ impl PlayerController { /// The wait grows with the attempt number so a short outage has time to /// clear, and the shared budget stops the retries when it doesn't. /// - /// Only *called* from the Android error callback (`#[cfg(android)]`), but - /// compiled and unit-tested on the host, hence `allow(dead_code)` off-Android. + /// Called from the Android error callback, which decides in-process, and from + /// `player_recover_stream`, which is how the same decision reaches the + /// backends whose event thread has no controller to call — MPV is built + /// before the controller exists, so on Linux the error is emitted, echoed by + /// the frontend, and decided here. /// - /// TRACES: UR-040, UR-004 | DR-129 | UT-117 - #[cfg_attr(not(target_os = "android"), allow(dead_code))] + /// TRACES: UR-040, UR-004 | DR-129, DR-130 | UT-117 pub fn recoverable_error_resume(&self) -> Option<(f64, u64)> { self.claim_stream_resume() .map(|(position, attempt)| (position, attempt as u64 * RESUME_BACKOFF_STEP_SECS)) diff --git a/src-tauri/src/player/mpv_backend.rs b/src-tauri/src/player/mpv_backend.rs index 466ed796..78414fee 100644 --- a/src-tauri/src/player/mpv_backend.rs +++ b/src-tauri/src/player/mpv_backend.rs @@ -561,7 +561,7 @@ impl PlayerBackend for MpvBackend { /// 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 + /// TRACES: UR-005 | DR-130 | UT-121 fn position(&self) -> f64 { let live = self.mpv.get_property::("time-pos").ok(); self.observed.lock_safe().position_or_last(live) @@ -570,7 +570,7 @@ impl PlayerBackend for MpvBackend { /// Total duration — live, or the last one observed. Unloaded at EOF for the /// same reason as `position`. /// - /// TRACES: UR-005 | DR-130 | UT-118 + /// TRACES: UR-005 | DR-130 | UT-121 fn duration(&self) -> Option { let live = self.mpv.get_property::("duration").ok(); self.observed.lock_safe().duration_or_last(live) diff --git a/src/lib/api/bindings.ts b/src/lib/api/bindings.ts index eb8b98da..1c2b6e6b 100644 --- a/src/lib/api/bindings.ts +++ b/src/lib/api/bindings.ts @@ -243,6 +243,29 @@ async playerPlayNextEpisode(item: PlayItemRequest) : Promise { async playerOnPlaybackEnded(itemId: string | null, repositoryHandle: string | null) : Promise { return await TAURI_INVOKE("player_on_playback_ended", { itemId, repositoryHandle }); }, +/** + * Try to recover playback after a **recoverable** player error, reporting + * whether it was handled. + * + * The frontend's error handler stops the player, which is right for a real + * failure and wrong for a network blip — it turned every hiccup into "playback + * died". This is the echo path for backends that cannot decide in-process: + * MpvBackend is constructed before `PlayerController` exists ([`lib.rs`]), so + * its event thread has no controller to ask. It emits the error, the frontend + * echoes it here, and the decision stays in Rust — the same shape as + * `PlaybackEnded` → `player_on_playback_ended`. + * + * Returns `true` when the stream was re-opened and the caller must NOT stop the + * player; `false` when the error is real and should be surfaced as before. + * Android decides inside its JNI callback and only emits errors it has already + * declined to recover, so this reports `false` for those without a second + * opinion — the shared attempt budget is spent by then either way. + * + * TRACES: UR-004, UR-040 | DR-130 | UT-117 + */ +async playerRecoverStream() : Promise { + return await TAURI_INVOKE("player_recover_stream"); +}, /** * Report an HTML5