[PATCH v3 0/2] MR3351: mfmediaengine: Implement RemoveAllEffects().

[Bernhard Kölbl (＠besentv)]

--
  v3: mfmediaengine: Implement RemoveAllEffects().
      mfmediaengine/tests: Add tests for RemoveAllEffects().

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

[PATCH v3 1/2] mfmediaengine/tests: Add tests for RemoveAllEffects().

[Bernhard Kölbl]

From: Bernhard Kölbl

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

diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index aa9de414fcb..ab045b55096 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -1823,6 +1823,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);

@@ -1837,6 +1840,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);

@@ -1891,8 +1910,13 @@ done:
     {
         IMFMediaEngineEx_Shutdown(media_engine_ex);

+        hr = IMFMediaEngineEx_RemoveAllEffects(media_engine_ex);
+        todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr);
+
+        Sleep(50); /* Give mediaengine some time to properly shutdown. */
+
         ref = IMFMediaEngineEx_Release(media_engine_ex);
-        todo_wine ok(!ref, "Unexpected ref %lu.\n", ref);
+        ok(!ref, "Unexpected ref %lu.\n", ref);
     }

if (texture)
@@ -1914,12 +1938,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(&notify->IMFMediaEngineNotify_iface);

[PATCH v3 2/2] 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 | 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 ab045b55096..db3f851dfc2 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -1824,7 +1824,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);
@@ -1841,20 +1841,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,12 +1908,12 @@ 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);

Sleep(50); /* Give mediaengine some time to properly shutdown. */

ref = IMFMediaEngineEx_Release(media_engine_ex);
-        ok(!ref, "Unexpected ref %lu.\n", ref);
+        todo_wine ok(!ref, "Unexpected ref %lu.\n", ref);
     }

if (texture)
@@ -1938,12 +1935,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(&notify->IMFMediaEngineNotify_iface);

[Bernhard Kölbl (＠besentv)]

v3:
- Add lock.
- Wait to let shutdown happen.

[Bernhard Kölbl (＠besentv)]

On Wed Jul 19 14:50:50 2023 +0000, Bernhard Kölbl wrote:
> Iirc, doing a little sleep, like 50, did make it very consistent.

I think i found a better solution. Instead of sleeping, the tests also get consistent, when you transfer all 4 frames of the test video. I guess that some were still queued in the Windows pipeline, making shutdown in inconsistent.

[Nikolay Sivov (＠nsivov)]

I suggest to drop new tests then, until someone figures it out.

[Bernhard Kölbl (＠besentv)]

On Tue Jul 25 21:14:06 2023 +0000, Nikolay Sivov wrote:
> I suggest to drop new tests then, until someone figures it out.

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.