[PATCH v3 0/2] MR2732: winecoreaudio: Implement per-channel volume control.

[Davide Beatrici (＠davidebeatrici)]

Requires testing, as I only have access to a remote macOS machine.

--
  v3: mmdevapi: Remove unused "channel" member in set_volumes_params.
      winecoreaudio: Implement per-channel volume control.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2732

[PATCH v3 1/2] winecoreaudio: Implement per-channel volume control.

[Davide Beatrici]

From: Davide Beatrici <git(a)davidebeatrici.dev>

---
 dlls/winecoreaudio.drv/coreaudio.c | 37 ++++++++++++++++--------------
 dlls/winecoreaudio.drv/mmdevdrv.c  | 16 ++++++-------
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c
index aef678e7ca7..902d01459c4 100644
--- a/dlls/winecoreaudio.drv/coreaudio.c
+++ b/dlls/winecoreaudio.drv/coreaudio.c
@@ -1665,28 +1665,31 @@ static NTSTATUS unix_set_volumes(void *args)
 {
     struct set_volumes_params *params = args;
     struct coreaudio_stream *stream = handle_get_stream(params->stream);
-    Float32 level = 1.0, tmp;
+    Float32 level = params->master_volume;
     OSStatus sc;
     UINT32 i;
+    AudioObjectPropertyAddress prop_addr = {
+        kAudioDevicePropertyVolumeScalar,
+        kAudioObjectPropertyScopeGlobal,
+        kAudioObjectPropertyElementMaster
+    };
 
-    if(params->channel >= stream->fmt->nChannels || params->channel < -1){
-        ERR("Incorrect channel %d\n", params->channel);
-        return STATUS_SUCCESS;
-    }
+    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);
 
-    if(params->channel == -1){
-        for(i = 0; i < stream->fmt->nChannels; ++i){
-            tmp = params->master_volume * params->volumes[i] * params->session_volumes[i];
-            level = tmp < level ? tmp : level;
-        }
-    }else
-        level = params->master_volume * params->volumes[params->channel] *
-            params->session_volumes[params->channel];
+    for (i = 1; i <= stream->fmt->nChannels; ++i) {
+        const float vol = level * params->session_volumes[i - 1] * params->volumes[i - 1];
+
+        prop_addr.mElement = i;
 
-    sc = AudioUnitSetParameter(stream->unit, kHALOutputParam_Volume,
-                               kAudioUnitScope_Global, 0, level, 0);
-    if(sc != noErr)
-        WARN("Couldn't set volume: %x\n", (int)sc);
+        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);
+        }
+    }
 
     return STATUS_SUCCESS;
 }
diff --git a/dlls/winecoreaudio.drv/mmdevdrv.c b/dlls/winecoreaudio.drv/mmdevdrv.c
index 5113132f09b..6f62a69028b 100644
--- a/dlls/winecoreaudio.drv/mmdevdrv.c
+++ b/dlls/winecoreaudio.drv/mmdevdrv.c
@@ -224,7 +224,7 @@ static void get_device_guid(EDataFlow flow, const char *dev, GUID *guid)
         RegCloseKey(key);
 }
 
-static void set_stream_volumes(ACImpl *This, int channel)
+static void set_stream_volumes(ACImpl *This)
 {
     struct set_volumes_params params;
 
@@ -232,7 +232,7 @@ static void set_stream_volumes(ACImpl *This, int channel)
     params.master_volume = This->session->mute ? 0.0f : This->session->master_vol;
     params.volumes = This->vols;
     params.session_volumes = This->session->channel_vols;
-    params.channel = channel;
+    params.channel = 0;
 
     UNIX_CALL(set_volumes, ¶ms);
 }
@@ -689,7 +689,7 @@ end:
         This->vols = NULL;
     }else{
         This->stream = stream;
-        set_stream_volumes(This, -1);
+        set_stream_volumes(This);
     }
 
     sessions_unlock();
@@ -1521,8 +1521,7 @@ static HRESULT WINAPI AudioStreamVolume_SetChannelVolume(
 
     This->vols[index] = level;
 
-    WARN("CoreAudio doesn't support per-channel volume control\n");
-    set_stream_volumes(This, index);
+    set_stream_volumes(This);
 
     sessions_unlock();
 
@@ -1566,7 +1565,7 @@ static HRESULT WINAPI AudioStreamVolume_SetAllVolumes(
     for(i = 0; i < count; ++i)
         This->vols[i] = levels[i];
 
-    set_stream_volumes(This, -1);
+    set_stream_volumes(This);
 
     sessions_unlock();
 
@@ -1682,9 +1681,8 @@ static HRESULT WINAPI ChannelAudioVolume_SetChannelVolume(
 
     session->channel_vols[index] = level;
 
-    WARN("CoreAudio doesn't support per-channel volume control\n");
     LIST_FOR_EACH_ENTRY(client, &session->clients, ACImpl, entry)
-        set_stream_volumes(client, index);
+        set_stream_volumes(client);
 
     sessions_unlock();
 
@@ -1737,7 +1735,7 @@ static HRESULT WINAPI ChannelAudioVolume_SetAllVolumes(
         session->channel_vols[i] = levels[i];
 
     LIST_FOR_EACH_ENTRY(client, &session->clients, ACImpl, entry)
-        set_stream_volumes(client, -1);
+        set_stream_volumes(client);
 
     sessions_unlock();
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/2732

[PATCH v3 2/2] mmdevapi: Remove unused "channel" member in set_volumes_params.

[Davide Beatrici]

From: Davide Beatrici <git(a)davidebeatrici.dev>

---
 dlls/mmdevapi/session.c            | 1 -
 dlls/mmdevapi/unixlib.h            | 1 -
 dlls/winealsa.drv/alsa.c           | 2 --
 dlls/winealsa.drv/mmdevdrv.c       | 1 -
 dlls/winecoreaudio.drv/coreaudio.c | 2 --
 dlls/winecoreaudio.drv/mmdevdrv.c  | 1 -
 dlls/wineoss.drv/mmdevdrv.c        | 1 -
 dlls/wineoss.drv/oss.c             | 2 --
 dlls/winepulse.drv/mmdevdrv.c      | 1 -
 dlls/winepulse.drv/pulse.c         | 2 --
 10 files changed, 14 deletions(-)

diff --git a/dlls/mmdevapi/session.c b/dlls/mmdevapi/session.c
index 825fd44d4cf..1f980282417 100644
--- a/dlls/mmdevapi/session.c
+++ b/dlls/mmdevapi/session.c
@@ -52,7 +52,6 @@ static void set_stream_volumes(struct audio_client *This)
     params.master_volume   = (This->session->mute ? 0.0f : This->session->master_vol);
     params.volumes         = This->vols;
     params.session_volumes = This->session->channel_vols;
-    params.channel         = 0;
 
     WINE_UNIX_CALL(set_volumes, ¶ms);
 }
diff --git a/dlls/mmdevapi/unixlib.h b/dlls/mmdevapi/unixlib.h
index 37b4c53cb35..b1e025a512b 100644
--- a/dlls/mmdevapi/unixlib.h
+++ b/dlls/mmdevapi/unixlib.h
@@ -207,7 +207,6 @@ struct set_volumes_params
     float master_volume;
     const float *volumes;
     const float *session_volumes;
-    int channel;
 };
 
 struct set_event_handle_params
diff --git a/dlls/winealsa.drv/alsa.c b/dlls/winealsa.drv/alsa.c
index 2b5f61eb213..200798a64e6 100644
--- a/dlls/winealsa.drv/alsa.c
+++ b/dlls/winealsa.drv/alsa.c
@@ -2767,7 +2767,6 @@ static NTSTATUS alsa_wow64_set_volumes(void *args)
         float master_volume;
         PTR32 volumes;
         PTR32 session_volumes;
-        int channel;
     } *params32 = args;
     struct set_volumes_params params =
     {
@@ -2775,7 +2774,6 @@ static NTSTATUS alsa_wow64_set_volumes(void *args)
         .master_volume = params32->master_volume,
         .volumes = ULongToPtr(params32->volumes),
         .session_volumes = ULongToPtr(params32->session_volumes),
-        .channel = params32->channel
     };
     return alsa_set_volumes(¶ms);
 }
diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c
index 6ea8882de59..96d71e98272 100644
--- a/dlls/winealsa.drv/mmdevdrv.c
+++ b/dlls/winealsa.drv/mmdevdrv.c
@@ -261,7 +261,6 @@ static void set_stream_volumes(ACImpl *This)
     params.master_volume = (This->session->mute ? 0.0f : This->session->master_vol);
     params.volumes = This->vols;
     params.session_volumes = This->session->channel_vols;
-    params.channel = 0;
 
     ALSA_CALL(set_volumes, ¶ms);
 }
diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c
index 902d01459c4..83c8048ff43 100644
--- a/dlls/winecoreaudio.drv/coreaudio.c
+++ b/dlls/winecoreaudio.drv/coreaudio.c
@@ -2046,7 +2046,6 @@ static NTSTATUS unix_wow64_set_volumes(void *args)
         float master_volume;
         PTR32 volumes;
         PTR32 session_volumes;
-        int channel;
     } *params32 = args;
     struct set_volumes_params params =
     {
@@ -2054,7 +2053,6 @@ static NTSTATUS unix_wow64_set_volumes(void *args)
         .master_volume = params32->master_volume,
         .volumes = ULongToPtr(params32->volumes),
         .session_volumes = ULongToPtr(params32->session_volumes),
-        .channel = params32->channel
     };
     return unix_set_volumes(¶ms);
 }
diff --git a/dlls/winecoreaudio.drv/mmdevdrv.c b/dlls/winecoreaudio.drv/mmdevdrv.c
index 6f62a69028b..f387e9f71ab 100644
--- a/dlls/winecoreaudio.drv/mmdevdrv.c
+++ b/dlls/winecoreaudio.drv/mmdevdrv.c
@@ -232,7 +232,6 @@ static void set_stream_volumes(ACImpl *This)
     params.master_volume = This->session->mute ? 0.0f : This->session->master_vol;
     params.volumes = This->vols;
     params.session_volumes = This->session->channel_vols;
-    params.channel = 0;
 
     UNIX_CALL(set_volumes, ¶ms);
 }
diff --git a/dlls/wineoss.drv/mmdevdrv.c b/dlls/wineoss.drv/mmdevdrv.c
index 89dbeac62bc..d41d0597721 100644
--- a/dlls/wineoss.drv/mmdevdrv.c
+++ b/dlls/wineoss.drv/mmdevdrv.c
@@ -271,7 +271,6 @@ static void set_stream_volumes(ACImpl *This)
     params.master_volume = (This->session->mute ? 0.0f : This->session->master_vol);
     params.volumes = This->vols;
     params.session_volumes = This->session->channel_vols;
-    params.channel = 0;
     OSS_CALL(set_volumes, ¶ms);
 }
 
diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c
index 96d84a07d00..035f6eced97 100644
--- a/dlls/wineoss.drv/oss.c
+++ b/dlls/wineoss.drv/oss.c
@@ -1974,7 +1974,6 @@ static NTSTATUS oss_wow64_set_volumes(void *args)
         float master_volume;
         PTR32 volumes;
         PTR32 session_volumes;
-        int channel;
     } *params32 = args;
     struct set_volumes_params params =
     {
@@ -1982,7 +1981,6 @@ static NTSTATUS oss_wow64_set_volumes(void *args)
         .master_volume = params32->master_volume,
         .volumes = ULongToPtr(params32->volumes),
         .session_volumes = ULongToPtr(params32->session_volumes),
-        .channel = params32->channel
     };
     return oss_set_volumes(¶ms);
 }
diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c
index 59e2d086d0b..c3bc02267b5 100644
--- a/dlls/winepulse.drv/mmdevdrv.c
+++ b/dlls/winepulse.drv/mmdevdrv.c
@@ -316,7 +316,6 @@ static void set_stream_volumes(ACImpl *This)
     params.master_volume   = This->session->mute ? 0.0f : This->session->master_vol;
     params.volumes         = This->vols;
     params.session_volumes = This->session->channel_vols;
-    params.channel         = 0;
     pulse_call(set_volumes, ¶ms);
 }
 
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c
index 28afb5b788d..a3354c5efe2 100644
--- a/dlls/winepulse.drv/pulse.c
+++ b/dlls/winepulse.drv/pulse.c
@@ -2723,7 +2723,6 @@ static NTSTATUS pulse_wow64_set_volumes(void *args)
         float master_volume;
         PTR32 volumes;
         PTR32 session_volumes;
-        int channel;
     } *params32 = args;
     struct set_volumes_params params =
     {
@@ -2731,7 +2730,6 @@ static NTSTATUS pulse_wow64_set_volumes(void *args)
         .master_volume = params32->master_volume,
         .volumes = ULongToPtr(params32->volumes),
         .session_volumes = ULongToPtr(params32->session_volumes),
-        .channel = params32->channel
     };
     return pulse_set_volumes(¶ms);
 }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/2732

[Davide Beatrici (＠davidebeatrici)]

On Wed May  3 08:28:24 2023 +0000, Huw Davies wrote:
> Right, I guess that's even worse than the deprecation warning ;-)  So
> let's switch to using `kAudioObjectPropertyElementMaster`.  I'd also
> missed that it was already in-use, so it's not quite as bad as adding a
> new warning.
> No problem.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2732#note_31904

[Davide Beatrici (＠davidebeatrici)]

Davide Beatrici (@davidebeatrici) commented about dlls/winecoreaudio.drv/coreaudio.c:
> {
>      struct set_volumes_params *params = args;
>      struct coreaudio_stream *stream = handle_get_stream(params->stream);
> -    Float32 level = 1.0, tmp;
> +    Float32 level = params->master_volume;
>      OSStatus sc;
>      UINT32 i;
> +    AudioObjectPropertyAddress prop_addr = {
> +        kAudioDevicePropertyVolumeScalar,
> +        kAudioObjectPropertyScopeGlobal,
> If it turns out the volume is altered for both input and output, we can use `kAudioDevicePropertyScopeInput` or `kAudioDevicePropertyScopeOutput` depending on the stream direction.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2732#note_31906

[PATCH v3 0/2] MR2732: winecoreaudio: Implement per-channel volume control. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2732

[Davide Beatrici (＠davidebeatrici)]

@huw Does the code work as expected?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2732#note_31952