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

--
  v4: winepulse: Don't set a floor for the period(s).
      mmdevapi: Set the default period to a minimum of 10 ms.

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

[PATCH v4 1/2] mmdevapi: Set the default period to a minimum of 10 ms.

[Davide Beatrici]

From: Davide Beatrici

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

diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c
index 0b849e47e3d..ed635bcf98a 100644
--- a/dlls/mmdevapi/client.c
+++ b/dlls/mmdevapi/client.c
@@ -99,25 +99,36 @@ 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,
+static HRESULT get_periods(struct audio_client *client,
+                           REFERENCE_TIME *def_period, REFERENCE_TIME *min_period)
+{
+    static const REFERENCE_TIME min_def_period = 100000; /* 10 ms */
+    struct get_device_period_params params;
+
+    params.device     = client->device_name;
+    params.flow       = client->dataflow;
+    params.def_period = def_period;
+    params.min_period = min_period;
+
+    wine_unix_call(get_device_period, &params);
+
+    if (def_period) *def_period = max(*def_period, min_def_period);
+
+    return params.result;
+}
+
+static HRESULT adjust_timing(struct audio_client *client,
                              REFERENCE_TIME *duration, REFERENCE_TIME *period,
                              const AUDCLNT_SHAREMODE mode, const DWORD flags,
                              const WAVEFORMATEX *fmt)
 {
-    struct get_device_period_params params;
     REFERENCE_TIME def_period, min_period;
+    HRESULT hr;

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;
-
-    wine_unix_call(get_device_period, &params);
-
-    if (FAILED(params.result))
-        return params.result;
+    if (FAILED(hr = get_periods(client, &def_period, &min_period)))
+        return hr;

TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)def_period, (ULONG)min_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,13 @@ 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;

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);
-
-    return params.result;
+    return get_periods(This, defperiod, minperiod);
 }

static HRESULT WINAPI client_Start(IAudioClient3 *iface)

[PATCH v4 2/2] winepulse: Don't set a floor for the period(s).

[Davide Beatrici]

From: Davide Beatrici

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

The default period floor is not needed anymore because it now lives in mmdevapi.
The minimum period floor 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);

[Huw Davies (＠huw)]

I've pushed a few fixups.

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

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.

[Davide Beatrici (＠davidebeatrici)]

On Sat Jun 15 16:24:53 2024 +0000, Huw Davies wrote:
> I've pushed a few fixups.

Thank you!

[PATCH v4 0/2] MR5810: Set the default period to a minimum of 10 ms in mmdevapi instead of the audio driver.

[Davide Beatrici (＠davidebeatrici)]

Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/client.c:
> +static HRESULT get_periods(struct audio_client *client,
>                             REFERENCE_TIME *def_period, REFERENCE_TIME *min_period)
>  {
>      static const REFERENCE_TIME min_def_period = 100000; /* 10 ms */
>      struct get_device_period_params params;
> - params.device     = This->device_name;
> - params.flow       = This->dataflow;
> -
>
> params.device     = client->device_name;
> -
>
> params.flow       = client->dataflow;
> params.def_period = def_period;
> params.min_period = min_period;
>
> wine_unix_call(get_device_period, &params);
> - TRACE("Device periods: %lu default and %lu minimum\n", (ULONG)*def_period, (ULONG)*min_period);

For reference, the reason why I moved this debug message here is to know the default period reported by the backend in case of problems (e.g. glitches, xruns).

However, I believe it's better to write one in the driver's code instead.

[PATCH v4 0/2] MR5810: Set the default period to a minimum of 10 ms in mmdevapi instead of the audio driver.

[Huw Davies (＠huw)]

On Sat Jun 15 16:30:30 2024 +0000, Davide Beatrici wrote:
> For reference, the reason why I moved this debug message here is to know
> the default period reported by the backend in case of problems (e.g.
> glitches, xruns).
> However, I believe it's better to write one in the driver's code instead.

`def_period` or `min_period` can be `NULL` when this is called from `IAudioClient_GetDefaultPeriod()` and I didn't want to add the conditionals to deal with this.