[PATCH v6 0/3] MR3351: mfmediaengine: Implement RemoveAllEffects().

[Bernhard Kölbl (＠besentv)]

--
  v6: mfmediaengine: Implement RemoveAllEffects().
      mfmediaengine/tests: Add tests for RemoveAllEffects().
      mfmediaengine/tests: Make effect tests more consistent.

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

[PATCH v6 1/3] mfmediaengine/tests: Make effect tests more consistent.

[Bernhard Kölbl]

From: Bernhard Kölbl

Do this by waiting for the first available frame with a
separate ready event and removing flaky ref checks.

Signed-off-by: Bernhard Kölbl 
---
 dlls/mfmediaengine/tests/mfmediaengine.c | 42 ++++++++----------------
 1 file changed, 14 insertions(+), 28 deletions(-)

diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index aa9de414fcb..4eb04c8e7ef 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))
@@ -1787,7 +1793,6 @@ static void test_effect(void)
     UINT token;
     HRESULT hr;
     DWORD res;
-    ULONG ref;
     BSTR url;

stream = load_resource(L"i420-64x64.avi", L"video/avi");
@@ -1858,14 +1863,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);
@@ -1890,9 +1890,7 @@ done:
     if (media_engine_ex)
     {
         IMFMediaEngineEx_Shutdown(media_engine_ex);
-
-        ref = IMFMediaEngineEx_Release(media_engine_ex);
-        todo_wine ok(!ref, "Unexpected ref %lu.\n", ref);
+        IMFMediaEngineEx_Release(media_engine_ex);
     }

if (texture)
@@ -1901,26 +1899,14 @@ done:
         ID3D11Device_Release(device);

if (audio_effect2)
-    {
-        ref = IMFTransform_Release(&audio_effect2->IMFTransform_iface);
-        todo_wine ok(!ref, "Unexpected ref %lu.\n", ref);
-    }
+        IMFTransform_Release(&audio_effect2->IMFTransform_iface);
     if (audio_effect)
-    {
-        ref = IMFTransform_Release(&audio_effect->IMFTransform_iface);
-        todo_wine ok(!ref, "Unexpected ref %lu.\n", ref);
-    }
+        IMFTransform_Release(&audio_effect->IMFTransform_iface);

if (video_effect2)
-    {
-        ref = IMFTransform_Release(&video_effect2->IMFTransform_iface);
-        todo_wine ok(!ref, "Unexpected ref %lu.\n", ref);
-    }
+        IMFTransform_Release(&video_effect2->IMFTransform_iface);
     if (video_effect)
-    {
-        ref = IMFTransform_Release(&video_effect->IMFTransform_iface);
-        todo_wine ok(!ref, "Unexpected ref %lu.\n", ref);
-    }
+        IMFTransform_Release(&video_effect->IMFTransform_iface);

IMFMediaEngineNotify_Release(&notify->IMFMediaEngineNotify_iface);
 }

[PATCH v6 2/3] mfmediaengine/tests: Add tests for RemoveAllEffects().

[Bernhard Kölbl]

From: Bernhard Kölbl

Signed-off-by: Bernhard Kölbl 
---
 dlls/mfmediaengine/tests/mfmediaengine.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index 4eb04c8e7ef..3fbcd8c4d1d 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -1828,6 +1828,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);

@@ -1842,6 +1845,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);

@@ -1890,6 +1909,10 @@ done:
     if (media_engine_ex)
     {
         IMFMediaEngineEx_Shutdown(media_engine_ex);
+
+        hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex);
+        todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr);
+
         IMFMediaEngineEx_Release(media_engine_ex);
     }

[PATCH v6 3/3] mfmediaengine: Implement RemoveAllEffects().

[Bernhard Kölbl]

From: Bernhard Kölbl

Signed-off-by: Bernhard Kölbl 
---
 dlls/mfmediaengine/main.c                | 17 +++++++++++++++--
 dlls/mfmediaengine/tests/mfmediaengine.c | 17 +++++++----------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c
index e664db81d1d..2f8addaf873 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 3fbcd8c4d1d..a3d9a2bbf75 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -1829,7 +1829,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);
@@ -1846,20 +1846,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);
@@ -1911,7 +1908,7 @@ 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);

IMFMediaEngineEx_Release(media_engine_ex);
     }

[Bernhard Kölbl (＠besentv)]

v6:
- I'm done so I just removed the annoying tests. :joy: