-
Notifications
You must be signed in to change notification settings - Fork 37
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
Cloned Playback Volume data model module into Capture module #175
base: master
Are you sure you want to change the base?
Conversation
-Adds default capture device to the data model -Works in the same way as (and reuses the code of) the playback device -Works on default capture device (eg. microphone, line-in, stereo mix)
-Done to match behaviour of my setup, where mic mute key affects that device specifically
Hello, Sorry for not getting back to you sooner. I went through it and I think this is a nice addition. To avoid this issue, could you refactor this to use an abstract class instead? Functionally, the only real difference is in I propose the following class signatures: This allows you to put all shared logic in Thank you! |
Happy to do that, just need to figure out the logic; I’m not the most experienced with classes and that, so I went for the most basic possible solution that produced the effect I was wanting at the time. I do understand it’s not the best for code maintainability, but as a proof of concept it worked. I’ll take a look at it shortly. Another benefit of the requested refactor is to allow more than just the specified devices to be exposed to the data model, if someone puts together a suitable interface for users to select devices; rather than the current two options of default playback device and default communication capture device. More reason to go ahead with the refactor. |
You could have additional devices available as dynamic data models but I'm not sure how useful this is |
Okay, I've given it a crack at refactoring. A couple of things I wasn't exactly happy with:
|
Artemis trying to load an abstract class as a feature seems like an oversight to me, it will be fixed in Core. I'm doing quite large changes to this plugin for linux compatibility, i'll see if I can get these changes in there as well. |
|
||
private bool SetAudioEndpointDevice() | ||
{ | ||
_audioDevice = _naudioDeviceEnumerationService.GetDefaultAudioEndpoint(DataFlow.Render, Role.Console); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_audioDevice = _naudioDeviceEnumerationService.GetDefaultAudioEndpoint(_flow, _role);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way at least the module will correctly capture the mic. Unfortunately, the volume doesn't seem to update unless i also have the native windows mixer open:
Artemis.UI.Windows_e0ZRENbMP9.mp4
I'm not sure if we need to listen for some event or call some update function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, missed that one (_flow/_role). That's a major issue. Fix implemented.
In regards to the volume not updating, I can confirm this issue occurs on my system. My suspicion is that nAudio is registering the device and pulling in data - but not actually opening access to the device itself, in regards to the security context. If you note the icon in the system tray for "Apps using your microphone", it will appear when the Windows recording device panel is enabled, and disappear when it is not. The same occurs if you use the Settings sound panel (the "replacement" control panel), or (for another example) if you have a Discord call open. I'll do a little more digging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, we do have to initialise a recording access in order for the volume data to be updated. This was outside my use case, as I only wanted the mute state or otherwise; but better to do it right the first time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my experience users don't like it when apps listen to the mic 24/7 so I would be careful with how you implement this. Maybe separate volume and actual sounds into separate modules or add an option to enable mic capture or something? It's not a trivial feature to add imo.
I've implemented a capture feature; on by default (for consistency with the render variant of the plugin), but with an option in the plugin settings to disable it. Description text could probably use some improvement, but it's there as the second option in the list. |
…/Artemis.Plugins into feature/audio-capture
-Side note: pretty sure I clobbered the Git history pretty bad here. Probably warrants a squash... if I could figure that out.
One other thing I noticed; if you disable the capture option while the data model debugger is open, it will appear to stop updating; this is only a visual bug, the data model is updating in the background. A quick way to test is using this profile I quickly put together (change the file extension to JSON), and then try toggling the enable/disable on and off. |
-Adds default capture device to the data model
-Works in the same way as (and reuses the code of) the playback device
-Works on default capture device (eg. microphone, line-in, stereo mix)
I created this because one of the RGB buttons on my laptop defaults to muting and unmuting the microphone, and I wanted to have the mute status reflected in the button colour; to do this, I needed to expose the capture device in the data model, in the same way that the playback device is exposed. It is simply a copy/paste, with appropriate identifiers find/replaced, and the default capture device being referenced in the initialisation rather than the default playback device.