[PATCH v4 0/1] MR9742: mf: Prevent invalid topologies from being started.

[Bernhard Kölbl]

Signed-off-by: Bernhard Kölbl <[e-mail]>

--
  v4: mf: Prevent invalid topologies from being started.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9742

[PATCH v4 1/1] mf: Prevent invalid topologies from being started.

[Bernhard Kölbl]

From: Bernhard Kölbl <[e-mail]>

Signed-off-by: Bernhard Kölbl <[e-mail]>
---
 dlls/mf/session.c  |  2 ++
 dlls/mf/tests/mf.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/dlls/mf/session.c b/dlls/mf/session.c
index 28195f914b0..378a54ab47a 100644
--- a/dlls/mf/session.c
+++ b/dlls/mf/session.c
@@ -2111,6 +2111,8 @@ static void session_set_topology(struct media_session *session, DWORD flags, IMF
                 hr = IMFTopoLoader_Load(session->topo_loader, topology, &resolved_topology, NULL /* FIXME? */);
             if (SUCCEEDED(hr))
                 hr = session_init_media_types(resolved_topology);
+            else
+                topology = NULL;
 
             if (SUCCEEDED(hr))
             {
diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c
index df1362381b2..eec7a4ea881 100644
--- a/dlls/mf/tests/mf.c
+++ b/dlls/mf/tests/mf.c
@@ -10892,6 +10892,47 @@ static void test_media_session_sample_request(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
 }
 
+static void test_media_session_invalid_topology(void)
+{
+    IMFAsyncCallback *callback;
+    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);
+
+    hr = MFCreateMediaSession(NULL, &session);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    callback = create_test_callback(TRUE);
+    source = create_test_source(FALSE);
+    topology = create_test_topology(source, NULL, &duration);
+
+    hr = IMFMediaSession_SetTopology(session, 0, topology);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = wait_media_event(session, callback, MESessionTopologySet, 5000, &propvar);
+    ok(hr == E_FAIL, "Unexpected hr %#lx.\n", hr);
+
+    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);
+
+    IMFTopology_Release(topology);
+    IMFMediaSource_Release(source);
+    IMFAsyncCallback_Release(callback);
+    IMFMediaSession_Release(session);
+
+    hr = MFShutdown();
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+}
+
 START_TEST(mf)
 {
     init_functions();
@@ -10935,4 +10976,5 @@ START_TEST(mf)
     test_media_session_seek();
     test_media_session_scrubbing();
     test_media_session_sample_request();
+    test_media_session_invalid_topology();
 }
-- 
GitLab

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

[Bernhard Kölbl]

v3:
- simplify tests
- set session to NULL

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_130516

[Bernhard Kölbl]

On Wed Feb 25 18:13:04 2026 +0000, Nikolay Sivov wrote:
> At this point you don't know if topology is going to work really. If
> Load() failed we should simply reset "topology" to null.
> okay, changed it

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_130517

[Bernhard Kölbl]

On Wed Feb 25 18:17:50 2026 +0000, Nikolay Sivov wrote:
> 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.
> I've simplified the tests. Should I add additional tests with MFSESSION_SETTOPOLOGY_NORESOLUTION? I've noticed it causes different behavior in Wine, so might be worth doing. On Windows there doesn't seem to be any difference.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_130518

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
> hr = IMFTopoLoader_Load(session->topo_loader, topology, &resolved_topology, NULL /* FIXME? */);
>              if (SUCCEEDED(hr))
>                  hr = session_init_media_types(resolved_topology);
> +            else
> +                topology = NULL;
>
> if (SUCCEEDED(hr))
>              {

I think it makes more sense to move this few lines down. I do `if (SUCCEEDED(hr)) topology = resolved_topology; else topology = NULL;`

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9742#note_131844