[PATCH 0/2] MR2736: Draft: Reset presentation flags when session_clear_presentation is called.
This prevents hangs when a program sets a new topology after stopping the current topology, because if we don't reset the flags to 0 the session will not subscribe to the events of the new topology sources. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2736
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/mf/tests/mf.c | 198 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 196 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index e8d7252aa3f..5e38e0a2a72 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -2037,6 +2037,11 @@ static IMFMediaSource *create_media_source(const WCHAR *name, const WCHAR *mime) static void test_media_session_events(void) { + media_type_desc audio_mp3_desc = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio), + ATTR_GUID(MF_MT_SUBTYPE, MFAudioFormat_MP3), + }; static const media_type_desc audio_float_44100 = { ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio), @@ -2062,17 +2067,19 @@ static void test_media_session_events(void) struct test_media_sink media_sink = test_media_sink; struct test_handler handler = test_handler; struct test_source *source_impl; + IMFActivate *sink_activate, *sink_activate2; IMFAsyncCallback *callback, *callback2; IMFMediaType *input_type, *output_type; IMFTopologyNode *src_node, *sink_node; + IMFTopology *topology, *topology2; + IMFMediaSource *source, *source2; IMFPresentationDescriptor *pd; IMFMediaSession *session; IMFStreamDescriptor *sd; IMFAsyncResult *result; - IMFMediaSource *source; - IMFTopology *topology; IMFMediaEvent *event; PROPVARIANT propvar; + BOOL selected; HRESULT hr; ULONG ref; @@ -2564,6 +2571,193 @@ static void test_media_session_events(void) ok(ref == 0, "Release returned %ld\n", ref); + /* Test for events when setting a new topology after stopping a media session */ + if (!(source = create_media_source(L"mp3encdata.bin", L"audio/mp3")) || + !(source2 = create_media_source(L"mp3encdata.bin", L"audio/mp3"))) + { + win_skip("MP3 media source is not supported, skipping tests.\n"); + hr = MFShutdown(); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + return; + } + + + callback = create_test_callback(TRUE); + + hr = MFCreateMediaSession(NULL, &session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + 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 = MFCreateTopology(&topology); + ok(hr == S_OK, "Unexpected 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); + + hr = IMFMediaSource_CreatePresentationDescriptor(source, &pd); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFPresentationDescriptor_GetStreamDescriptorByIndex(pd, 0, &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); + IMFTopologyNode_Release(src_node); + IMFPresentationDescriptor_Release(pd); + IMFStreamDescriptor_Release(sd); + + hr = MFCreateMediaType(&output_type); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + init_media_type(output_type, audio_mp3_desc, -1); + hr = MFCreateAudioRendererActivate(&sink_activate); + ok(hr == S_OK, "Failed to create grabber sink, hr %#lx.\n", hr); + IMFMediaType_Release(output_type); + + 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); + IMFTopologyNode_Release(sink_node); + + hr = IMFTopology_SetUINT32(topology, &MF_TOPOLOGY_ENUMERATE_SOURCE_TYPES, TRUE); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + + 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 = MFCreateTopology(&topology2); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFTopology_AddNode(topology2, sink_node); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFTopology_AddNode(topology2, 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); + + hr = IMFMediaSource_CreatePresentationDescriptor(source2, &pd); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFPresentationDescriptor_GetStreamDescriptorByIndex(pd, 0, &selected, &sd); + ok(selected, "got selected %u.\n", !!selected); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + init_source_node(source2, -1, src_node, pd, sd); + IMFTopologyNode_Release(src_node); + IMFPresentationDescriptor_Release(pd); + IMFStreamDescriptor_Release(sd); + + hr = MFCreateMediaType(&output_type); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + init_media_type(output_type, audio_mp3_desc, -1); + IMFMediaType_Release(output_type); + + hr = MFCreateAudioRendererActivate(&sink_activate2); + ok(hr == S_OK, "Failed to create grabber sink, hr %#lx.\n", hr); + + hr = IMFTopologyNode_SetObject(sink_node, (IUnknown *)sink_activate2); + 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); + IMFTopologyNode_Release(sink_node); + + hr = IMFTopology_SetUINT32(topology2, &MF_TOPOLOGY_ENUMERATE_SOURCE_TYPES, TRUE); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + + propvar.vt = VT_EMPTY; + 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); + ok(propvar.punkVal != (IUnknown *)topology, "got punkVal %p\n", propvar.punkVal); + PropVariantClear(&propvar); + + 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); + ok(propvar.punkVal != (IUnknown *)topology2, "got punkVal %p\n", propvar.punkVal); + PropVariantClear(&propvar); + + propvar.vt = VT_EMPTY; + hr = IMFMediaSession_Stop(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event_until_blocking(session, callback, MESessionStopped, 1000, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); + ok(propvar.punkVal != (IUnknown *)topology2, "got punkVal %p\n", propvar.punkVal); + PropVariantClear(&propvar); + + propvar.vt = VT_EMPTY; + hr = IMFMediaSession_SetTopology(session, MFSESSION_SETTOPOLOGY_IMMEDIATE, topology2); + 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); + ok(propvar.punkVal != (IUnknown *)topology2, "got punkVal %p\n", propvar.punkVal); + PropVariantClear(&propvar); + + 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); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); + ok(propvar.punkVal != (IUnknown *)topology2, "got punkVal %p\n", propvar.punkVal); + PropVariantClear(&propvar); + + propvar.vt = VT_EMPTY; + hr = IMFMediaSession_Close(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine + { + hr = wait_media_event_until_blocking(session, callback, MESessionClosed, 1000, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + } + ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); + ok(propvar.punkVal != (IUnknown *)topology2, "got punkVal %p\n", propvar.punkVal); + PropVariantClear(&propvar); + + hr = IMFMediaSession_Shutdown(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ref = IMFMediaSession_Release(session); + todo_wine ok(ref == 0, "Release returned %ld\n", ref); + + hr = IMFTopology_Clear(topology); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ref = IMFTopology_Release(topology); + ok(ref == 0, "Release returned %ld\n", ref); + hr = IMFMediaSource_Shutdown(source); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ref = IMFMediaSource_Release(source); + ok(ref == 0, "Release returned %ld\n", ref); + hr = IMFActivate_ShutdownObject(sink_activate); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ref = IMFActivate_Release(sink_activate); + ok(ref == 0, "Release returned %ld\n", ref); + + hr = IMFTopology_Clear(topology2); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ref = IMFTopology_Release(topology2); + ok(ref == 0, "Release returned %ld\n", ref); + hr = IMFMediaSource_Shutdown(source2); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ref = IMFMediaSource_Release(source2); + todo_wine ok(ref == 0, "Release returned %ld\n", ref); + hr = IMFActivate_ShutdownObject(sink_activate2); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ref = IMFActivate_Release(sink_activate2); + todo_wine ok(ref == 0, "Release returned %ld\n", ref); + hr = MFShutdown(); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2736
From: Santino Mazza <smazza(a)codeweavers.com> This prevents hangs when a program sets a new topology after stopping the current topology, because if we don't reset the flags to 0 the session will not subscribe to the events of the new topology sources. --- dlls/mf/session.c | 1 + dlls/mf/tests/mf.c | 9 +++------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 7d6a65d3f55..8b29c8487fb 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -800,6 +800,7 @@ static void session_clear_presentation(struct media_session *session) IMFTopology_Clear(session->presentation.current_topology); session->presentation.topo_status = MF_TOPOSTATUS_INVALID; + session->presentation.flags = 0; LIST_FOR_EACH_ENTRY_SAFE(source, source2, &session->presentation.sources, struct media_source, entry) { diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 5e38e0a2a72..0f4c9129398 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -2710,7 +2710,7 @@ static void test_media_session_events(void) 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); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); ok(propvar.punkVal != (IUnknown *)topology2, "got punkVal %p\n", propvar.punkVal); PropVariantClear(&propvar); @@ -2718,11 +2718,8 @@ static void test_media_session_events(void) propvar.vt = VT_EMPTY; hr = IMFMediaSession_Close(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine - { hr = wait_media_event_until_blocking(session, callback, MESessionClosed, 1000, &propvar); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - } ok(propvar.vt == VT_EMPTY, "got vt %u\n", propvar.vt); ok(propvar.punkVal != (IUnknown *)topology2, "got punkVal %p\n", propvar.punkVal); PropVariantClear(&propvar); @@ -2752,11 +2749,11 @@ static void test_media_session_events(void) hr = IMFMediaSource_Shutdown(source2); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ref = IMFMediaSource_Release(source2); - todo_wine ok(ref == 0, "Release returned %ld\n", ref); + ok(ref == 0, "Release returned %ld\n", ref); hr = IMFActivate_ShutdownObject(sink_activate2); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ref = IMFActivate_Release(sink_activate2); - todo_wine ok(ref == 0, "Release returned %ld\n", ref); + ok(ref == 0, "Release returned %ld\n", ref); hr = MFShutdown(); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2736
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=132328 Your paranoid android. === w11pro64_amd (64 bit report) === mf: mf.c:2677: Test failed: Unexpected hr 0xc00d36fa. mf.c:2679: Test failed: got punkVal 000000000433E290 mf.c:2686: Test failed: Unexpected hr 0xc00d36b2. mf.c:2695: Test failed: Unexpected hr 0xc00d36b2. mf.c:2704: Test failed: Unexpected hr 0xc00d36fa. mf.c:2706: Test failed: got punkVal 000000000433E6A0 mf.c:2713: Test failed: Unexpected hr 0xc00d36b2. mf.c:2730: Test failed: Release returned 1
I think this one is good to go without tests commit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2736#note_31829
On Wed May 3 00:50:41 2023 +0000, Nikolay Sivov wrote:
I think this one is good to go without tests commit. Oh okay
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2736#note_31830
participants (4)
-
Marvin -
Nikolay Sivov (＠nsivov) -
Santino Mazza
-
Santino Mazza (＠tati1454)