Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Throttle the bell sound (and visual indication) to avoid overloading audio stack #9776

Closed
benthillerkus opened this issue Apr 11, 2021 · 3 comments
Closed

Throttle the bell sound (and visual indication) to avoid overloading audio stack #9776

benthillerkus opened this issue Apr 11, 2021 · 3 comments
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v2.0

Comments

@benthillerkus
Copy link

Windows Terminal version (or Windows build number)

1.6.10571.0

Other Software

Software

NVidia Game Ready Drivers v465.89
Windows Build 19042.867

Hardware

NVidia Geforce GTX 970
I'm using HDMI on the GPU for video and audio

Steps to reproduce

  1. Trigger the bell repeatedly (for example by pressing backspace on an empty line in SSH)
  2. Brace your ears

Expected Behavior

Lots of Windows Alert sounds

Actual Behavior

First the sounds will appear normally, but then it's just break down in loud audio glitching / corruption. To get rid of it you'll have to shut down the PC (restarting didn't help in my experience)
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 11, 2021
@DHowett
Copy link
Member

DHowett commented Apr 13, 2021

Thanks for the report! I'm not sure how we're tripping over some driver issues, but I think it would be a good idea for us to limit the number of bells we emit (throttle them to 1/second or so?)

I'm going to repurpose this issue as the "throttle bells" task. I hope that it'll help!

@DHowett DHowett added Issue-Task It's a feature request, but it doesn't really need a major design. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 13, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 13, 2021
@DHowett DHowett added this to the Terminal v2.0 milestone Apr 13, 2021
@DHowett DHowett changed the title Bell crashes audio drivers (?) and causes loud glitch noises Throttle the bell sound (and visual indication) to avoid overloading audio stack Apr 13, 2021
@Don-Vito Don-Vito mentioned this issue Apr 14, 2021
Merged
6 tasks
@ghost ghost added In-PR This issue has a related PR Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Apr 14, 2021
@ghost ghost closed this as completed in 9a2d27e Apr 14, 2021
mpela81 pushed a commit to mpela81/terminal that referenced this issue Apr 17, 2021
@Don-Vito @mpela81
Limit terminal warning bells to one per second (microsoft#9812)
6116231 
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes microsoft#9776
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated. 
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. 

## Detailed Description of the Pull Request / Additional comments
Use `ThrottledFunc` in `TermControl` to limit bell emission callback to one per second.
DHowett pushed a commit that referenced this issue May 14, 2021
@Don-Vito @DHowett
Limit terminal warning bells to one per second (#9812)
0d451cc 
* [x] Closes #9776
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.

Use `ThrottledFunc` in `TermControl` to limit bell emission callback to one per second.

(cherry picked from commit 9a2d27e)
DHowett pushed a commit that referenced this issue May 14, 2021
@Don-Vito @DHowett
Limit terminal warning bells to one per second (#9812)
76a609b 
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #9776
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.

## Detailed Description of the Pull Request / Additional comments
Use `ThrottledFunc` in `TermControl` to limit bell emission callback to one per second.

(cherry picked from commit 9a2d27e)
@ghost
Copy link

ghost commented May 25, 2021

🎉This issue was addressed in #9812, which has now been successfully released as Windows Terminal v1.8.1444.0.:tada:

Handy links:

@ghost
Copy link

ghost commented May 25, 2021

🎉This issue was addressed in #9812, which has now been successfully released as Windows Terminal Preview v1.9.1445.0.:tada:

Handy links:

@zadjii-msft zadjii-msft mentioned this issue Aug 16, 2021
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v2.0
Development

No branches or pull requests
2 participants
@DHowett @benthillerkus