macOS: separate input and output audio device selection - #3908
macOS: separate input and output audio device selection#3908jshipman42 wants to merge 6 commits into
Conversation
LoadAndInitializeFirstValidDriver() returned the error messages of the drivers it tried as a plain list, and the caller paired each message with a device name by indexing strDriverNames[] at the same position. That couples the error reporting to the layout of the driver list. Return the device name together with its error message instead, so the caller no longer has to know how the driver list is organized. No change in behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012i8guRg33BV5Prf7tqkkcH
Sound APIs differ in how they present devices: ASIO offers one driver which covers both directions, while CoreAudio manages the input and the output device independently of each other. Add an optional interface for the latter, which a sound API can enable by returning true from IsInOutDevSelectionSeparate() and filling one device list per direction. The selected device is still identified by one single (combined) device name, so GetDev()/SetDev() and the stored settings stay unchanged. No sound API implements the interface yet, so there is no change in behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012i8guRg33BV5Prf7tqkkcH
CoreAudio manages the input and the output device independently, but the
device list was built as the cartesian product of both so that a single
combo box was sufficient in the GUI:
// we add combined entries for input and output for each device so that we
// do not need two combo boxes in the GUI for input and output (therefore
// all possible combinations are required which can be a large number)
The number of entries grows quadratically and the list is capped at
MAX_NUMBER_SOUND_CARDS, so on a system with 12 input and 15 output devices
the required 181 entries exceed the limit of 129 and the remaining
combinations are silently dropped. Building the list also queries every
device twice per combination, on each device change and dialog refresh.
Enumerate the devices per direction instead and offer one combo box for
the input and one for the output device. The selected device is still
identified by one single combined name, so previously stored settings
(including the "System Default In/Out Devices" entry) remain valid.
CheckDeviceCapabilities() is split into an input and an output part, which
also lets the search for the first usable device look for one device per
direction instead of trying all combinations.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012i8guRg33BV5Prf7tqkkcH
Selecting a device resets the channel mapping to the first two available channels, which was correct as long as the input and the output device were always selected together. Now that both can be selected on their own, only reset the mapping of the direction whose device actually changed, so that changing the output device no longer discards the input channel mapping (and vice versa). The device notifications are registered per direction as well, but keyed on the device ID rather than on the direction: one device can serve both directions and CoreAudio then keeps one single registration for it, so removing it for one direction would also silence the notifications of the other one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012i8guRg33BV5Prf7tqkkcH
The check whether the selected device changed compared the device names, but the "System Default" entry follows whatever macOS has configured, so the device behind it can change while its name stays the same. Reloading the driver then kept the previous device IDs, and since the IO callback compares the device it is called for against them, it would discard the audio of the device it had just been started on. Note that this only covers the paths which reload the driver. A change of the macOS default device itself still only triggers RS_ONLY_RESTART, which does not reload, so the system default entry does not follow the change until the driver is reloaded for another reason. Fixing that is a change to the notification handling and out of scope here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012i8guRg33BV5Prf7tqkkcH
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012i8guRg33BV5Prf7tqkkcH
|
Perfect. Thank you very much. It might take some time to review this though. Please disclose the use of Claude in the PR body. |
ann0see
left a comment
There was a problem hiding this comment.
It's a large PR diff though. So it might take a while. Do you think you could somehow logically split it otherwise into multiple PRs/not just commits? Higher chance to get it reviewed.
|
Great, thank you! This has been on my mental ToDo list for a long time. This week is busy, but I will try it out soon. |
I guess that would depend on whether it could sensibly be divided. If all the parts work together to achieve a single goal, they might not make sense separately. I haven't looked at the code yet, so I can't say whether that principle applies in this particular case, but I can imagine it might. |
|
Thanks both, and no rush on the review. @ann0see — disclosure added to the PR body. On splitting: I had a look at where the diff actually sits, and it is very concentrated.
Two things make the big one look larger than it is: the existing 208-line What I think can sensibly come out as its own PR is the first commit.
If you would like that separately, I will open it as its own PR and rebase this one on top — this PR then simply loses those 40 lines, with no rework needed. Worth weighing in the decision, though: it is a prerequisite rather than an independent cleanup, since this PR relies on it to report device errors correctly. The two would therefore have a merge-order dependency, in that this PR could not merge until the prerequisite had, and it would put two reviews in front of you rather than one. The rest I would keep together, and @softins's instinct about parts not making sense separately applies to both remaining candidates:
So — happy to pull the first commit out, happy to leave it as it stands, and happy to go further if you see a division I have missed. You know the review load better than I do, so I will follow whichever you prefer. |
|
🤖 AI: Runtime results from a macOS setup unlike yours: macOS 12.7.6, x86_64, Qt 5.15.2 — the combination the MacOS Legacy leg ships. The overflow reproduces, and this branch clears it. Ten aggregate devices via Across 11 stored-name cases on both builds, a settings file written by Query counts, by dyld interposition on |
Disclosure
This change was implemented with Claude (Claude Code); the commits carry
Co-Authored-By: Claude Opus 5trailers. I set the goal and the constraints, reviewed the result, and ran it against my own setup (12 input-capable and 15 output-capable CoreAudio devices) before opening this.Before submitting, the diff went through several independent review passes by both Claude and Codex, and their findings were addressed in the commits where they originated. Noting it for transparency, not as a substitute for your review.
Short description of changes
Fixes the incomplete audio device list on macOS reported in #3216, by enumerating the input and output devices separately instead of listing every combination of the two.
The macOS device list contains one entry per combination of input and output device (
in: <input>/out: <output>), which was a deliberate simplification so that a single combo box was enough:The number of entries grows quadratically while the list is capped at
MAX_NUMBER_SOUND_CARDS, so beyond roughly a dozen devices the enumeration stops early and the remaining combinations are silently absent from the combo box. This is the cause @softins identified in #3216:On my machine (12 input-capable and 15 output-capable CoreAudio devices) the combined list needs 181 entries against the limit of 129. Split per direction, the two lists hold 13 and 16.
Approach
This follows what was agreed in #3216 — split selection on macOS, unchanged single-device selection everywhere else — with one deviation. @ann0see suggested an
#ifdef:I made it a runtime capability instead:
CSoundBase::IsInOutDevSelectionSeparate()returnsfalseby default, CoreAudio overrides it totrue, and the settings dialog shows one form or the other accordingly. The GUI code stays single-source rather than split by platform, and iOS or a future WASAPI/portaudio backend (#821) can adopt the split by overriding one method. ASIO, JACK, oboe and iOS are untouched and keep the combined list and the single "Audio Device" combo box.This is also the "bigger refactoring with the sound card abstraction" @ann0see anticipated in #3216, kept as small as it can be: the base class gains six virtuals that all default to no-op, and one existing method changes its return type to report a device name with each driver error.
On
MAX_NUMBER_SOUND_CARDSIt stays at 129, and this PR removes the reason to raise it. @ann0see noted that bumping it "could have performance implications" — that concern is well founded, since building the combined list calls
GetAudioDeviceInfos()twice per combination (~360 CoreAudio property queries on my machine), on every device change and dialog refresh, and that cost grows quadratically. Enumerating per direction makes it linear: 27 queries for the same machine.The constant now bounds the devices per direction rather than the combinations, so it is no longer reachable in practice. Only its comment changes, because it documented the combination arithmetic (
// e.g. 16 inputs, 8 outputs + default entry).Compatibility
The selected device is still identified by one single combined name, so
auddev_base64keeps its format and previously stored values — including the legacySystem Default In/Out Devicesentry — load unchanged. In the other direction there is one new case: selecting the system default for only one of the two directions produces a name an older version cannot resolve, in which case it warns and falls back to a valid device, exactly as it does for a device that has been unplugged.Scope
Device selection only. The channel mapping, the
Audio Channelssetting and the mono/stereo handling proposed in the older discussion (#681, now https://github.com/orgs/jamulussoftware/discussions/1039) are deliberately not touched.Known limitation (pre-existing, not addressed here)
Selecting the system default device for a direction does not follow a later change of the macOS default device.
deviceNotification()mapskAudioHardwarePropertyDefaultInputDevice/DefaultOutputDevicetoRS_ONLY_RESTART, andCClient::OnSndCrdReinitRequest()deliberately skipsSound.SetDev()for that reset type, so the driver is not reloaded and the previously resolved device stays in use until a reload happens for another reason. This behaves exactly as it did before this PR; changing it means changing the notification handling, which seemed better kept separate from a device-enumeration change. Happy to follow up if you would like it fixed.Commits
Six, separable so they can be reviewed or taken independently. Two of them stand alone:
System Defaultentry can change while its name does not. Reloading the driver then kept the previous device IDs, and since the IO callback compares the device it is called for against them, it would discard the audio of the device it had just been started on. This covers the reload paths only, see the limitation above.The last commit adds the author to the in-app contributor list, as asked for in CONTRIBUTING.md; happy to drop it if you would rather it were separate.
CHANGELOG: macOS: Fixed audio devices missing from the device list, and split the device selection into separate input and output menus
Context: Fixes an issue?
Fixes: #3216
Earlier background: #681, moved to https://github.com/orgs/jamulussoftware/discussions/1039 in 2021.
Does this change need documentation? What needs to be documented and how?
Yes, a small update to the Software Manual: on macOS the "Audio Device" setting becomes two settings, "Input Device" and "Output Device". Happy to open a PR on the website repo if this is accepted.
Status of this Pull Request
Working implementation.
What is missing until this pull request can be merged?
Review, and testing on macOS setups other than mine. I have tested on macOS 26.5 / Qt 6.11 (arm64): selecting devices per direction, channel mapping preserved for the direction that did not change, settings written by the previous version loading unchanged, and the fallback when a stored device is no longer present. Every commit builds on its own.
@heepm — you reported #3216 and offered to help test; if you still have the aggregate-device setup from that thread, this would be the ideal case to confirm, since every device should now appear in the lists.
The other platforms are compile-tested only, through the
autobuildworkflow on my fork: Windows, Windows JACK, macOS, macOS Legacy, iOS, Android and Linux amd64/arm64/armhf all pass, including CodeQL. I have no ASIO or JACK hardware to confirm at runtime that those paths behave as before, though they are unchanged by design.Checklist