[PATCH v8 0/6] MR7466: mf: Fix some issues with seeking.
-- v8: mf: Initialise the grabber sample count when setting state after a seek. mf: Do not count draining a downstream node as a request fulfillment. mf: Do not count placing the stream sink ENDOFSEGMENT marker as a request fulfillment. 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(). https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mf/tests/mf.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index a02f2859f91..e12a389f95f 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}; @@ -3042,6 +3057,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); @@ -6627,6 +6648,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); @@ -6664,6 +6687,7 @@ 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); + check_sample_delivery(grabber_callback->ready_event); /* Seek to beyond duration */ propvar.vt = VT_I8; @@ -6694,6 +6718,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); @@ -6719,6 +6745,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); @@ -6761,6 +6790,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); @@ -6792,6 +6823,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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)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 e12a389f95f..898934b6173 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6748,6 +6748,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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)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 898934b6173..0876de45e59 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6938,6 +6938,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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Placing the marker is not the result of a request but of receiving MEEndOfStream. Decrementing the request count can halt later sample delivery. --- dlls/mf/session.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 87af5b95eab..664f25a4c32 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -3888,21 +3888,21 @@ static void session_deliver_sample_to_node(struct media_session *session, struct switch (topo_node->type) { case MF_TOPOLOGY_OUTPUT_NODE: - if (topo_node->u.sink.requests) + if (!sample) { - if (sample) - { - if (FAILED(hr = IMFStreamSink_ProcessSample(topo_node->object.sink_stream, sample))) - { - WARN("Stream sink failed to process sample, hr %#lx.\n", hr); - IMFMediaEventQueue_QueueEventParamVar(session->event_queue, MEError, &GUID_NULL, hr, NULL); - } - } - else if (FAILED(hr = IMFStreamSink_PlaceMarker(topo_node->object.sink_stream, MFSTREAMSINK_MARKER_ENDOFSEGMENT, + if (FAILED(hr = IMFStreamSink_PlaceMarker(topo_node->object.sink_stream, MFSTREAMSINK_MARKER_ENDOFSEGMENT, NULL, NULL))) { WARN("Failed to place sink marker, hr %#lx.\n", hr); } + } + else if (topo_node->u.sink.requests) + { + if (FAILED(hr = IMFStreamSink_ProcessSample(topo_node->object.sink_stream, sample))) + { + WARN("Stream sink failed to process sample, hr %#lx.\n", hr); + IMFMediaEventQueue_QueueEventParamVar(session->event_queue, MEError, &GUID_NULL, hr, NULL); + } topo_node->u.sink.requests--; } break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)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 | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 664f25a4c32..21ea1cd1043 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -3850,10 +3850,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 0876de45e59..0ad5feefb8c 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6718,7 +6718,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 */ @@ -6745,7 +6744,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. @@ -6766,7 +6764,7 @@ 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 + flaky_wine check_sample_delivery(grabber_callback->ready_event); hr = IMFMediaSession_Stop(session); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
From: Conor McCarthy <cmccarthy(a)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 | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) 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 0ad5feefb8c..ca115787fa9 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6764,7 +6764,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); - flaky_wine check_sample_delivery(grabber_callback->ready_event); hr = IMFMediaSession_Stop(session); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7466
Rebased again and removed some changes that are now unnecessary. This is ready. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7466#note_110770
participants (2)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)