[PATCH v2 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
```

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

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

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

[Davide Beatrici]

From: Davide Beatrici

---
 dlls/mmdevapi/client.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c
index 0b849e47e3d..a760256540a 100644
--- a/dlls/mmdevapi/client.c
+++ b/dlls/mmdevapi/client.c
@@ -47,6 +47,8 @@ extern HRESULT get_audio_session(const GUID *sessionguid, IMMDevice *device, UIN
                                  struct audio_session **out);
 extern struct audio_session_wrapper *session_wrapper_create(struct audio_client *client);

+static const REFERENCE_TIME min_def_period = 100000;
+
 static HANDLE main_loop_thread;

void main_loop_stop(void)
@@ -122,7 +124,7 @@ static HRESULT adjust_timing(struct audio_client *This,
     TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)def_period, (ULONG)min_period);

if (mode == AUDCLNT_SHAREMODE_SHARED) {
-        *period = def_period;
+        *period = max(def_period, min_def_period);
         if (*duration < 3 * *period)
             *duration = 3 * *period;
     } else {
@@ -132,7 +134,7 @@ static HRESULT adjust_timing(struct audio_client *This,
             params.result = AUDCLNT_E_UNSUPPORTED_FORMAT;
         else {
             if (*period == 0)
-                *period = def_period;
+                *period = max(def_period, min_def_period);
             if (*period < min_period || *period > 5000000)
                 params.result = AUDCLNT_E_INVALID_DEVICE_PERIOD;
             else if (*duration > 20000000) /* The smaller the period, the lower this limit. */
@@ -762,6 +764,9 @@ static HRESULT WINAPI client_GetDevicePeriod(IAudioClient3 *iface, REFERENCE_TIM

wine_unix_call(get_device_period, &params);

+    if (defperiod && *defperiod < min_def_period)
+        *defperiod = min_def_period;
+
     return params.result;
 }

[PATCH v2 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);

[Davide Beatrici (＠davidebeatrici)]

> Do we not need to do this in the exclusive case?

Good catch, we do as there is no way to communicate to the application that we chose a period other than 10 ms (which is why `IAudioClient3` exists in the first place).
> Also, 'cap' to me implies a maximum value. I'd try `min_def_period` and change the commit msg.

Agreed.

[Huw Davies (＠huw)]

> > Do we not need to do this in the exclusive case?
>
> Good catch, we do as there is no way to communicate to the application that we chose a period other than 10 ms [].

That being the case, I think it would make sense to add a helper function to do the unix call and fix-up the default period.

[Paul Gofman (＠gofman)]

> Good catch, we do as there is no way to communicate to the application that we chose a period other than 10 ms (which is why `IAudioClient3` exists in the first place).

At least for older interfaces across sound APIs there were a few games that firmly assume 10ms (and overflow buffers with larger) periods even if there is a way to communicate a different period.

[Davide Beatrici (＠davidebeatrici)]

On Fri Jun 14 18:55:45 2024 +0000, Paul Gofman wrote:
> > Good catch, we do as there is no way to communicate to the application
>
> that we chose a period other than 10 ms (which is why `IAudioClient3`
> exists in the first place).
> At least for older interfaces across sound APIs there were a few games
> that firmly assume 10ms (and overflow buffers with larger) periods even
> if there is a way to communicate a different period.

On that note, I was thinking: should we perhaps force 10 ms even if the UNIX driver reports a higher period?

[Paul Gofman (＠gofman)]

On Fri Jun 14 19:03:55 2024 +0000, Davide Beatrici wrote:
> On that note, I was thinking: should we perhaps force 10 ms even if the
> UNIX driver reports a higher period?

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).