[PATCH 0/2] MR3319: Draft: Leave critical section before waiting for streaming thread.

[Santino Mazza (＠tati1454)]

This is just a draft to receive feedback, I'm not sure how could I test this behavior, right now the test I wrote works but there are times where it doesn't enter into the deadlock.

Also I'm not sure if leaving the critical section before waiting for the streaming thread is the correct way to solve the problem.

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

[PATCH 1/2] mfplat/test: Test for topology changes.

[Santino Mazza]

From: Santino Mazza <smazza(a)codeweavers.com>

---
 dlls/mf/tests/mf.c | 130 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c
index fa538e8ad9f..48c87573ea8 100644
--- a/dlls/mf/tests/mf.c
+++ b/dlls/mf/tests/mf.c
@@ -7050,6 +7050,135 @@ static void test_mpeg4_media_sink(void)
     IMFMediaType_Release(audio_type);
 }
 
+static void add_source_topology(IMFMediaSource *source, HWND hwnd, IMFTopology *topology)
+{
+    IMFTopologyNode *src_node, *sink_node;
+    IMFMediaTypeHandler *typehandler;
+    IMFPresentationDescriptor *pd;
+    IMFActivate *sink_activate;
+    IMFMediaType *output_type;
+    IMFStreamDescriptor *sd;
+    GUID major_type;
+    DWORD sd_count;
+    BOOL selected;
+    HRESULT hr;
+
+    hr = IMFMediaSource_CreatePresentationDescriptor(source, &pd);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IMFPresentationDescriptor_GetStreamDescriptorCount(pd, &sd_count);
+    ok(hr == S_OK, "Unexpected hr %#lx\n", hr);
+
+    for (int i = 0; i < sd_count; ++i)
+    {
+        hr = MFCreateTopologyNode(MF_TOPOLOGY_OUTPUT_NODE, &sink_node);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        hr = MFCreateTopologyNode(MF_TOPOLOGY_SOURCESTREAM_NODE, &src_node);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+        hr = IMFPresentationDescriptor_GetStreamDescriptorByIndex(pd, i, &selected, &sd);
+        ok(selected, "got selected %u.\n", !!selected);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        init_source_node(source, -1, src_node, pd, sd);
+
+        hr = IMFStreamDescriptor_GetMediaTypeHandler(sd, &typehandler);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        hr = IMFMediaTypeHandler_GetCurrentMediaType(typehandler, &output_type);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+        hr = IMFMediaType_GetMajorType(output_type, &major_type);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+        if (IsEqualGUID(&major_type, &MFMediaType_Video))
+        {
+            hr = MFCreateVideoRendererActivate(hwnd, &sink_activate);
+            ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        }
+        else if (IsEqualGUID(&major_type, &MFMediaType_Audio))
+        {
+            hr = MFCreateAudioRendererActivate(&sink_activate);
+            ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        }
+
+        hr = IMFTopologyNode_SetObject(sink_node, (IUnknown *)sink_activate);
+        ok(hr == S_OK, "Failed to set object, hr %#lx.\n", hr);
+        hr = IMFTopologyNode_SetUINT32(sink_node, &MF_TOPONODE_CONNECT_METHOD, MF_CONNECT_ALLOW_DECODER);
+        ok(hr == S_OK, "Failed to set connect method, hr %#lx.\n", hr);
+
+        hr = IMFTopology_AddNode(topology, sink_node);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        hr = IMFTopology_AddNode(topology, src_node);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        hr = IMFTopologyNode_ConnectOutput(src_node, 0, sink_node, 0);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        IMFTopologyNode_Release(src_node);
+        IMFTopologyNode_Release(sink_node);
+        IMFStreamDescriptor_Release(sd);
+        IMFMediaTypeHandler_Release(typehandler);
+        IMFMediaType_Release(output_type);
+        IMFActivate_Release(sink_activate);
+    }
+
+    IMFPresentationDescriptor_Release(pd);
+}
+
+static void test_topology_change(void)
+{
+    IMFAsyncCallback *callback;
+    IMFMediaSession *session;
+    IMFMediaSource *source;
+    IMFTopology *topology;
+    PROPVARIANT propvar;
+    HRESULT hr;
+    HWND hwnd;
+
+    hr = MFStartup(MF_VERSION, MFSTARTUP_FULL);
+    ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr);
+
+    if (!(source = create_media_source(L"test.mp4", L"video/mp4")))
+    {
+        win_skip("MP4 media source is not supported, skipping tests.\n");
+        return;
+    }
+
+    callback = create_test_callback(TRUE);
+
+    hr = MFCreateMediaSession(NULL, &session);
+    ok(hr == S_OK, "Failed to create media session, hr %#lx.\n", hr);
+
+    hr = MFCreateTopology(&topology);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hwnd = create_window();
+    add_source_topology(source, hwnd, topology);
+
+    hr = IMFMediaSession_SetTopology(session, MFSESSION_SETTOPOLOGY_IMMEDIATE, topology);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = wait_media_event_until_blocking(session, callback, MESessionTopologySet, 1000, &propvar);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(propvar.vt == VT_UNKNOWN, "got vt %u\n", propvar.vt);
+
+    propvar.vt = VT_EMPTY;
+    hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = wait_media_event_until_blocking(session, callback, MESessionStarted, 1000, &propvar);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt);
+
+    hr = IMFMediaSession_Stop(session);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = wait_media_event_until_blocking(session, callback, MESessionStopped, 1000, &propvar);
+    todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt);
+
+    hr = IMFMediaSession_ClearTopologies(session);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    IMFTopology_Release(topology);
+    CloseWindow(hwnd);
+    DestroyWindow(hwnd);
+}
+
 START_TEST(mf)
 {
     init_functions();
@@ -7084,4 +7213,5 @@ START_TEST(mf)
     test_MFGetTopoNodeCurrentType();
     test_MFRequireProtectedEnvironment();
     test_mpeg4_media_sink();
+    test_topology_change();
 }
-- 
GitLab


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

[PATCH 2/2] evr: Leave critical section before waiting for streaming thread.

[Santino Mazza]

From: Santino Mazza <smazza(a)codeweavers.com>

Because the streaming thread enters the critical section when it
receives an EVRM_PROCESS_INPUT message, it's possible to get into
a deadlock when waiting for the streaming thread while being in the
critical section.
---
 dlls/evr/presenter.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c
index 06592f8766e..674bd298c33 100644
--- a/dlls/evr/presenter.c
+++ b/dlls/evr/presenter.c
@@ -747,8 +747,11 @@ static HRESULT video_presenter_end_streaming(struct video_presenter *presenter)
 
     PostThreadMessageW(presenter->thread.tid, EVRM_STOP, 0, 0);
 
+    /* Make sure streaming thread is not blocked by critical section before waiting for it. */
+    LeaveCriticalSection(&presenter->cs);
     WaitForSingleObject(presenter->thread.hthread, INFINITE);
     CloseHandle(presenter->thread.hthread);
+    EnterCriticalSection(&presenter->cs);
 
     TRACE("Terminated streaming thread tid %#lx.\n", presenter->thread.tid);
 
-- 
GitLab

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

[PATCH 2/2] evr: Leave critical section before waiting for streaming thread.

[Marvin]

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=134783

Your paranoid android.


=== debian11 (32 bit report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11 (32 bit ar:MA report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11 (32 bit de report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11 (32 bit fr report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11 (32 bit he:IL report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11 (32 bit hi:IN report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11 (32 bit ja:JP report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11 (32 bit zh:CN report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11b (32 bit WoW report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.

=== debian11b (64 bit WoW report) ===

mf:
mf.c:7158: Test failed: Unexpected hr 0xc00d5212.
mf.c:7171: Test succeeded inside todo block: Unexpected hr 0.