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.

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

Specify an audio file for the audible bell #8366

Closed
KalleOlaviNiemitalo opened this issue Nov 22, 2020 · 22 comments · Fixed by #11511
Closed

Specify an audio file for the audible bell #8366

KalleOlaviNiemitalo opened this issue Nov 22, 2020 · 22 comments · Fixed by #11511
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. 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

Comments

@KalleOlaviNiemitalo
Copy link

Description of the new feature/enhancement

Please let me configure the audible bell so that it plays a sound other than "Critical Stop". I'd like to select a sound with a shorter duration and perhaps lower volume.

This request is for Windows Terminal, not the console host.

Proposed technical implementation details (optional)

Ever since #7679, the implementation of the audible bell hardcodes SND_ALIAS_SYSTEMHAND:

const auto soundAlias = reinterpret_cast<LPCTSTR>(SND_ALIAS_SYSTEMHAND);
PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY);

I can think of two ways to make this configurable:

  • Let me specify the path of a sound file in settings.json. I think this should be configurable per profile, like the bellStyle setting (Feature Request: disable bell #2360) is.

  • Alternatively, register Windows Terminal with the "Change system sounds" feature in Control Panel so that I can choose a sound there. According to C++ at Work: Web Version Checking, Adding Sound to an App, this involves the HKEY_CURRENT_USER\AppEvents\Schemes\Apps Registry key and the SND_APPLICATION flag. I suspect that the MSIX packaging of Windows Terminal won't allow writing to such Registry keys, though.

@KalleOlaviNiemitalo KalleOlaviNiemitalo added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Nov 22, 2020
@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 Nov 22, 2020
@zadjii-msft
Copy link
Member

I absolutely agree that this is a reasonable feature request. From our team's internal Teams channel:

image

@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Nov 23, 2020
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Nov 23, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 23, 2020
@electronic-dk
Copy link

Which audio file types will the terminal be supporting in this case? Only wav?
P.S. The bruh sound looks like a perfect choice for the bell, but I'd definitely like to set john cena theme as the bell sound.

@zadjii-msft
Copy link
Member

It probably shouldn't just be .wav - hopefully XAML/WinRT have some convenient APIs we could use for loading and playing audio files that'll accept a wide gamut of file types

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 24, 2020
@WSLUser
Copy link
Contributor

WSLUser commented Nov 24, 2020

Alternatively, register Windows Terminal with the "Change system sounds" feature in Control Panel so that I can choose a sound there.

I would like this to be an option as well if possible or make the sounds available at least.

@KalleOlaviNiemitalo
Copy link
Author

If WinRT has spatial audio APIs, I might be able to hear which monitor contains the terminal that is ringing the bell.

@j4james
Copy link
Collaborator

j4james commented Nov 24, 2020

If there's not a lot of demand for a per-profile setting, I'd also be on favour of us handling this with a custom sound event that could be configured from the control panel. That way it could (hopefully) be shared with conhost too.

There's no reason why it couldn't also be set from within the Terminal settings GUI, once that's in place. It's just that the underlying config would be stored in the registry rather than the current json file.

See also: https://docs.microsoft.com/en-us/windows/win32/uxguide/vis-sound

@PhMajerus
Copy link

If WinRT has spatial audio APIs, I might be able to hear which monitor contains the terminal that is ringing the bell.

I think this should be global in Windows instead of a Terminal feature.

I've suggested years ago, especially for multi-monitor setups, to have all apps audio be spatially located according to their position. It can be difficult since Windows typically knows which process the sound is coming from but not which window precisely, but anchoring the sound to the top level window of each process would be a great option.
Locating the sound according to the desktop coordinate would require some calibration step to know where the monitors and speakers are and how their coordinates map, so it would be better as a global Windows setting.

Note Windows Mixed Reality actually does this for UWP apps already, as it is even more important when your apps are all around you in the WinMR home 3D environment instead of in a 2D desktop.

@KalleOlaviNiemitalo
Copy link
Author

PSReadLine 2.1.0 seems to be using Console.Beep(1221, 50) by default: Cmdlets.cs, Render.cs. Perhaps Windows Terminal could offer a configurable beep as an alternative to an audio file.

  • How to specify the parameters in settings,json? Could be "bellFrequency": 1221, "bellDuration": 50. IIRC, some media player supported encoding frequencies and durations in a URI, but I presume that would be more difficult to map to the settings UI, and I can't find it in IANA's list of URI schemes.
  • The Beep function waits until the sound ends. That could slow down Windows Terminal. Could be avoided by having Windows Terminal instead generate a waveform in memory and then play it with some other API.

@steevenlee

This comment has been minimized.

@Don-Vito

This comment has been minimized.

@steevenlee

This comment has been minimized.

@Don-Vito

This comment has been minimized.

@steevenlee

This comment has been minimized.

@Don-Vito

This comment has been minimized.

@bbhoss
Copy link

bbhoss commented Mar 7, 2021

It would be nice if the bell were configurable per-profile. That way if you had something that emitted a bell when it was finished executing a long-running process, you could easily distinguish it from the default bell.

@zadjii-msft
Copy link
Member

@bbhoss yep, we're definitely planning on making this per-profile. In fact, the "bellSound": "bruh.wav" example from earlier was exactly what convinced the team to make the bell settings per-profile in the first place 😉

@Lexicality
Copy link

If possible, I would very much like the ability to set a bunch of bell sounds and have it pick a random one each time. I think it would make situations where you get a bunch of bell tones one after the other nicer if you had a selection.

My particular use case is picking all the Untitled Goose Game harmonica honk noises so I get a harmony when I hit ^c a bunch of times

@zadjii-msft
Copy link
Member

set a bunch of bell sounds and have it pick a random one each time ... picking all the Untitled Goose Game harmonica honk noises so I get a harmony when I hit ^c a bunch of times

Okay at first I was like "this is an okay feature request but not one we'd likely get to" but now I'm all "this is a P0 must-ship for v2" 😄

@zadjii-msft
Copy link
Member

zadjii-msft commented Oct 14, 2021

honk-001.mp4

(obviously, audio on)

@ghost ghost added the In-PR This issue has a related PR label Oct 14, 2021
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@ghost ghost closed this as completed in #11511 Jan 6, 2022
@ghost ghost added 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 Jan 6, 2022
ghost pushed a commit that referenced this issue Jan 6, 2022
## Summary of the Pull Request

Adds a per-profile setting for setting the audio sound for the bell. The setting is `bellSound`, it accepts a path. We'll use the file at that path as the sound for the bell. If it doesn't exist, then oh well, so sound for you. 

It'll also secretly accept an array of paths. If you provide an array, it will pick one at random. 

## PR Checklist
* [x] Closes #8366
* [x] I work here
* [ ] Tests - lol this is the hackathon, I'm just messing around
* [ ] Requires documentation to be updated

## Validation Steps Performed

I'm not suggesting that anyone go to [this post](https://www.reddit.com/r/untitledgoosegame/comments/d77le4/honk_ringtones/) and download a zip full of `honk.mp3`s. I'm definitely not suggesting you add it to your settings like 

```jsonc
"bellSound": [
    "C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk1.mp3",
    "C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk2.mp3",
    "C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk3.mp3",
    "C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk4.mp3",
    "C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled1.mp3",
    "C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled2.mp3",
    "C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled3.mp3"
]
```

No, don't do that.

https://user-images.githubusercontent.com/18356694/137389503-91e43dba-8f7b-4078-9d35-23ceb2ac9432.mp4



* [x] It surprisingly works elevated
* [x] We should probably accept env vars in these paths
* [x] We may only want one `MediaPlayer` per terminal, rather than one per pane
* [ ] We may want to validate the paths, and discard ones that don't exist.
  * [x] alternatively, _meh_
@Lexicality
Copy link

🎉 Thank you so much for implementing my suggestion! Do you know when the next release is likely to be?

@zadjii-msft
Copy link
Member

@Lexicality At this moment, no. We've been on a bit of a slow burn leading up to the holidays. I imagine it won't be for at least a few more weeks.

@ghost
Copy link

ghost commented Feb 3, 2022

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

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. 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
Development

Successfully merging a pull request may close this issue.