This MR fix some video play crashes for KiriKiri games.
If we don't correctly return failure in dmo_wrapper_sink_Receive() call, the worker thread which calls Receive() will continue running and crashes then. With this MR, the worker thread will terminate as expected because Receive() fails.
From: Ziqing Hui zhui@codeweavers.com
--- dlls/qasf/dmowrapper.c | 2 ++ dlls/qasf/tests/dmowrapper.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index 18d40a5671a..dc3f02d164b 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -311,6 +311,8 @@ static HRESULT WINAPI dmo_wrapper_sink_Receive(struct strmbase_sink *iface, IMed
if (filter->filter.state == State_Stopped) return VFW_E_WRONG_STATE; + if (iface->flushing) + return S_FALSE;
IUnknown_QueryInterface(filter->dmo, &IID_IMediaObject, (void **)&dmo);
diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index 388004de783..72dabd6658f 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1823,7 +1823,7 @@ static void test_streaming_events(IMediaControl *control, IPin *sink, IMemInputP ok(testsink2->got_begin_flush == 1, "Got %u calls to IPin::BeginFlush().\n", testsink2->got_begin_flush);
hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == S_FALSE, "Got hr %#lx.\n", hr); + ok(hr == S_FALSE, "Got hr %#lx.\n", hr);
hr = IPin_EndOfStream(sink); todo_wine ok(hr == S_FALSE, "Got hr %#lx.\n", hr);
From: Ziqing Hui zhui@codeweavers.com
--- dlls/qasf/tests/dmowrapper.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index 72dabd6658f..bfa107626cf 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1262,6 +1262,8 @@ static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample
if (testmode == 13 && got_Receive > 1) WaitForSingleObject(filter->event, INFINITE); + if (testmode == 14) + return E_FAIL;
return S_OK; } @@ -1747,6 +1749,22 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, 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); + hr = IMediaControl_Pause(control); + ok(hr == S_OK, "Pause returned %#lx.\n", hr); + got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0; + + /* Test receive if downstream receive fails. */ + testmode = 14; + hr = IMemInputPin_Receive(input, sample); + todo_wine + ok(hr == E_FAIL, "Receive returned %#lx.\n", hr); + todo_wine + ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); + todo_wine + ok(got_ProcessOutput == 1, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); + todo_wine + ok(got_Receive == 1, "Got %u calls to Receive().\n", got_Receive); + ok(got_Discontinuity == 1, "Got %u calls to Discontinuity().\n", got_Discontinuity); got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0;
testmode = 0;
From: Ziqing Hui zhui@codeweavers.com
--- dlls/qasf/dmowrapper.c | 11 ++++++++--- dlls/qasf/tests/dmowrapper.c | 4 ---- 2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index dc3f02d164b..8d2d24cc12f 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -305,9 +305,9 @@ static HRESULT WINAPI dmo_wrapper_sink_Receive(struct strmbase_sink *iface, IMed struct dmo_wrapper *filter = impl_from_strmbase_filter(iface->pin.filter); DWORD index = iface - filter->sinks; REFERENCE_TIME start = 0, stop = 0; + HRESULT process_hr, hr; IMediaObject *dmo; DWORD flags = 0; - HRESULT hr;
if (filter->filter.state == State_Stopped) return VFW_E_WRONG_STATE; @@ -329,7 +329,11 @@ static HRESULT WINAPI dmo_wrapper_sink_Receive(struct strmbase_sink *iface, IMed * 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(process_hr = process_output(filter, dmo))) + { + hr = process_hr; + goto out; + } }
if (FAILED(hr = get_output_samples(filter))) @@ -353,7 +357,8 @@ static HRESULT WINAPI dmo_wrapper_sink_Receive(struct strmbase_sink *iface, IMed goto out; }
- process_output(filter, dmo); + if (FAILED(process_hr = process_output(filter, dmo))) + hr = process_hr;
out: filter->input_buffer.sample = NULL; diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index bfa107626cf..78d138b104e 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1756,13 +1756,9 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, /* Test receive if downstream receive fails. */ testmode = 14; hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == E_FAIL, "Receive returned %#lx.\n", hr); - todo_wine ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); - todo_wine ok(got_ProcessOutput == 1, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); - todo_wine ok(got_Receive == 1, "Got %u calls to Receive().\n", got_Receive); ok(got_Discontinuity == 1, "Got %u calls to Discontinuity().\n", got_Discontinuity); got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0;
Do we need the separate "process_hr" variable? It doesn't look to me like we do.
On Tue Dec 3 03:02:06 2024 +0000, Elizabeth Figura wrote:
Do we need the separate "process_hr" variable? It doesn't look to me like we do.
If we don't use a separate "process_hr", https://gitlab.winehq.org/zhui/wine/-/blob/0e3ed50a318cc3b9a6a7be52a179f4fbd... will get S_FALSE, it expect S_OK.
This merge request was approved by Elizabeth Figura.
-
Elizabeth Figura (＠zfigura) -
Ziqing Hui
-
Ziqing Hui (＠zhui)