Fix recorder start/stop failures and debounce toggle shortcuts#644
Open
Korben00 wants to merge 3 commits intocjpais:mainfrom
Open
Fix recorder start/stop failures and debounce toggle shortcuts#644Korben00 wants to merge 3 commits intocjpais:mainfrom
Korben00 wants to merge 3 commits intocjpais:mainfrom
Conversation
- Refactor AudioRecorder::stop() to use map_err for cleaner error handling - Simplify shortcut debounce logic and remove unnecessary variable extraction - Remove extraneous whitespace in audio manager - Add missing closing brace and comment in shortcut handler
- Extract magic number 250 to DEBOUNCE_MS constant - Move active_toggles update inside lock scope to prevent race conditions - Fix brace alignment and remove duplicate closing braces
Contributor
|
Debouncing could make sense as a UX improvement, but it doesn't directly fix race conditions - it can mask them and make them harder to test. |
virenpepper
approved these changes
Jan 28, 2026
virenpepper
left a comment
virenpepper
left a comment
There was a problem hiding this comment.
hey, i reviewed this fix in detail and verified it compiles and works
the root cause is in recorder.rs - the consumer loop blocks on sample_rx.recv() waiting for audio, and only checks commands after receiving a sample. if the audio stream stops producing samples (race condition, device issue), stop() blocks forever because the command is never processed...
- recv_timeout(50ms) instead of blocking recv() - critical fix
- 5s timeout on stop() response - prevents infinite hang
- debouncing in handler (250ms) - prevents rapid triggers
- ui state changes after recording actually starts - correct state management
- early exit in on_end if not recording - prevents invalid stop calls
one minor note: 250ms debounce might feel slightly sluggish for power users, we should prob update to 150/200ms if possible
LGTM nice work! this should fix #641
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed that in some cases the recorder could fail the start/stop and leave the app in an inconsistent state (microstream stopped, toggle stuck). So I reinforced error management by restoring the microphone stream when it’s fairing, and I stabilized the toggle shortcut with a debounce to avoid double triggers.
Related Issues/Discussions
Fixes #
Discussion: #641
Testing
AI Assistance
If AI was used: