Skip to content

Conversation

@abdulkadiryugruk
Copy link
Contributor

This PR addresses critical volume control issues and crashes, specifically tested on macOS Tahoe 26.0.1.

Environment & Context:

  • OS: macOS Tahoe 26.0.1
  • Status Before Fix: System sounds, general computer audio, and Google Chrome were working correctly. However, volume control was completely non-functional for Discord and Brave Browser.
  • Fix Verification: This patch successfully restores volume control for Discord (by targeting the correct renderer process) and resolves the more complex broadcasting issues required for Brave Browser.

Technical Changes:

  1. Process Broadcasting: Added a mechanism to broadcast volume changes to all related processes (fixing Brave/Discord helpers not responding to the main app slider).
  2. Crash Protection: - Fixed a crash caused by passing nullptr as BundleID to the driver (replaced with a safe placeholder).
    • Added try-catch blocks and safe memory management around process iteration to prevent app crashes during volume adjustments.
  3. UI Cleanup: Improved filtering to show only the main "Discord" and "Brave Browser" apps in the list, hiding unnecessary "Helper/GPU" processes while maintaining control over them.

Fix Discord/Brave volume control & crashes on macOS Tahoe
Copy link

@LoackyBit LoackyBit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe the changes are too specific

@abdulkadiryugruk
Copy link
Contributor Author

@LoackyBit Thanks for the feedback!

Actually, that was exactly my observation during testing. Standard apps and even some Electron apps like Google Chrome and Spotify work perfectly out of the box with the current system.

However, I noticed that Discord and Brave specifically were completely unresponsive due to their non-standard process naming structure. Since these are very popular applications, I thought addressing them explicitly would be a significant quality-of-life improvement for many users, even if it's specific.

Also, considering this PR fixes a critical crash (nullptr BundleID) that affects the stability of the whole app, I believe it's worth merging these specific fixes alongside the crash protection.

@kyleneideck
Copy link
Owner

Thanks for the PR. This looks useful. I'll try to test it and clean it up a bit today so we can get it merged. Or I can add review comments if you'd prefer to do that clean up yourself.

@abdulkadiryugruk
Copy link
Contributor Author

Hi Kyle, thanks for the quick response!

I'm glad to hear that you found the PR useful. Since you are more familiar with the project's coding style and best practices, it would be great if you could handle the clean-up/refactoring. I'd be happy to leave that to you to ensure it fits perfectly with the rest of the codebase.

Thanks again for maintaining this awesome project!

@kyleneideck
Copy link
Owner

When I tested it, the helper processes Discord and Brave used to play their audio both/all had bundle IDs, so we can just add them to the map of helper bundle IDs in BGMBackgroundMusicDevice::ResponsibleBundleIDsOf. That is, unless Discord or Brave don't always use those helpers when they play audio. I didn't test making a voice call in Discord, for example. I only played a video.

Can you test again with my new changes and see if the Discord and Brave volumes still always work for you? If they don't always work, let me know what you did in Discord/Brave to get it to play the audio that bypassed the volume control, and I'll try to reproduce it.

Also, considering this PR fixes a critical crash (nullptr BundleID) that affects the stability of the whole app, I believe it's worth merging these specific fixes alongside the crash protection.

Can you check and see if you can still reproduce that crash with the current code (on the master branch of either your fork or mine)? Does it happen when you change the volume of an app/process that doesn't have a bundle ID? No worries if you can't reproduce it, but if you can, I'd like to try reproducing it myself.

@kyleneideck
Copy link
Owner

Also, I think you can ignore the failing PR checks. It's an existing issue.

@abdulkadiryugruk
Copy link
Contributor Author

Hi Kyle, thanks for looking into this!

I want to clarify one specific detail regarding your test to make sure we are on the same page:

1. Playback vs. Volume Control:
The issue was NOT about audio playback. In my tests, audio was always transmitted/heard correctly for all apps.
The issue was specifically Volume Control.

  • The Bug: When I moved the main slider for Discord or Brave, the volume level did not change. It remained at 100% (or whatever the app's internal volume was).
  • The Test: When you test, please ensure you move the BGM slider and confirm if the audio level actually changes.

2. The "Helper" Behavior (Discord vs. Brave):

  • Discord: In the "More Apps" list, I found that the specific helper process (labeled Renderer) DID control the volume correctly if I adjusted it manually. However, the main "Discord" slider wasn't linked to it.
  • Brave Browser: This was different. Even the helper process shown in "More Apps" DID NOT control the volume in my tests.
  • Why Broadcasting?: Since Brave's helper wasn't responsive and Discord required finding the specific "Renderer" process dynamically, I implemented the "broadcast" loop as a catch-all solution.

3. The Crash (Reproduction):
The crash happens specifically when my loop tries to SetAppVolume on a process that does not have a Bundle ID (passing nullptr to the driver). On macOS Tahoe, iterating through all processes inevitably hits some system processes without Bundle IDs. Without the try-catch block and the safe placeholder ID I added, this crashes the app instantly.

4. Current Status with this PR:
Just to confirm: With this fix applied, I am currently able to control the volume levels of all applications (Chrome, Discord, Brave, System) seamlessly directly from the Main Apps list. The UI behaves exactly as intended.

Conclusion:
Adding IDs to ResponsibleBundleIDsOf will likely fix Discord cleanly. However, for Brave, since the visible helper wasn't working for me, I'm curious if the ID mapping will be enough. I’d be happy to test your implementation if you push it! But for now, the try-catch in my PR is what stops the crashes on my end.

BGMApp was setting the `kAudioDeviceCustomPropertyAppVolumes` property
with only a process ID in the CFDictionary when the app had no bundle
ID. BGMDriver would then cause a null deref in
`BGM_ClientMap::GetClients`.

See kyleneideck#839.
@kyleneideck
Copy link
Owner

@abdulkadiryugruk My changes are already on your fork (master branch). You should be able to get them by running git pull.

abdulkadiryugruk@d17ad6a
abdulkadiryugruk@1989b78

  1. The Crash (Reproduction):
    The crash happens specifically when my loop tries to SetAppVolume on a process that does not have a Bundle ID (passing nullptr to the driver). On macOS Tahoe, iterating through all processes inevitably hits some system processes without Bundle IDs. Without the try-catch block and the safe placeholder ID I added, this crashes the app instantly.

Thanks, I've reproduced it locally by hardcoding BGMApp to set the property without including a bundle ID. I think what was happening was BGMApp was setting the kAudioDeviceCustomPropertyAppVolumes property with only a process ID in the CFDictionary, no bundle ID. Some code in BGMDriver assumed a bundle ID would be included and crashed here: https://github.com/kyleneideck/BackgroundMusic/blob/master/BGMDriver/BGMDriver/DeviceClients/BGM_ClientMap.cpp#L255. I've just pushed a fix to this PR.

Let me know if Discord and Brave both still work for you after you update and we should be able to merge the PR.

@abdulkadiryugruk
Copy link
Contributor Author

Hi Kyle, I've updated to your latest changes and tested thoroughly.

Great news:

  1. No Crashes: The app is extremely stable. I tried to trigger the crash by interacting with various helper processes and system apps, and it's rock solid.
  2. Main Sliders Work: I can control "Discord" and "Brave Browser" volume directly from the main list without any issues.
  3. PWA Support: I also verified that PWAs (like YouTube Music installed via Brave) are correctly controlled through the main Brave slider.

Everything looks perfect on my end. I think we are ready to merge! 🚀

A Personal Note:
I also want to thank you for building such a fantastic application. It’s been an honor to communicate with you, and I'm truly proud to have contributed a small part to this project.

Thanks again for the quick and solid fix.

@kyleneideck kyleneideck merged commit bcaef2c into kyleneideck:master Dec 29, 2025
4 checks passed
@kyleneideck
Copy link
Owner

Great, thanks for testing all that. I've merged the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants