[PATCH v4 0/3] MR3351: mfmediaengine: Implement RemoveAllEffects().
-- v4: mfmediaengine: Implement RemoveAllEffects(). mfmediaengine/tests: Add tests for RemoveAllEffects(). mfmediaengine/tests: Make ref checks more consistent. https://gitlab.winehq.org/wine/wine/-/merge_requests/3351
From: Bernhard Kölbl <bkoelbl(a)codeweavers.com> Do this by waiting for the first available frame with a separate ready event. Signed-off-by: Bernhard Kölbl <bkoelbl(a)codeweavers.com> --- dlls/mfmediaengine/tests/mfmediaengine.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c index aa9de414fcb..38063d1d21a 100644 --- a/dlls/mfmediaengine/tests/mfmediaengine.c +++ b/dlls/mfmediaengine/tests/mfmediaengine.c @@ -1139,7 +1139,7 @@ struct test_transfer_notify LONG refcount; IMFMediaEngineEx *media_engine; - HANDLE ready_event; + HANDLE ready_event, frame_ready_event; HRESULT error; }; @@ -1175,6 +1175,7 @@ static ULONG WINAPI test_transfer_notify_Release(IMFMediaEngineNotify *iface) if (!refcount) { + CloseHandle(notify->frame_ready_event); CloseHandle(notify->ready_event); free(notify); } @@ -1211,6 +1212,8 @@ static HRESULT WINAPI test_transfer_notify_EventNotify(IMFMediaEngineNotify *ifa notify->error = param2; /* fallthrough */ case MF_MEDIA_ENGINE_EVENT_FIRSTFRAMEREADY: + SetEvent(notify->frame_ready_event); + break; case MF_MEDIA_ENGINE_EVENT_TIMEUPDATE: SetEvent(notify->ready_event); break; @@ -1237,6 +1240,9 @@ static struct test_transfer_notify *create_transfer_notify(void) object->ready_event = CreateEventW(NULL, FALSE, FALSE, NULL); ok(!!object->ready_event, "Failed to create an event, error %lu.\n", GetLastError()); + object->frame_ready_event = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(!!object->frame_ready_event, "Failed to create an event, error %lu.\n", GetLastError()); + return object; } @@ -1295,7 +1301,7 @@ static void test_TransferVideoFrame(void) SysFreeString(url); IMFByteStream_Release(stream); - res = WaitForSingleObject(notify->ready_event, 5000); + res = WaitForSingleObject(notify->frame_ready_event, 5000); ok(!res, "Unexpected res %#lx.\n", res); if (FAILED(notify->error)) @@ -1858,14 +1864,9 @@ static void test_effect(void) IMFByteStream_Release(stream); /* Wait for MediaEngine to be ready. */ - res = WaitForSingleObject(notify->ready_event, 5000); + res = WaitForSingleObject(notify->frame_ready_event, 5000); ok(!res, "Unexpected res %#lx.\n", res); - /* Wait for another update. This makes MediaEngine shutdown more consistent on Windows. */ - res = WaitForSingleObject(notify->ready_event, 500); - /* Timeupdates are missing in Wine. */ - todo_wine ok(!res, "Unexpected res %#lx.\n", res); - SetRect(&dst_rect, 0, 0, desc.Width, desc.Height); hr = IMFMediaEngineEx_TransferVideoFrame(notify->media_engine, (IUnknown *)texture, NULL, &dst_rect, NULL); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3351
From: Bernhard Kölbl <bkoelbl(a)codeweavers.com> Signed-off-by: Bernhard Kölbl <bkoelbl(a)codeweavers.com> --- dlls/mfmediaengine/tests/mfmediaengine.c | 28 +++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c index 38063d1d21a..829f056d2a2 100644 --- a/dlls/mfmediaengine/tests/mfmediaengine.c +++ b/dlls/mfmediaengine/tests/mfmediaengine.c @@ -1829,6 +1829,9 @@ static void test_effect(void) hr = ID3D11Device_CreateTexture2D(device, &desc, NULL, &texture); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = passthrough_mft_create(0, &video_effect); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -1843,6 +1846,22 @@ static void test_effect(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); EXPECT_REF(&video_effect2->IMFTransform_iface, 2); + hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine EXPECT_REF(&video_effect->IMFTransform_iface, 1); + todo_wine EXPECT_REF(&video_effect2->IMFTransform_iface, 1); + + if (FAILED(hr)) /* Temporary skip */ + goto done; + + hr = IMFMediaEngineEx_InsertVideoEffect(media_engine_ex, (IUnknown *)&video_effect->IMFTransform_iface, FALSE); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine EXPECT_REF(&video_effect->IMFTransform_iface, 2); + + hr = IMFMediaEngineEx_InsertVideoEffect(media_engine_ex, (IUnknown *)&video_effect2->IMFTransform_iface, FALSE); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine EXPECT_REF(&video_effect2->IMFTransform_iface, 2); + hr = passthrough_mft_create(0, &audio_effect); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -1892,8 +1911,11 @@ done: { IMFMediaEngineEx_Shutdown(media_engine_ex); + hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex); + todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + ref = IMFMediaEngineEx_Release(media_engine_ex); - todo_wine ok(!ref, "Unexpected ref %lu.\n", ref); + ok(!ref, "Unexpected ref %lu.\n", ref); } if (texture) @@ -1915,12 +1937,12 @@ done: if (video_effect2) { ref = IMFTransform_Release(&video_effect2->IMFTransform_iface); - todo_wine ok(!ref, "Unexpected ref %lu.\n", ref); + ok(!ref, "Unexpected ref %lu.\n", ref); } if (video_effect) { ref = IMFTransform_Release(&video_effect->IMFTransform_iface); - todo_wine ok(!ref, "Unexpected ref %lu.\n", ref); + ok(!ref, "Unexpected ref %lu.\n", ref); } IMFMediaEngineNotify_Release(¬ify->IMFMediaEngineNotify_iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3351
From: Bernhard Kölbl <bkoelbl(a)codeweavers.com> Signed-off-by: Bernhard Kölbl <bkoelbl(a)codeweavers.com> --- dlls/mfmediaengine/main.c | 17 +++++++++++++++-- dlls/mfmediaengine/tests/mfmediaengine.c | 23 ++++++++++------------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c index 925d9c3566e..32127a716ea 100644 --- a/dlls/mfmediaengine/main.c +++ b/dlls/mfmediaengine/main.c @@ -2719,9 +2719,22 @@ static HRESULT WINAPI media_engine_InsertAudioEffect(IMFMediaEngineEx *iface, IU static HRESULT WINAPI media_engine_RemoveAllEffects(IMFMediaEngineEx *iface) { - FIXME("%p stub.\n", iface); + struct media_engine *engine = impl_from_IMFMediaEngineEx(iface); + HRESULT hr = S_OK; - return E_NOTIMPL; + TRACE("%p.\n", iface); + + EnterCriticalSection(&engine->cs); + if (engine->flags & FLAGS_ENGINE_SHUT_DOWN) + hr = MF_E_SHUTDOWN; + else + { + media_engine_clear_effects(&engine->audio_effects); + media_engine_clear_effects(&engine->video_effects); + } + LeaveCriticalSection(&engine->cs); + + return hr; } static HRESULT WINAPI media_engine_SetTimelineMarkerTimer(IMFMediaEngineEx *iface, double timeout) diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c index 829f056d2a2..f4914496ca9 100644 --- a/dlls/mfmediaengine/tests/mfmediaengine.c +++ b/dlls/mfmediaengine/tests/mfmediaengine.c @@ -1830,7 +1830,7 @@ static void test_effect(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = passthrough_mft_create(0, &video_effect); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -1847,20 +1847,17 @@ static void test_effect(void) EXPECT_REF(&video_effect2->IMFTransform_iface, 2); hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine EXPECT_REF(&video_effect->IMFTransform_iface, 1); - todo_wine EXPECT_REF(&video_effect2->IMFTransform_iface, 1); - - if (FAILED(hr)) /* Temporary skip */ - goto done; + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + EXPECT_REF(&video_effect->IMFTransform_iface, 1); + EXPECT_REF(&video_effect2->IMFTransform_iface, 1); hr = IMFMediaEngineEx_InsertVideoEffect(media_engine_ex, (IUnknown *)&video_effect->IMFTransform_iface, FALSE); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine EXPECT_REF(&video_effect->IMFTransform_iface, 2); + EXPECT_REF(&video_effect->IMFTransform_iface, 2); hr = IMFMediaEngineEx_InsertVideoEffect(media_engine_ex, (IUnknown *)&video_effect2->IMFTransform_iface, FALSE); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine EXPECT_REF(&video_effect2->IMFTransform_iface, 2); + EXPECT_REF(&video_effect2->IMFTransform_iface, 2); hr = passthrough_mft_create(0, &audio_effect); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -1912,10 +1909,10 @@ done: IMFMediaEngineEx_Shutdown(media_engine_ex); hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex); - todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); + ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); ref = IMFMediaEngineEx_Release(media_engine_ex); - ok(!ref, "Unexpected ref %lu.\n", ref); + todo_wine ok(!ref, "Unexpected ref %lu.\n", ref); } if (texture) @@ -1937,12 +1934,12 @@ done: if (video_effect2) { ref = IMFTransform_Release(&video_effect2->IMFTransform_iface); - ok(!ref, "Unexpected ref %lu.\n", ref); + todo_wine ok(!ref, "Unexpected ref %lu.\n", ref); } if (video_effect) { ref = IMFTransform_Release(&video_effect->IMFTransform_iface); - ok(!ref, "Unexpected ref %lu.\n", ref); + todo_wine ok(!ref, "Unexpected ref %lu.\n", ref); } IMFMediaEngineNotify_Release(¬ify->IMFMediaEngineNotify_iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3351
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=135258 Your paranoid android. === debian11b (64 bit WoW report) === win32u: win32u.c:1057: Test failed: windows.devices.bluetooth:bluetooth start dlls/windows.devices.bluetooth/tests/bluetooth.c
v4: - Improve test consistency. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3351#note_40240
On Tue Jul 25 21:14:06 2023 +0000, Bernhard Kölbl wrote:
I found the issue, ready event waits for first frame ready and timeupdate at the same time, which is wrong. we only should wait for the first frame. Little side note, I ran the reworked test on my local Win 11 machine 50 times in a row without failure, so I think it can be considered consistent. But feel free To test it yourself.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3351#note_40266
This now managed to hang for me, once. With: ``` 00d4:fixme:mfplat:bytestream_file_Close 00000000008F7320 0140:err:sync:RtlpWaitForCriticalSection section 00000000008FD9F0 "../../wine-git/dlls/winegstreamer/media_source.c: cs" wait timed out in thread 0140, blocked by 00d4, retrying (60 sec) 0114:err:sync:RtlpWaitForCriticalSection section 00000000008F85C0 (null) wait timed out in thread 0114, blocked by 00d4, retrying (60 sec) ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3351#note_40333
On Wed Jul 26 17:03:33 2023 +0000, Nikolay Sivov wrote:
This now manages to hang for me, not on every run. With: ``` 00d4:fixme:mfplat:bytestream_file_Close 00000000008F7320 0140:err:sync:RtlpWaitForCriticalSection section 00000000008FD9F0 "../../wine-git/dlls/winegstreamer/media_source.c: cs" wait timed out in thread 0140, blocked by 00d4, retrying (60 sec) 0114:err:sync:RtlpWaitForCriticalSection section 00000000008F85C0 (null) wait timed out in thread 0114, blocked by 00d4, retrying (60 sec) ``` Okay i got this in 1/100 tries, I'll see what i can do.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3351#note_40355
On Wed Jul 26 17:03:33 2023 +0000, Bernhard Kölbl wrote:
Okay i got this in 1/100 tries, I'll see what i can do. So, why was it locking up?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3351#note_40877
participants (3)
-
Bernhard Kölbl -
Marvin -
Nikolay Sivov (＠nsivov)