[PATCH v3 0/2] MR5810: Cap default period in mmdevapi instead of the audio driver.

[Davide Beatrici (＠davidebeatrici)]

Trace output with PulseAudio:

```
0144:trace:mmdevapi:adjust_timing Requested duration 1000000 and period 0
0144:trace:mmdevapi:adjust_timing Device periods: 26666 default and 26666 minimum
0144:trace:mmdevapi:adjust_timing Adjusted duration 1000000 and period 100000
```

--
  v3: winepulse: Don't cap period(s).
      mmdevapi: Cap default period to 10 ms.

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

[PATCH v3 1/2] mmdevapi: Cap default period to 10 ms.

[Davide Beatrici]

From: Davide Beatrici

---
 dlls/mmdevapi/client.c | 59 +++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c
index 0b849e47e3d..a4dadeb4916 100644
--- a/dlls/mmdevapi/client.c
+++ b/dlls/mmdevapi/client.c
@@ -99,27 +99,38 @@ static inline struct audio_client *impl_from_IAudioStreamVolume(IAudioStreamVolu
     return CONTAINING_RECORD(iface, struct audio_client, IAudioStreamVolume_iface);
 }

-static HRESULT adjust_timing(struct audio_client *This,
-                             REFERENCE_TIME *duration, REFERENCE_TIME *period,
-                             const AUDCLNT_SHAREMODE mode, const DWORD flags,
-                             const WAVEFORMATEX *fmt)
+static HRESULT get_periods(struct audio_client *This,
+                           REFERENCE_TIME *def_period, REFERENCE_TIME *min_period)
 {
+    static const REFERENCE_TIME min_def_period = 100000; /* 10 ms */
     struct get_device_period_params params;
-    REFERENCE_TIME def_period, min_period;
-
-    TRACE("Requested duration %lu and period %lu\n", (ULONG)*duration, (ULONG)*period);

params.device     = This->device_name;
     params.flow       = This->dataflow;
-    params.def_period = &def_period;
-    params.min_period = &min_period;
+    params.def_period = def_period;
+    params.min_period = min_period;

wine_unix_call(get_device_period, &params);

-    if (FAILED(params.result))
-        return params.result;
+    TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)*def_period, (ULONG)*min_period);
+
+    *def_period = max(*def_period, min_def_period);
+
+    return params.result;
+}
+
+static HRESULT adjust_timing(struct audio_client *This,
+                             REFERENCE_TIME *duration, REFERENCE_TIME *period,
+                             const AUDCLNT_SHAREMODE mode, const DWORD flags,
+                             const WAVEFORMATEX *fmt)
+{
+    HRESULT hr;
+    REFERENCE_TIME def_period, min_period;

-    TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)def_period, (ULONG)min_period);
+    TRACE("Requested duration %lu and period %lu\n", (ULONG)*duration, (ULONG)*period);
+
+    if (FAILED(hr = get_periods(This, &def_period, &min_period)))
+        return hr;

if (mode == AUDCLNT_SHAREMODE_SHARED) {
         *period = def_period;
@@ -129,21 +140,21 @@ static HRESULT adjust_timing(struct audio_client *This,
         const WAVEFORMATEXTENSIBLE *fmtex = (WAVEFORMATEXTENSIBLE *)fmt;
         if (fmtex->Format.wFormatTag == WAVE_FORMAT_EXTENSIBLE &&
            (fmtex->dwChannelMask == 0 || fmtex->dwChannelMask & SPEAKER_RESERVED))
-            params.result = AUDCLNT_E_UNSUPPORTED_FORMAT;
+            hr = AUDCLNT_E_UNSUPPORTED_FORMAT;
         else {
             if (*period == 0)
                 *period = def_period;
             if (*period < min_period || *period > 5000000)
-                params.result = AUDCLNT_E_INVALID_DEVICE_PERIOD;
+                hr = AUDCLNT_E_INVALID_DEVICE_PERIOD;
             else if (*duration > 20000000) /* The smaller the period, the lower this limit. */
-                params.result = AUDCLNT_E_BUFFER_SIZE_ERROR;
+                hr = AUDCLNT_E_BUFFER_SIZE_ERROR;
             else if (flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK) {
                 if (*duration != *period)
-                    params.result = AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL;
+                    hr = AUDCLNT_E_BUFDURATION_PERIOD_NOT_EQUAL;

FIXME("EXCLUSIVE mode with EVENTCALLBACK\n");

-                params.result = AUDCLNT_E_DEVICE_IN_USE;
+                hr = AUDCLNT_E_DEVICE_IN_USE;
             } else if (*duration < 8 * *period)
                 *duration = 8 * *period; /* May grow above 2s. */
         }
@@ -151,7 +162,7 @@ static HRESULT adjust_timing(struct audio_client *This,

TRACE("Adjusted duration %lu and period %lu\n", (ULONG)*duration, (ULONG)*period);

-    return params.result;
+    return hr;
 }

static void dump_fmt(const WAVEFORMATEX *fmt)
@@ -748,21 +759,17 @@ static HRESULT WINAPI client_GetDevicePeriod(IAudioClient3 *iface, REFERENCE_TIM
                                       REFERENCE_TIME *minperiod)
 {
     struct audio_client *This = impl_from_IAudioClient3(iface);
-    struct get_device_period_params params;
+    HRESULT hr;

TRACE("(%p)->(%p, %p)\n", This, defperiod, minperiod);

if (!defperiod && !minperiod)
         return E_POINTER;

-    params.device     = This->device_name;
-    params.flow       = This->dataflow;
-    params.def_period = defperiod;
-    params.min_period = minperiod;
-
-    wine_unix_call(get_device_period, &params);
+    if (FAILED(hr = get_periods(This, defperiod, minperiod)))
+        return hr;

-    return params.result;
+    return hr;
 }

static HRESULT WINAPI client_Start(IAudioClient3 *iface)

[PATCH v3 2/2] winepulse: Don't cap period(s).

[Davide Beatrici]

From: Davide Beatrici

This driver, unlike the others, queries the engine for the actual device period.

The default period cap is not needed anymore because it now lives in mmdevapi.
The minimum period cap never mattered because exclusive mode is not supported.
---
 dlls/winepulse.drv/pulse.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c
index dd8d0b4441d..068245f03b2 100644
--- a/dlls/winepulse.drv/pulse.c
+++ b/dlls/winepulse.drv/pulse.c
@@ -113,9 +113,6 @@ static pa_mainloop *pulse_ml;
 static struct list g_phys_speakers = LIST_INIT(g_phys_speakers);
 static struct list g_phys_sources = LIST_INIT(g_phys_sources);

-static const REFERENCE_TIME MinimumPeriod = 30000;
-static const REFERENCE_TIME DefaultPeriod = 100000;
-
 static pthread_mutex_t pulse_mutex;
 static pthread_cond_t pulse_cond = PTHREAD_COND_INITIALIZER;

@@ -750,12 +747,6 @@ static void pulse_probe_settings(int render, const char *pulse_name, WAVEFORMATE
     if (length)
         *def_period = *min_period = pa_bytes_to_usec(10 * length, &ss);

-    if (*min_period < MinimumPeriod)
-        *min_period = MinimumPeriod;
-
-    if (*def_period < DefaultPeriod)
-        *def_period = DefaultPeriod;
-
     wfx->wFormatTag = WAVE_FORMAT_EXTENSIBLE;
     wfx->cbSize = sizeof(WAVEFORMATEXTENSIBLE) - sizeof(WAVEFORMATEX);

[PATCH v3 2/2] winepulse: Don't cap period(s).

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=146305

Your paranoid android.

=== debian11 (32 bit report) ===

Report validation errors:
mmdevapi:capture crashed (c0000005)
mmdevapi:render crashed (c0000005)

=== debian11b (64 bit WoW report) ===

Report validation errors:
mmdevapi:capture crashed (c0000005)
mmdevapi:render crashed (c0000005)

[Davide Beatrici (＠davidebeatrici)]

On Fri Jun 14 19:07:19 2024 +0000, Paul Gofman wrote:
> We should, yeah, it should depend strictly on what Windows has as buffer
> length and not what backend hints. Back then it was fixed this way in
> xaudio / faudio. I am not sure if mmdevapi ever queried that from
> backends but back then it had voluntary buffer size and that was also
> causing problems (1d66a108b901ebfb11b134e9a6895b1d87333638).

I addressed @huw's comment. I will create a separate merge request to enforce 10 ms as the default period.