alexmarsev / sanear

Robust DirectShow audio renderer. Used in MPC-HC.
GNU Lesser General Public License v2.1
55 stars 20 forks source link

Make Sanear listen to Default Audio Change. #9

Closed Belphemur closed 9 years ago

Belphemur commented 9 years ago

Using the documentation given here: https://msdn.microsoft.com/en-us/library/windows/desktop/dd371417%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396 and here: https://msdn.microsoft.com/en-us/library/windows/desktop/dd370810%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396

I Implemented the IMMNotificationClient to make sanear change the output audio device when it's changed.

Only quirk: The trayIcon won't keep the Default Audio Device as the selected audio device, but will change to the one set by the IMMNotificationClient. I don't know your code enough to find an easy and elegant way to resolve this little problem.

Other than that, it works flawlessly, I followed your Factory pattern.

alexmarsev commented 9 years ago

Thanks for your contribution!

I reworked your code a bit here https://github.com/alexmarsev/sanear/commit/65d1c3b1fb7802f420602db94ace04f6ad999712. That way it checks for default device changes and settings changes together, and doesn't write anything to the config.

Belphemur commented 9 years ago

I'm happy I helped a little. I'm not really familiar with Windows API (and its macro like DECLARE_IUNKNOWN (damn useful !))

If I understand the logic correctly, you're using an atomic to check for change made by other thread (like the one holding the DeviceEnumerator) made into Sanear (like a change of default device).

Why not keeping the deviceID in a field in the AudioDeviceNotificationClient and returning it with a method GetDefaultDeviceId() ?

(It's really curiosity, and for learning purpose)

alexmarsev commented 9 years ago

To be fair, DECLARE_IUNKNOWN is part of DirectShow Base Classes, not the most renown thing these days.

Regarding GetDefaultDeviceId(), yes the value from IMMNotificationClient::OnDefaultDeviceChanged() could be reused, but:

  1. We need default device id even before it's called - for the situation when the user switches from default device to explicit device which is currently system default. Meaning the current GetDefaultDeviceId() is still needed in some form.
  2. We need to copy the string to use it in GetDefaultDeviceId(), meaning handling out-of-memory gracefully.
  3. We need to serialize access to that copied string. Meaning mutex or shared_ptr atomic (the latter is not very nice).

All in all, potential speed gain didn't justify code complexity for me and I discarded it as premature optimization.