[PATCH v2 0/1] MR8742: mf: Handle start request when session is already running.

[Bernhard Kölbl (＠besentv)]

Starting the session while it's already running should not resume anything.

--
  v2: mf: Handle start request when session is already running.

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

[PATCH v2 1/1] mf: Handle start request when session is already running.

[Bernhard Kölbl]

From: Bernhard Kölbl

Signed-off-by: Bernhard Kölbl 
---
 dlls/mf/session.c  |  3 +++
 dlls/mf/tests/mf.c | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/dlls/mf/session.c b/dlls/mf/session.c
index 87af5b95eab..dd6b9b98284 100644
--- a/dlls/mf/session.c
+++ b/dlls/mf/session.c
@@ -1119,6 +1119,9 @@ static void session_start(struct media_session *session, const GUID *time_format

break;
             }
+            else if (session->state == SESSION_STATE_STARTED)
+                return session_command_complete_with_event(session, MESessionStarted, S_OK, NULL);
+
             /* fallthrough; we're resuming from the current position */
         case SESSION_STATE_STOPPED:
             /* Start request with no current topology. */
diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c
index a02f2859f91..c56e59fbc6c 100644
--- a/dlls/mf/tests/mf.c
+++ b/dlls/mf/tests/mf.c
@@ -7919,6 +7919,21 @@ static void test_media_session_seek(void)

compare_object_states(&actual_object_state_record, &expected_sample_request_and_delivery_records);

+    SET_EXPECT(test_media_sink_GetPresentationClock);
+    SET_EXPECT(test_media_sink_GetStreamSinkCount);
+
+    PropVariantClear(&propvar);
+    hr = IMFMediaSession_Start(session, NULL, &propvar);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = wait_media_event(session, callback, MESessionStarted, 1000, &propvar);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    todo_wine
+    CHECK_CALLED(test_media_sink_GetPresentationClock);
+    todo_wine
+    CHECK_CALLED(test_media_sink_GetStreamSinkCount);
+
     memset(&actual_object_state_record, 0, sizeof(actual_object_state_record));
     hr = IMFMediaSession_Pause(session);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

[Bernhard Kölbl (＠besentv)]

v2: Keep the fall through when in the paused state.

[Nikolay Sivov (＠nsivov)]

Could we maybe use an early return like this instead. Switch is already no very readable because of a number of fallthough conditions.

[session.diff](/uploads/16f7b84f7bfcb5beb33fd4d904004a5f/session.diff)

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/mf/tests/mf.c:
>  compare_object_states(&actual_object_state_record, &expected_sample_request_and_delivery_records);
>
> - SET_EXPECT(test_media_sink_GetPresentationClock);
> - SET_EXPECT(test_media_sink_GetStreamSinkCount);
> -
> - PropVariantClear(&propvar);
> - hr = IMFMediaSession_Start(session, NULL, &propvar);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - hr = wait_media_event(session, callback, MESessionStarted, 1000, &propvar);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - todo_wine
> - CHECK_CALLED(test_media_sink_GetPresentationClock);
> - todo_wine
> - CHECK_CALLED(test_media_sink_GetStreamSinkCount);

Do we need to check for that? Any idea what this means?

[Bernhard Kölbl (＠besentv)]

On Wed Sep 17 16:50:37 2025 +0000, Nikolay Sivov wrote:
> Do we need to check for that? Any idea what this means?

Well, these checks are already upstream. They need to be "re-set" otherwise tests will fail.