[PATCH 0/1] MR2869: winepulse: Implement is_format_supported in unixlib.

[Davide Beatrici (＠davidebeatrici)]

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

[PATCH 1/1] winepulse: Implement is_format_supported in unixlib.

[Davide Beatrici]

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

---
 dlls/winepulse.drv/mmdevdrv.c | 175 +++++-----------------------------
 dlls/winepulse.drv/pulse.c    | 156 +++++++++++++++++++++++++++++-
 2 files changed, 179 insertions(+), 152 deletions(-)

diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c
index 9ec0e3b7f46..55685d76482 100644
--- a/dlls/winepulse.drv/mmdevdrv.c
+++ b/dlls/winepulse.drv/mmdevdrv.c
@@ -620,27 +620,6 @@ static void dump_fmt(const WAVEFORMATEX *fmt)
     }
 }
 
-static WAVEFORMATEX *clone_format(const WAVEFORMATEX *fmt)
-{
-    WAVEFORMATEX *ret;
-    size_t size;
-
-    if (fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE)
-        size = sizeof(WAVEFORMATEXTENSIBLE);
-    else
-        size = sizeof(WAVEFORMATEX);
-
-    ret = CoTaskMemAlloc(size);
-    if (!ret)
-        return NULL;
-
-    memcpy(ret, fmt, size);
-
-    ret->cbSize = size - sizeof(WAVEFORMATEX);
-
-    return ret;
-}
-
 static void session_init_vols(AudioSession *session, UINT channels)
 {
     if (session->channel_count < channels) {
@@ -888,152 +867,48 @@ static HRESULT WINAPI AudioClient_IsFormatSupported(IAudioClient3 *iface,
         WAVEFORMATEX **out)
 {
     ACImpl *This = impl_from_IAudioClient3(iface);
-    HRESULT hr = S_OK;
-    WAVEFORMATEX *closest = NULL;
-    BOOL exclusive;
+    struct is_format_supported_params params;
 
     TRACE("(%p)->(%x, %p, %p)\n", This, mode, fmt, out);
 
     if (!fmt)
         return E_POINTER;
 
-    if (out)
-        *out = NULL;
-
-    if (mode == AUDCLNT_SHAREMODE_EXCLUSIVE) {
-        exclusive = 1;
-        out = NULL;
-    } else if (mode == AUDCLNT_SHAREMODE_SHARED) {
-        exclusive = 0;
-        if (!out)
-            return E_POINTER;
-    } else
-        return E_INVALIDARG;
-
-    if (fmt->nChannels == 0)
-        return AUDCLNT_E_UNSUPPORTED_FORMAT;
-
-    closest = clone_format(fmt);
-    if (!closest)
-        return E_OUTOFMEMORY;
-
     dump_fmt(fmt);
 
-    switch (fmt->wFormatTag) {
-    case WAVE_FORMAT_EXTENSIBLE: {
-        WAVEFORMATEXTENSIBLE *ext = (WAVEFORMATEXTENSIBLE*)closest;
+    switch (mode) {
+        case AUDCLNT_SHAREMODE_SHARED:
+            if (!out)
+                return E_POINTER;
 
-        if ((fmt->cbSize != sizeof(WAVEFORMATEXTENSIBLE) - sizeof(WAVEFORMATEX) &&
-             fmt->cbSize != sizeof(WAVEFORMATEXTENSIBLE)) ||
-            fmt->nBlockAlign != fmt->wBitsPerSample / 8 * fmt->nChannels ||
-            ext->Samples.wValidBitsPerSample > fmt->wBitsPerSample ||
-            fmt->nAvgBytesPerSec != fmt->nBlockAlign * fmt->nSamplesPerSec) {
-            hr = E_INVALIDARG;
+        params.fmt_out = CoTaskMemAlloc(sizeof(*params.fmt_out));
+        break;
+        case AUDCLNT_SHAREMODE_EXCLUSIVE:
+            params.fmt_out = NULL;
             break;
-        }
-
-        if (exclusive) {
-            UINT32 mask = 0, i, channels = 0;
-
-            if (!(ext->dwChannelMask & (SPEAKER_ALL | SPEAKER_RESERVED))) {
-                for (i = 1; !(i & SPEAKER_RESERVED); i <<= 1) {
-                    if (i & ext->dwChannelMask) {
-                        mask |= i;
-                        channels++;
-                    }
-                }
-
-                if (channels != fmt->nChannels || (ext->dwChannelMask & ~mask)) {
-                    hr = AUDCLNT_E_UNSUPPORTED_FORMAT;
-                    break;
-                }
-            } else {
-                hr = AUDCLNT_E_UNSUPPORTED_FORMAT;
-                break;
-            }
-        }
-
-        if (IsEqualGUID(&ext->SubFormat, &KSDATAFORMAT_SUBTYPE_IEEE_FLOAT)) {
-            if (fmt->wBitsPerSample != 32) {
-                hr = E_INVALIDARG;
-                break;
-            }
+        default:
+            return E_INVALIDARG;
+    }
 
-            if (ext->Samples.wValidBitsPerSample != fmt->wBitsPerSample) {
-                hr = S_FALSE;
-                ext->Samples.wValidBitsPerSample = fmt->wBitsPerSample;
-            }
-        } else if (IsEqualGUID(&ext->SubFormat, &KSDATAFORMAT_SUBTYPE_PCM)) {
-            if (!fmt->wBitsPerSample || fmt->wBitsPerSample > 32 || fmt->wBitsPerSample % 8) {
-                hr = E_INVALIDARG;
-                break;
-            }
+    params.device  = This->device_name;
+    params.flow    = This->dataflow;
+    params.share   = mode;
+    params.fmt_in  = fmt;
 
-            if (ext->Samples.wValidBitsPerSample != fmt->wBitsPerSample &&
-                !(fmt->wBitsPerSample == 32 &&
-                  ext->Samples.wValidBitsPerSample == 24)) {
-                hr = S_FALSE;
-                ext->Samples.wValidBitsPerSample = fmt->wBitsPerSample;
-                break;
-            }
-        } else {
-            hr = AUDCLNT_E_UNSUPPORTED_FORMAT;
-            break;
-        }
-
-        break;
-    }
+    pulse_call(is_format_supported, ¶ms);
 
-    case WAVE_FORMAT_ALAW:
-    case WAVE_FORMAT_MULAW:
-        if (fmt->wBitsPerSample != 8) {
-            hr = E_INVALIDARG;
-            break;
-        }
-        /* Fall-through */
-    case WAVE_FORMAT_IEEE_FLOAT:
-        if (fmt->wFormatTag == WAVE_FORMAT_IEEE_FLOAT && fmt->wBitsPerSample != 32) {
-            hr = E_INVALIDARG;
-            break;
-        }
-        /* Fall-through */
-    case WAVE_FORMAT_PCM:
-        if (fmt->wFormatTag == WAVE_FORMAT_PCM &&
-            (!fmt->wBitsPerSample || fmt->wBitsPerSample > 32 || fmt->wBitsPerSample % 8)) {
-            hr = E_INVALIDARG;
-            break;
-        }
+    if (params.result == S_FALSE) {
+        *out = ¶ms.fmt_out->Format;
+    } else {
+        if (out)
+            *out = NULL;
 
-        if (fmt->nChannels > 2) {
-            hr = AUDCLNT_E_UNSUPPORTED_FORMAT;
-            break;
-        }
-        /*
-         * fmt->cbSize, fmt->nBlockAlign and fmt->nAvgBytesPerSec seem to be
-         * ignored, invalid values are happily accepted.
-         */
-        break;
-    default:
-        hr = AUDCLNT_E_UNSUPPORTED_FORMAT;
-        break;
+        CoTaskMemFree(params.fmt_out);
     }
 
-    if (exclusive && hr != S_OK) {
-        hr = AUDCLNT_E_UNSUPPORTED_FORMAT;
-        CoTaskMemFree(closest);
-    } else if (hr != S_FALSE)
-        CoTaskMemFree(closest);
-    else
-        *out = closest;
-
-    /* Winepulse does not currently support exclusive mode, if you know of an
-     * application that uses it, I will correct this..
-     */
-    if (hr == S_OK && exclusive)
-        return This->dataflow == eCapture ? AUDCLNT_E_UNSUPPORTED_FORMAT : AUDCLNT_E_EXCLUSIVE_MODE_NOT_ALLOWED;
+    TRACE("returning: %08lx %p\n", params.result, out ? *out : NULL);
 
-    TRACE("returning: %08lx %p\n", hr, out ? *out : NULL);
-    return hr;
+    return params.result;
 }
 
 static HRESULT WINAPI AudioClient_GetMixFormat(IAudioClient3 *iface,
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c
index a3354c5efe2..1b3749e19d7 100644
--- a/dlls/winepulse.drv/pulse.c
+++ b/dlls/winepulse.drv/pulse.c
@@ -2077,6 +2077,134 @@ static NTSTATUS pulse_release_capture_buffer(void *args)
     return STATUS_SUCCESS;
 }
 
+static NTSTATUS pulse_is_format_supported(void *args)
+{
+    struct is_format_supported_params *params = args;
+    WAVEFORMATEXTENSIBLE in;
+    WAVEFORMATEXTENSIBLE *out;
+    const WAVEFORMATEX *fmt = &in.Format;
+    const BOOLEAN exclusive = params->share == AUDCLNT_SHAREMODE_EXCLUSIVE;
+
+    if (params->fmt_in->nChannels == 0)
+        return AUDCLNT_E_UNSUPPORTED_FORMAT;
+
+    memcpy(&in, params->fmt_in, params->fmt_in->wFormatTag == WAVE_FORMAT_EXTENSIBLE ?
+    sizeof(in) : sizeof(in.Format));
+
+    if (exclusive)
+        out = ∈
+    else {
+        out = params->fmt_out;
+        memcpy(out, fmt, fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE ?
+        sizeof(*out) : sizeof((*out).Format));
+    }
+
+    params->result = S_OK;
+
+    switch (fmt->wFormatTag) {
+        case WAVE_FORMAT_EXTENSIBLE: {
+            if ((fmt->cbSize != sizeof(WAVEFORMATEXTENSIBLE) - sizeof(WAVEFORMATEX) &&
+                fmt->cbSize != sizeof(WAVEFORMATEXTENSIBLE)) ||
+                fmt->nBlockAlign != fmt->wBitsPerSample / 8 * fmt->nChannels ||
+                in.Samples.wValidBitsPerSample > fmt->wBitsPerSample ||
+                fmt->nAvgBytesPerSec != fmt->nBlockAlign * fmt->nSamplesPerSec) {
+                params->result = E_INVALIDARG;
+                break;
+            }
+
+            if (exclusive) {
+                UINT32 mask = 0, i, channels = 0;
+
+                if (!(in.dwChannelMask & (SPEAKER_ALL | SPEAKER_RESERVED))) {
+                    for (i = 1; !(i & SPEAKER_RESERVED); i <<= 1) {
+                        if (i & in.dwChannelMask) {
+                            mask |= i;
+                            channels++;
+                        }
+                    }
+
+                    if (channels != fmt->nChannels || (in.dwChannelMask & ~mask)) {
+                        params->result = AUDCLNT_E_UNSUPPORTED_FORMAT;
+                        break;
+                    }
+                } else {
+                    params->result = AUDCLNT_E_UNSUPPORTED_FORMAT;
+                    break;
+                }
+            }
+
+            if (IsEqualGUID(&in.SubFormat, &KSDATAFORMAT_SUBTYPE_IEEE_FLOAT)) {
+                if (fmt->wBitsPerSample != 32) {
+                    params->result = E_INVALIDARG;
+                    break;
+                }
+
+                if (in.Samples.wValidBitsPerSample != fmt->wBitsPerSample) {
+                    params->result = S_FALSE;
+                    out->Samples.wValidBitsPerSample = fmt->wBitsPerSample;
+                }
+            } else if (IsEqualGUID(&in.SubFormat, &KSDATAFORMAT_SUBTYPE_PCM)) {
+                if (!fmt->wBitsPerSample || fmt->wBitsPerSample > 32 || fmt->wBitsPerSample % 8) {
+                    params->result = E_INVALIDARG;
+                    break;
+                }
+
+                if (in.Samples.wValidBitsPerSample != fmt->wBitsPerSample &&
+                    !(fmt->wBitsPerSample == 32 &&
+                    in.Samples.wValidBitsPerSample == 24)) {
+                    params->result = S_FALSE;
+                    out->Samples.wValidBitsPerSample = fmt->wBitsPerSample;
+                    break;
+                }
+            } else {
+                params->result = AUDCLNT_E_UNSUPPORTED_FORMAT;
+                break;
+            }
+
+            break;
+        }
+
+        case WAVE_FORMAT_ALAW:
+        case WAVE_FORMAT_MULAW:
+            if (fmt->wBitsPerSample != 8) {
+                params->result = E_INVALIDARG;
+                break;
+            }
+            /* Fall-through */
+            case WAVE_FORMAT_IEEE_FLOAT:
+                if (fmt->wFormatTag == WAVE_FORMAT_IEEE_FLOAT && fmt->wBitsPerSample != 32) {
+                    params->result = E_INVALIDARG;
+                    break;
+                }
+                /* Fall-through */
+                case WAVE_FORMAT_PCM: {
+                    if (fmt->wFormatTag == WAVE_FORMAT_PCM &&
+                        (!fmt->wBitsPerSample || fmt->wBitsPerSample > 32 || fmt->wBitsPerSample % 8)) {
+                        params->result = E_INVALIDARG;
+                        break;
+                    }
+
+                    if (fmt->nChannels > 2) {
+                        params->result = AUDCLNT_E_UNSUPPORTED_FORMAT;
+                        break;
+                    }
+
+                    /* fmt->cbSize, fmt->nBlockAlign and fmt->nAvgBytesPerSec seem to be
+                     * ignored, invalid values are happily accepted. */
+                    break;
+                }
+                default:
+                    params->result = AUDCLNT_E_UNSUPPORTED_FORMAT;
+                    break;
+    }
+
+    /* This driver does not support exclusive mode. */
+    if (exclusive && params->result == S_OK)
+        params->result = params->flow == eCapture ? AUDCLNT_E_UNSUPPORTED_FORMAT : AUDCLNT_E_EXCLUSIVE_MODE_NOT_ALLOWED;
+
+    return STATUS_SUCCESS;
+}
+
 static NTSTATUS pulse_get_mix_format(void *args)
 {
     struct get_mix_format_params *params = args;
@@ -2395,7 +2523,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] =
     pulse_release_render_buffer,
     pulse_get_capture_buffer,
     pulse_release_capture_buffer,
-    pulse_not_implemented,
+    pulse_is_format_supported,
     pulse_get_mix_format,
     pulse_get_device_period,
     pulse_get_buffer_size,
@@ -2561,6 +2689,30 @@ static NTSTATUS pulse_wow64_get_capture_buffer(void *args)
     return STATUS_SUCCESS;
 };
 
+static NTSTATUS pulse_wow64_is_format_supported(void *args)
+{
+    struct
+    {
+        PTR32 device;
+        EDataFlow flow;
+        AUDCLNT_SHAREMODE share;
+        PTR32 fmt_in;
+        PTR32 fmt_out;
+        HRESULT result;
+    } *params32 = args;
+    struct is_format_supported_params params =
+    {
+        .device = ULongToPtr(params32->device),
+        .flow = params32->flow,
+        .share = params32->share,
+        .fmt_in = ULongToPtr(params32->fmt_in),
+        .fmt_out = ULongToPtr(params32->fmt_out)
+    };
+    pulse_is_format_supported(¶ms);
+    params32->result = params.result;
+    return STATUS_SUCCESS;
+}
+
 static NTSTATUS pulse_wow64_get_mix_format(void *args)
 {
     struct
@@ -2840,7 +2992,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] =
     pulse_release_render_buffer,
     pulse_wow64_get_capture_buffer,
     pulse_release_capture_buffer,
-    pulse_not_implemented,
+    pulse_wow64_is_format_supported,
     pulse_wow64_get_mix_format,
     pulse_wow64_get_device_period,
     pulse_wow64_get_buffer_size,
-- 
GitLab

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

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/winepulse.drv/pulse.c:
> return STATUS_SUCCESS;
>  }
>
> +static NTSTATUS pulse_is_format_supported(void *args)
> +{
> +    struct is_format_supported_params *params = args;
> +    WAVEFORMATEXTENSIBLE in;
> +    WAVEFORMATEXTENSIBLE *out;
> +    const WAVEFORMATEX *fmt = &in.Format;
> +    const BOOLEAN exclusive = params->share == AUDCLNT_SHAREMODE_EXCLUSIVE;
> +
> +    if (params->fmt_in->nChannels == 0)
> +        return AUDCLNT_E_UNSUPPORTED_FORMAT;

```suggestion:-1+0
    if (params->fmt_in->nChannels == 0) {
        params->result = AUDCLNT_E_UNSUPPORTED_FORMAT;
        return STATUS_SUCCESS;
    }
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2869#note_33367

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/winepulse.drv/mmdevdrv.c:
> -    if (fmt->nChannels == 0)
> -        return AUDCLNT_E_UNSUPPORTED_FORMAT;
> -
> -    closest = clone_format(fmt);
> -    if (!closest)
> -        return E_OUTOFMEMORY;
> -
>      dump_fmt(fmt);
>
> -    switch (fmt->wFormatTag) {
> -    case WAVE_FORMAT_EXTENSIBLE: {
> -        WAVEFORMATEXTENSIBLE *ext = (WAVEFORMATEXTENSIBLE*)closest;
> +    switch (mode) {
> +        case AUDCLNT_SHAREMODE_SHARED:
> +            if (!out)
> +                return E_POINTER;
> Since the other drivers have fairly consistent implementations on the PE-side and unixlib prologues, it would make more sense to copy those.  If you want to change them after merging you could do that later.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2869#note_33368

[Davide Beatrici (＠davidebeatrici)]

On Mon May 22 08:54:41 2023 +0000, Huw Davies wrote:
> ```suggestion:-1+0
>     if (params->fmt_in->nChannels == 0) {
>         params->result = AUDCLNT_E_UNSUPPORTED_FORMAT;
>         return STATUS_SUCCESS;
>     }
> ```

Oops, the tests didn't catch that. We may want to add a case for 0 channels.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2869#note_33426

[Davide Beatrici (＠davidebeatrici)]

On Mon May 22 09:19:03 2023 +0000, Huw Davies wrote:
> Since the other drivers have fairly consistent implementations on the
> PE-side and unixlib prologues, it would make more sense to copy those.
>  If you want to change them after merging you could do that later.
> Good idea.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2869#note_33427