fix(player): recover a failed stream on Linux instead of stopping (DR-130)
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
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<bool, String> {
|
||||
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
|
||||
|
||||
@@ -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<tauri::Wry> {
|
||||
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,
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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::<f64>("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<f64> {
|
||||
let live = self.mpv.get_property::<f64>("duration").ok();
|
||||
self.observed.lock_safe().duration_or_last(live)
|
||||
|
||||
@@ -243,6 +243,29 @@ async playerPlayNextEpisode(item: PlayItemRequest) : Promise<PlayerStatus> {
|
||||
async playerOnPlaybackEnded(itemId: string | null, repositoryHandle: string | null) : Promise<null> {
|
||||
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<boolean> {
|
||||
return await TAURI_INVOKE("player_recover_stream");
|
||||
},
|
||||
/**
|
||||
* Report an HTML5 <video> state change (playing/paused/loading/stopped/idle).
|
||||
*/
|
||||
|
||||
@@ -145,3 +145,76 @@ describe("Player Events — pause must not zero the slider duration", () => {
|
||||
expect(get(playbackDuration)).toBe(70);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A recoverable error is a network hiccup, not the end of playback. The handler
|
||||
* used to stop the player unconditionally, so a blip on wifi killed the track —
|
||||
* on Linux especially, where MPV's EndFile(ERROR) is the only signal a stream
|
||||
* died and there is no in-process controller for the backend to consult.
|
||||
*
|
||||
* TRACES: UR-004, UR-040 | DR-130
|
||||
*/
|
||||
describe("Player Events — recoverable errors get one chance before stopping", () => {
|
||||
beforeEach(async () => {
|
||||
const { cleanupPlayerEvents } = await import("./playerEvents");
|
||||
const { player } = await import("$lib/stores/player");
|
||||
cleanupPlayerEvents();
|
||||
player.setIdle();
|
||||
vi.clearAllMocks();
|
||||
registeredHandler = null;
|
||||
currentQueueItemStore.set(null);
|
||||
});
|
||||
|
||||
it("does not stop the player when Rust re-opened the stream", async () => {
|
||||
const { invoke } = await import("@tauri-apps/api/core");
|
||||
vi.mocked(invoke).mockImplementation(async (cmd: string) =>
|
||||
cmd === "player_recover_stream" ? true : null
|
||||
);
|
||||
|
||||
const { initPlayerEvents } = await import("./playerEvents");
|
||||
const { player } = await import("$lib/stores/player");
|
||||
await initPlayerEvents();
|
||||
await fire({ type: "state_changed", state: "playing", media_id: "track-1" });
|
||||
|
||||
await fire({ type: "error", message: "Playback stream failed", recoverable: true });
|
||||
|
||||
const calls = vi.mocked(invoke).mock.calls.map(([cmd]) => cmd);
|
||||
expect(calls).toContain("player_recover_stream");
|
||||
expect(calls).not.toContain("player_stop");
|
||||
expect(get(player).state.kind).not.toBe("error");
|
||||
});
|
||||
|
||||
it("stops the player when recovery declines", async () => {
|
||||
const { invoke } = await import("@tauri-apps/api/core");
|
||||
vi.mocked(invoke).mockImplementation(async (cmd: string) =>
|
||||
cmd === "player_recover_stream" ? false : null
|
||||
);
|
||||
|
||||
const { initPlayerEvents } = await import("./playerEvents");
|
||||
await initPlayerEvents();
|
||||
|
||||
await fire({ type: "error", message: "Playback stream failed", recoverable: true });
|
||||
await Promise.resolve();
|
||||
|
||||
const calls = vi.mocked(invoke).mock.calls.map(([cmd]) => cmd);
|
||||
expect(calls).toContain("player_recover_stream");
|
||||
expect(calls).toContain("player_stop");
|
||||
});
|
||||
|
||||
it("does not attempt recovery for an unrecoverable error", async () => {
|
||||
// Android decides in its JNI callback and reports the errors it already
|
||||
// declined as unrecoverable, so this must not ask a second time.
|
||||
const { invoke } = await import("@tauri-apps/api/core");
|
||||
vi.mocked(invoke).mockResolvedValue(null);
|
||||
|
||||
const { initPlayerEvents } = await import("./playerEvents");
|
||||
await initPlayerEvents();
|
||||
|
||||
await fire({ type: "error", message: "Decoder failed", recoverable: false });
|
||||
await Promise.resolve();
|
||||
|
||||
const calls = vi.mocked(invoke).mock.calls.map(([cmd]) => cmd);
|
||||
expect(calls).not.toContain("player_recover_stream");
|
||||
expect(calls).toContain("player_stop");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -276,9 +276,32 @@ async function handlePlaybackEnded(): Promise<void> {
|
||||
|
||||
/**
|
||||
* Handle error events.
|
||||
*
|
||||
* A recoverable error gets one chance at recovery before anything is torn down.
|
||||
* Backends whose event thread cannot reach the controller (MpvBackend is built
|
||||
* before PlayerController exists) report the failure and rely on this echo to
|
||||
* put the decision back in Rust — the same shape as PlaybackEnded →
|
||||
* playerOnPlaybackEnded. Nothing is decided here: if Rust re-opened the stream
|
||||
* it says so, and stopping the player would kill the playback it just restored.
|
||||
*
|
||||
* TRACES: UR-004, UR-040 | DR-130
|
||||
*/
|
||||
async function handleError(message: string, recoverable: boolean): Promise<void> {
|
||||
console.error(`Playback error (recoverable: ${recoverable}): ${message}`);
|
||||
|
||||
if (recoverable) {
|
||||
try {
|
||||
if (await commands.playerRecoverStream()) {
|
||||
console.log("Stream re-opened after a recoverable error - not stopping");
|
||||
return;
|
||||
}
|
||||
} catch (e) {
|
||||
// Fall through to the normal stop: a failed recovery attempt is still an
|
||||
// error, and leaving the player running would strand it mid-failure.
|
||||
console.error("Stream recovery attempt failed:", e);
|
||||
}
|
||||
}
|
||||
|
||||
player.setError(message);
|
||||
|
||||
// Stop backend player to prevent orphaned playback
|
||||
|
||||
Reference in New Issue
Block a user