Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions BGMApp/BGMApp/BGMAppDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,18 @@ - (void) initVolumesMenuSection {

- (void) applicationWillTerminate:(NSNotification*)aNotification {
#pragma unused (aNotification)

DebugMsg("BGMAppDelegate::applicationWillTerminate");

// Deactivate control sync and playthrough before changing the default device back. This
// prevents the default device change from triggering volume sync listeners that could leave
// the output device at the wrong volume. See
// https://github.com/kyleneideck/BackgroundMusic/issues/841
[audioDevices prepareForTermination];

// Change the user's default output device back.
NSError* error = [audioDevices unsetBGMDeviceAsOSDefault];

if (error) {
[self showSetDeviceAsDefaultError:error
message:@"Failed to reset your system's audio output device."
Expand Down
2 changes: 0 additions & 2 deletions BGMApp/BGMApp/BGMAppVolumesController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
// System Includes
#include <libproc.h>


#pragma clang assume_nonnull begin

@implementation BGMAppVolumesController {
Expand Down Expand Up @@ -256,4 +255,3 @@ - (void) observeValueForKeyPath:(NSString* __nullable)keyPath
@end

#pragma clang assume_nonnull end

33 changes: 32 additions & 1 deletion BGMApp/BGMApp/BGMAudioDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@ bool BGMAudioDevice::CanBeOutputDeviceInBGMApp() const
canBeDefault;
}

#pragma mark Device Type

bool BGMAudioDevice::IsAggregate() const
{
try
{
return GetTransportType() == kAudioDeviceTransportTypeAggregate;
}
catch(CAException)
{
return false;
}
}

#pragma mark Available Controls

bool BGMAudioDevice::HasSettableMasterVolume(AudioObjectPropertyScope inScope) const
Expand Down Expand Up @@ -115,6 +129,12 @@ bool BGMAudioDevice::HasSettableMasterMute(AudioObjectPropertyScope inScope)
void BGMAudioDevice::CopyMuteFrom(const BGMAudioDevice inDevice,
AudioObjectPropertyScope inScope)
{
// Skip aggregate devices for the same reason as in CopyVolumeFrom.
if(IsAggregate() || inDevice.IsAggregate())
{
return;
}

// TODO: Support for devices that have per-channel mute controls but no master mute control
if(HasSettableMasterMute(inScope) && inDevice.HasMuteControl(inScope, kMasterChannel))
{
Expand All @@ -127,6 +147,17 @@ void BGMAudioDevice::CopyMuteFrom(const BGMAudioDevice inDevice,
void BGMAudioDevice::CopyVolumeFrom(const BGMAudioDevice inDevice,
AudioObjectPropertyScope inScope)
{
// Don't try to copy volume to/from aggregate devices. Aggregate devices rely on macOS to
// manage their virtual volume control through the sub-devices. Using the deprecated
// AudioHardwareService APIs (which we use for virtual master volume) to write to an aggregate
// device can corrupt its volume control state, causing the volume slider to become permanently
// disabled in System Settings. See https://github.com/kyleneideck/BackgroundMusic/issues/848
if(IsAggregate() || inDevice.IsAggregate())
{
DebugMsg("BGMAudioDevice::CopyVolumeFrom: Skipping volume copy for aggregate device");
return;
}

// Get the volume of the other device.
bool didGetVolume = false;
Float32 volume = FLT_MIN;
Expand Down Expand Up @@ -155,7 +186,7 @@ void BGMAudioDevice::CopyVolumeFrom(const BGMAudioDevice inDevice,

if(numChannels > 0) // Avoid divide by zero.
{
volume /= numChannels;
volume /= static_cast<Float32>(numChannels);
}
}

Expand Down
8 changes: 8 additions & 0 deletions BGMApp/BGMApp/BGMAudioDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ class BGMAudioDevice
*/
bool CanBeOutputDeviceInBGMApp() const;

#pragma mark Device Type

/*!
@return True if this device is an aggregate device (i.e. a device created in Audio MIDI Setup
that combines multiple audio devices).
*/
bool IsAggregate() const;

#pragma mark Available Controls

bool HasSettableMasterVolume(AudioObjectPropertyScope inScope) const;
Expand Down
5 changes: 5 additions & 0 deletions BGMApp/BGMApp/BGMAudioDeviceManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ static const int kBGMErrorCode_ReturningEarly = 2;
// Replace BGMDevice as the default device with the output device
- (NSError* __nullable) unsetBGMDeviceAsOSDefault;

// Prepare for app termination by deactivating control sync and playthrough before the default
// device is changed back. This prevents stale volume state from being left on the output device.
// Must be called before unsetBGMDeviceAsOSDefault.
- (void) prepareForTermination;

#ifdef __cplusplus
// The virtual device published by BGMDriver.
- (BGMBackgroundMusicDevice) bgmDevice;
Expand Down
27 changes: 27 additions & 0 deletions BGMApp/BGMApp/BGMAudioDeviceManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,33 @@ - (NSError* __nullable) setBGMDeviceAsOSDefault {
return nil;
}

- (void) prepareForTermination {
DebugMsg("BGMAudioDeviceManager::prepareForTermination: Deactivating control sync and "
"playthrough before termination");

@try {
[stateLock lock];

// Deactivate control sync BEFORE changing the default device. This removes the volume/mute
// listeners so that the default device change doesn't trigger any stale volume sync that
// could leave the output device at the wrong volume.
// See https://github.com/kyleneideck/BackgroundMusic/issues/841
BGMLogAndSwallowExceptions("BGMAudioDeviceManager::prepareForTermination", [&] {
deviceControlSync.Deactivate();
});

// Stop playthrough so we don't try to pass audio during shutdown.
BGMLogAndSwallowExceptions("BGMAudioDeviceManager::prepareForTermination", [&] {
playThrough.Deactivate();
});
BGMLogAndSwallowExceptions("BGMAudioDeviceManager::prepareForTermination", [&] {
playThrough_UISounds.Deactivate();
});
} @finally {
[stateLock unlock];
}
}

- (NSError* __nullable) unsetBGMDeviceAsOSDefault {
// Copy the devices so we can call the HAL without holding stateLock. See startPlayThroughSync.
BGMBackgroundMusicDevice* bgmDeviceCopy;
Expand Down
2 changes: 1 addition & 1 deletion BGMApp/BGMApp/BGMAutoPauseMusic.mm
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ - (void) queueUnpauseBlock {
// TODO: Fading in and out would make short pauses a lot less jarring because, if they were short enough, we wouldn't
// actually pause the music player. So you'd hear a dip in the music's volume rather than a gap.
UInt64 unpauseDelayNsec =
static_cast<UInt64>((wentSilent - wentAudible) * kUnpauseDelayWeightingFactor);
static_cast<UInt64>(static_cast<Float64>(wentSilent - wentAudible) * kUnpauseDelayWeightingFactor);

// Convert from absolute time to nanos.
mach_timebase_info_data_t info;
Expand Down
34 changes: 28 additions & 6 deletions BGMApp/BGMApp/BGMBackgroundMusicDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,14 @@ BGMBackgroundMusicDevice::ResponsibleBundleIDsOf(CACFString inParentBundleID)
{ "com.apple.finder",
{ "com.apple.quicklook.ui.helper",
"com.apple.quicklook.QuickLookUIService" } },
// Safari
{ "com.apple.Safari", { "com.apple.WebKit.WebContent" } },
// Safari. Recent macOS versions can run normal tabs in either the classic WebContent
// service or the EnhancedSecurity variant, and Tahoe can also hand media playback off to
// the GPU helper (surfaced as “Safari Graphics and Media”). Safari's own app process
// doesn't emit the tab audio directly.
{ "com.apple.Safari",
{ "com.apple.WebKit.WebContent",
"com.apple.WebKit.WebContent.EnhancedSecurity",
"com.apple.WebKit.GPU" } },
// Firefox
{ "org.mozilla.firefox", { "org.mozilla.plugincontainer" } },
// Firefox Nightly
Expand All @@ -240,19 +246,36 @@ BGMBackgroundMusicDevice::ResponsibleBundleIDsOf(CACFString inParentBundleID)
{ "com.hnc.Discord",
{ "com.hnc.Discord.helper",
"com.hnc.Discord.helper.Renderer",
"com.hnc.Discord.helper.renderer",
"com.hnc.Discord.helper.plugin",
"com.hnc.Discord.helper.Plugin" } },
// Brave
{ "com.brave.Browser",
{ "com.brave.Browser.helper",
"com.brave.Browser.helper.renderer",
"com.brave.Browser.helper.plugin" } },
// NAVER Whale
{ "com.naver.Whale",
{ "com.naver.Whale.helper",
"com.naver.Whale.helper.renderer",
"com.naver.Whale.helper.plugin" } },
// Skype
{ "com.skype.skype", { "com.skype.skype.Helper" } },
// Google Chrome
{ "com.google.Chrome", { "com.google.Chrome.helper" } },
{ "com.google.Chrome",
{ "com.google.Chrome.helper",
"com.google.Chrome.helper.renderer",
"com.google.Chrome.helper.plugin" } },
// Microsoft Edge
{ "com.microsoft.edgemac", { "com.microsoft.edgemac.helper" } },
{ "com.microsoft.edgemac",
{ "com.microsoft.edgemac.helper",
"com.microsoft.edgemac.helper.renderer",
"com.microsoft.edgemac.helper.plugin" } },
// Arc
{ "company.thebrowser.Browser", { "company.thebrowser.browser.helper" } }
{ "company.thebrowser.Browser",
{ "company.thebrowser.browser.helper",
"company.thebrowser.browser.helper.renderer",
"company.thebrowser.browser.helper.plugin" } }
};

// Parallels' VM "dock helper" apps have bundle IDs like
Expand Down Expand Up @@ -331,4 +354,3 @@ CFStringRef BGMBackgroundMusicDevice::GetMusicPlayerBundleID() const
}

#pragma clang assume_nonnull end

16 changes: 14 additions & 2 deletions BGMApp/BGMApp/BGMDeviceControlsList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,21 @@ bool BGMDeviceControlsList::MatchControlsListOf(AudioObjectID inDeviceID)

// Check which controls the other device has.
BGMAudioDevice device(inDeviceID);
bool hasMute = device.HasSettableMasterMute(inScope);

bool hasVolume =
// Aggregate devices (created in Audio MIDI Setup) rely on macOS to provide virtual volume
// control through the sub-devices. The deprecated AudioHardwareService APIs we use to detect
// volume support can return incorrect results for aggregate devices on newer macOS versions,
// which causes us to incorrectly disable BGMDevice's volume control. This then triggers
// PropagateControlListChange (which toggles the default device through a null device), and
// that can permanently break the aggregate device's volume slider in System Settings.
//
// To avoid this, always report that aggregate devices have volume and mute controls, since
// macOS provides virtual volume/mute for them regardless of what the HAL reports.
bool isAggregate = device.IsAggregate();

bool hasMute = isAggregate || device.HasSettableMasterMute(inScope);

bool hasVolume = isAggregate ||
device.HasSettableMasterVolume(inScope) || device.HasSettableVirtualMasterVolume(inScope);

if(!hasVolume)
Expand Down
6 changes: 3 additions & 3 deletions BGMApp/BGMApp/BGMPlayThrough.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,8 @@ OSStatus BGMPlayThrough::WaitForOutputDeviceToStart() noexcept

DebugMsg("BGMPlayThrough::WaitForOutputDeviceToStart: Started %f ms after notification, %f "
"ms after entering WaitForOutputDeviceToStart.",
static_cast<Float64>(startedBy - mToldOutputDeviceToStartAt) * base / NSEC_PER_MSEC,
static_cast<Float64>(startedBy - startedAt) * base / NSEC_PER_MSEC);
static_cast<Float64>(startedBy - mToldOutputDeviceToStartAt) * static_cast<Float64>(base) / NSEC_PER_MSEC,
static_cast<Float64>(startedBy - startedAt) * static_cast<Float64>(base) / NSEC_PER_MSEC);
}

// Figure out which error code to return.
Expand Down Expand Up @@ -1121,7 +1121,7 @@ OSStatus BGMPlayThrough::OutputDeviceIOProc(AudioObjectID inDevice,
refCon->mInToOutSampleOffset);

// Recalculate the in-to-out offset and read head.
refCon->mInToOutSampleOffset = inOutputTime->mSampleTime - lastInputSampleTime;
refCon->mInToOutSampleOffset = inOutputTime->mSampleTime - static_cast<Float64>(lastInputSampleTime);
readHeadSampleTime = static_cast<CARingBuffer::SampleTime>(
inOutputTime->mSampleTime - refCon->mInToOutSampleOffset);
}
Expand Down
4 changes: 4 additions & 0 deletions BGMApp/BGMApp/BGMTermination.mm
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@
// it's better for things to work even if BGMXPCHelper isn't installed.
if(sAudioDevices)
{
// Deactivate control sync and playthrough before changing the default device back.
// This prevents the default device change from triggering volume sync that could leave
// the output device at the wrong volume.
[sAudioDevices prepareForTermination];
[sAudioDevices unsetBGMDeviceAsOSDefault];
}
}
Expand Down
23 changes: 23 additions & 0 deletions BGMApp/BGMApp/_uninstall-non-interactive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,29 @@ osascript -e 'tell application id "com.apple.finder"
|| rm -rf "${trash_dir}" \
|| true

# Clean up BGMDevice entries from macOS audio preferences to prevent persistent low volume
# after uninstall. See https://github.com/kyleneideck/BackgroundMusic/issues/841
echo "Cleaning up audio device preferences."
for plist_dir in "$HOME/Library/Preferences" "/Library/Preferences/Audio"; do
for plist in "com.apple.audio.DeviceSettings.plist" "com.apple.audio.SystemSettings.plist"; do
plist_path="${plist_dir}/${plist}"
if [ -f "${plist_path}" ]; then
# Remove entries keyed by BGMDevice UIDs
for uid in "BGMDevice" "BGMDevice_UISounds"; do
defaults delete "${plist_path}" "${uid}" &>/dev/null || true
done
fi
done
done
# Also clean up ByHost audio preferences
for byhost_plist in "$HOME/Library/Preferences/ByHost"/com.apple.audio.*.plist; do
if [ -f "${byhost_plist}" ]; then
for uid in "BGMDevice" "BGMDevice_UISounds"; do
defaults delete "${byhost_plist}" "${uid}" &>/dev/null || true
done
fi
done

echo "Restarting Core Audio."
# Wait a little because moving files to the trash plays a short sound.
sleep 2
Expand Down
55 changes: 53 additions & 2 deletions BGMApp/BGMAppTests/UnitTests/BGMMusicPlayersUnitTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
// Copyright © 2016-2020 Kyle Neideck
//

// Unit include
// Unit includes
#define private public
#import "BGMBackgroundMusicDevice.h"
#undef private
#import "BGMMusicPlayers.h"

// BGM includes
Expand All @@ -45,6 +48,23 @@
// CAHALAudioSystemObject, are also mocked. The unit tests are compiled with mock implementations:
// Mock_CAHALAudioObject.cpp and Mock_CAHALAudioSystemObject.cpp.

static NSArray<NSString*>* BGMResponsibleBundleIDs(NSString* bundleID) {
std::vector<CACFString> responsibleBundleIDs =
BGMBackgroundMusicDevice::ResponsibleBundleIDsOf(CACFString((__bridge CFStringRef)bundleID));

NSMutableArray<NSString*>* result =
[NSMutableArray arrayWithCapacity:(NSUInteger)responsibleBundleIDs.size()];

for (const CACFString& responsibleBundleID : responsibleBundleIDs) {
NSString* nsBundleID = (__bridge NSString*)responsibleBundleID.GetCFString();
if (nsBundleID) {
[result addObject:nsBundleID];
}
}

return result;
}

@interface BGMMockUserDefaults : BGMUserDefaults

@property NSUUID* selectedPlayerID;
Expand Down Expand Up @@ -211,7 +231,38 @@ - (void) testSelectedMusicPlayerInBGMDeviceProperties {
XCTAssertEqualObjects(players.selectedMusicPlayer.name, @"VLC");
}

- (void) testResponsibleBundleIDsOfSafariIncludeMediaHelperBundleIDs {
NSArray<NSString*>* responsibleBundleIDs = BGMResponsibleBundleIDs(@"com.apple.Safari");

XCTAssertTrue([responsibleBundleIDs containsObject:@"com.apple.WebKit.WebContent"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.apple.WebKit.WebContent.EnhancedSecurity"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.apple.WebKit.GPU"]);
}

- (void) testResponsibleBundleIDsOfChromeIncludeModernHelperBundleIDs {
NSArray<NSString*>* responsibleBundleIDs = BGMResponsibleBundleIDs(@"com.google.Chrome");

XCTAssertTrue([responsibleBundleIDs containsObject:@"com.google.Chrome.helper"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.google.Chrome.helper.renderer"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.google.Chrome.helper.plugin"]);
}

- (void) testResponsibleBundleIDsOfBraveIncludeModernHelperBundleIDs {
NSArray<NSString*>* responsibleBundleIDs = BGMResponsibleBundleIDs(@"com.brave.Browser");

XCTAssertTrue([responsibleBundleIDs containsObject:@"com.brave.Browser.helper"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.brave.Browser.helper.renderer"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.brave.Browser.helper.plugin"]);
}

- (void) testResponsibleBundleIDsOfWhaleIncludeModernHelperBundleIDs {
NSArray<NSString*>* responsibleBundleIDs = BGMResponsibleBundleIDs(@"com.naver.Whale");

XCTAssertTrue([responsibleBundleIDs containsObject:@"com.naver.Whale.helper"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.naver.Whale.helper.renderer"]);
XCTAssertTrue([responsibleBundleIDs containsObject:@"com.naver.Whale.helper.plugin"]);
}

// TODO: Test setting the selectedMusicPlayer property

@end

Loading