diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c522857..d7591ca9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,18 +48,29 @@ jobs: steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v6 - - name: Cache APT packages + - name: Install Linux build dependencies if: runner.os == 'Linux' # Tauri 2 on Linux still uses GTK 3 + WebKitGTK; cpal needs ALSA # development headers; the rest mirrors what `cargo check` will # link against transitively from the Tauri runtime. - uses: awalsh128/cache-apt-pkgs-action@553a35bb8ebd9fcabcb1c9451aa4c98e1b4ca8a9 # v1.6.3 - with: - packages: >- - libgtk-3-dev libwebkit2gtk-4.1-dev libsoup-3.0-dev - libayatana-appindicator3-dev librsvg2-dev libasound2-dev + # + # Installed directly rather than via cache-apt-pkgs-action on + # purpose: the runner image's pre-baked apt index pins package + # versions (e.g. gstreamer1.0-plugins-good, a libwebkit2gtk dep) + # that the Ubuntu mirror rotates out, so a cached install with no + # `apt-get update` 404s. The action also keys its cache purely on + # the package list, so a failed run saves an empty cache that + # poisons every later run under the same immutable key. `apt-get + # update` first refreshes the index to whatever the mirror + # currently serves; skipping the apt cache removes the poison + # trap. The expensive cache — compiled Rust artifacts — stays on + # Swatinem/rust-cache below. + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends \ + libgtk-3-dev libwebkit2gtk-4.1-dev libsoup-3.0-dev \ + libayatana-appindicator3-dev librsvg2-dev libasound2-dev \ libssl-dev pkg-config - version: v1 - uses: dtolnay/rust-toolchain@2c7215f132e9ebf062739d9130488b56d53c060c # stable with: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 93796b09..1908ef73 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -88,14 +88,19 @@ jobs: # proc macros (#[tauri::command], sqlx::query!, etc.) and external # crate symbols. Without it, analysis quality drops below the 50 % # call-target threshold and GitHub flags the run as low-quality. - - name: Cache APT packages - uses: awalsh128/cache-apt-pkgs-action@553a35bb8ebd9fcabcb1c9451aa4c98e1b4ca8a9 # v1.6.3 - with: - packages: >- - libgtk-3-dev libwebkit2gtk-4.1-dev libsoup-3.0-dev - libayatana-appindicator3-dev librsvg2-dev libasound2-dev + # Installed directly rather than via cache-apt-pkgs-action: the + # runner image's pre-baked apt index pins package versions the + # Ubuntu mirror rotates out, so a cached install with no `apt-get + # update` 404s, and the action's package-list-keyed cache saves an + # empty entry on failure that poisons later runs. Same rationale as + # ci.yml. + - name: Install Linux build dependencies + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends \ + libgtk-3-dev libwebkit2gtk-4.1-dev libsoup-3.0-dev \ + libayatana-appindicator3-dev librsvg2-dev libasound2-dev \ libssl-dev pkg-config - version: v1 - uses: dtolnay/rust-toolchain@2c7215f132e9ebf062739d9130488b56d53c060c # stable with: diff --git a/docs/features/playback.md b/docs/features/playback.md index ccf343da..e5bea8d0 100644 --- a/docs/features/playback.md +++ b/docs/features/playback.md @@ -56,6 +56,32 @@ The chosen device's name is persisted in `profile_setting['audio.output_device'] On Linux, enumeration uses ALSA's hint database (`snd_device_name_hint("pcm")`) instead of cpal's `output_devices()` to avoid a 1-2 s freeze + `pcm_dmix` / `pcm_route` stderr spam from probing every PCM card. +## Output-stream lifecycle & recovery + +Three paths replace the output stream, and they must all end in the same place: `wasapi_exclusive_active` updated and a `player:audio-mode-changed` event emitted, because that event is the only thing that keeps Settings' Exclusive-mode toggle honest ([`ExclusiveModeCard`](../../src/components/views/settings/ExclusiveModeCard.tsx) re-reads on it). + +| Path | Trigger | Order | +| --- | --- | --- | +| `set_output_device` | user picks another endpoint | spawn first, then release — the two streams target different devices, so a failed spawn can roll back to the working one | +| `set_wasapi_exclusive` | user toggles the mode | **release first when the old stream is exclusive**, then spawn | +| `force_rebuild_output` | automatic recovery after a device error | **release first when the old stream is exclusive**, then spawn | + +The release-first rule is the #322 / #405 lesson: a WASAPI exclusive client owns its endpoint outright, so no other client — shared *or* exclusive — can open it until that client is released. Re-opening the **same** endpoint while an exclusive stream still holds it always fails, and when it failed inside `set_wasapi_exclusive` the command returned `Err` before persisting anything, leaving the toggle latched on the mode the user was trying to leave (#405). + +Device loss reaches the recovery path from two independent places, since the two backends have separate failure surfaces: + +- **cpal shared** — the stream's `err_fn` callback fires on an arbitrary thread. +- **WASAPI exclusive** — [`wasapi_exclusive::run_event_loop`](../../src-tauri/crates/app/src/audio/wasapi_exclusive.rs) returns an `ExitReason`; `DeviceLost` covers a failed `wait_for_event` / `write_to_device`. Each is re-checked against the shutdown channel first so a deliberate teardown isn't mistaken for a failure. + +Both then call the shared [`output::notify_device_lost`](../../src-tauri/crates/app/src/audio/output.rs) (park the player, emit `player:state` + `player:error`, sync the OS media controls) and [`output::schedule_device_rebuild`](../../src-tauri/crates/app/src/audio/output.rs) (300 ms backoff, then a same-device rebuild). + +Two gates keep the recovery from thrashing: + +- **`RebuildGate`** (`REBUILD_SETTLE_WINDOW`, 2 s) — one rebuild per burst of device errors. `begin_deliberate_output_change()` opens the same window around a mode toggle, because seizing the endpoint exclusively kicks the outgoing shared client off it and that self-inflicted `DeviceNotAvailable` would otherwise schedule a rebuild that undoes the switch. +- **`FlapWindow`** (`EXCLUSIVE_FLAP_THRESHOLD` / `EXCLUSIVE_FLAP_WINDOW`) — a device that resets on every exclusive grab gives up on exclusive for the rest of the session. Cleared by an explicit toggle or device switch. + +Every failure path that ends with no output thread at all publishes `wasapi_exclusive_active = false` + the event before returning the error — a toggle describing a stream that no longer exists is the exact shape of #405. + ## OS media controls [`media_controls.rs`](../../src-tauri/crates/app/src/media_controls.rs) bridges the engine to [`souvlaki 0.8`](https://crates.io/crates/souvlaki): diff --git a/src-tauri/crates/app/src/audio/engine.rs b/src-tauri/crates/app/src/audio/engine.rs index fa1295c2..d902611c 100644 --- a/src-tauri/crates/app/src/audio/engine.rs +++ b/src-tauri/crates/app/src/audio/engine.rs @@ -247,6 +247,17 @@ impl RebuildGate { self.armed = false; self.last_finished = Some(now); } + + /// Drop the settle window entirely so the very next [`try_arm`] + /// succeeds. Used when a deliberate output change we suppressed for + /// (via [`AudioEngine::begin_deliberate_output_change`]) turns out + /// to have installed no stream at all: the recovery we then schedule + /// must not be swallowed by the window we opened for a swap that + /// never happened (#405). + fn reopen(&mut self) { + self.armed = false; + self.last_finished = None; + } } /// Handle stored in Tauri state. Cloning an `Arc` is cheap. @@ -538,7 +549,10 @@ impl AudioEngine { /// cycles in 14 seconds) only triggers one rebuild attempt /// instead of stacking three concurrent SwapProducer cmds onto /// the decoder. - pub fn try_rebuild_after_device_error(&self) -> AppResult<()> { + pub(super) fn try_rebuild_after_device_error( + &self, + target: super::output::RebuildTarget, + ) -> AppResult<()> { use std::sync::atomic::Ordering; // Release the #365 gate on EVERY exit path below (including the @@ -577,7 +591,15 @@ impl AudioEngine { } let _guard = ResetGuard(&self.rebuild_in_progress); - let pinned = self.current_output_device(); + // `Resolve` reads the device off the live (dead) handle still + // parked in `self.output` — right for the cpal / exclusive + // device-loss callers. `Device` is passed when the caller already + // released the handle, so a self-resolve would see `None` and + // reopen the OS default instead of the user's pick (#405). + let pinned = match target { + super::output::RebuildTarget::Resolve => self.current_output_device(), + super::output::RebuildTarget::Device(device) => device, + }; let pref_exclusive = self.wasapi_exclusive.load(Ordering::Relaxed); // #322: an exclusive-mode flap storm. A device that resets on every @@ -640,6 +662,55 @@ impl AudioEngine { .try_arm(Instant::now(), REBUILD_SETTLE_WINDOW) } + /// Publish "no output thread at all" when a rebuild bailed out after + /// the old handle was already released. + /// + /// The exclusive flag must not keep claiming a stream that no longer + /// exists — a toggle describing one is the exact shape of #405 — and + /// the event is what makes Settings re-read. No-ops when `guard` + /// still holds a handle, i.e. the failure happened before the old + /// stream was given up and the flag is still accurate. + fn publish_output_lost_if_gone(&self, guard: &Option) { + if guard.is_none() { + self.wasapi_exclusive_active + .store(false, std::sync::atomic::Ordering::Release); + let _ = self.app.emit("player:audio-mode-changed", ()); + } + } + + /// Open the #365 settle window around an output change WE are making + /// on purpose (a mode toggle, a device switch). + /// + /// Replacing a stream makes the outgoing one fire + /// `DeviceNotAvailable` — seizing the endpoint in exclusive mode + /// kicks the shared client off it, and tearing a stream down can do + /// the same. That error is a consequence of the change, not a device + /// failure, but the cpal callback can't tell the difference and + /// schedules a recovery rebuild that fights the mode the user just + /// picked (visible in the #405 report as a second exclusive open + /// ~300 ms after the first). Marking the gate finished starts the + /// same quiet period a real rebuild would, so those echoes are + /// ignored. + fn begin_deliberate_output_change(&self) { + self.rebuild_gate + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .finish(Instant::now()); + } + + /// Undo a [`begin_deliberate_output_change`] when the change ended up + /// installing no stream (the spawn failed after the old handle was + /// already released). The settle window was there to absorb the echo + /// error from a successful swap; with no swap there is no echo, and + /// leaving the window open would suppress the recovery rebuild we're + /// about to schedule — the exact silent-no-output tail of #405. + fn cancel_deliberate_output_change(&self) { + self.rebuild_gate + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .reopen(); + } + /// Clear the #322 session-level exclusive suppression and its flap /// counter. Called when the user explicitly re-toggles WASAPI exclusive /// or switches output device — both are a fresh chance for exclusive to @@ -696,12 +767,20 @@ impl AudioEngine { } } - let (producer, handle) = spawn_output_with_mode( + let (producer, handle) = match spawn_output_with_mode( self.shared.clone(), self.app.clone(), device_name, exclusive, - )?; + ) { + Ok(pair) => pair, + Err(err) => { + // `pre_release` (an old exclusive stream) already took the + // handle above, so this is the no-output-at-all case. + self.publish_output_lost_if_gone(&guard); + return Err(err); + } + }; // The freshly-spawned `handle` owns a live cpal output // thread. If either send below fails (decoder dead, channel @@ -727,6 +806,7 @@ impl AudioEngine { })(); if let Err(err) = send_result { handle.stop(); + self.publish_output_lost_if_gone(&guard); return Err(err); } *guard = Some(handle); @@ -887,6 +967,13 @@ impl AudioEngine { })(); if let Err(err) = send_result { handle.stop(); + // Mirror force_rebuild_output / set_wasapi_exclusive: if the + // SwapProducer send failed the closure had already run + // `guard.take()`, so no output thread remains — the exclusive + // flag must stop claiming one and Settings must re-read (#405). + // No-ops when an earlier Stop send failed before guard.take(), + // where the old stream is still installed and the flag accurate. + self.publish_output_lost_if_gone(&guard); return Err(err); } @@ -968,10 +1055,6 @@ impl AudioEngine { if previous == enabled { return Ok(()); } - // An explicit user toggle clears any #322 flap-storm suppression so - // re-enabling exclusive actually retries it (and disabling resets - // the counter for next time). - self.reset_exclusive_suppression(); // Reuse `set_output_device` with the active device name — the // current/requested equality check inside it would short-circuit // a same-device call, so go straight to the rebuild path by @@ -996,21 +1079,131 @@ impl AudioEngine { .load(std::sync::atomic::Ordering::Acquire); let position_ms = self.shared.current_position_ms(); - let (producer, handle) = - spawn_output_with_mode(self.shared.clone(), self.app.clone(), active, enabled)?; + // #405 — the stuck Settings toggle. Leaving exclusive mode re-opens + // the SAME endpoint in shared mode, and a WASAPI exclusive client + // owns its endpoint outright: no other client, shared OR exclusive, + // can open it until that client is released (the #322 lesson, which + // `force_rebuild_output` already applies). Spawning first — the + // order that's correct for a device *switch*, where the two streams + // target different endpoints — therefore fails every time here. The + // command returned `Err`, so the preference was never persisted and + // `wasapi_exclusive_active` kept reporting the old mode: the toggle + // sat latched on the very mode the user was trying to leave, with a + // restart as the only way out. Release the old exclusive stream + // FIRST so the new open finds a free device. + let pre_release = guard.as_ref().is_some_and(|h| h.wasapi_exclusive); + if pre_release { + if was_playing { + if let Err(e) = self.cmd_tx.send(AudioCmd::Stop) { + // Nothing has been torn down yet — the old exclusive + // stream is still installed and running. Roll the + // preference swap back so the in-memory pref keeps + // matching the live stream; without this a later + // device-error rebuild would read the new pref and + // silently flip to the mode this toggle failed to + // apply. (The spawn / send_result failure paths below + // deliberately keep the new pref instead, because by + // then the old stream is already gone.) + self.wasapi_exclusive + .store(previous, std::sync::atomic::Ordering::Relaxed); + return Err(AppError::Audio(format!( + "audio command channel closed: {e}" + ))); + } + } + if let Some(old) = guard.take() { + old.stop(); + } + } + + // An explicit user toggle clears any #322 flap-storm suppression so + // re-enabling exclusive actually retries it (and disabling resets + // the counter for next time). Deferred until after the pre-release + // teardown so a failed Stop send above — which leaves the old stream + // intact and rolls the preference back — doesn't also wipe the + // suppression state that still describes that surviving stream. + self.reset_exclusive_suppression(); + + // Both directions of this toggle disturb the outgoing stream on + // purpose — don't let the resulting device error trigger a + // recovery rebuild that undoes the switch. + self.begin_deliberate_output_change(); + + let (producer, handle) = match spawn_output_with_mode( + self.shared.clone(), + self.app.clone(), + active.clone(), + enabled, + ) { + Ok(pair) => pair, + Err(err) => { + // No replacement stream was installed, so the settle + // window we opened has no swap-echo to absorb. Reopen the + // gate either way, so it can't suppress a genuine device + // error — on the surviving old stream (shared → exclusive + // where even the shared fallback failed, `guard` still + // holds it) or on the recovery we schedule below. + self.cancel_deliberate_output_change(); + if guard.is_none() { + // `pre_release` already released the old exclusive + // handle, so there is no output thread at all. Tell + // Settings the flag is stale (#405), then schedule a + // rebuild that re-opens in the mode now recorded in + // `wasapi_exclusive`. Pass `active` explicitly: the + // teardown emptied `self.output`, so a self-resolve + // would reopen the OS default instead of the user's + // device. + self.publish_output_lost_if_gone(&guard); + super::output::schedule_device_rebuild( + &self.app, + super::output::RebuildTarget::Device(active), + ); + } else { + // shared → exclusive where even the shared fallback + // failed: the old shared stream is untouched and still + // running. Roll the preference back to match it — same + // as the failed-Stop path — otherwise a later + // device-error rebuild would read the new pref and flip + // to the exclusive mode this toggle never applied. + self.wasapi_exclusive + .store(previous, std::sync::atomic::Ordering::Relaxed); + } + return Err(err); + } + }; let active_mode = handle.wasapi_exclusive; - if was_playing { + // Group the whole hand-off so ANY failing step still runs the + // `handle.stop()` below. `handle` owns a live output thread on a + // `!Send` thread that can't be reaped from Drop, so an early + // return here would leak it until process exit — and when the new + // stream is the exclusive one, that leaked thread keeps the device + // locked while `guard` still points at the old stream. Same shape + // as `force_rebuild_output`. + let send_result = (|| { + if was_playing && !pre_release { + self.cmd_tx + .send(AudioCmd::Stop) + .map_err(|e| AppError::Audio(format!("audio command channel closed: {e}")))?; + } + // No-op when `pre_release` already took the old handle above. + if let Some(old) = guard.take() { + old.stop(); + } self.cmd_tx - .send(AudioCmd::Stop) + .send(AudioCmd::SwapProducer(producer)) .map_err(|e| AppError::Audio(format!("audio command channel closed: {e}")))?; + Ok::<(), AppError>(()) + })(); + if let Err(err) = send_result { + handle.stop(); + // A failing `Stop` send bails before the old handle is taken, + // so `guard` still describes a live stream and the flag stays + // accurate; the later steps leave no output at all. The helper + // tells those two apart and only publishes for the second. + self.publish_output_lost_if_gone(&guard); + return Err(err); } - if let Some(old) = guard.take() { - old.stop(); - } - self.cmd_tx - .send(AudioCmd::SwapProducer(producer)) - .map_err(|e| AppError::Audio(format!("audio command channel closed: {e}")))?; *guard = Some(handle); self.wasapi_exclusive_active .store(active_mode, std::sync::atomic::Ordering::Release); @@ -1219,6 +1412,20 @@ mod rebuild_gate_tests { assert!(g.try_arm(t + SETTLE, SETTLE)); } + #[test] + fn reopen_lets_a_rebuild_arm_inside_the_settle_window() { + let mut g = RebuildGate::default(); + let t = Instant::now(); + // A deliberate output change opens the settle window... + g.finish(t); + // ...but the spawn failed and installed nothing, so we reopen. + g.reopen(); + // The scheduled recovery lands 300 ms later — well inside what + // would have been the 2 s window — and MUST now arm, otherwise + // the engine is left with no output thread at all (#405). + assert!(g.try_arm(t + Duration::from_millis(300), SETTLE)); + } + #[test] fn collapses_a_full_cascade_into_a_single_rebuild() { // Replays the reporter's pattern: an error every ~300 ms, each one diff --git a/src-tauri/crates/app/src/audio/output.rs b/src-tauri/crates/app/src/audio/output.rs index 6b9b22b9..da994ef6 100644 --- a/src-tauri/crates/app/src/audio/output.rs +++ b/src-tauri/crates/app/src/audio/output.rs @@ -262,6 +262,98 @@ pub fn spawn_output_with_mode( spawn_output_thread(shared, app, device_name) } +/// Surface a lost audio device to the user: park the player, tell the +/// UI, and keep the OS media controls in sync. +/// +/// Shared by the cpal error callback and the WASAPI-exclusive event +/// loop ([`super::wasapi_exclusive`]) so both backends report a device +/// loss identically — the exclusive thread used to just exit, leaving +/// the UI convinced playback was still running. +pub(super) fn notify_device_lost(app: &AppHandle, shared: &Arc, message: String) { + shared.set_state(PlayerState::Paused); + let _ = app.emit( + "player:state", + json!({ "state": "paused", "track_id": null }), + ); + let _ = app.emit("player:error", json!({ "message": message })); + if let Some(controls) = app.try_state::() { + controls.update_playback(PlayerState::Paused, shared.current_position_ms()); + } +} + +/// Which device a scheduled rebuild should reopen. +/// +/// The two callers differ in whether the engine can still resolve the +/// device from its live handle at rebuild time: +/// - the cpal error callback and the WASAPI-exclusive `DeviceLost` exit +/// both fire while the (now-dead) handle is still parked in +/// `self.output`, so its `device_name` is still readable → [`Resolve`]; +/// - the `set_wasapi_exclusive` failure path has already `take()`n the +/// old handle, emptying `self.output`, so a self-resolve would return +/// `None` and reopen the OS default instead of the user's pick (#405) +/// → [`Device`] carries the device captured before the teardown. +/// +/// [`Resolve`]: RebuildTarget::Resolve +/// [`Device`]: RebuildTarget::Device +pub(super) enum RebuildTarget { + /// Resolve the device from the engine's live handle at rebuild time. + Resolve, + /// Reopen this specific device (`None` = OS default). + Device(Option), +} + +/// Schedule a same-device rebuild of the output stream after a device +/// loss (#175). Returns immediately — the work happens on a tokio task. +/// +/// 300 ms backoff first (covers the OS settling a USB replug / driver +/// restart / Windows session reset), then a SAME-DEVICE rebuild through +/// the engine. Re-querying the OS default here would land the user on a +/// different output every time their pinned device flapped. +/// +/// The engine is resolved AFTER the delay on purpose — an error can fire +/// while `AudioEngine::new` is still running, i.e. before the engine has +/// been registered in Tauri's managed state, and looking it up eagerly +/// would silently drop the recovery for that window. +pub(super) fn schedule_device_rebuild(app: &AppHandle, target: RebuildTarget) { + let app = app.clone(); + tauri::async_runtime::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + let Some(engine) = app.try_state::>() else { + return; + }; + // #365: one rebuild per burst of device errors. A DAC that + // resets on every exclusive grab makes our own re-open + // trigger the next error, so without a gate the passes + // chase each other until one opens exclusive while the + // previous stream is still settling and collides with + // AUDCLNT_E_DEVICE_IN_USE. Gating here (rather than before + // the sleep) still collapses the burst: whichever task + // wakes first arms, and the others hit either `armed` or + // the settle window. + if !engine.try_arm_device_rebuild() { + tracing::debug!( + "device rebuild already armed or within the settle window; \ + ignoring this device loss" + ); + return; + } + // The rebuild is synchronous and genuinely slow: it joins + // the old output thread — which, in WASAPI exclusive mode, + // can sit in `wait_for_event` up to its 2 s timeout — and + // then opens the device inline (COM init + the #174 format + // negotiation ladder). Running that directly on a tokio + // worker would park the worker for the whole duration, so + // hand it to the blocking pool. + let engine = engine.inner().clone(); + let _ = tokio::task::spawn_blocking(move || { + if let Err(err) = engine.try_rebuild_after_device_error(target) { + tracing::warn!(%err, "auto-rebuild after device loss failed"); + } + }) + .await; + }); +} + /// Handle retained by the engine so it can tear the output thread down /// cleanly on shutdown or device switch. Separate from the decoder-side /// `Producer` which is handed off independently — see the tuple returned @@ -488,76 +580,19 @@ where // to be dropped by cpal anyway. // // On `DeviceNotAvailable` (#175) we additionally schedule an - // auto-rebuild via tokio: 300 ms backoff (covers the OS settling - // a USB replug / driver restart / Windows session reset), then a - // SAME-DEVICE rebuild through the engine. Re-querying the OS - // default here would land the user on a different output every - // time their pinned device flapped — the engine's debounce guard - // also coalesces a quick double-flap into a single rebuild - // attempt. + // auto-rebuild — see [`schedule_device_rebuild`]. The engine's + // debounce guard coalesces a quick double-flap into a single + // rebuild attempt. let err_shared = shared.clone(); let err_app = app.clone(); let err_fn = move |err: cpal::StreamError| { tracing::warn!(?err, "cpal stream error"); - err_shared.set_state(PlayerState::Paused); - let _ = err_app.emit( - "player:state", - json!({ "state": "paused", "track_id": null }), - ); - let _ = err_app.emit( - "player:error", - json!({ "message": format!("audio device error: {err}") }), - ); - if let Some(controls) = err_app.try_state::() { - controls.update_playback(PlayerState::Paused, err_shared.current_position_ms()); - } + notify_device_lost(&err_app, &err_shared, format!("audio device error: {err}")); if matches!(err, cpal::StreamError::DeviceNotAvailable) { - let app_clone = err_app.clone(); - // Everything below runs off this callback, on the tokio task: - // the engine lookup, the #365 gate, the rebuild and its logs. - // The engine is resolved AFTER the delay on purpose — an error - // can fire while `AudioEngine::new` is still running, i.e. - // before the engine has been registered in Tauri's managed - // state, and looking it up eagerly here would silently drop - // the recovery for that window. - tauri::async_runtime::spawn(async move { - tokio::time::sleep(std::time::Duration::from_millis(300)).await; - let Some(engine) = app_clone.try_state::>() - else { - return; - }; - // #365: one rebuild per burst of device errors. A DAC that - // resets on every exclusive grab makes our own re-open - // trigger the next error, so without a gate the passes - // chase each other until one opens exclusive while the - // previous stream is still settling and collides with - // AUDCLNT_E_DEVICE_IN_USE. Gating here (rather than before - // the sleep) still collapses the burst: whichever task - // wakes first arms, and the others hit either `armed` or - // the settle window. - if !engine.try_arm_device_rebuild() { - tracing::debug!( - "device rebuild already armed or within the settle window; \ - ignoring this DeviceNotAvailable" - ); - return; - } - // The rebuild is synchronous and genuinely slow: it joins - // the old output thread — which, in WASAPI exclusive mode, - // can sit in `wait_for_event` up to its 2 s timeout — and - // then opens the device inline (COM init + the #174 format - // negotiation ladder). Running that directly on a tokio - // worker would park the worker for the whole duration, so - // hand it to the blocking pool. - let engine = engine.inner().clone(); - let _ = tokio::task::spawn_blocking(move || { - if let Err(err) = engine.try_rebuild_after_device_error() { - tracing::warn!(%err, "auto-rebuild after DeviceNotAvailable failed"); - } - }) - .await; - }); + // The erroring handle is still parked in the engine's + // `self.output`, so the rebuild can self-resolve its device. + schedule_device_rebuild(&err_app, RebuildTarget::Resolve); } }; diff --git a/src-tauri/crates/app/src/audio/wasapi_exclusive.rs b/src-tauri/crates/app/src/audio/wasapi_exclusive.rs index e1782b92..5af7232e 100644 --- a/src-tauri/crates/app/src/audio/wasapi_exclusive.rs +++ b/src-tauri/crates/app/src/audio/wasapi_exclusive.rs @@ -144,9 +144,50 @@ fn output_thread_main( "wasapi exclusive stream opened" ); - run_event_loop(session, consumer, shutdown_rx, &shared, &app); + let exit = run_event_loop(session, consumer, shutdown_rx, &shared); - tracing::debug!("wasapi exclusive output thread exiting"); + match exit { + ExitReason::Shutdown => { + tracing::debug!("wasapi exclusive output thread exiting"); + } + // #405: the exclusive backend runs its own event loop, entirely + // separate from cpal's error callback — so when the device goes + // away here (`wait_for_event` / `write_to_device` failing), the + // thread used to just return and nobody ever heard about it. The + // engine kept its `wasapi_exclusive = true` handle for a thread + // that no longer exists, `wasapi_exclusive_active` stayed latched + // on, and the Settings toggle was stuck showing "on" until the + // app restarted. Report the loss down the same two paths the cpal + // callback uses: tell the UI, then schedule a rebuild (which is + // what re-syncs the toggle, via `player:audio-mode-changed`). + ExitReason::DeviceLost(reason) => { + tracing::warn!( + %reason, + "wasapi exclusive output thread lost the device; requesting rebuild" + ); + super::output::notify_device_lost( + &app, + &shared, + format!("audio device error: {reason}"), + ); + // The dead handle is still parked in the engine's + // `self.output`, so the rebuild can self-resolve its device. + super::output::schedule_device_rebuild(&app, super::output::RebuildTarget::Resolve); + } + } +} + +/// Why [`run_event_loop`] returned. +/// +/// The distinction matters: a clean shutdown is the engine tearing this +/// thread down on purpose (device switch, mode toggle, app exit) and +/// must NOT trigger a recovery, whereas a device loss must (#405). +enum ExitReason { + /// The engine asked us to stop via the shutdown channel. + Shutdown, + /// The device stopped accepting buffers. Carries the failure for + /// the user-facing `player:error` message. + DeviceLost(String), } /// Bit depth + container layout actually accepted by the device in @@ -634,13 +675,15 @@ fn pick_device(device_name: &Option) -> AppResult { /// Mirrors the cpal callback's logic for `paused_output`, /// `drain_silent`, `volume`, `normalize_enabled`, and `mono_enabled` /// so the user-facing behaviour is identical regardless of backend. +/// +/// Returns why the loop stopped so the caller can decide whether a +/// recovery is warranted — see [`ExitReason`]. fn run_event_loop( session: ExclusiveSession, mut consumer: Consumer, shutdown_rx: Receiver<()>, shared: &Arc, - _app: &AppHandle, -) { +) -> ExitReason { let ExclusiveSession { client, render, @@ -672,16 +715,25 @@ fn run_event_loop( // whether to re-init or fall back. const EVENT_TIMEOUT_MS: u32 = 2000; - loop { + let exit = loop { if shutdown_rx.try_recv().is_ok() { - break; + break ExitReason::Shutdown; } match event.wait_for_event(EVENT_TIMEOUT_MS) { Ok(()) => {} Err(err) => { + // A teardown racing the failure: the engine sent the + // shutdown while we were parked in `wait_for_event`, and + // releasing the device is what made the wait fail. That's + // a clean stop, not a device loss — reporting it as one + // would schedule a rebuild against a stream the engine is + // deliberately replacing. + if shutdown_rx.try_recv().is_ok() { + break ExitReason::Shutdown; + } tracing::warn!(?err, "wasapi exclusive wait_for_event failed"); - break; + break ExitReason::DeviceLost(format!("wasapi wait_for_event failed: {err:?}")); } } @@ -765,17 +817,21 @@ fn run_event_loop( }; if let Err(err) = render.write_to_device(need_frames, bytes, None) { + // Same teardown race as the `wait_for_event` arm above. + if shutdown_rx.try_recv().is_ok() { + break ExitReason::Shutdown; + } tracing::warn!(?err, "wasapi write_to_device failed"); - break; + break ExitReason::DeviceLost(format!("wasapi write_to_device failed: {err:?}")); } // Quick non-blocking shutdown check after every buffer so a // user-initiated stop takes effect within one device period // (~10 ms) rather than waiting up to EVENT_TIMEOUT_MS. if shutdown_rx.try_recv().is_ok() { - break; + break ExitReason::Shutdown; } - } + }; // Stop the stream so the next exclusive opener doesn't fight us // for the device. Errors are non-fatal — we're tearing down anyway. @@ -785,6 +841,8 @@ fn run_event_loop( // uninit; not strictly required, but tidier under tracing. std::thread::sleep(Duration::from_millis(5)); wasapi::deinitialize(); + + exit } /// Pack the decoded `f32` sample buffer into the little-endian byte diff --git a/src/components/views/settings/ExclusiveModeCard.tsx b/src/components/views/settings/ExclusiveModeCard.tsx index 652b5bfa..9019791b 100644 --- a/src/components/views/settings/ExclusiveModeCard.tsx +++ b/src/components/views/settings/ExclusiveModeCard.tsx @@ -104,6 +104,16 @@ export function ExclusiveModeCard() { } catch (err) { console.error("[ExclusiveModeCard] toggle failed", err); setError(String(err)); + // A failed toggle still moves the engine (issue #405): it may have + // torn the old stream down before failing to open the new one. The + // success path re-reads for exactly this reason — do it here too, + // otherwise the switch keeps showing the mode the user just tried + // to leave and looks stuck. + try { + setEnabled(await playerGetWasapiExclusive()); + } catch (refreshErr) { + console.error("[ExclusiveModeCard] refresh after failed toggle", refreshErr); + } } finally { setBusy(false); }