[PATCH v4 0/1] MR8365: mmdevapi: Round min_period_frames.

[Paul Gofman (＠gofman)]

--
  v4: mmdevapi: Round min_period_frames up in client_GetSharedModeEnginePeriod().
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8365

[PATCH v4 1/1] mmdevapi: Round min_period_frames up in client_GetSharedModeEnginePeriod().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/mmdevapi/client.c       |  3 ++-
 dlls/mmdevapi/tests/render.c | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c
index 39cbae27b34..15ae213a1dc 100644
--- a/dlls/mmdevapi/client.c
+++ b/dlls/mmdevapi/client.c
@@ -1064,7 +1064,8 @@ static HRESULT WINAPI client_GetSharedModeEnginePeriod(IAudioClient3 *iface,
         return hr;
 
     *default_period_frames = def_period * format->nSamplesPerSec / (REFERENCE_TIME)10000000;
-    *min_period_frames     = min_period * format->nSamplesPerSec / (REFERENCE_TIME)10000000;
+    *min_period_frames     = (min_period * format->nSamplesPerSec + 10000000 - 1) / (REFERENCE_TIME)10000000;
+    *default_period_frames = max( *default_period_frames, *min_period_frames );
     *max_period_frames     = *default_period_frames;
     *unit_period_frames    = 1;
 
diff --git a/dlls/mmdevapi/tests/render.c b/dlls/mmdevapi/tests/render.c
index f9939fbcff6..9d9b5119afc 100644
--- a/dlls/mmdevapi/tests/render.c
+++ b/dlls/mmdevapi/tests/render.c
@@ -383,6 +383,20 @@ static void test_audioclient(void)
         hr = IMMDevice_Activate(dev, &IID_IAudioClient, CLSCTX_INPROC_SERVER,
                 NULL, (void**)&ac);
         ok(hr == S_OK, "Activation failed with %08lx\n", hr);
+
+        hr = IAudioClient_QueryInterface(ac, &IID_IAudioClient3, (void**)&ac3);
+        ok(hr == S_OK, "Failed to query IAudioClient3 interface: %08lx\n", hr);
+
+        hr = IAudioClient3_InitializeSharedAudioStream(
+            ac3, AUDCLNT_SHAREMODE_SHARED, min_period, pwfx, NULL);
+        ok(hr == S_OK, "InitializeSharedAudioStream returns %08lx\n", hr);
+
+        IAudioClient3_Release(ac3);
+        IAudioClient_Release(ac);
+
+        hr = IMMDevice_Activate(dev, &IID_IAudioClient, CLSCTX_INPROC_SERVER,
+                NULL, (void**)&ac);
+        ok(hr == S_OK, "Activation failed with %08lx\n", hr);
     }
     else
         win_skip("IAudioClient3 is not present\n");
-- 
GitLab

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

[Paul Gofman (＠gofman)]

v4:
 - rebase and drop test changes which are probably now not needed.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8365#note_116146

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/mmdevapi/tests/render.c:
> NULL, (void**)&ac);
>          ok(hr == S_OK, "Activation failed with %08lx\n", hr);
> +
> +        hr = IAudioClient_QueryInterface(ac, &IID_IAudioClient3, (void**)&ac3);
> +        ok(hr == S_OK, "Failed to query IAudioClient3 interface: %08lx\n", hr);
> +
> +        hr = IAudioClient3_InitializeSharedAudioStream(
> +            ac3, AUDCLNT_SHAREMODE_SHARED, min_period, pwfx, NULL);
> +        ok(hr == S_OK, "InitializeSharedAudioStream returns %08lx\n", hr);
> +
> +        IAudioClient3_Release(ac3);
> +        IAudioClient_Release(ac);
> +
> +        hr = IMMDevice_Activate(dev, &IID_IAudioClient, CLSCTX_INPROC_SERVER,
> +                NULL, (void**)&ac);
> +        ok(hr == S_OK, "Activation failed with %08lx\n", hr);

These tests don't seem to test the implemention change.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8365#note_116535

[Paul Gofman (＠gofman)]

On Sat Sep 20 13:21:32 2025 +0000, Huw Davies wrote:
> These tests don't seem to test the implemention change.
> Well, it does here with the implementation part of the patch reverted:
render.c:392: Test failed: InitializeSharedAudioStream returns 88890020

That currently depends on min_period received from get_periods() inside AudioClient3_GetSharedModeEnginePeriod() which depends on the host configured backend periods. Here get_periods() returns 26666 value for min_period, which I suppose a rather common pulseaudio default (no manual changes to pulse config here) and the problem with Malody V was reproduced on different setups as well. That, without the patch, results in 127 samples returned min period (for 48kHz sample rate), and then using that in stream initialization results in the too low value. With the patch it gets rounded up to 128 which reliably avoids the problem. 
Of course with some different backend / host sample rate the problem might not reproduce, that's why I didn't put the test separately with todo_wine before the fix.

Separately, I am not entirely sure if the current min period is correct, on Windows I see the same values for min and max periods and I don't know how to change that for shared mode stream. But assuming there is some cases when that period is different (maybe might happen on some different hardware on Windows) I guess we do need rounding here: the period in samples depends on sample rate and we probably can't instead be measuring the device periods in samples.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8365#note_116543