Run background process to keep conhost audio session active. #452
Conversation
| if (isAudioSessionMuted != null) | ||
| { | ||
| audioSessionFound = true; | ||
| } | ||
| } | ||
| catch (Exception) { } |
There was a problem hiding this comment.
Is hiding exceptions like this really a good idea? Can you be more specific about the kind of exceptions you're wanting to squash here? What about logging?
There was a problem hiding this comment.
Finally, I was not able to get any exception on 1809/1903 and just removed try/catch block
|
|
||
| cmdProcess.Kill(); | ||
| if (deviceMuted != true) | ||
| { | ||
| VolumeControl.SetDefaultAudioEndpointMute(false); |
There was a problem hiding this comment.
This should probably be a in try-finally to ensure the audio endpoint is always unmuted even if something goes wrong.
There was a problem hiding this comment.
Have removed muting of device as it was discussed ( in next PR will attempt to mute audio session on its early start )
| @@ -569,44 +569,58 @@ internal static void SaveFile(string path, string content) | |||
| } | |||
| } | |||
|
|
|||
| internal static void MuteTerminal(bool mute) | |||
| private static bool MuteConhost(bool mute, int attemptCount) | |||
There was a problem hiding this comment.
Why are there different attemptCount values for different call sites?
There was a problem hiding this comment.
have removed looping completely
| bool? deviceMuted = VolumeControl.GetDefaultAudioEndpointMute(); | ||
| if (deviceMuted != true) | ||
| { | ||
| VolumeControl.SetDefaultAudioEndpointMute(true); |
There was a problem hiding this comment.
@mjs what do you think if to remove muting of audio device completely and let it play bell sound? So far it's 3-4 reports from different people to explain why the sound suddenly disappears.
There was a problem hiding this comment.
Yeah, I saw those reports.
If the bell plays then people will probably complain about that... it might be less confusing than muting I guess.
Terminus lets the user disable the terminal bell (or use a visual bell) and they use xtermjs too. It might be good to dig into how they do it.
There was a problem hiding this comment.
@mjs will check Terminus. Initially, I have seen that Fluent passes some option to xterm to configure bell, but I haven't seen any difference in real behavior.
There was a problem hiding this comment.
There was a problem hiding this comment.
Never mind about Terminus. I've just played around with it and its terminal beep suppression doesn't work.
Don't mute audio device.
maxhora
force-pushed
the
mr/enhance-muting-upstream
branch
from
86477f4
to
1650cef
Compare
|
@felixse please take a look to have this merged, thanks |
…#452) Don't mute audio device.
"Mute Terminal" functionality enhancement discussed in previous PR #437 (comment)