[PATCH v3 0/1] MR9742: mf: Prevent invalid topologies from being started.
Signed-off-by: Bernhard Kölbl <bkoelbl@codeweavers.com> -- v3: mf: Prevent invalid topologies from being started. https://gitlab.winehq.org/wine/wine/-/merge_requests/9742
From: Bernhard Kölbl <bkoelbl@codeweavers.com> Signed-off-by: Bernhard Kölbl <bkoelbl@codeweavers.com> --- dlls/mf/session.c | 4 +- dlls/mf/tests/mf.c | 121 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 8d2ca079634..b7012e69883 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -2117,7 +2117,7 @@ static void session_set_topology(struct media_session *session, DWORD flags, IMF session_raise_topology_set(session, topology, hr); /* With no current topology set it right away, otherwise queue. */ - if (topology) + if (SUCCEEDED(hr) && topology) { struct queued_topology *queued_topology; @@ -4932,6 +4932,8 @@ HRESULT WINAPI MFCreateMediaSession(IMFAttributes *config, IMFMediaSession **ses goto failed; } + session_clear_presentation(object); + *session = &object->IMFMediaSession_iface; return S_OK; diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 0c0590ed195..10a0e7775bb 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -8436,6 +8436,126 @@ static void test_media_session_seek(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); } +static void test_media_session_invalid_topology(void) +{ + media_type_desc video_rgb32_desc = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), + ATTR_GUID(MF_MT_SUBTYPE, MFVideoFormat_RGB32), + }; + struct test_grabber_callback *grabber_callback; + IMFActivate *sink_activate; + IMFAsyncCallback *callback; + IMFMediaType *output_type; + IMFMediaSession *session; + IMFMediaSource *source; + IMFTopology *topology; + PROPVARIANT propvar; + UINT64 duration; + HRESULT hr; + + 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"))) + { + todo_wine /* Gitlab CI Debian runner */ + win_skip("MP4 media source is not supported, skipping tests.\n"); + MFShutdown(); + return; + } + + /* test starting an invalid topology */ + source = create_test_source(TRUE); + callback = create_test_callback(TRUE); + + hr = MFCreateMediaSession(NULL, &session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = MFCreateAudioRendererActivate(&sink_activate); + ok(hr == S_OK, "Failed to create audio renderer sink, hr %#lx.\n", hr); + + topology = create_test_topology(source, sink_activate, &duration); + hr = IMFMediaSession_SetTopology(session, 0, topology); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IMFTopology_Release(topology); + + propvar.vt = VT_I8; + propvar.hVal.QuadPart = 0; + hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(session, callback, MESessionStarted, 5000, &propvar); + ok(hr == MF_E_INVALIDREQUEST, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaSession_Stop(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + IMFMediaSource_Release(source); + IMFActivate_ShutdownObject(sink_activate); + IMFActivate_Release(sink_activate); + + /* then start a valid topology */ + source = create_test_source(FALSE); + + grabber_callback = impl_from_IMFSampleGrabberSinkCallback(create_test_grabber_callback()); + hr = MFCreateMediaType(&output_type); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + init_media_type(output_type, video_rgb32_desc, -1); + hr = MFCreateSampleGrabberSinkActivate(output_type, &grabber_callback->IMFSampleGrabberSinkCallback_iface, &sink_activate); + ok(hr == S_OK, "Failed to create grabber sink, hr %#lx.\n", hr); + IMFMediaType_Release(output_type); + + topology = create_test_topology(source, sink_activate, &duration); + hr = IMFMediaSession_SetTopology(session, 0, topology); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IMFTopology_Release(topology); + + propvar.vt = VT_I8; + propvar.hVal.QuadPart = 0; + hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(session, callback, MESessionStarted, 5000, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + IMFMediaSource_Release(source); + IMFActivate_ShutdownObject(sink_activate); + IMFActivate_Release(sink_activate); + + hr = IMFMediaSession_Stop(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + /* try invalid topology once again */ + source = create_test_source(TRUE); + + hr = MFCreateAudioRendererActivate(&sink_activate); + ok(hr == S_OK, "Failed to create audio renderer sink, hr %#lx.\n", hr); + + topology = create_test_topology(source, sink_activate, &duration); + hr = IMFMediaSession_SetTopology(session, 0, topology); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IMFTopology_Release(topology); + + propvar.vt = VT_I8; + propvar.hVal.QuadPart = 10; + hr = IMFMediaSession_Start(session, &GUID_NULL, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(session, callback, MESessionStarted, 5000, &propvar); + ok(hr == E_FAIL, "Unexpected hr %#lx.\n", hr); /* fails because unseekable source is still in use */ + + hr = IMFMediaSession_Stop(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + IMFMediaSource_Release(source); + IMFActivate_ShutdownObject(sink_activate); + IMFActivate_Release(sink_activate); + + IMFAsyncCallback_Release(callback); + IMFMediaSession_Release(session); + + hr = MFShutdown(); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); +} + START_TEST(mf) { init_functions(); @@ -8475,4 +8595,5 @@ START_TEST(mf) test_media_session_source_shutdown(); test_media_session_thinning(); test_media_session_seek(); + test_media_session_invalid_topology(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9742
On Tue Jan 13 16:37:13 2026 +0000, Bernhard Kölbl wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/9742/diffs?diff_id=237317&start_sha=6ded171a9924b12a55fbecbb597354502ac63fbb#38af2002cdac18e4819839775ba914609caa73df_7282_7282) done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_126721
On Tue Jan 13 16:10:06 2026 +0000, Bernhard Kölbl wrote:
Because the topo_status needs to be set. ``` session->presentation.topo_status = MF_TOPOSTATUS_INVALID; ``` I don't think it makes sense as a default.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_129049
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
session_raise_topology_set(session, topology, hr);
/* With no current topology set it right away, otherwise queue. */ - if (topology) + if (SUCCEEDED(hr) && topology) { struct queued_topology *queued_topology;
At this point you don't know if topology is going to work really. If Load() failed we should simply reset "topology" to null. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_129050
For the tests, I think something much shorter will show the problem already, if you create an incomplete geometry and load with MFSESSION_SETTOPOLOGY_NORESOLUTION for example. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_129051
participants (2)
-
Bernhard Kölbl -
Nikolay Sivov (＠nsivov)