Skip to content

fix(audio): release the exclusive stream before re-opening the same device#429

Merged
InstaZDLL merged 8 commits into
mainfrom
fix/405-wasapi-toggle-release-first
Jul 25, 2026
Merged

fix(audio): release the exclusive stream before re-opening the same device#429
InstaZDLL merged 8 commits into
mainfrom
fix/405-wasapi-toggle-release-first

Conversation

@InstaZDLL

@InstaZDLL InstaZDLL commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Closes #405

Joël's 2026-07-25 log shows the Settings toggle still stuck on after v1.6.6-beta.1. The audio/lyrics/seek-bar recovery fixed in the earlier betas holds — this is a different failure, and reading his log line by line turned up three separate defects.

1. Leaving exclusive mode could never succeed

set_wasapi_exclusive spawned the new output stream before releasing the old one:

let (producer, handle) =
    spawn_output_with_mode(self.shared.clone(), self.app.clone(), active, enabled)?;
// ...only then: guard.take() / old.stop()

That order is correct for a device switch — two different endpoints, so a failed spawn can roll back to the working stream. But a mode toggle re-opens the same endpoint, and a WASAPI exclusive client owns its endpoint outright: no other client, shared or exclusive, can open it until that client is released. So the shared open failed every time.

The consequences chain neatly into the report:

  • the ? returned Err before player_set_wasapi_exclusive persisted the preference,
  • wasapi_exclusive_active was never updated and no player:audio-mode-changed was emitted,
  • the frontend's catch set an error string but left enabled untouched,
  • → the toggle stayed latched on the mode the user was trying to leave, restart being the only way out. Exactly what Joël describes, twice.

It's visible in his log at 05:26:32.931: a bare cpal output stream opened with no WASAPI line and no rebuild line before it — a manual toggle-off attempt. (That message is logged in build_stream_inner before build_output_stream runs, so it appears even when the open then fails.)

force_rebuild_output already learned this in #322. Same release-first rule applied here.

2. The exclusive backend died silently

The WASAPI exclusive backend runs its own event loop in wasapi_exclusive::run_event_loop, completely separate from cpal's err_fn. When the device went away there — wait_for_event or write_to_device failing — the thread just break'd out and returned. No player:error, no player:state, no recovery scheduled. Its AppHandle parameter was literally named _app.

The engine then kept an OutputHandle { wasapi_exclusive: true } for a thread that no longer existed, and wasapi_exclusive_active stayed latched on forever.

run_event_loop now returns an ExitReason, and DeviceLost goes down the same two paths the cpal callback uses. Those were extracted out of err_fn so both backends report a device loss identically:

  • output::notify_device_lost — park the player, emit player:state + player:error, sync the OS media controls;
  • output::schedule_device_rebuild — 300 ms backoff, #365 gate, same-device rebuild on the blocking pool.

Both failure arms re-check the shutdown channel first, so a deliberate teardown racing the failure is reported as Shutdown and doesn't schedule a rebuild against a stream the engine is replacing on purpose.

3. The mode switch triggered a rebuild that fought it

Seizing the endpoint exclusively kicks the outgoing shared client off it, so the toggle itself made the old stream fire DeviceNotAvailable — and err_fn dutifully scheduled a recovery rebuild for it. In Joël's log that's the second exclusive open ~300 ms after the first:

05:26:08.532  WASAPI Exclusive Mode engaged
05:26:08.814  rebuilding cpal output after DeviceNotAvailable  exclusive=true
05:26:08.855  WASAPI Exclusive Mode engaged      ← again

begin_deliberate_output_change() opens the existing REBUILD_SETTLE_WINDOW around the toggle so those self-inflicted echoes are ignored. No new mechanism — it just marks the gate finished, which is what a real rebuild already does on its way out.

Failure paths now publish honest state

Every path that ends with no output thread at all — a spawn that fails after the pre-release, a SwapProducer hand-off to a dead decoder — now stores wasapi_exclusive_active = false and emits player:audio-mode-changed before returning the error. A toggle describing a stream that no longer exists is the exact shape of #405. The SwapProducer rollback in set_wasapi_exclusive also gained the handle.stop() that force_rebuild_output already had, so a failed hand-off no longer leaks the output thread until process exit.

ExclusiveModeCard re-reads the engine state on a failed toggle too, not just a successful one, so the UI converges on the truth either way.

Docs

New Output-stream lifecycle & recovery section in docs/features/playback.md — the three replacement paths and which of them must release first, the two independent device-loss surfaces, and what RebuildGate / FlapWindow each protect against.

Validation

  • cargo check --workspace --all-targets
  • cargo test -p waveflow --lib ✅ 120 passed
  • bun run typecheck ✅ · bun run lint

⚠️ wasapi_exclusive.rs is #[cfg(target_os = "windows")], so it does not compile locally — this box is Linux with no rustup and no Windows target available. CI's windows-latest matrix slot runs cargo check --workspace --all-targets and is what actually compiles that file; please let it go green before merging.

Runtime confirmation needs a Windows machine — the toggle should now switch off cleanly while exclusive mode is engaged.

Summary by CodeRabbit

  • Améliorations
    • Reprise plus robuste après perte ou changement de périphérique : passage en pause, notifications d’état/erreur et synchronisation des contrôles médias OS.
    • Reconstruction mieux orchestrée en cas de “device lost”, avec cibles de reprise, backoff, garde-fous anti-boucles et traitement distinct des bascules volontaires (dont WASAPI exclusif).
    • Mode WASAPI exclusif : resynchronisation automatique de l’interface après échec d’activation pour éviter un toggle bloqué.
  • Documentation
    • Nouvelle section sur le cycle de vie de l’output stream et les scénarios de récupération.
  • Tests
    • Test unitaire pour valider l’armement d’une reconstruction même durant la fenêtre de stabilisation.
  • Chores
    • Mise à jour des installations Linux dans les workflows CI et CodeQL.

…evice

Joël's 2026-07-25 log on #405 shows the Settings toggle still stuck on
after the beta. Three separate defects, all reachable from that report.

1. `set_wasapi_exclusive` spawned the new output stream BEFORE releasing
   the old one. That order is right for a device *switch* (two different
   endpoints), but leaving exclusive mode re-opens the SAME endpoint —
   and a WASAPI exclusive client owns its endpoint outright, so the
   shared open always failed. The command returned `Err` before
   persisting anything, `wasapi_exclusive_active` kept reporting the old
   mode, and the toggle sat latched on the very mode the user was trying
   to leave, with a restart as the only way out. `force_rebuild_output`
   already learned this in #322; apply the same release-first rule here.

2. The WASAPI exclusive backend runs its own event loop, entirely
   separate from cpal's error callback. When the device went away there
   (`wait_for_event` / `write_to_device` failing) the thread just
   returned and nobody heard about it: no `player:error`, no recovery,
   and the engine kept an `exclusive = true` handle for a thread that no
   longer existed. `run_event_loop` now reports an `ExitReason`, and a
   `DeviceLost` goes down the same two paths the cpal callback uses —
   extracted as `output::notify_device_lost` + `schedule_device_rebuild`.
   Both failure arms re-check the shutdown channel first so a deliberate
   teardown isn't mistaken for a device loss.

3. Seizing the endpoint exclusively kicks the outgoing shared client off
   it, so the mode switch made the old stream fire `DeviceNotAvailable`
   and scheduled a recovery rebuild that fought the switch — visible in
   the log as a second exclusive open ~300 ms after the first.
   `begin_deliberate_output_change()` opens the existing #365 settle
   window around the toggle so those echoes are ignored.

Every failure path that ends with no output thread at all now publishes
`wasapi_exclusive_active = false` + `player:audio-mode-changed` before
returning, and the Settings card re-reads on a failed toggle too — a
toggle describing a stream that no longer exists is the shape of #405.

Refs #405

Claude-Session: https://claude.ai/code/session_01F9cgAgcbjFFe74k9vTEcjV
@InstaZDLL InstaZDLL added scope: frontend React/Vite frontend (src/) scope: backend Rust/Tauri backend (src-tauri/) scope: docs Docs, README, assets type: fix Bug fix size: l 200-500 lines labels Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Cette mise à jour centralise la récupération après perte de périphérique audio, distingue les arrêts WASAPI volontaires des pertes réelles, sécurise les reconstructions et resynchronise l’interface après un échec du mode exclusif. Les workflows Linux installent directement leurs dépendances APT.

Changes

Récupération audio

Layer / File(s) Summary
Notification et planification communes
src-tauri/crates/app/src/audio/output.rs
Les pertes audio mettent la lecture en pause, publient les événements d’état et d’erreur, synchronisent les contrôles médias et planifient une reconstruction après 300 ms avec garde anti-doublon.
Détection CPAL et classification WASAPI
src-tauri/crates/app/src/audio/output.rs, src-tauri/crates/app/src/audio/wasapi_exclusive.rs
Le callback CPAL et la boucle WASAPI distinguent les pertes de périphérique des arrêts volontaires, y compris lors des courses avec shutdown.
Reconstruction et changements volontaires
src-tauri/crates/app/src/audio/engine.rs, docs/features/playback.md
Les changements d’output appliquent l’ordre release-first, neutralisent les rebuilds concurrents, rouvrent le gate après un échec sans sortie et publient l’absence de sortie.
Resynchronisation du mode exclusif
src/components/views/settings/ExclusiveModeCard.tsx
Après l’échec d’un toggle, l’interface relit l’état WASAPI effectif et journalise séparément l’échec de cette relecture.

Environnements de build

Layer / File(s) Summary
Installation des dépendances Linux
.github/workflows/ci.yml, .github/workflows/codeql.yml
Les workflows remplacent le cache APT par apt-get update puis une installation directe avec les paquets système requis.

Estimated code review effort: 4 (Complexe) | ~60 minutes

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant CPAL_or_WASAPI
  participant RecoveryHelpers
  participant AudioEngine
  participant Interface
  CPAL_or_WASAPI->>RecoveryHelpers: signaler DeviceNotAvailable ou DeviceLost
  RecoveryHelpers->>Interface: publier player:state et player:error
  RecoveryHelpers->>AudioEngine: planifier le rebuild après 300 ms
  AudioEngine->>AudioEngine: armer le rebuild et recréer la sortie
  AudioEngine->>Interface: publier l’état effectif de sortie
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Les workflows CI/CodeQL modifiés pour installer les paquets APT sans cache sont hors du périmètre audio de #405. Déplace ces changements de CI dans un PR séparé ou justifie-les explicitement dans l’objectif du ticket.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Le titre est clair, en scope et résume bien le changement principal sur le stream exclusif audio.
Description check ✅ Passed La description couvre le contexte, le correctif, la validation et le lien #405; elle est un peu narrative mais suffisante.
Linked Issues check ✅ Passed Les changements répondent à #405: release-first, pertes de device exclusif signalées, rebuild planifié et état audio rétabli.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/405-wasapi-toggle-release-first

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src-tauri/crates/app/src/audio/engine.rs`:
- Around line 1097-1101: Dans le flux de reconstruction de sortie autour de
was_playing, remplace le retour anticipé de l’envoi AudioCmd::Stop par le patron
de force_rebuild_output : regroupe Stop, récupération/arrêt de handle et
SwapProducer dans une closure unique, puis appelle handle.stop() même si une
étape échoue. Préserve la propagation de l’erreur et émets l’état audio attendu
afin que la préférence wasapi_exclusive et l’événement player:audio-mode-changed
restent synchronisés.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9bb211cd-d699-46e0-90f0-a7b33e79e802

📥 Commits

Reviewing files that changed from the base of the PR and between 280fc0f and fbae307.

📒 Files selected for processing (5)
  • docs/features/playback.md
  • src-tauri/crates/app/src/audio/engine.rs
  • src-tauri/crates/app/src/audio/output.rs
  • src-tauri/crates/app/src/audio/wasapi_exclusive.rs
  • src/components/views/settings/ExclusiveModeCard.tsx

Comment thread src-tauri/crates/app/src/audio/engine.rs Outdated
@InstaZDLL InstaZDLL self-assigned this Jul 25, 2026
@InstaZDLL InstaZDLL added this to the v1.7.0 milestone Jul 25, 2026
`set_wasapi_exclusive` sent `AudioCmd::Stop` with `?` after the new
output thread was already spawned. A closed command channel therefore
returned early and dropped `handle` without `stop()`: the output thread
lives on a `!Send` thread that can't be reaped from Drop, so it leaked
until process exit — and when the new stream was the exclusive one, that
leaked thread kept the device locked while `guard` still pointed at the
old stream.

Group Stop / old-handle release / SwapProducer into one closure and run
`handle.stop()` on any failure, matching `force_rebuild_output`. The
rollback also publishes the audio-mode state, so `wasapi_exclusive_active`
and `player:audio-mode-changed` stay in sync with what actually survived.

That publish rule was duplicated in three places, so it moves to
`AudioEngine::publish_output_lost_if_gone` — it only fires when the guard
is empty, which distinguishes a `Stop` send failing before the old handle
is taken (stream still live, flag already accurate) from the later steps
leaving no output at all.

Claude-Session: https://claude.ai/code/session_01F9cgAgcbjFFe74k9vTEcjV
@InstaZDLL

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src-tauri/crates/app/src/audio/engine.rs (1)

1006-1191: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Ne fermez pas la porte avant d’avoir libéré l’ancien thread.

set_wasapi_exclusive() appelle begin_deliberate_output_change() avant spawn_output_with_mode() ; si l’échec survient après guard.take(), schedule_device_rebuild() ne peut plus armer dans les 2 s suivant cette fermeture artificielle de la porte, et try_rebuild_after_device_error() ignore la faute. Ne la clôturez que si le changement est effectivement parvenu dans un nouveau stream, ou réinitialisez la porte après l’échec quand guard est vide.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/crates/app/src/audio/engine.rs` around lines 1006 - 1191, Update
set_wasapi_exclusive so begin_deliberate_output_change does not permanently
suppress recovery when spawn_output_with_mode fails after the old handle was
released. On that failure path, when guard is empty, reset or reopen the
deliberate-change suppression before schedule_device_rebuild; preserve the
suppression only after a new stream has been successfully installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src-tauri/crates/app/src/audio/engine.rs`:
- Around line 1006-1191: Update set_wasapi_exclusive so
begin_deliberate_output_change does not permanently suppress recovery when
spawn_output_with_mode fails after the old handle was released. On that failure
path, when guard is empty, reset or reopen the deliberate-change suppression
before schedule_device_rebuild; preserve the suppression only after a new stream
has been successfully installed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 98987f73-cb96-44f9-a672-b22259541523

📥 Commits

Reviewing files that changed from the base of the PR and between fbae307 and de43498.

📒 Files selected for processing (1)
  • src-tauri/crates/app/src/audio/engine.rs

cache-apt-pkgs-action installs the pinned versions from the runner
image's pre-baked apt index without an `apt-get update`. When the
Ubuntu mirror rotates a package out — gstreamer1.0-plugins-good
1.24.2-1ubuntu1.4, a libwebkit2gtk-4.1-dev dependency — that pinned
version 404s and the whole install aborts, taking glib-2.0 with it and
failing `cargo check` on ubuntu-latest.

The action also keys its cache purely on the package list, so a failed
run saves an empty (0-package) cache under an immutable key that then
poisons every subsequent run until the entry is deleted by hand.

Replace it in both ci.yml and codeql.yml with a plain `apt-get update`
+ `apt-get install`: the update refreshes the index to whatever the
mirror currently serves, and dropping the apt cache removes the
empty-cache poison trap. The expensive cache (compiled Rust artifacts
via Swatinem/rust-cache) is untouched — the system .debs it replaces
cost ~20s to fetch.
@InstaZDLL InstaZDLL added the scope: ci CI/CD, workflows label Jul 25, 2026
…s no stream

set_wasapi_exclusive opens the #365 settle window via
begin_deliberate_output_change to absorb the echo DeviceNotAvailable a
successful stream swap fires. But on the failure path — spawn fails
after the old exclusive handle was already released, leaving no output
thread — it then calls schedule_device_rebuild to recover. That rebuild
defers 300 ms and lands well inside the 2 s window we just opened, so
try_arm refuses it: the recovery is swallowed and the engine is left
with no output and no way back short of a restart, the silent tail of
the same #405 report.

With no swap there is no echo to suppress, so reopen the gate
(cancel_deliberate_output_change) before scheduling, letting the
recovery actually arm. The suppression is preserved on the success path,
where the swap does happen and the echo does need absorbing.
@InstaZDLL

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Action performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

… too

playback.md documents the invariant that every output-replace path
ending with no thread at all publishes wasapi_exclusive_active = false
and emits player:audio-mode-changed before returning the error. Two of
the three paths (force_rebuild_output, set_wasapi_exclusive) honour it;
set_output_device did not.

Its send_result closure runs guard.take() before sending SwapProducer,
so a failed SwapProducer send leaves the old handle already dropped and
the new one stopped — no output thread — yet the exclusive flag kept its
stale value and Settings never re-read. Call publish_output_lost_if_gone
on that path like the sibling functions; it no-ops when an earlier Stop
send failed before guard.take() (old stream still installed).

Refs #405

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src-tauri/crates/app/src/audio/engine.rs (3)

1111-1128: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Préserver le périphérique épinglé lors de la récupération.

active contient le périphérique sélectionné, mais le chemin d’échec appelle schedule_device_rebuild(&self.app) sans transmettre cette valeur. Une fois guard vide, try_rebuild_after_device_error() relit current_output_device() et obtient None, ce qui peut rouvrir le périphérique audio par défaut au lieu du périphérique choisi par l’utilisateur.

Transmettez active au scheduler ou conservez le dernier périphérique demandé dans AudioEngine avant de libérer l’ancien handle.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/crates/app/src/audio/engine.rs` around lines 1111 - 1128, Preserve
the user-selected output device during the failure recovery path in the
surrounding output rebuild logic. Update the call to schedule_device_rebuild
when guard.is_none() so it receives the active device, or persist that requested
device on AudioEngine before releasing the old handle, and ensure
try_rebuild_after_device_error() reopens that device instead of falling back to
current_output_device() returning None.

1075-1102: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Ouvrir la fenêtre de stabilisation avant le teardown.

old.stop() s’exécute avant begin_deliberate_output_change() (Line 1102). Si l’arrêt WASAPI déclenche DeviceNotAvailable et dure plus de 300 ms, le rebuild différé peut s’armer pendant ce changement volontaire. L’appel ultérieur à finish() ne peut pas annuler une tâche déjà passée par try_arm, qui pourra ensuite reconstruire la sortie avec un état périmé.

Ouvrez la fenêtre avant Stop/old.stop, annulez-la sur toute sortie n’ayant pas installé de nouveau flux, ou invalidez les rebuilds déjà en file via un compteur de génération.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/crates/app/src/audio/engine.rs` around lines 1075 - 1102, Move
begin_deliberate_output_change() before the pre_release teardown in the toggle
flow, so the stabilization window covers both AudioCmd::Stop and old.stop().
Ensure every path that exits without installing the replacement stream cancels
or invalidates the deliberate-change state, using finish() or the existing
rebuild-generation mechanism as appropriate; update the surrounding logic rather
than changing unrelated device-switch behavior.

1087-1109: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Tirez aussi l’ancien flux partagé avant le changement de mode.

Lors d’un passage partagé → exclusif sur le même endpoint, pre_release reste false et spawn_output_with_mode tente l’open exclusif alors que le thread cpal courant utilise encore le périphérique. WASAPI peut refuser cette init et le fallback silencieux maintient le mode partagé malgré la préférence. Libérez tout handle existant avant spawn_output_with_mode pour ce flip de mode.

Correction minimale
-        let pre_release = guard.as_ref().is_some_and(|h| h.wasapi_exclusive);
+        let pre_release = guard.is_some();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/crates/app/src/audio/engine.rs` around lines 1087 - 1109, Release
the existing shared-output handle before calling spawn_output_with_mode when
switching modes on the same endpoint, not only when pre_release is true. Ensure
the current stream is stopped and removed from guard before the new mode is
opened, while preserving the existing wasapi_exclusive-specific handling and
playback-state behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src-tauri/crates/app/src/audio/engine.rs`:
- Around line 1111-1128: Preserve the user-selected output device during the
failure recovery path in the surrounding output rebuild logic. Update the call
to schedule_device_rebuild when guard.is_none() so it receives the active
device, or persist that requested device on AudioEngine before releasing the old
handle, and ensure try_rebuild_after_device_error() reopens that device instead
of falling back to current_output_device() returning None.
- Around line 1075-1102: Move begin_deliberate_output_change() before the
pre_release teardown in the toggle flow, so the stabilization window covers both
AudioCmd::Stop and old.stop(). Ensure every path that exits without installing
the replacement stream cancels or invalidates the deliberate-change state, using
finish() or the existing rebuild-generation mechanism as appropriate; update the
surrounding logic rather than changing unrelated device-switch behavior.
- Around line 1087-1109: Release the existing shared-output handle before
calling spawn_output_with_mode when switching modes on the same endpoint, not
only when pre_release is true. Ensure the current stream is stopped and removed
from guard before the new mode is opened, while preserving the existing
wasapi_exclusive-specific handling and playback-state behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 27a40248-e038-4e79-81bb-88f1b5e5c536

📥 Commits

Reviewing files that changed from the base of the PR and between de43498 and a72518d.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • .github/workflows/codeql.yml
  • src-tauri/crates/app/src/audio/engine.rs

Two gaps on set_wasapi_exclusive's spawn-failure path (going exclusive →
shared, where the old exclusive handle was already released):

1. Wrong device on recovery. The deferred recovery ran
   try_rebuild_after_device_error, which resolves its target via
   current_output_device(). The teardown had already take()n the handle,
   emptying self.output, so that resolved to None and the rebuild
   reopened the OS default instead of the device the user had pinned.
   schedule_device_rebuild now takes a RebuildTarget: the cpal and
   WASAPI-exclusive device-loss callers pass Resolve (their dead handle
   is still parked in self.output), while the toggle-failure path passes
   Device(active) captured before the teardown.

2. Suppression window left open. begin_deliberate_output_change opens the
   #365 settle window to absorb a swap echo. When the spawn failed no
   replacement was installed, so there is no echo — but the window stayed
   open on the branch where the old shared stream survives (shared →
   exclusive, even the shared fallback failed), suppressing genuine
   errors on that surviving stream for 2 s. Reopen the gate on every
   spawn-failure exit, not only when the engine is left with no output.

Left as-is by design: the release-first order is kept exclusive-only
(the same-endpoint echo is handled by the settle window, a path
confirmed working on real hardware), and the exclusive teardown exits
via Shutdown with no echo, so begin_deliberate need not move earlier.

Refs #405
@InstaZDLL InstaZDLL added size: xl > 500 lines and removed size: l 200-500 lines labels Jul 25, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src-tauri/crates/app/src/audio/engine.rs (1)

694-712: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Le gate ne neutralise pas un rebuild déjà armé.

begin_deliberate_output_change() modifie seulement RebuildGate. Une tâche ayant déjà passé try_arm_device_rebuild() poursuit try_rebuild_after_device_error() sans revalidation ; elle peut donc exécuter Stop/SwapProducer pendant ou juste après un changement manuel. Utilisez un jeton de génération ou une sérialisation commune, vérifiée juste avant le rebuild.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/crates/app/src/audio/engine.rs` around lines 694 - 712, Update the
rebuild flow around begin_deliberate_output_change,
cancel_deliberate_output_change, try_arm_device_rebuild, and
try_rebuild_after_device_error so an already-armed rebuild is invalidated or
blocked when a deliberate output change begins. Add a shared generation token or
serialization mechanism and revalidate it immediately before executing
Stop/SwapProducer, ensuring stale rebuild tasks cannot run during or after the
manual change.
src-tauri/crates/app/src/audio/wasapi_exclusive.rs (1)

163-175: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Ne signalez pas la perte de périphérique depuis les contextes audio.

wasapi_exclusive.rs#L163-175 et output.rs#L588-595 exécutent dans le thread WASAPI et le callback CPAL : format!, logs tracing, accès à l’état Tauri, emissions JSON et planification de tâches. Envoyez uniquement l’erreur au superviseur depuis ces contextes; le superviseur doit ensuite appeler la notification et le rebuild hors temps réel.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/crates/app/src/audio/wasapi_exclusive.rs` around lines 163 - 175,
Remove notification and rebuild work from the WASAPI audio context in
wasapi_exclusive.rs, including logging, formatting, Tauri state access, and task
scheduling; send only the device-loss error to the supervisor. Apply the same
constraint to the CPAL callback path in output.rs, then have the non-real-time
supervisor perform the logging, notify_device_lost call, and
schedule_device_rebuild operation for both sites.

Sources: Coding guidelines, Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src-tauri/crates/app/src/audio/engine.rs`:
- Around line 694-712: Update the rebuild flow around
begin_deliberate_output_change, cancel_deliberate_output_change,
try_arm_device_rebuild, and try_rebuild_after_device_error so an already-armed
rebuild is invalidated or blocked when a deliberate output change begins. Add a
shared generation token or serialization mechanism and revalidate it immediately
before executing Stop/SwapProducer, ensuring stale rebuild tasks cannot run
during or after the manual change.

In `@src-tauri/crates/app/src/audio/wasapi_exclusive.rs`:
- Around line 163-175: Remove notification and rebuild work from the WASAPI
audio context in wasapi_exclusive.rs, including logging, formatting, Tauri state
access, and task scheduling; send only the device-loss error to the supervisor.
Apply the same constraint to the CPAL callback path in output.rs, then have the
non-real-time supervisor perform the logging, notify_device_lost call, and
schedule_device_rebuild operation for both sites.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e15f6073-1c1a-44e9-8900-1a4a7311059f

📥 Commits

Reviewing files that changed from the base of the PR and between a72518d and 5b53cc7.

📒 Files selected for processing (3)
  • src-tauri/crates/app/src/audio/engine.rs
  • src-tauri/crates/app/src/audio/output.rs
  • src-tauri/crates/app/src/audio/wasapi_exclusive.rs

…nd fails

set_wasapi_exclusive swaps the wasapi_exclusive preference up front (for
the no-op early-exit), then in the pre_release branch sends AudioCmd::Stop
before tearing the old exclusive stream down. If that send fails (decoder
channel closed) the `?` returned before guard.take() ran — so the old
exclusive stream was still installed and running, yet the in-memory
preference now claimed the new mode. A later device-error rebuild reads
that preference and would silently flip to the mode the toggle never
actually applied.

Unlike the spawn / send_result failure paths, which keep the new pref on
purpose because the old stream is already gone and a rebuild will apply
it, this path leaves the old stream intact — so restore the preference to
match it before propagating the error.

reset_exclusive_suppression() is also deferred until after the
pre-release teardown: on this same failure it must not wipe the flap-storm
suppression that still describes the surviving old stream. Its effect is
unchanged for every success path (nothing between the old and new call
site reads the suppression state).

Refs #405
…o fails

The shared → exclusive toggle spawns with exclusive = true, which on
Windows falls back to cpal shared internally and only returns Err when
even that shared fallback can't open the device. In that case
pre_release is false, so `guard` still holds the old shared stream,
untouched and running — but the wasapi_exclusive preference was already
swapped to exclusive up top.

The guard.is_none() arm keeps the new pref on purpose (no stream left, a
rebuild will apply it), but the surviving-stream arm did not, leaving the
pref claiming exclusive while the live stream is shared — a later
device-error rebuild would then read that pref and flip to the exclusive
mode the toggle never managed to apply. Roll the preference back to match
the surviving stream, mirroring the failed-Stop path.

Refs #405
@InstaZDLL
InstaZDLL merged commit 27a5dd4 into main Jul 25, 2026
14 checks passed
@InstaZDLL
InstaZDLL deleted the fix/405-wasapi-toggle-release-first branch July 25, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: backend Rust/Tauri backend (src-tauri/) scope: ci CI/CD, workflows scope: docs Docs, README, assets scope: frontend React/Vite frontend (src/) size: xl > 500 lines type: fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WASAPI: audio recovers after a device flap but player state does not (lyrics, seek bar, toggle)

1 participant