[PATCH v6 0/5] MR8862: mmdevapi/tests: Rewrite capturing tests, part 1/4.
I began working on this to address some flakyness (see e.g. https://gitlab.winehq.org/wine/wine/-/jobs/183916), and eventually I ended up rewriting `test_capture()` because the previous implementation looked somewhat incomplete. -- v6: mmdevapi/tests: Check that the received packet isn't larger than the padding. mmdevapi/tests: Check that GetBuffer() returns a packet of the expected size. mmdevapi/tests: Check that GetBuffer() fails when no packet is available. mmdevapi/tests: Introduce a helper to read many packets when capturing. https://gitlab.winehq.org/wine/wine/-/merge_requests/8862
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> Which makes sense to me given the interface documentation and works on the TestBot and physical machines I have, but doesn't work on the GitLab CI. --- dlls/mmdevapi/tests/render.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/mmdevapi/tests/render.c b/dlls/mmdevapi/tests/render.c index 3febeb4c4fb..f9939fbcff6 100644 --- a/dlls/mmdevapi/tests/render.c +++ b/dlls/mmdevapi/tests/render.c @@ -1460,10 +1460,11 @@ static void test_clock(int share) /* ok(hr == AUDCLNT_E_BUFFER_TOO_LARGE || (hr == S_OK && i==0) without todo_wine */ ok(hr == S_OK || hr == AUDCLNT_E_BUFFER_TOO_LARGE, "GetBuffer large (%u) failed: %08lx\n", avail, hr); - if(hr == S_OK && i) ok(FALSE, "GetBuffer large (%u) at iteration %d\n", avail, i); - /* Only the first iteration should allow that large a buffer + /* In theory only the first iteration should allow that large a buffer * as prefill was drained during the first 350+100ms sleep. - * Afterwards, only 100ms of data should find room per iteration. */ + * Afterwards, only 100ms of data should find room per iteration. + * However on some drivers a large buffer is allowed even at later + * iterations, so we don't explicitly check this. */ if(hr == S_OK) { trace("data at %p\n", data); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8862
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> test_capture() currently captures only one audio packet in each of its phases. That means that it the packet succeeds no check is performed on failures, and if it fails no check is performed on successes. By reading many packets each time we test more possible control flows. --- dlls/mmdevapi/tests/capture.c | 56 ++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/dlls/mmdevapi/tests/capture.c b/dlls/mmdevapi/tests/capture.c index e4f8eea5556..a2933468535 100644 --- a/dlls/mmdevapi/tests/capture.c +++ b/dlls/mmdevapi/tests/capture.c @@ -106,6 +106,44 @@ static void test_uninitialized(IAudioClient *ac) CloseHandle(handle); } +static void read_packets(IAudioClient *ac, IAudioCaptureClient *acc, HANDLE handle, + unsigned int packet_count) +{ + unsigned int idx = 0; + HRESULT hr; + + while (idx < packet_count) + { + UINT32 next_packet_size, frames; + UINT64 dev_pos, qpc_pos; + DWORD flags; + BYTE *ptr; + + winetest_push_context("packet %u", idx); + + hr = IAudioCaptureClient_GetNextPacketSize(acc, &next_packet_size); + ok(hr == S_OK, "GetNextPacketSize returns %08lx\n", hr); + + if (next_packet_size == 0) + { + ok(WaitForSingleObject(handle, 1000) == WAIT_OBJECT_0, "Waiting on event handle failed!\n"); + + winetest_pop_context(); + continue; + } + + hr = IAudioCaptureClient_GetBuffer(acc, &ptr, &frames, &flags, &dev_pos, &qpc_pos); + ok(hr == S_OK, "GetBuffer returns %08lx\n", hr); + + hr = IAudioCaptureClient_ReleaseBuffer(acc, frames); + ok(hr == S_OK, "Releasing buffer returns %08lx\n", hr); + + ++idx; + + winetest_pop_context(); + } +} + static void test_capture(IAudioClient *ac, HANDLE handle, WAVEFORMATEX *wfx) { IAudioCaptureClient *acc; @@ -396,23 +434,7 @@ static void test_capture(IAudioClient *ac, HANDLE handle, WAVEFORMATEX *wfx) hr = IAudioClient_Start(ac); ok(hr == S_OK, "Start on a stopped stream returns %08lx\n", hr); - Sleep(180); - - hr = IAudioClient_GetCurrentPadding(ac, &pad); - ok(hr == S_OK, "GetCurrentPadding call returns %08lx\n", hr); - - hr = IAudioCaptureClient_GetBuffer(acc, &data, &frames, &flags, &pos, &qpc); - flaky_wine - ok(hr == S_OK, "Valid IAudioCaptureClient_GetBuffer returns %08lx\n", hr); - trace("Running position %d pad %u flags %lx, amount of frames locked: %u\n", - SUCCEEDED(hr) ? (UINT)pos : -1, pad, flags, frames); - - if(SUCCEEDED(hr)){ - /* Some w7 machines signal DATA_DISCONTINUITY here following the - * previous AUDCLNT_S_BUFFER_EMPTY, others not. What logic? */ - ok(pos >= sum, "Position %u gap %d\n", (UINT)pos, (UINT)pos - sum); - IAudioCaptureClient_ReleaseBuffer(acc, frames); - } + read_packets(ac, acc, handle, 10); IAudioCaptureClient_Release(acc); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8862
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mmdevapi/tests/capture.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/mmdevapi/tests/capture.c b/dlls/mmdevapi/tests/capture.c index a2933468535..1ef9fd26184 100644 --- a/dlls/mmdevapi/tests/capture.c +++ b/dlls/mmdevapi/tests/capture.c @@ -126,6 +126,20 @@ static void read_packets(IAudioClient *ac, IAudioCaptureClient *acc, HANDLE hand if (next_packet_size == 0) { + /* There is some room for flakyness here, in theory a packet could arrive between the + * GetNextPacketSize() call and the GetBuffer() call. Hopefully this is not very probable. */ + ptr = (void*)0xdeadf00ddeadf00d; + flags = 0xdeadf11d; + dev_pos = 0xdeadf22ddeadf22d; + qpc_pos = 0xdeadf33ddeadf33d; + hr = IAudioCaptureClient_GetBuffer(acc, &ptr, &frames, &flags, &dev_pos, &qpc_pos); + ok(hr == AUDCLNT_S_BUFFER_EMPTY, "GetBuffer returns %08lx\n", hr); + ok(broken((uintptr_t)ptr == (uintptr_t)0xdeadf00ddeadf00d) || /* <= win8 */ + !ptr, "Unexpected data after GetBuffer: %p\n", ptr); + ok(flags == 0xdeadf11d, "Unexpected flags after GetBuffer: %08lx\n", flags); + ok(dev_pos == 0xdeadf22ddeadf22d, "Unexpected device position after GetBuffer: %I64u\n", dev_pos); + ok(qpc_pos == 0xdeadf33ddeadf33d, "Unexpected QPC position after GetBuffer: %I64u\n", qpc_pos); + ok(WaitForSingleObject(handle, 1000) == WAIT_OBJECT_0, "Waiting on event handle failed!\n"); winetest_pop_context(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8862
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mmdevapi/tests/capture.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/mmdevapi/tests/capture.c b/dlls/mmdevapi/tests/capture.c index 1ef9fd26184..600ed24d05f 100644 --- a/dlls/mmdevapi/tests/capture.c +++ b/dlls/mmdevapi/tests/capture.c @@ -149,6 +149,9 @@ static void read_packets(IAudioClient *ac, IAudioCaptureClient *acc, HANDLE hand hr = IAudioCaptureClient_GetBuffer(acc, &ptr, &frames, &flags, &dev_pos, &qpc_pos); ok(hr == S_OK, "GetBuffer returns %08lx\n", hr); + ok(next_packet_size == frames, "GetNextPacketSize returns %u, GetBuffer returns %u frames\n", + next_packet_size, frames); + hr = IAudioCaptureClient_ReleaseBuffer(acc, frames); ok(hr == S_OK, "Releasing buffer returns %08lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8862
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mmdevapi/tests/capture.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/mmdevapi/tests/capture.c b/dlls/mmdevapi/tests/capture.c index 600ed24d05f..c9ee23b8d51 100644 --- a/dlls/mmdevapi/tests/capture.c +++ b/dlls/mmdevapi/tests/capture.c @@ -114,7 +114,7 @@ static void read_packets(IAudioClient *ac, IAudioCaptureClient *acc, HANDLE hand while (idx < packet_count) { - UINT32 next_packet_size, frames; + UINT32 next_packet_size, frames, padding; UINT64 dev_pos, qpc_pos; DWORD flags; BYTE *ptr; @@ -152,6 +152,11 @@ static void read_packets(IAudioClient *ac, IAudioCaptureClient *acc, HANDLE hand ok(next_packet_size == frames, "GetNextPacketSize returns %u, GetBuffer returns %u frames\n", next_packet_size, frames); + hr = IAudioClient_GetCurrentPadding(ac, &padding); + ok(hr == S_OK, "GetCurrentPadding returns %08lx\n", hr); + ok(padding >= frames, "GetCurrentPadding returns %u, GetBuffer returns %u frames\n", + padding, frames); + hr = IAudioCaptureClient_ReleaseBuffer(acc, frames); ok(hr == S_OK, "Releasing buffer returns %08lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8862
On Tue Sep 9 11:28:57 2025 +0000, Giovanni Mascellani wrote:
Ok. Ignore the latest push, though, that's not ready. Done, but the commit count rose to 20, so they'll come in installments.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8862#note_115781
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8862
participants (3)
-
Giovanni Mascellani -
Giovanni Mascellani (＠giomasce)
-
Huw Davies (＠huw)