[PATCH 0/1] MR7920: winecoreaudio: Fallback to pre-per-channel audio if the audio device does not support it
I was able to replicate the issue seen in [Bug 58113](https://bugs.winehq.org/show_bug.cgi?id=58113) on my M1. The issue stems from the usage of AudioDevicePropertyVolumeScalar, which the audio driver for the M1 does not support (at least so it appears.) Using AudioObjectIsPropertySettable allows for fast checking for this situation, including preemptively disabling main channel audio if it appears to be unsupported. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920
From: Ada <yretenai(a)gmail.com> --- dlls/winecoreaudio.drv/coreaudio.c | 72 +++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c index 6b81a7d1d5c..1479a77dd52 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -76,6 +76,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(coreaudio); +enum coreaudio_channel_state +{ + COREAUDIO_CHANNEL_STATE_UNCHECKED, /* have not yet performed any checks to see if we can set volume properties */ + COREAUDIO_CHANNEL_STATE_FALLBACK, /* per-channel volume control is unavailble, fallback to setting the whole audio unit volume */ + COREAUDIO_CHANNEL_STATE_IGNORE_MAIN, /* per-channel volume control is available, but not for the main ("master") channel */ + COREAUDIO_CHANNEL_STATE_OK, /* all OK, per-channel volume control is available for all channels including main */ +}; + struct coreaudio_stream { os_unfair_lock lock; @@ -90,6 +98,7 @@ struct coreaudio_stream HANDLE event; BOOL playing, please_quit; + enum coreaudio_channel_state channel_volume_state; REFERENCE_TIME period; UINT32 period_frames; UINT32 bufsize_frames, resamp_bufsize_frames; @@ -1758,6 +1767,58 @@ static NTSTATUS unix_get_prop_value(void *args) return STATUS_SUCCESS; } +static NTSTATUS unix_set_volumes_to_unit( struct coreaudio_stream *stream, struct set_volumes_params *params) +{ + Float32 level = 1.0, tmp; + OSStatus sc; + UINT32 i; + + for(i = 0; i < stream->fmt->nChannels; ++i){ + tmp = params->master_volume * params->volumes[i] * params->session_volumes[i]; + level = tmp < level ? tmp : level; + } + + sc = AudioUnitSetParameter(stream->unit, kHALOutputParam_Volume, kAudioUnitScope_Global, 0, level, 0); + if(sc != noErr) { + WARN("Couldn't set volume: %x\n", (int)sc); + } + + return STATUS_SUCCESS; +} + +static void unix_check_channel_properties(struct coreaudio_stream *stream, struct set_volumes_params *params) +{ + Boolean set; + OSStatus sc; + UINT32 i; + // NOTE: keep in sync with unix_set_volumes + AudioObjectPropertyAddress prop_addr = { + kAudioDevicePropertyVolumeScalar, + kAudioObjectPropertyScopeGlobal, + kAudioObjectPropertyElementMain + }; + + stream->channel_volume_state = COREAUDIO_CHANNEL_STATE_OK; + + sc = AudioObjectIsPropertySettable(stream->dev_id, &prop_addr, &set); + if (sc != noErr || !set) { + WARN("Audio device lacks main channel control, checking if per-channel audio is available: %x\n", (int)sc); + stream->channel_volume_state = COREAUDIO_CHANNEL_STATE_IGNORE_MAIN; + } + + // check if per-channel audio control is available + for (i = 1; i <= stream->fmt->nChannels; ++i) { + prop_addr.mElement = i; + + sc = AudioObjectIsPropertySettable(stream->dev_id, &prop_addr, &set); + if (sc != noErr || !set) { + stream->channel_volume_state = COREAUDIO_CHANNEL_STATE_FALLBACK; + WARN("Audio device lacks per-channel control on channel #%u, falling back to old behavior: %x\n", i, (int)sc); + return; + } + } +} + static NTSTATUS unix_set_volumes(void *args) { struct set_volumes_params *params = args; @@ -1771,7 +1832,16 @@ static NTSTATUS unix_set_volumes(void *args) kAudioObjectPropertyElementMain }; - sc = AudioObjectSetPropertyData(stream->dev_id, &prop_addr, 0, NULL, sizeof(float), &level); + if (stream->channel_volume_state == COREAUDIO_CHANNEL_STATE_UNCHECKED) + unix_check_channel_properties(stream, params); + + if (stream->channel_volume_state == COREAUDIO_CHANNEL_STATE_FALLBACK) + return unix_set_volumes_to_unit(stream, params); + else if (stream->channel_volume_state == COREAUDIO_CHANNEL_STATE_IGNORE_MAIN) + sc = kAudioHardwareUnknownPropertyError; + else + sc = AudioObjectSetPropertyData(stream->dev_id, &prop_addr, 0, NULL, sizeof(float), &level); + if (sc == noErr) level = 1.0f; else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7920
Thanks for looking at this--I was actually able to use `kAudioDevicePropertyVolumeScalar` by changing `kAudioObjectPropertyScopeGlobal` to `kAudioObjectPropertyScopeOutput` (or presumably `Input` for an input device). Just curious, did you find any combination of hardware where `VolumeScalar`+`ScopeGlobal` worked? But, I don't think we want to use `kAudioDevicePropertyVolumeScalar` in any case, since it's actually controlling the system-wide volume rather than just the volume of our audio stream. Looking at `winealsa` and `winepulse`, they only use volume to scale audio data before sending it to ALSA/PulseAudio, not to control system volume. Although there are some Windows APIs that we could expose system volume control to, in general it doesn't seem like allowing apps (especially Windows apps/games of dubious quality) to change system volume is a good idea. That just leaves `kHALOutputParam_Volume`, which can't adjust individual channels but only affects our stream. This is the method being used when I was testing your patch. Could you change your patch to always use `kHALOutputParam_Volume` (in essence, reverting f46e9b8f120605d984f4ae14b4c815a67fbd1b59)? (Also, I found that HALLab.app (included with the Xcode "Additional Tools" downloadable from https://developer.apple.com/download/all/) can be used to show the available volume controls. With AirPods or a USB audio device, I see channel volume controls. On the Studio Display or MacBook Pro (M2 Pro) built-in speakers, I only see the master.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_102722
It's a bit confusing. `kAudioObjectPropertyScopeOutput` set the global volume for the whole system to 1.0 on my M1, and still did not actually change volume past that. `kAudioObjectPropertyScopeGlobal` ends up affecting the stream volume. (I think "Output" in this sense refers to the output device, and "Global" refers to the stream regardless of output device.)
Could you change your patch to always use `kHALOutputParam_Volume`
This would regress per-channel support on audio devices that do support it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_102790
On Wed May 14 23:46:11 2025 +0000, Ada Ahmed wrote:
It's a bit confusing. `kAudioObjectPropertyScopeOutput` set the global volume for the whole system to 1.0 on my M1, and still did not actually change volume past that. `kAudioObjectPropertyScopeGlobal` ends up affecting the stream volume. (I think "Output" in this sense refers to the output device itself and not the stream, where "Global" refers to the stream regardless of output device. I am uncertain if this is actually the reality but it's the behavior I have observed, and I could not find full documentation on these parameters so it is a bit like navigating in the dark)
Could you change your patch to always use `kHALOutputParam_Volume` This would regress per-channel support on audio devices that do support it, but given the very narrow support pool it probably(?) would go unmissed. (Famous last words.) Yeah this is all quite undocumented, but it seems odd that setting `kAudioObjectPropertyScopeGlobal` instead of `Input`/`Output` (on an `AudioDevice`) would end up setting the stream volume.
You've successfully tested setting `kAudioDevicePropertyVolumeScalar`+`kAudioObjectPropertyScopeGlobal`, and it affected only the stream volume? What kind of hardware was it on? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103448
You've successfully tested setting `kAudioDevicePropertyVolumeScalar`+`kAudioObjectPropertyScopeGlobal`, and it affected only the stream volume?
I have not, I have only observed `kAudioObjectPropertyScopeOutput` setting my system output volume to 1.0 from a previously muted state. `kAudioObjectPropertyScopeGlobal` is a no-op for me, I just blindly assumed that it worked as expected at the time of implementation on hardware that does support it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103462
On Thu May 15 01:06:49 2025 +0000, Ada Ahmed wrote:
You've successfully tested setting `kAudioDevicePropertyVolumeScalar`+`kAudioObjectPropertyScopeGlobal`, and it affected only the stream volume? I have not, I have only observed `kAudioObjectPropertyScopeOutput` setting my system output volume to 1.0 from a previously muted state. `kAudioObjectPropertyScopeGlobal` is a no-op for me, I just blindly assumed that it worked as expected at the time of implementation on hardware that does support it. @davidebeatrici, what hardware/OS were you testing with when you implemented f46e9b8f120605d984f4ae14b4c815a67fbd1b59?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103524
On Thu May 15 15:37:34 2025 +0000, Brendan Shanks wrote:
@davidebeatrici, what hardware/OS were you testing with when you implemented f46e9b8f120605d984f4ae14b4c815a67fbd1b59? From my merge request (!2732) message:
Requires testing, as I only have access to a remote macOS machine.
{width=2602 height=554} {width=2260 height=723} -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103576
On Fri May 16 05:22:37 2025 +0000, Davide Beatrici wrote:
From my merge request (!2732) message:
Requires testing, as I only have access to a remote macOS machine. {width=2602 height=554} {width=2260 height=723} Starting to look more like it never properly worked. Probably best to essentially revert this part of the patch then.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103594
On Fri May 16 08:35:43 2025 +0000, Ada Ahmed wrote:
Starting to look more like it never properly worked. Probably best to essentially revert this part of the patch then. Does the functionality work properly with your patch?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103638
On Fri May 16 17:03:04 2025 +0000, Davide Beatrici wrote:
Does the functionality work properly with your patch? For builtin audio devices yes, but I think it will always revert to the old behavior as a fallback.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103712
On Sat May 17 04:57:36 2025 +0000, Ada Ahmed wrote:
For builtin audio devices yes, but I think it will always revert to the old behavior as a fallback. I see. If there's no way to fix that, which is not straightforward without proper documentation, let's just remove the dead code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_103730
participants (4)
-
Ada -
Ada Ahmed (＠yretenai)
-
Brendan Shanks (＠bshanks)
-
Davide Beatrici (＠davidebeatrici)