When the output device is changed, update its volume slider.

The label above the slider is set to the name of the new output device
and the slider's value is set to its volume.

Also,
 - clean up some code in BGMAudioDeviceManager and
   BGMOutputVolumeMenuItem, and
 - return from BGMAppDelegate::applicationDidFinishLaunching early if
   the launch is being aborted.

BGMApp/BGMApp/BGMAppDelegate.mm

@@ -141,7 +141,9 @@ static float const kStatusBarIconPadding = 0.25;
 
     // Set up audioDevices, which coordinates BGMDevice and the output device. It manages
     // playthrough, volume/mute controls, etc.
-    [self initAudioDeviceManager];
+    if (![self initAudioDeviceManager]) {
+        return;
+    }
 
     // Handle some of the unusual reasons BGMApp might have to exit, mostly crashes.
     BGMTermination::SetUpTerminationCleanUp(audioDevices);
@@ -176,6 +178,7 @@ static float const kStatusBarIconPadding = 0.25;
                                                          view:self.outputVolumeView
                                                        slider:self.outputVolumeSlider
                                                   deviceLabel:self.outputVolumeLabel];
+    [audioDevices setOutputVolumeMenuItem:outputVolume];
 
     // Add it to the main menu below the "Volumes" heading.
     [self.bgmMenu insertItem:outputVolume
@@ -202,14 +205,14 @@ static float const kStatusBarIconPadding = 0.25;
     return [[BGMUserDefaults alloc] initWithDefaults:wrappedDefaults];
 }
 
-- (void) initAudioDeviceManager {
+// Returns NO if (and only if) BGMApp is about to terminate because of a fatal error.
+- (BOOL) initAudioDeviceManager {
     NSError* error;
-
     audioDevices = [[BGMAudioDeviceManager alloc] initWithError:&error];
 
-    if (audioDevices == nil) {
+    if (!audioDevices) {
         [self showDeviceNotFoundErrorMessageAndExit:error.code];
-        return;
+        return NO;
     }
 
     error = [audioDevices setBGMDeviceAsOSDefault];
@@ -220,6 +223,8 @@ static float const kStatusBarIconPadding = 0.25;
                                            "default audio device."
                           informativeText:@"You might be able to set it yourself."];
     }
+
+    return YES;
 }
 
 - (void) applicationWillTerminate:(NSNotification*)aNotification {
@@ -255,6 +260,8 @@ static float const kStatusBarIconPadding = 0.25;
             [alert setInformativeText:@"If you do have one installed, this is probably a bug. Sorry about that. Feel free to file an issue on GitHub."];
         }
 
+        // This crashes if built with Xcode 9.0.1, but works with versions of Xcode before 9 and
+        // with 9.1.
         [alert runModal];
         [NSApp terminate:self];
     });

BGMApp/BGMApp/BGMAudioDeviceManager.h

@@ -37,6 +37,9 @@
 #import <Foundation/Foundation.h>
 #import <CoreAudio/AudioHardwareBase.h>
 
+// Forward Declarations
+@class BGMOutputVolumeMenuItem;
+
 
 #pragma clang assume_nonnull begin
 
@@ -48,6 +51,9 @@ static const int kBGMErrorCode_ReturningEarly       = 3;
 
 - (instancetype) initWithError:(NSError**)error;
 
+// Set the BGMOutputVolumeMenuItem to be notified when the output device is changed.
+- (void) setOutputVolumeMenuItem:(BGMOutputVolumeMenuItem*)item;
+
 // Set BGMDevice as the default audio device for all processes
 - (NSError* __nullable) setBGMDeviceAsOSDefault;
 // Replace BGMDevice as the default device with the output device

BGMApp/BGMApp/BGMAudioDeviceManager.mm

@@ -24,16 +24,17 @@
 #import "BGMAudioDeviceManager.h"
 
 // Local Includes
-#include "BGM_Types.h"
+#import "BGM_Types.h"
-#include "BGM_Utils.h"
+#import "BGM_Utils.h"
-#include "BGMDeviceControlSync.h"
+#import "BGMDeviceControlSync.h"
-#include "BGMPlayThrough.h"
+#import "BGMPlayThrough.h"
-#include "BGMAudioDevice.h"
+#import "BGMAudioDevice.h"
-#include "BGMXPCProtocols.h"
+#import "BGMXPCProtocols.h"
+#import "BGMOutputVolumeMenuItem.h"
 
 // PublicUtility Includes
-#include "CAHALAudioSystemObject.h"
+#import "CAHALAudioSystemObject.h"
-#include "CAAutoDisposer.h"
+#import "CAAutoDisposer.h"
 
 
 #pragma clang assume_nonnull begin
@@ -49,6 +50,8 @@
     // A connection to BGMXPCHelper so we can send it the ID of the output device.
     NSXPCConnection* __nullable bgmXPCHelperConnection;
 
+    BGMOutputVolumeMenuItem* __nullable outputVolumeMenuItem;
+
     NSRecursiveLock* stateLock;
 }
 
@@ -58,6 +61,7 @@
     if ((self = [super init])) {
         stateLock = [NSRecursiveLock new];
         bgmXPCHelperConnection = nil;
+        outputVolumeMenuItem = nil;
 
         try {
             bgmDevice = BGMBackgroundMusicDevice();
@@ -175,6 +179,10 @@
     }
 }
 
+- (void) setOutputVolumeMenuItem:(BGMOutputVolumeMenuItem*)item {
+    outputVolumeMenuItem = item;
+}
+
 #pragma mark Systemwide Default Device
 
 // Note that there are two different "default" output devices on OS X: "output" and "system output". See
@@ -309,9 +317,6 @@ forAppWithProcessID:(pid_t)processID
     
     AudioDeviceID currentDeviceID = outputDevice.GetObjectID();  // (GetObjectID doesn't throw.)
 
-    // Set up playthrough and control sync
-    BGMAudioDevice newOutputDevice(newDeviceID);
-    
     @try {
         [stateLock lock];
         
@@ -321,25 +326,8 @@ forAppWithProcessID:(pid_t)processID
             currentDeviceID = outputDevice.GetObjectID();
             
             if (newDeviceID != currentDeviceID) {
-                // Deactivate playthrough rather than stopping it so it can't be started by HAL
+                BGMAudioDevice newOutputDevice(newDeviceID);
-                // notifications while we're updating deviceControlSync.
+                [self setOutputDeviceForPlaythroughAndControlSync:newOutputDevice];
-                playThrough.Deactivate();
-                playThrough_UISounds.Deactivate();
-
-                deviceControlSync.SetDevices(bgmDevice, newOutputDevice);
-                deviceControlSync.Activate();
-
-                // Stream audio from BGMDevice to the new output device. This blocks while the old device
-                // stops IO.
-                playThrough.SetDevices(&bgmDevice, &newOutputDevice);
-                playThrough.Activate();
-
-                // TODO: Support setting different devices as the default output device and the
-                //       default system output device, the way OS X does.
-                BGMAudioDevice uiSoundsDevice = bgmDevice.GetUISoundsBGMDeviceInstance();
-                playThrough_UISounds.SetDevices(&uiSoundsDevice, &newOutputDevice);
-                playThrough_UISounds.Activate();
-
                 outputDevice = newOutputDevice;
             }
             
@@ -373,8 +361,7 @@ forAppWithProcessID:(pid_t)processID
                                         revertTo:(revertOnFailure ? &currentDeviceID : nullptr)];
         }
 
-        // Tell BGMXPCHelper about the new output device.
+        [self propagateOutputDeviceChange];
-        [self sendOutputDeviceToBGMXPCHelper];
     } @finally {
         [stateLock unlock];
     }
@@ -382,7 +369,30 @@ forAppWithProcessID:(pid_t)processID
     return nil;
 }
 
-- (void) setDataSource:(UInt32)dataSourceID device:(BGMAudioDevice)device {
+// Changes the output device that playthrough plays audio to and that BGMDevice's controls are
+// kept in sync with. Throws CAException.
+- (void) setOutputDeviceForPlaythroughAndControlSync:(const BGMAudioDevice&)newOutputDevice {
+    // Deactivate playthrough rather than stopping it so it can't be started by HAL notifications
+    // while we're updating deviceControlSync.
+    playThrough.Deactivate();
+    playThrough_UISounds.Deactivate();
+
+    deviceControlSync.SetDevices(bgmDevice, newOutputDevice);
+    deviceControlSync.Activate();
+
+    // Stream audio from BGMDevice to the new output device. This blocks while the old device stops
+    // IO.
+    playThrough.SetDevices(&bgmDevice, &newOutputDevice);
+    playThrough.Activate();
+
+    // TODO: Support setting different devices as the default output device and the default system
+    //       output device the way OS X does?
+    BGMAudioDevice uiSoundsDevice = bgmDevice.GetUISoundsBGMDeviceInstance();
+    playThrough_UISounds.SetDevices(&uiSoundsDevice, &newOutputDevice);
+    playThrough_UISounds.Activate();
+}
+
+- (void) setDataSource:(UInt32)dataSourceID device:(BGMAudioDevice&)device {
     BGMLogAndSwallowExceptions("BGMAudioDeviceManager::setDataSource", [&] {
         AudioObjectPropertyScope scope = kAudioObjectPropertyScopeOutput;
         UInt32 channel = 0;
@@ -396,6 +406,14 @@ forAppWithProcessID:(pid_t)processID
     });
 }
 
+- (void) propagateOutputDeviceChange {
+    // Tell BGMXPCHelper that the output device has changed.
+    [self sendOutputDeviceToBGMXPCHelper];
+
+    // Update the menu item for the volume of the output device.
+    [outputVolumeMenuItem outputDeviceDidChange];
+}
+
 - (NSError*) failedToSetOutputDevice:(AudioDeviceID)deviceID
                            errorCode:(OSStatus)errorCode
                             revertTo:(AudioDeviceID*)revertTo {

BGMApp/BGMApp/BGMOutputVolumeMenuItem.h

@@ -38,5 +38,7 @@
                                slider:(NSSlider*)slider
                           deviceLabel:(NSTextField*)label;
 
+- (void) outputDeviceDidChange;
+
 @end
 

BGMApp/BGMApp/BGMOutputVolumeMenuItem.mm

@@ -35,13 +35,14 @@
 #import <CoreAudio/AudioHardware.h>
 
 
-const float                    SLIDER_EPSILON = 1e-10f;
+const float                    kSliderEpsilon           = 1e-10f;
-const AudioObjectPropertyScope SCOPE          = kAudioDevicePropertyScopeOutput;
+const AudioObjectPropertyScope kScope                   = kAudioDevicePropertyScopeOutput;
-const UInt32                   CHANNEL        = kMasterChannel;
+NSString* const __nonnull      kGenericOutputDeviceName = @"Output Device";
 
 @implementation BGMOutputVolumeMenuItem {
     BGMAudioDeviceManager* audioDevices;
     NSTextField* outputVolumeLabel;
+    NSSlider* volumeSlider;
 }
 
 // TODO: Update the UI when the output device is changed.
@@ -59,32 +60,30 @@ const UInt32                   CHANNEL        = kMasterChannel;
     if ((self = [super initWithTitle:@"" action:nil keyEquivalent:@""])) {
         audioDevices = devices;
         outputVolumeLabel = label;
+        volumeSlider = slider;
 
         // Apply our custom view from MainMenu.xib.
         self.view = view;
 
-        try {
+        [self initSlider];
-            [self initSlider:slider];
+        [self setOutputVolumeLabel];
-            [self setOutputVolumeLabel];
-        } catch (const CAException& e) {
-            NSLog(@"BGMOutputVolumeMenuItem::initWithBGMMenu: Exception: %d", e.GetError());
-        }
     }
 
     return self;
 }
 
-- (void) initSlider:(NSSlider*)slider {
+- (void) initSlider {
     BGMAssert([NSThread isMainThread],
               "initSlider must be called from the main thread because it calls UI functions.");
 
-    slider.target = self;
+    volumeSlider.target = self;
-    slider.action = @selector(sliderChanged:);
+    volumeSlider.action = @selector(sliderChanged:);
 
-    BGMAudioDevice bgmDevice = [audioDevices bgmDevice];
+    // Initialise the slider.
+    [self updateVolumeSlider];
 
-    // This block updates the value of the output volume slider. Note that it can only run on the
+    // Register a listener that will update the slider when the user changes the volume or
-    // main thread/queue because it calls UI functions
+    // mutes/unmutes their audio.
     AudioObjectPropertyListenerBlock updateSlider =
         ^(UInt32 inNumberAddresses, const AudioObjectPropertyAddress* inAddresses) {
             // The docs for AudioObjectPropertyListenerBlock say inAddresses will always contain
@@ -92,54 +91,102 @@ const UInt32                   CHANNEL        = kMasterChannel;
             // inAddresses.
             #pragma unused (inNumberAddresses, inAddresses)
 
-            try {
+            dispatch_async(dispatch_get_main_queue(), ^{
-                if (bgmDevice.GetMuteControlValue(SCOPE, kMasterChannel)) {
+                [self updateVolumeSlider];
-                    // The output device is muted, so show the volume as 0 on the slider.
+            });
-                    slider.doubleValue = 0.0;
-                } else {
-                    // The slider values and volume values are both from 0 to 1, so we can use the
-                    // volume as is.
-                    slider.doubleValue =
-                        bgmDevice.GetVolumeControlScalarValue(SCOPE, kMasterChannel);
-                }
-            } catch (const CAException& e) {
-                NSLog(@"BGMOutputVolumeMenuItem::initSlider: Failed to update slider. (%d)",
-                      e.GetError());
-            }
         };
 
-    // Initialise the slider. (The args are ignored.)
+    // Instead of swallowing exceptions, we could try again later, but I doubt it would be worth the
-    updateSlider(0, {});
+    // effort. And the documentation doesn't actually explain what could cause this to fail.
+    BGMLogAndSwallowExceptions("BGMOutputVolumeMenuItem::initSlider", ([&] {
+        // Register the listener to receive volume notifications.
+        audioDevices.bgmDevice.AddPropertyListenerBlock(
+            CAPropertyAddress(kAudioDevicePropertyVolumeScalar, kScope),
+            dispatch_get_main_queue(),
+            updateSlider);
 
-    // Register a listener that will update the slider when the user changes the volume from
+        // Register the same listener for mute/unmute notifications.
-    // somewhere else.
+        audioDevices.bgmDevice.AddPropertyListenerBlock(
-    audioDevices.bgmDevice.AddPropertyListenerBlock(
+            CAPropertyAddress(kAudioDevicePropertyMute, kScope),
-        CAPropertyAddress(kAudioDevicePropertyVolumeScalar, SCOPE),
+            dispatch_get_main_queue(),
-        dispatch_get_main_queue(),
+            updateSlider);
-        updateSlider);
+    }));
-
-    // Register the same listener for mute/unmute.
-    audioDevices.bgmDevice.AddPropertyListenerBlock(
-        CAPropertyAddress(kAudioDevicePropertyMute, SCOPE),
-        dispatch_get_main_queue(),
-        updateSlider);
 }
 
-// Sets the label to the name of the output device.
+// Updates the value of the output volume slider. Should only be called on the main thread because
+// it calls UI functions.
+- (void) updateVolumeSlider {
+    BGMAssert([[NSThread currentThread] isMainThread], "updateVolumeSlider on non-main thread.");
+
+    BGMAudioDevice bgmDevice = [audioDevices bgmDevice];
+
+    // BGMDevice should never return an error for these calls, so we just swallow any exceptions and
+    // give up. (That said, we do check mute last so that, if it did throw, it wouldn't affect the
+    // more important calls.)
+    BGMLogAndSwallowExceptions("BGMOutputVolumeMenuItem::updateVolumeSlider", ([&] {
+        BOOL hasVolume = bgmDevice.HasSettableMasterVolume(kScope);
+
+        // If the device doesn't have a master volume control, we disable the slider and set it to
+        // full (or to zero, if muted).
+        volumeSlider.enabled = hasVolume;
+
+        if (hasVolume) {
+            // Set the slider to the current output volume. The slider values and volume values are
+            // both from 0 to 1, so we can use the volume as is.
+            volumeSlider.doubleValue =
+                bgmDevice.GetVolumeControlScalarValue(kScope, kMasterChannel);
+        } else {
+            volumeSlider.doubleValue = 1.0;
+        }
+
+        // Set the slider to zero if the device is muted.
+        if (bgmDevice.HasSettableMasterMute(kScope) &&
+            bgmDevice.GetMuteControlValue(kScope, kMasterChannel)) {
+            volumeSlider.doubleValue = 0.0;
+        }
+    }));
+};
+
+- (void) outputDeviceDidChange {
+    dispatch_async(dispatch_get_main_queue(), ^{
+        // Update the label to use the name of the new output device.
+        [self setOutputVolumeLabel];
+        // Set the slider to the volume of the new device.
+        [self updateVolumeSlider];
+    });
+}
+
+// Sets the label to the name of the output device. Falls back to a generic name if the device
+// returns an error when queried.
 - (void) setOutputVolumeLabel {
     BGMAudioDevice device = audioDevices.outputDevice;
+    BOOL didSetLabel = NO;
 
-    if (device.HasDataSourceControl(SCOPE, CHANNEL)) {
+    try {
-        UInt32 dataSourceID = device.GetCurrentDataSourceID(SCOPE, CHANNEL);
+        if (device.HasDataSourceControl(kScope, kMasterChannel)) {
+            // The device has datasources, so use the current datasource's name like macOS does.
+            UInt32 dataSourceID = device.GetCurrentDataSourceID(kScope, kMasterChannel);
 
-        outputVolumeLabel.stringValue =
+            outputVolumeLabel.stringValue =
-            (__bridge_transfer NSString*)device.CopyDataSourceNameForID(SCOPE,
+                (__bridge_transfer NSString*)device.CopyDataSourceNameForID(kScope,
-                                                                        CHANNEL,
+                                                                            kMasterChannel,
-                                                                        dataSourceID);
+                                                                            dataSourceID);
+            didSetLabel = YES;  // So we know not to change the text if setting the tooltip fails.
 
-        outputVolumeLabel.toolTip = (__bridge_transfer NSString*)device.CopyName();
+            outputVolumeLabel.toolTip = (__bridge_transfer NSString*)device.CopyName();
-    } else {
+        } else {
-        outputVolumeLabel.stringValue = (__bridge_transfer NSString*)device.CopyName();
+            outputVolumeLabel.stringValue = (__bridge_transfer NSString*)device.CopyName();
+        }
+    } catch (const CAException& e) {
+        BGMLogException(e);
+
+        // The device returned an error, so set the label to a generic device name, since we don't
+        // want to leave it set to the previous device's name.
+        outputVolumeLabel.toolTip = nil;
+
+        if (!didSetLabel) {
+            outputVolumeLabel.stringValue = kGenericOutputDeviceName;
+        }
     }
 
     // Take the label out of the accessibility hierarchy, which also moves the slider up a level.
@@ -163,13 +210,15 @@ const UInt32                   CHANNEL        = kMasterChannel;
     try {
         // The slider values and volume values are both from 0.0f to 1.0f, so we can use the slider
         // value as is.
-        audioDevices.bgmDevice.SetVolumeControlScalarValue(SCOPE, CHANNEL, newValue);
+        audioDevices.bgmDevice.SetVolumeControlScalarValue(kScope, kMasterChannel, newValue);
 
         // Mute BGMDevice if they set the slider to zero, and unmute it for non-zero. Muting makes
         // sure the audio doesn't play very quietly instead being completely silent. This matches
         // the behaviour of the Volume menu built-in to macOS.
-        if (audioDevices.bgmDevice.HasMuteControl(SCOPE, CHANNEL)) {
+        if (audioDevices.bgmDevice.HasMuteControl(kScope, kMasterChannel)) {
-            audioDevices.bgmDevice.SetMuteControlValue(SCOPE, CHANNEL, (newValue < SLIDER_EPSILON));
+            audioDevices.bgmDevice.SetMuteControlValue(kScope,
+                                                       kMasterChannel,
+                                                       (newValue < kSliderEpsilon));
         }
     } catch (const CAException& e) {
         NSLog(@"BGMOutputVolumeMenuItem::sliderChanged: Failed to set volume (%d)", e.GetError());

BGMApp/BGMApp/BGMPlayThrough.cpp

@@ -373,8 +373,8 @@ bool    BGMPlayThrough::CheckIOProcsAreStopped() const noexcept
     return statesOK;
 }
 
-void    BGMPlayThrough::SetDevices(BGMAudioDevice* __nullable inInputDevice,
+void    BGMPlayThrough::SetDevices(const BGMAudioDevice* __nullable inInputDevice,
-                                   BGMAudioDevice* __nullable inOutputDevice)
+                                   const BGMAudioDevice* __nullable inOutputDevice)
 {
     CAMutex::Locker stateLocker(mStateMutex);
     

BGMApp/BGMApp/BGMPlayThrough.h

@@ -104,8 +104,8 @@ public:
      Pass null for either param to only change one of the devices.
      @throws CAException
      */
-    void                SetDevices(BGMAudioDevice* __nullable inInputDevice,
+    void                SetDevices(const BGMAudioDevice* __nullable inInputDevice,
-                                   BGMAudioDevice* __nullable inOutputDevice);
+                                   const BGMAudioDevice* __nullable inOutputDevice);
     
     /*! @throws CAException */
     void                Start();