This fixes failure to play the prologue video in Planet of the Apes: Last Frontier, which unpauses by calling Start() with a location. The exact state leading to this issue does not occur in the Start() tests, and it's not clear how to reproduce it reliably.
-- v7: mf: Do not count draining a downstream node as a request fulfillment. mf: Fill the sample request queue also when seeking while running. mf: Initialise the grabber sample count when setting state after a seek. mf/tests: Wait for session closure at the end of test_media_session_Start(). mf/tests: Test pause followed by immediate restart at current time in test_media_session_Start(). mf/tests: Test sample delivery where applicable in test_media_session_Start().
From: Conor McCarthy cmccarthy@codeweavers.com
--- dlls/mf/tests/mf.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 1070e9c1b2d..b04514853a8 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -131,6 +131,21 @@ static void check_service_interface_(unsigned int line, void *iface_ptr, REFGUID IUnknown_Release(unk); }
+#define check_sample_delivery(a) check_sample_delivery_(__LINE__, a) +static void check_sample_delivery_(unsigned int line, HANDLE event) +{ + HRESULT hr; + UINT i; + + WaitForSingleObject(event, 0); + /* Get five samples since Wine's mf session implementation sends four requests initially. */ + for (i = 0; i < 5; ++i) + { + hr = WaitForSingleObject(event, 200); + ok_(__FILE__, line)(hr == WAIT_OBJECT_0, "%u: Unexpected hr %#lx.\n", i, hr); + } +} + static HWND create_window(void) { RECT r = {0, 0, 640, 480}; @@ -2899,6 +2914,12 @@ static HRESULT WINAPI test_grabber_callback_OnProcessSample(IMFSampleGrabberSink if (!grabber->ready_event) return E_NOTIMPL;
+ if (!grabber->done_event) + { + SetEvent(grabber->ready_event); + return S_OK; + } + sample = create_sample(buffer, sample_size); hr = IMFSample_SetSampleFlags(sample, sample_flags); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -6345,6 +6366,8 @@ static void test_media_session_Start(void) }
grabber_callback = impl_from_IMFSampleGrabberSinkCallback(create_test_grabber_callback()); + grabber_callback->ready_event = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(!!grabber_callback->ready_event, "CreateEventW failed, error %lu\n", GetLastError()); hr = MFCreateMediaType(&output_type); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); init_media_type(output_type, video_rgb32_desc, -1); @@ -6382,6 +6405,8 @@ static void test_media_session_Start(void) hr = IMFPresentationClock_GetTime(presentation_clock, &time); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(llabs(time - 10000000) <= allowed_error, "Unexpected time %I64d.\n", time); + todo_wine + check_sample_delivery(grabber_callback->ready_event);
/* Seek to beyond duration */ propvar.vt = VT_I8; @@ -6412,6 +6437,8 @@ static void test_media_session_Start(void) hr = IMFPresentationClock_GetTime(presentation_clock, &time); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(llabs(time) <= allowed_error, "Unexpected time %I64d.\n", time); + todo_wine + check_sample_delivery(grabber_callback->ready_event);
/* Seek to 1s while in paused state */ hr = IMFMediaSession_Pause(session); @@ -6437,6 +6464,9 @@ static void test_media_session_Start(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(time > old_time, "Unexpected time %I64d.\n", time);
+ todo_wine + check_sample_delivery(grabber_callback->ready_event); + hr = IMFMediaSession_Stop(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSession_Close(session); @@ -6479,6 +6509,8 @@ static void test_media_session_Start(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok((caps & MFMEDIASOURCE_CAN_SEEK) == 0, "Got unexpected caps %#lx.\n", caps); grabber_callback = impl_from_IMFSampleGrabberSinkCallback(create_test_grabber_callback()); + grabber_callback->ready_event = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(!!grabber_callback->ready_event, "CreateEventW failed, error %lu\n", GetLastError()); hr = MFCreateMediaType(&output_type); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); init_media_type(output_type, video_rgb32_desc, -1); @@ -6510,6 +6542,8 @@ static void test_media_session_Start(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok((caps & MFSESSIONCAP_SEEK) == 0, "Got unexpected caps %#lx\n", caps);
+ check_sample_delivery(grabber_callback->ready_event); + /* Seek to 1s */ propvar.vt = VT_I8; propvar.hVal.QuadPart = 10000000;
From: Conor McCarthy cmccarthy@codeweavers.com
--- dlls/mf/tests/mf.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index b04514853a8..59e909bc66c 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6467,6 +6467,27 @@ static void test_media_session_Start(void) todo_wine check_sample_delivery(grabber_callback->ready_event);
+ /* Pause followed by immediate restart at current time. + * Planet of the Apes: Last Frontier does this in a cutscene. */ + 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 = IMFMediaSession_Pause(session); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(session, callback, MESessionPaused, 1000, &propvar); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFPresentationClock_GetTime(presentation_clock, &time); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + propvar.vt = VT_I8; + propvar.hVal.QuadPart = time; + hr = IMFMediaSession_Start(session, &GUID_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_sample_delivery(grabber_callback->ready_event); + hr = IMFMediaSession_Stop(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSession_Close(session);
From: Conor McCarthy cmccarthy@codeweavers.com
MFShutdown() may be called too soon after seeking has been fixed. --- dlls/mf/tests/mf.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 59e909bc66c..3945b539266 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6658,6 +6658,9 @@ static void test_media_session_Start(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSession_Close(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + /* Wait for close to avoid the occasional assertion when MFShutdown() is called in Wine. + * Waiting for MESessionClosed would be correct, but Windows doesn't send it here. */ + Sleep(10); hr = IMFMediaSession_Shutdown(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSource_Shutdown(source);
From: Conor McCarthy cmccarthy@codeweavers.com
If sample_count is zero in this case, we end up with no samples being requested. --- dlls/mf/samplegrabber.c | 3 +++ dlls/mf/tests/mf.c | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/dlls/mf/samplegrabber.c b/dlls/mf/samplegrabber.c index a8c797f25ed..52f287dac26 100644 --- a/dlls/mf/samplegrabber.c +++ b/dlls/mf/samplegrabber.c @@ -1195,6 +1195,9 @@ static HRESULT sample_grabber_set_state(struct sample_grabber *grabber, enum sin
if (state == SINK_STATE_RUNNING && grabber->state != SINK_STATE_RUNNING) { + /* Seek operations flush all pending sample requests. */ + if (offset != PRESENTATION_CURRENT_POSITION) + grabber->sample_count = MAX_SAMPLE_QUEUE_LENGTH; /* Every transition to running state sends a bunch requests to build up initial queue. */ for (i = 0; i < grabber->sample_count; ++i) { diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 3945b539266..6893500426d 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6464,7 +6464,6 @@ static void test_media_session_Start(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(time > old_time, "Unexpected time %I64d.\n", time);
- todo_wine check_sample_delivery(grabber_callback->ready_event);
/* Pause followed by immediate restart at current time. @@ -6485,7 +6484,6 @@ static void test_media_session_Start(void) 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_sample_delivery(grabber_callback->ready_event);
hr = IMFMediaSession_Stop(session);
From: Conor McCarthy cmccarthy@codeweavers.com
--- dlls/mf/samplegrabber.c | 3 ++- dlls/mf/session.c | 18 ++++++++++++++++++ dlls/mf/tests/mf.c | 1 - 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/dlls/mf/samplegrabber.c b/dlls/mf/samplegrabber.c index 52f287dac26..a125acf54a2 100644 --- a/dlls/mf/samplegrabber.c +++ b/dlls/mf/samplegrabber.c @@ -1193,7 +1193,8 @@ static HRESULT sample_grabber_set_state(struct sample_grabber *grabber, enum sin grabber->sample_count = MAX_SAMPLE_QUEUE_LENGTH; }
- if (state == SINK_STATE_RUNNING && grabber->state != SINK_STATE_RUNNING) + if (state == SINK_STATE_RUNNING + && (grabber->state != SINK_STATE_RUNNING || offset != PRESENTATION_CURRENT_POSITION)) { /* Seek operations flush all pending sample requests. */ if (offset != PRESENTATION_CURRENT_POSITION) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 2e95d763984..d5776ceaa70 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1033,6 +1033,21 @@ static void session_flush_nodes(struct media_session *session) } }
+static void session_reset_request_count(struct media_session *session) +{ + struct topo_node *node; + UINT i; + + LIST_FOR_EACH_ENTRY(node, &session->presentation.nodes, struct topo_node, entry) + { + if (node->type == MF_TOPOLOGY_TRANSFORM_NODE) + { + for (i = 0; i < node->u.transform.output_count; ++i) + node->u.transform.outputs[i].requests = 0; + } + } +} + static void session_handle_source_shutdown(struct media_session *session);
static void session_start(struct media_session *session, const GUID *time_format, const PROPVARIANT *start_position) @@ -3154,6 +3169,9 @@ static void session_set_source_object_state(struct media_session *session, IUnkn if (!session_is_source_nodes_state(session, OBJ_STATE_STARTED)) break;
+ /* In a paused state, the request count may be non-zero from when the session was running. */ + session_reset_request_count(session); + session_set_topo_status(session, S_OK, MF_TOPOSTATUS_STARTED_SOURCE);
session_set_presentation_clock(session); diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 6893500426d..e5c61b9356b 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6405,7 +6405,6 @@ static void test_media_session_Start(void) hr = IMFPresentationClock_GetTime(presentation_clock, &time); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(llabs(time - 10000000) <= allowed_error, "Unexpected time %I64d.\n", time); - todo_wine check_sample_delivery(grabber_callback->ready_event);
/* Seek to beyond duration */
From: Conor McCarthy cmccarthy@codeweavers.com
Calling session_deliver_sample_to_node() on the downstream node does not fulfill requests from the current node. It sets the downstream node to draining, and delivers all its pending requests according to its own request count. --- dlls/mf/session.c | 3 +-- dlls/mf/tests/mf.c | 1 - 2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/dlls/mf/session.c b/dlls/mf/session.c index d5776ceaa70..49ea14f6872 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -3810,10 +3810,9 @@ static void transform_node_deliver_samples(struct media_session *session, struct IMFMediaEvent_Release(event); }
- while (stream->requests && drained) + if (drained) { session_deliver_sample_to_node(session, down_node, input, NULL); - stream->requests--; } }
diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index e5c61b9356b..7b9412e8a1d 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6436,7 +6436,6 @@ static void test_media_session_Start(void) hr = IMFPresentationClock_GetTime(presentation_clock, &time); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(llabs(time) <= allowed_error, "Unexpected time %I64d.\n", time); - todo_wine check_sample_delivery(grabber_callback->ready_event);
/* Seek to 1s while in paused state */
I have completely redone this, and added extra testing to `test_media_session_Start()` which revealed some issues. But tests are failing after rebase, so it needs more work.
I am looking at the possibility of removing the transform sample request count, since it's caused a lot of trouble, and maybe the sink request count too. I think all requests from the sink should propagate all the way to the source, so we can rely on the source's internal request count only. Samples sent from the source will propagate down to the sink, and we need only ensure they are always delivered. Currently, if the source's internal request count is less than the count kept by the transform, sample delivery gets stuck.
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)