[PATCH v2 0/1] MR7920: winecoreaudio: revert unix_set_volumes
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. -- v2: winecoreaudio: revert unix_set_volumes from f46e9b8f120605d984f4ae14b4c815a67fbd1b59 https://gitlab.winehq.org/wine/wine/-/merge_requests/7920
From: Ada <yretenai(a)gmail.com> --- dlls/winecoreaudio.drv/coreaudio.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c index 6b81a7d1d5c..0061395d7fe 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -1762,30 +1762,20 @@ static NTSTATUS unix_set_volumes(void *args) { struct set_volumes_params *params = args; struct coreaudio_stream *stream = handle_get_stream(params->stream); - Float32 level = params->master_volume; + Float32 level = 1.0, tmp; OSStatus sc; UINT32 i; - AudioObjectPropertyAddress prop_addr = { - kAudioDevicePropertyVolumeScalar, - kAudioObjectPropertyScopeGlobal, - kAudioObjectPropertyElementMain - }; - - sc = AudioObjectSetPropertyData(stream->dev_id, &prop_addr, 0, NULL, sizeof(float), &level); - if (sc == noErr) - level = 1.0f; - else - WARN("Couldn't set master volume, applying it directly to the channels: %x\n", (int)sc); - for (i = 1; i <= stream->fmt->nChannels; ++i) { - const float vol = level * params->session_volumes[i - 1] * params->volumes[i - 1]; + WARN("CoreAudio doesn't support per-channel volume control\n"); - prop_addr.mElement = 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 = AudioObjectSetPropertyData(stream->dev_id, &prop_addr, 0, NULL, sizeof(float), &vol); - if (sc != noErr) { - WARN("Couldn't set channel #%u volume: %x\n", i, (int)sc); - } + 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7920
Brendan Shanks (@bshanks) commented about dlls/winecoreaudio.drv/coreaudio.c:
- }; - - sc = AudioObjectSetPropertyData(stream->dev_id, &prop_addr, 0, NULL, sizeof(float), &level); - if (sc == noErr) - level = 1.0f; - else - WARN("Couldn't set master volume, applying it directly to the channels: %x\n", (int)sc);
- for (i = 1; i <= stream->fmt->nChannels; ++i) { - const float vol = level * params->session_volumes[i - 1] * params->volumes[i - 1]; + WARN("CoreAudio doesn't support per-channel volume control\n");
- prop_addr.mElement = 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; I think this could be replaced by `fminf()`
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_104156
Brendan Shanks (@bshanks) commented about dlls/winecoreaudio.drv/coreaudio.c:
UINT32 i; - AudioObjectPropertyAddress prop_addr = { - kAudioDevicePropertyVolumeScalar, - kAudioObjectPropertyScopeGlobal, - kAudioObjectPropertyElementMain - }; - - sc = AudioObjectSetPropertyData(stream->dev_id, &prop_addr, 0, NULL, sizeof(float), &level); - if (sc == noErr) - level = 1.0f; - else - WARN("Couldn't set master volume, applying it directly to the channels: %x\n", (int)sc);
- for (i = 1; i <= stream->fmt->nChannels; ++i) { - const float vol = level * params->session_volumes[i - 1] * params->volumes[i - 1]; + WARN("CoreAudio doesn't support per-channel volume control\n");
We don't need to print this warning if all channels have the same volume/session volume (which they often do). This could also be a good candidate to be in an `if (once)` block, so there aren't constant warnings while moving a volume slider around. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_104157
I think it would be better to have the commit message be the typical git revert, like: ``` Revert "winecoreaudio: Implement per-channel volume control." This reverts commit f46e9b8f120605d984f4ae14b4c815a67fbd1b59. ``` Also, I think it could be good to have the revert just revert the commit and then put the other improvements (`fminf`, avoid `WARN` when possible) in a separate commit -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_104155
On Wed May 21 23:42:38 2025 +0000, Brendan Shanks wrote:
We don't need to print this warning if all channels have the same volume/session volume (which they often do). This could also be a good candidate to be in an `if (once)` block, so there aren't constant warnings while moving a volume slider around. Good points, I'll make the commit probably around Monday. Likely will just warn once regardless of volume levels to keep the code relatively simple.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7920#note_104430
participants (3)
-
Ada -
Ada Ahmed (＠yretenai)
-
Brendan Shanks (＠bshanks)