From: Brendan McGrath <bmcgrath(a)codeweavers.com> The total of: 1. unsatisfied sample requests made to MF; plus 2. samples queued (i.e. satisfied requests) should always equal 4. Thus, for example, if we flush 3 samples, we need to request 3 more. The stop state appears to flush all samples from sample grabber and all requests outstanding with MF. Hence, starting playback from the stop state always makes four new sample requests. Pause and resume always makes four new sample requests, but will also not flush the queue. This appears to be a Windows bug, as looping pause and resume can therefore result in samples being queued until we are OOM. --- dlls/mf/samplegrabber.c | 37 ++++++++++++----------- dlls/mf/tests/mf.c | 65 +++++++++++------------------------------ 2 files changed, 37 insertions(+), 65 deletions(-) diff --git a/dlls/mf/samplegrabber.c b/dlls/mf/samplegrabber.c index 88da642b25e..4fc7428b4b7 100644 --- a/dlls/mf/samplegrabber.c +++ b/dlls/mf/samplegrabber.c @@ -91,7 +91,8 @@ struct sample_grabber CRITICAL_SECTION cs; UINT32 sample_count; IMFSample *samples[MAX_SAMPLE_QUEUE_LENGTH]; - byte samples_queued; + UINT64 samples_queued; + BYTE pending_sample_deliveries; }; static IMFSampleGrabberSinkCallback *sample_grabber_get_callback(const struct sample_grabber *sink) @@ -414,12 +415,13 @@ static HRESULT stream_queue_sample(struct sample_grabber *grabber, IMFSample *sa static void sample_grabber_stream_request_sample(struct sample_grabber *grabber) { IMFStreamSink_QueueEvent(&grabber->IMFStreamSink_iface, MEStreamSinkRequestSample, &GUID_NULL, S_OK, NULL); + grabber->pending_sample_deliveries++; } static HRESULT WINAPI sample_grabber_stream_ProcessSample(IMFStreamSink *iface, IMFSample *sample) { struct sample_grabber *grabber = impl_from_IMFStreamSink(iface); - BOOL sample_delivered; + BOOL sample_delivered = FALSE; LONGLONG sampletime; HRESULT hr = S_OK; @@ -434,6 +436,9 @@ static HRESULT WINAPI sample_grabber_stream_ProcessSample(IMFStreamSink *iface, hr = MF_E_STREAMSINK_REMOVED; else if (grabber->state != SINK_STATE_STOPPED) { + if (grabber->pending_sample_deliveries) + grabber->pending_sample_deliveries--; + hr = IMFSample_GetSampleTime(sample, &sampletime); if (SUCCEEDED(hr)) @@ -1157,6 +1162,7 @@ static HRESULT sample_grabber_set_state(struct sample_grabber *grabber, enum sin [SINK_STATE_RUNNING] = MEStreamSinkStarted, }; BOOL do_callback = FALSE; + byte required_requests; HRESULT hr = S_OK; unsigned int i; @@ -1173,25 +1179,22 @@ static HRESULT sample_grabber_set_state(struct sample_grabber *grabber, enum sin sample_grabber_cancel_timer(grabber); release_samples(grabber); grabber->sample_count = MAX_SAMPLE_QUEUE_LENGTH; + sample_grabber_release_pending_items(grabber); + grabber->pending_sample_deliveries = 0; } - - if (state == SINK_STATE_RUNNING && grabber->state != SINK_STATE_RUNNING) + else if (state == SINK_STATE_RUNNING && + (grabber->state != SINK_STATE_RUNNING || offset != PRESENTATION_CURRENT_POSITION)) { - /* Every transition to running state sends a bunch requests to build up initial queue. */ - for (i = 0; i < grabber->sample_count; ++i) + if (offset != PRESENTATION_CURRENT_POSITION) { - if (grabber->state == SINK_STATE_PAUSED && offset == PRESENTATION_CURRENT_POSITION) - { - assert(grabber->samples[i]); - stream_queue_sample(grabber, grabber->samples[i]); - } - else - { - sample_grabber_stream_request_sample(grabber); - } + sample_grabber_cancel_timer(grabber); + sample_grabber_release_pending_items(grabber); } - release_samples(grabber); - grabber->sample_count = 0; + + required_requests = MAX_SAMPLE_QUEUE_LENGTH - grabber->pending_sample_deliveries; + + for (i = 0; i < required_requests; i++) + sample_grabber_stream_request_sample(grabber); } do_callback = state != grabber->state || state != SINK_STATE_PAUSED; diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 6089e4e77d6..5b5cec7f44e 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -4334,7 +4334,6 @@ static HRESULT WINAPI timer_CancelTimer(IMFTimer *iface, IUnknown *cancel_key) { struct presentation_clock* pc = impl_from_IMFTimer(iface); - todo_wine CHECK_EXPECT(timer_CancelTimer); ok(cancel_key == pc->cancel_key, "Unexpected cancel key %p\n", cancel_key); @@ -4559,8 +4558,6 @@ static void supply_samples(IMFStreamSink *stream, int num_samples) } } -static BOOL ignore_clock = FALSE; - #define count_samples_requested(stream) _count_samples_requested(__LINE__, stream) static int _count_samples_requested(int line, IMFStreamSink *stream) { @@ -4585,7 +4582,6 @@ static int _count_samples_requested(int line, IMFStreamSink *stream) } else if (met == MEStreamSinkMarker) { - todo_wine_if(!ignore_clock) CHECK_EXPECT(MEStreamSinkMarker); break; } @@ -4737,18 +4733,15 @@ static void test_sample_grabber_seek(void) SET_EXPECT(timer_CancelTimer); hr = IMFClockStateSink_OnClockStart(clock_sink, 0, 1234); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine CHECK_CALLED(timer_CancelTimer); samples_requested = count_samples_requested(stream); - todo_wine ok(samples_requested == 4, "Unexpected number of samples requested %d\n", samples_requested); /* test number of new sample requests on seek when in running state and 3 samples have been provided */ sample_pts = 0; SET_EXPECT(timer_SetTimer); supply_samples(stream, 2); - todo_wine CHECK_CALLED(timer_SetTimer); /* this marker gets silently discarded on the next seek */ hr = IMFStreamSink_PlaceMarker(stream, MFSTREAMSINK_MARKER_DEFAULT, NULL, NULL); @@ -4776,18 +4769,15 @@ static void test_sample_grabber_seek(void) SET_EXPECT(timer_CancelTimer); hr = IMFClockStateSink_OnClockStart(clock_sink, 0, 1234); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine CHECK_CALLED(timer_CancelTimer); samples_requested = count_samples_requested(stream); - todo_wine ok(samples_requested == 2, "Unexpected number of samples requested %d\n", samples_requested); /* test number of new sample requests after a flush then seek */ sample_pts = expected_pts = 0; SET_EXPECT(timer_SetTimer); supply_samples(stream, 2); - todo_wine CHECK_CALLED(timer_SetTimer); /* there is no cancel timer, or sample requests during a flush */ @@ -4800,11 +4790,9 @@ static void test_sample_grabber_seek(void) SET_EXPECT(timer_CancelTimer); hr = IMFClockStateSink_OnClockStart(clock_sink, 0, 1234); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine CHECK_CALLED(timer_CancelTimer); samples_requested = count_samples_requested(stream); - todo_wine ok(samples_requested == 3, "Unexpected number of samples requested %d\n", samples_requested); /* test number of new sample requests on seek whilst stopped */ @@ -4821,40 +4809,30 @@ static void test_sample_grabber_seek(void) sample_pts = expected_pts = 0; SET_EXPECT(timer_SetTimer); supply_samples(stream, 1); - todo_wine CHECK_CALLED(timer_SetTimer); hr = IMFStreamSink_PlaceMarker(stream, MFSTREAMSINK_MARKER_DEFAULT, NULL, NULL); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); supply_samples(stream, 2); - todo_wine hr = trigger_timer(mock_clock); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = WaitForSingleObject(grabber_callback_impl->ready_event, 1000); - todo_wine ok(hr == WAIT_OBJECT_0, "Unexpected hr %#lx.\n", hr); - if (hr == WAIT_OBJECT_0) - { - expected_pts = 41667; - ResetEvent(mock_clock->set_timer_event); - SET_EXPECT(timer_SetTimer); - SetEvent(grabber_callback_impl->done_event); + expected_pts = 41667; + ResetEvent(mock_clock->set_timer_event); + SET_EXPECT(timer_SetTimer); + SetEvent(grabber_callback_impl->done_event); - SET_EXPECT(MEStreamSinkMarker); - samples_requested = count_samples_requested(stream); - ok(samples_requested == 1, "Unexpected number of samples requested %d\n", samples_requested); - hr = WaitForSingleObject(mock_clock->set_timer_event, 1000); - ok(hr == WAIT_OBJECT_0, "Unexpected hr %#lx.\n", hr); - CHECK_CALLED(MEStreamSinkMarker); - CHECK_CALLED(timer_SetTimer); - } - else - { - skip("skipping MEStreamSinkMarker test\n"); - } + SET_EXPECT(MEStreamSinkMarker); + samples_requested = count_samples_requested(stream); + todo_wine + ok(samples_requested == 1, "Unexpected number of samples requested %d\n", samples_requested); + hr = WaitForSingleObject(mock_clock->set_timer_event, 1000); + ok(hr == WAIT_OBJECT_0, "Unexpected hr %#lx.\n", hr); + CHECK_CALLED(MEStreamSinkMarker); + CHECK_CALLED(timer_SetTimer); hr = IMFClockStateSink_OnClockPause(clock_sink, 0); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -4888,14 +4866,12 @@ static void test_sample_grabber_seek(void) expected_pts = sample_pts; SET_EXPECT(timer_SetTimer); supply_samples(stream, 4); - todo_wine CHECK_CALLED(timer_SetTimer); hr = IMFClockStateSink_OnClockRestart(clock_sink, 0); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); samples_requested = count_samples_requested(stream); - todo_wine ok(samples_requested == 4, "Unexpected number of samples requested %d\n", samples_requested); /* test pause and resume with 4 samples queued */ @@ -4915,7 +4891,6 @@ static void test_sample_grabber_seek(void) SET_EXPECT(timer_CancelTimer); hr = IMFClockStateSink_OnClockStart(clock_sink, 0, 1234567); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine CHECK_CALLED(timer_CancelTimer); samples_requested = count_samples_requested(stream); @@ -4936,24 +4911,19 @@ static void test_sample_grabber_seek(void) /* check contents of collection */ hr = IMFCollection_GetElementCount(grabber_callback_impl->samples, &count); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(count == ARRAY_SIZE(use_clock_samples), "Unexpected total of samples delivered %ld\n", count); for (i = 0; i < ARRAY_SIZE(use_clock_samples); i++) { hr = IMFCollection_GetElement(grabber_callback_impl->samples, i, (IUnknown**)&sample); - todo_wine_if(i) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - if (hr == S_OK) - { - hr = IMFSample_GetSampleTime(sample, &pts); - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - ok(pts == use_clock_samples[i], "%d: Unexpected pts %I64d, expected %I64d\n", i, pts, use_clock_samples[i]); + hr = IMFSample_GetSampleTime(sample, &pts); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(pts == use_clock_samples[i], "%d: Unexpected pts %I64d, expected %I64d\n", i, pts, use_clock_samples[i]); - ref = IMFSample_Release(sample); - ok(ref == 1, "Release returned %ld\n", ref); - } + ref = IMFSample_Release(sample); + ok(ref == 1, "Release returned %ld\n", ref); } /* required for the sink to be fully released */ @@ -4969,7 +4939,6 @@ static void test_sample_grabber_seek(void) /* test with MF_SAMPLEGRABBERSINK_IGNORE_CLOCK */ - ignore_clock = TRUE; grabber_callback = create_test_grabber_callback(); grabber_callback_impl = impl_from_IMFSampleGrabberSinkCallback(grabber_callback); grabber_callback_impl->do_event = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9489