[PATCH v2 0/6] MR5717: Improve dmo_wrapper_sink_Receive.
This reduce the chance of buffer allocating issue for Kirikiri2 games(eg. Super Naughty Maid) when the game is trying to play videos. -- v2: qasf/dmowrapper: Sync Stop() and Receive() for dmo wrapper filter. qasf/dmowrapper: Return VFW_E_WRONG_STATE in dmo_wrapper_sink_Receive. qasf/dmowrapper: Return failure in dmo_wrapper_sink_Receive if samples allocating fails. qasf/dmowrapper: Introduce release_output_samples. qasf/dmowrapper: Introduce get_output_samples. qasf/tests: Add more tests for dmo_wrapper_sink_Receive. https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/qasf/tests/dmowrapper.c | 101 ++++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-) diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index 9fade2f7f1f..632b7e2e558 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1122,6 +1122,7 @@ struct testfilter struct strmbase_sink sink; const AM_MEDIA_TYPE *sink_mt; unsigned int got_new_segment, got_eos, got_begin_flush, got_end_flush; + HANDLE event; }; static inline struct testfilter *impl_from_strmbase_filter(struct strmbase_filter *iface) @@ -1145,6 +1146,7 @@ static void testfilter_destroy(struct strmbase_filter *iface) strmbase_source_cleanup(&filter->source); strmbase_sink_cleanup(&filter->sink); strmbase_filter_cleanup(&filter->filter); + CloseHandle(filter->event); } static const struct strmbase_filter_ops testfilter_ops = @@ -1199,6 +1201,7 @@ static HRESULT testsink_get_media_type(struct strmbase_pin *iface, unsigned int static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample *sample) { + struct testfilter *filter = impl_from_strmbase_filter(iface->pin.filter); REFERENCE_TIME start, stop; LONG len, i; HRESULT hr; @@ -1257,6 +1260,9 @@ static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample if (testmode == 10) testmode = 11; + if (testmode == 13 && got_Receive > 1) + WaitForSingleObject(filter->event, INFINITE); + return S_OK; } @@ -1311,6 +1317,7 @@ static void testfilter_init(struct testfilter *filter) strmbase_filter_init(&filter->filter, NULL, &clsid, &testfilter_ops); strmbase_source_init(&filter->source, &filter->filter, L"source", &testsource_ops); strmbase_sink_init(&filter->sink, &filter->filter, L"sink", &testsink_ops, NULL); + filter->event = CreateEventA(NULL, TRUE, FALSE, NULL); } static void test_sink_allocator(IMemInputPin *input) @@ -1518,15 +1525,46 @@ static void test_filter_state(IMediaControl *control) ok(state == State_Stopped, "Got state %lu.\n", state); } +struct receive_proc_arg +{ + IMemInputPin *input_pin; + IMediaSample *sample; +}; + +static DWORD WINAPI receive_proc(void *arg) +{ + struct receive_proc_arg *proc_arg = arg; + HRESULT hr; + + hr = IMemInputPin_Receive(proc_arg->input_pin, proc_arg->sample); + ok(hr == S_OK, "Receive returned %#lx.\n", hr); + + return 0; +} + +static DWORD WINAPI stop_filter_proc(void *arg) +{ + IBaseFilter *filter = arg; + HRESULT hr; + + hr = IBaseFilter_Stop(filter); + ok(hr == S_OK, "Stop returned %#lx.\n", hr); + + return 0; +} + static void test_sample_processing(IMediaControl *control, IMemInputPin *input, - struct testfilter *testsink) + struct testfilter *testsink, IBaseFilter *dmo_filter) { + struct receive_proc_arg receive_proc_arg; + HANDLE stop_thread, receive_thread; REFERENCE_TIME start, stop; IMemAllocator *allocator; IMediaSample *sample; LONG size, i; HRESULT hr; BYTE *data; + DWORD ret; hr = IMemInputPin_ReceiveCanBlock(input); ok(hr == S_OK, "Got hr %#lx.\n", hr); @@ -1661,6 +1699,65 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, ok(got_Discontinuity == 1, "Got %u calls to Discontinuity().\n", got_Discontinuity); got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0; + /* Test receive with downstream input allocator decommitted. */ + hr = IMemAllocator_Decommit(testsink->sink.pAllocator); + ok(hr == S_OK, "Decommit returned %#lx.\n", hr); + hr = IMemInputPin_Receive(input, sample); + todo_wine + ok(hr == VFW_E_NOT_COMMITTED, "Receive returned %#lx.\n", hr); + todo_wine + ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); + ok(got_ProcessOutput == 0, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); + ok(got_Receive == 0, "Got %u calls to Receive().\n", got_Receive); + ok(got_Discontinuity == 1, "Got %u calls to Discontinuity().\n", got_Discontinuity); + hr = IMemAllocator_Commit(testsink->sink.pAllocator); + ok(hr == S_OK, "Commit returned %#lx.\n", hr); + got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0; + + /* Test receive with filter stopped. */ + hr = IBaseFilter_Stop(dmo_filter); + ok(hr == S_OK, "Stop returned %#lx.\n", hr); + hr = IMemInputPin_Receive(input, sample); + todo_wine + ok(hr == VFW_E_WRONG_STATE, "Receive returned %#lx.\n", hr); + todo_wine + ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); + ok(got_ProcessOutput == 0, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); + ok(got_Receive == 0, "Got %u calls to Receive().\n", got_Receive); + todo_wine + ok(got_Discontinuity == 0, "Got %u calls to Discontinuity().\n", got_Discontinuity); + hr = IMediaControl_Run(control); + ok(hr == S_OK, "Run returned %#lx.\n", hr); + got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0; + + /* Test stop filter during receiving. */ + testmode = 13; + /* Call Receive() in new thread, it will be blocked + * by event waiting in testsink_Receive(). */ + receive_proc_arg.input_pin = input; + receive_proc_arg.sample = sample; + receive_thread = CreateThread(NULL, 0, receive_proc, &receive_proc_arg, 0, NULL); + ok(!!receive_thread, "CreateThread returned NULL thread.\n"); + ret = WaitForSingleObject(receive_thread, 200); + ok(ret == WAIT_TIMEOUT, "WaitForSingleObject returned %#lx.\n", ret); + /* Call Stop() in new thread, it will be blocked + * because Receive() is still processing. */ + stop_thread = CreateThread(NULL, 0, stop_filter_proc, dmo_filter, 0, NULL); + ok(!!stop_thread, "CreateThread returned NULL thread.\n"); + ret = WaitForSingleObject(stop_thread, 200); + todo_wine + ok(ret == WAIT_TIMEOUT, "WaitForSingleObject returned %#lx.\n", ret); + /* Signal event to end Receive(). */ + SetEvent(testsink->event); + ret = WaitForSingleObject(receive_thread, 200); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %#lx.\n", ret); + ret = WaitForSingleObject(stop_thread, 200); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %#lx.\n", ret); + got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0; + + testmode = 0; + CloseHandle(stop_thread); + CloseHandle(receive_thread); hr = IMediaControl_Stop(control); ok(hr == S_OK, "Got hr %#lx.\n", hr); IMediaSample_Release(sample); @@ -1882,7 +1979,7 @@ static void test_connect_pin(void) ok(hr == S_OK, "Got hr %#lx.\n", hr); test_filter_state(control); - test_sample_processing(control, meminput, &testsink); + test_sample_processing(control, meminput, &testsink, filter); /* Streaming event tests are more interesting if multiple source pins are * connected. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/qasf/dmowrapper.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index 95e12e860ae..b98b18a7cac 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -202,12 +202,10 @@ static void dmo_wrapper_sink_disconnect(struct strmbase_sink *iface) IMediaObject_Release(dmo); } -static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) +static HRESULT get_output_samples(struct dmo_wrapper *filter) { - DMO_OUTPUT_DATA_BUFFER *buffers = filter->buffers; - DWORD status, i; - BOOL more_data; HRESULT hr; + DWORD i; for (i = 0; i < filter->source_count; ++i) { @@ -216,16 +214,29 @@ static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) if (FAILED(hr = IMemAllocator_GetBuffer(filter->sources[i].pin.pAllocator, &filter->sources[i].buffer.sample, NULL, NULL, 0))) { - ERR("Failed to get sample, hr %#lx.\n", hr); - goto out; + ERR("Failed to get sample for source %lu, hr %#lx.\n", i, hr); + return hr; } - buffers[i].pBuffer = &filter->sources[i].buffer.IMediaBuffer_iface; + filter->buffers[i].pBuffer = &filter->sources[i].buffer.IMediaBuffer_iface; IMediaSample_SetActualDataLength(filter->sources[i].buffer.sample, 0); } else - buffers[i].pBuffer = NULL; + filter->buffers[i].pBuffer = NULL; } + return S_OK; +} + +static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) +{ + DMO_OUTPUT_DATA_BUFFER *buffers = filter->buffers; + DWORD status, i; + BOOL more_data; + HRESULT hr; + + if (FAILED(hr = get_output_samples(filter))) + goto out; + do { more_data = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/qasf/dmowrapper.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index b98b18a7cac..1d4ad7f75e6 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -202,6 +202,20 @@ static void dmo_wrapper_sink_disconnect(struct strmbase_sink *iface) IMediaObject_Release(dmo); } +static void release_output_samples(struct dmo_wrapper *filter) +{ + DWORD i; + + for (i = 0; i < filter->source_count; ++i) + { + if (filter->sources[i].buffer.sample) + { + IMediaSample_Release(filter->sources[i].buffer.sample); + filter->sources[i].buffer.sample = NULL; + } + } +} + static HRESULT get_output_samples(struct dmo_wrapper *filter) { HRESULT hr; @@ -215,6 +229,7 @@ static HRESULT get_output_samples(struct dmo_wrapper *filter) &filter->sources[i].buffer.sample, NULL, NULL, 0))) { ERR("Failed to get sample for source %lu, hr %#lx.\n", i, hr); + release_output_samples(filter); return hr; } filter->buffers[i].pBuffer = &filter->sources[i].buffer.IMediaBuffer_iface; @@ -235,7 +250,7 @@ static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) HRESULT hr; if (FAILED(hr = get_output_samples(filter))) - goto out; + return hr; do { @@ -275,7 +290,8 @@ static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) if (FAILED(hr = IMemInputPin_Receive(filter->sources[i].pin.pMemInputPin, sample))) { WARN("Downstream sink returned %#lx.\n", hr); - goto out; + release_output_samples(filter); + return hr; } IMediaSample_SetActualDataLength(sample, 0); } @@ -283,16 +299,7 @@ static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) } } while (more_data); -out: - for (i = 0; i < filter->source_count; ++i) - { - if (filter->sources[i].buffer.sample) - { - IMediaSample_Release(filter->sources[i].buffer.sample); - filter->sources[i].buffer.sample = NULL; - } - } - + release_output_samples(filter); return hr; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/qasf/dmowrapper.c | 12 ++++++++---- dlls/qasf/tests/dmowrapper.c | 3 --- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index 1d4ad7f75e6..f94b00dbb44 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -249,9 +249,6 @@ static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) BOOL more_data; HRESULT hr; - if (FAILED(hr = get_output_samples(filter))) - return hr; - do { more_data = FALSE; @@ -325,9 +322,14 @@ static HRESULT WINAPI dmo_wrapper_sink_Receive(struct strmbase_sink *iface, IMed /* Calling Discontinuity() might change the DMO's mind about whether it * has more data to process. The DirectX documentation explicitly * states that we should call ProcessOutput() again in this case. */ + if (FAILED(hr = get_output_samples(filter))) + goto out; process_output(filter, dmo); } + if (FAILED(hr = get_output_samples(filter))) + goto out; + if (IMediaSample_IsSyncPoint(sample) == S_OK) flags |= DMO_INPUT_DATA_BUFFERF_SYNCPOINT; @@ -366,7 +368,9 @@ static HRESULT dmo_wrapper_sink_eos(struct strmbase_sink *iface) if (FAILED(hr = IMediaObject_Discontinuity(dmo, index))) ERR("Discontinuity() failed, hr %#lx.\n", hr); - process_output(filter, dmo); + if (SUCCEEDED(get_output_samples(filter))) + process_output(filter, dmo); + if (FAILED(hr = IMediaObject_Flush(dmo))) ERR("Flush() failed, hr %#lx.\n", hr); diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index 632b7e2e558..ada45c447b1 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1703,9 +1703,7 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, hr = IMemAllocator_Decommit(testsink->sink.pAllocator); ok(hr == S_OK, "Decommit returned %#lx.\n", hr); hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == VFW_E_NOT_COMMITTED, "Receive returned %#lx.\n", hr); - todo_wine ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); ok(got_ProcessOutput == 0, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); ok(got_Receive == 0, "Got %u calls to Receive().\n", got_Receive); @@ -1720,7 +1718,6 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, hr = IMemInputPin_Receive(input, sample); todo_wine ok(hr == VFW_E_WRONG_STATE, "Receive returned %#lx.\n", hr); - todo_wine ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); ok(got_ProcessOutput == 0, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); ok(got_Receive == 0, "Got %u calls to Receive().\n", got_Receive); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/qasf/dmowrapper.c | 3 +++ dlls/qasf/tests/dmowrapper.c | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index f94b00dbb44..76704f88cd4 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -309,6 +309,9 @@ static HRESULT WINAPI dmo_wrapper_sink_Receive(struct strmbase_sink *iface, IMed DWORD flags = 0; HRESULT hr; + if (filter->filter.state == State_Stopped) + return VFW_E_WRONG_STATE; + IUnknown_QueryInterface(filter->dmo, &IID_IMediaObject, (void **)&dmo); if (IMediaSample_IsDiscontinuity(sample) == S_OK) diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index ada45c447b1..70dcc0c453e 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1716,12 +1716,10 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, hr = IBaseFilter_Stop(dmo_filter); ok(hr == S_OK, "Stop returned %#lx.\n", hr); hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == VFW_E_WRONG_STATE, "Receive returned %#lx.\n", hr); ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); ok(got_ProcessOutput == 0, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); ok(got_Receive == 0, "Got %u calls to Receive().\n", got_Receive); - todo_wine ok(got_Discontinuity == 0, "Got %u calls to Discontinuity().\n", got_Discontinuity); hr = IMediaControl_Run(control); ok(hr == S_OK, "Run returned %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/qasf/dmowrapper.c | 2 ++ dlls/qasf/tests/dmowrapper.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index 76704f88cd4..18d40a5671a 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -703,6 +703,7 @@ static HRESULT dmo_wrapper_cleanup_stream(struct strmbase_filter *iface) IUnknown_QueryInterface(filter->dmo, &IID_IMediaObject, (void **)&dmo); + EnterCriticalSection(&filter->filter.stream_cs); for (i = 0; i < filter->source_count; ++i) { if (filter->sources[i].pin.pin.peer) @@ -712,6 +713,7 @@ static HRESULT dmo_wrapper_cleanup_stream(struct strmbase_filter *iface) IMediaObject_Flush(dmo); IMediaObject_Release(dmo); + LeaveCriticalSection(&filter->filter.stream_cs); return S_OK; } diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index 70dcc0c453e..388004de783 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1740,7 +1740,6 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, stop_thread = CreateThread(NULL, 0, stop_filter_proc, dmo_filter, 0, NULL); ok(!!stop_thread, "CreateThread returned NULL thread.\n"); ret = WaitForSingleObject(stop_thread, 200); - todo_wine ok(ret == WAIT_TIMEOUT, "WaitForSingleObject returned %#lx.\n", ret); /* Signal event to end Receive(). */ SetEvent(testsink->event); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5717
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146041 Your paranoid android. === debian11b (64 bit WoW report) === Report validation errors: shell32:shelllink crashed (c0000005)
To be clear, patch 4/6 does two things: call get_output_samples() before ProcessInput() [which isn't mentioned in the subject], and propagate failure from process_output(). The former really should be spelled out explicitly, in the git history, and I'd prefer it to be a separate change. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5717#note_72410
On Wed Jun 5 03:29:58 2024 +0000, Elizabeth Figura wrote:
To be clear, patch 4/6 does two things: call get_output_samples() before ProcessInput() [which isn't mentioned in the subject], and propagate failure from process_output(). The former really should be spelled out explicitly, in the git history, and I'd prefer it to be a separate change. OK, I'll do it right away.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5717#note_72412
participants (4)
-
Elizabeth Figura (＠zfigura) -
Marvin -
Ziqing Hui
-
Ziqing Hui (＠zhui)