[Feature] NAudio For Volume Changed Instand Of Keyboard Hook#466
[Feature] NAudio For Volume Changed Instand Of Keyboard Hook#466kitUIN wants to merge 13 commits intounchihugo:masterfrom
Conversation
There was a problem hiding this comment.
Hi @kitUIN, looks great so far, thank you! By migrating to NAudio events we solve the issue of flyouts only displaying from keypress events. There's some issues I've found during testing that needs addressing before we merge this update! I've written them down as comments.
I've noticed that headphones or trackpad volume manipulation successfully get detected now, though media tracking still only works from keyboard presses, so the new code unfortunately only solves the problem partially. The new method also detects volume changes from things like manually changing volume from the volume mixer, do you think we can avoid that?
Also, are you sure we still need the legacy keyboard hooks system in place? I'd like to hear your thoughts on this!
Finally, to my understanding from reading your code, there's little to no overlap with my Volume Flyout PR (#387) as you've only changed the hook method. I assume that you have a better understanding of how to structure the code properly, so could you have a quick look into this just to make sure? Thank you!
|
|
||
| private bool _isStarted; | ||
|
|
||
| private const int MEDIA_KEY_PLAY_PAUSE = 0xB3; |
There was a problem hiding this comment.
Do you think we should add these constants to NativeMethods.cs or keep them here?
| { | ||
| DispatchEventAsync(DispatchVolumeChanged); | ||
| } | ||
| else // 此时必然是 mediaKeyPressed 为 true |
There was a problem hiding this comment.
Could you translate this comment as well?
| return CallNextHookEx(_hookId, nCode, wParam, lParam); | ||
| } | ||
|
|
||
| private void RaiseLockKeyPressed(LockKeyType keyType, bool isToggled) |
There was a problem hiding this comment.
I've noticed an issue with displaying lock keys status with this new method: if you press the key quickly (like a normal tap), it almost always reports the state it was previously in, and not the new state. This might be because the throttle is set too long for the lock keys, and because (like the hook version) we're listening for key presses instead of state changes.
| <System:String x:Key="MediaFlyoutExcludeVolumeDescription">Do not display the flyout when pressing volume up/down/mute</System:String> | ||
| <System:String x:Key="MediaFlyoutInputSourceTitle">Input Monitor Source</System:String> | ||
| <System:String x:Key="MediaFlyoutInputSourceDescription">Choose how FluentFlyout detects volume changes</System:String> | ||
| <System:String x:Key="MediaFlyoutInputSourceOptionNAudio">NAudio</System:String> |
There was a problem hiding this comment.
I think we should label the hook option as "Keyboard Hook (Legacy)" so users don't necessarily think that both options are equally as good.
| } | ||
|
|
||
| _nAudioRenderDevice.AudioEndpointVolume.OnVolumeNotification -= OnNVolumeNotification; | ||
| _nAudioRenderDevice.Dispose(); |
There was a problem hiding this comment.
Sometimes throws user-unhandled exception "System.Runtime.InteropServices.InvalidComObjectException: 'COM object that has been separated from its underlying RCW cannot be used'" when disconnecting an audio device. It would probably be wise to wrap some of these methods in try/catch blocks
Second Changed
|
This is a quite important edge case actually, maybe we should combine both methods instead then?
The native volume flyout and quick panel ( |
For this edge case, using both methods might be ok
Regrettably, at present I haven't found a way to ignore them. Using NAudio to detect volume changes is a limited approach. If the sole purpose is to support the touchpad and headphones, perhaps there are better ways to do it. |
I've just found out about WM_APPCOMMAND, most notably the WM_MEDIA and WM_VOLUME messages. If this works, leveraging this instead of NAudio will help solve all these issues we currently have with keyboard hooks and NAudio volume change events. Would you want to develop this or should I have a go at it? |
New Feature Support
InputMonitorService. This service notifies the main program through events. And the new mode of NAudio is also included in this service.VolumeChangedEvent: mute / volume up / volume downMediaKeyPressedEvent: play/pause, next, previous, stopLockKeyPressedEvent: CapsLock / NumLock / ScrollLock / InsertRelated issues