Skip to content

fix(audio): keep the audio-feedback output stream open instead of reopening it per chime (cross-platform) - #1740

Closed
kydechuk01 wants to merge 1 commit into
cjpais:mainfrom
kydechuk01:fix/audio-feedback-crossplatform
Closed

fix(audio): keep the audio-feedback output stream open instead of reopening it per chime (cross-platform)#1740
kydechuk01 wants to merge 1 commit into
cjpais:mainfrom
kydechuk01:fix/audio-feedback-crossplatform

Conversation

@kydechuk01

Copy link
Copy Markdown

Before Submitting This PR

  • I have searched existing issues and pull requests (including closed ones) to ensure this isn't a duplicate
  • I have read CONTRIBUTING.md

Human Written Description

This is the audio-feedback side of the hang, redone so it builds everywhere. In my first PR (#1713) I only compiled on Windows and it broke your macOS build — sorry about that. The real problem on my machine: a fresh WASAPI output stream gets opened for every start/stop chime, and when one stalls it drags the whole audio stack down with it. This opens the stream once and reuses it. Kept it minimal, and this time it never moves the stream across threads — that was the !Send compile error last time (cpal's stream is Send on WASAPI but not on CoreAudio/ALSA).

Related Issues/Discussions

Relates to #1712. Complements #1713 (the recorder-side bounded-waits fix for the same hang — that PR stays focused on the recorder change; this one is the audio-feedback half).

Fixes #1712

Community Feedback

You asked for a patch in #1712 ("If you have a patch would love to see it and pull it in"). #1213 / #1228 / #1349 are independent reports of the same symptom.

Testing

  • Windows 10 (Ryzen 9800X3D, RTX 5070): running these exact changes daily for over a week with audio feedback on — hundreds of transcriptions, GigaAM v3 and Whisper Large on Vulkan — zero hangs, where before it hung every 2–30.
  • Linux (Ubuntu, your test.yml): green on my fork — this is the platform that actually catches the bug, since cpal's Stream is !Send on ALSA just like on CoreAudio.
  • macOS (Apple Silicon, aarch64): ran your Build Test on my fork — the app compiles and links (Built application at .../aarch64-apple-darwin/release/handy); the job only fails at codesign because my fork has no Apple certificate. That was the exact step that failed to compile before, so this confirms the !Send fix.
  • cargo clippy clean on the touched file, cargo fmt applied.

Behaviour notes: the stream is pre-warmed once at startup, recreated only on device change, on a "Default"→OS-default switch, or on a playback stall (bounded, so a wedged device can at worst drop a chime — the pipeline is protected separately by the recorder-side change in #1713).

AI Assistance

  • AI was used

  • Tools used: Claude Code

  • How extensively: root-cause analysis and the patch were written with Claude Code under my direction and review; I built it and field-tested it on my machine over a week of daily use, and drove the cross-platform CI verification. Commits carry a Co-Authored-By: Claude trailer.

Opening a WASAPI output stream (or enumerating output devices for an
explicit selection) on every chime races concurrent audio session state
on Windows; when it stalls it can take the whole audio stack down with it.
Move playback to a dedicated long-lived worker that opens the output
stream once (pre-warmed at startup) and reuses it, recreating only on
device change, on a default-device switch, or on a playback stall.

Dispose of a scrapped stream on the worker thread itself: cpal::Stream is
!Send on CoreAudio and ALSA, so it must not be moved to another thread to
be dropped (this is what failed to compile on macOS in the first pass).
The transcription pipeline is protected independently by the recorder-side
bounded waits, so a slow teardown here can at worst drop a chime.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cjpais

cjpais commented Jul 20, 2026

Copy link
Copy Markdown
Owner

I need to take a look at this but my concern is keeping devices awake can affect sleep states on some systems and also conflict with other audio apps, which is mainly the reason why we open and close.

If we don't already we probably should open once at the start of transcription and only close at the end of it. At least this would be better, but mostly I don't think we should keep the device warm all the time unless we add a setting to disable this (and actually should be disabled by default)

@kydechuk01

Copy link
Copy Markdown
Author

Fair point — I hadn't thought about sleep states and conflicts with other audio apps holding the device open. That one's on me, and I agree it shouldn't stay warm forever.

Before I rework it: what about reusing the pattern you already have for the mic stream? schedule_lazy_close() in managers/audio.rs keeps the input stream open between recordings and closes it after 30s of idle, gated by the existing lazy_stream_close setting (which defaults to false). The same thing for the chime output stream would give:

  • lazy_stream_close = false (default): the stream is opened for a transcription and closed once it's done — essentially what you suggested, and one stream creation per transcription instead of two.
  • opt-in true: the stream survives 30s of idle, so during an active dictation session there's no per-chime WASAPI stream creation at all — which is the part that stalls on my machine.
  • either way the close lands on the idle timer or after the pipeline is finished, not simultaneously with the mic stream being torn down — that simultaneity is where the race actually bites.

That gives you the "setting, disabled by default" behaviour you asked for without adding a new setting, and it's the same semantics the mic stream already has, so it should read as familiar rather than as a new mechanism.

Happy to rework the PR that way. If you'd rather keep it dead simple — open at start of transcription, close at the end, no setting involved — I'll do that instead. Your call, just tell me which and I'll push it.

MaxITService added a commit to MaxITService/AIVORelay that referenced this pull request Jul 22, 2026
Manually adapted open upstream PR cjpais#1740 head e9934a6. Retained AIVO's independent result-ready feedback cue.
@cjpais

cjpais commented Aug 5, 2026

Copy link
Copy Markdown
Owner

I don't think that I'm going to be able to merge this right now. Largely that I don't think we should be keeping output streams active, and this kind of changes a lot of core stuff. If you want to submit a more targeted fix, let's do that

@cjpais cjpais closed this Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants