[PATCH 0/2] MR6957: qasf: Correctly return failure in process_output.
The tests indicate that for dmo_wrapper_sink_Receive: (1) If ProcessOutput succeeds, including S_FALSE, it continue processing. (2) If ProcessOutput fails, it returns S_OK. (3) If downstream Receive fails or return S_FALSE, it returns downstream hr. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6957
From: Ziqing Hui <zhui(a)codeweavers.com> The tests indicate that for dmo_wrapper_sink_Receive: (1) If ProcessOutput succeeds, including S_FALSE, it continue processing. (2) If ProcessOutput fails, it returns S_OK. (3) If downstream Receive fails or return S_FALSE, it returns downstream hr. --- dlls/qasf/tests/dmowrapper.c | 106 ++++++++++++++++++++++++++--------- 1 file changed, 80 insertions(+), 26 deletions(-) diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index 78d138b104e..afea0b86a4c 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -22,6 +22,7 @@ #include "dshow.h" #include "dmo.h" #include "dmodshow.h" +#include "mferror.h" #include "wine/strmbase.h" #include "wine/test.h" @@ -81,6 +82,8 @@ static unsigned int got_Flush, got_Discontinuity, got_ProcessInput, got_ProcessO static IMediaBuffer *testdmo_buffer; static int testmode; +static HRESULT process_output_hr = S_OK; +static HRESULT sink_receive_hr = S_OK; static HRESULT WINAPI dmo_inner_QueryInterface(IUnknown *iface, REFIID iid, void **out) { @@ -391,33 +394,30 @@ static HRESULT WINAPI dmo_ProcessOutput(IMediaObject *iface, DWORD flags, buffers[0].dwStatus |= DMO_OUTPUT_DATA_BUFFERF_INCOMPLETE; return S_OK; } - else if (testmode == 5) + if (testmode == 5) { hr = IMediaBuffer_SetLength(buffers[0].pBuffer, 0); ok(hr == S_OK, "Got hr %#lx.\n", hr); IMediaBuffer_Release(testdmo_buffer); return S_FALSE; } - else - { - if (testmode == 7) - buffers[0].dwStatus |= DMO_OUTPUT_DATA_BUFFERF_SYNCPOINT; - else if (testmode == 8) - buffers[0].dwStatus = DMO_OUTPUT_DATA_BUFFERF_TIME; - else if (testmode == 9) - buffers[0].dwStatus = 0; - - for (i = 0; i < 300; ++i) - data[i] = 111 - i; - hr = IMediaBuffer_SetLength(buffers[0].pBuffer, 300); - ok(hr == S_OK, "Got hr %#lx.\n", hr); - if (testdmo_buffer) - IMediaBuffer_Release(testdmo_buffer); - testdmo_buffer = NULL; - return S_OK; - } - return S_OK; + if (testmode == 7) + buffers[0].dwStatus |= DMO_OUTPUT_DATA_BUFFERF_SYNCPOINT; + else if (testmode == 8) + buffers[0].dwStatus = DMO_OUTPUT_DATA_BUFFERF_TIME; + else if (testmode == 9) + buffers[0].dwStatus = 0; + + for (i = 0; i < 300; ++i) + data[i] = 111 - i; + hr = IMediaBuffer_SetLength(buffers[0].pBuffer, 300); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + if (testdmo_buffer) + IMediaBuffer_Release(testdmo_buffer); + testdmo_buffer = NULL; + + return process_output_hr; } static HRESULT WINAPI dmo_Lock(IMediaObject *iface, LONG lock) @@ -1262,10 +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; + return sink_receive_hr; } static HRESULT testsink_new_segment(struct strmbase_sink *iface, @@ -1753,8 +1751,10 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, 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; + testmode = 0xdeadbeef; + + /* Test Receive if downstream Receive fails. */ + sink_receive_hr = E_FAIL; hr = IMemInputPin_Receive(input, sample); ok(hr == E_FAIL, "Receive returned %#lx.\n", hr); ok(got_ProcessInput == 0, "Got %u calls to ProcessInput().\n", got_ProcessInput); @@ -1763,7 +1763,61 @@ 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; - testmode = 0; + /* Test Receive if downstream Receive return S_FALSE. */ + sink_receive_hr = S_FALSE; + hr = IMemInputPin_Receive(input, sample); + todo_wine + ok(hr == S_FALSE, "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; + + sink_receive_hr = S_OK; + + /* Test Receive if ProcessOutput return S_FALSE. */ + process_output_hr = S_FALSE; + hr = IMemInputPin_Receive(input, sample); + ok(hr == S_OK, "Receive returned %#lx.\n", hr); + ok(got_ProcessInput == 1, "Got %u calls to ProcessInput().\n", got_ProcessInput); + ok(got_ProcessOutput == 2, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); + todo_wine + ok(got_Receive == 2, "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; + + /* Test Receive if ProcessOutput fails. */ + process_output_hr = E_FAIL; + hr = IMemInputPin_Receive(input, sample); + todo_wine + ok(hr == S_OK, "Receive returned %#lx.\n", hr); + todo_wine + ok(got_ProcessInput == 1, "Got %u calls to ProcessInput().\n", got_ProcessInput); + todo_wine + ok(got_ProcessOutput == 2, "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); + got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0; + + /* Test Receive if ProcessOutput needs more inputs. */ + process_output_hr = MF_E_TRANSFORM_NEED_MORE_INPUT; + hr = IMemInputPin_Receive(input, sample); + todo_wine + ok(hr == S_OK, "Receive returned %#lx.\n", hr); + todo_wine + ok(got_ProcessInput == 1, "Got %u calls to ProcessInput().\n", got_ProcessInput); + todo_wine + ok(got_ProcessOutput == 2, "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); + got_ProcessInput = got_ProcessOutput = got_Receive = got_Discontinuity = 0; + + process_output_hr = S_OK; + CloseHandle(stop_thread); CloseHandle(receive_thread); hr = IMediaControl_Stop(control); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6957
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/qasf/dmowrapper.c | 19 +++++++------------ dlls/qasf/tests/dmowrapper.c | 11 ----------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/dlls/qasf/dmowrapper.c b/dlls/qasf/dmowrapper.c index 8d2d24cc12f..4b9c3507160 100644 --- a/dlls/qasf/dmowrapper.c +++ b/dlls/qasf/dmowrapper.c @@ -245,17 +245,16 @@ static HRESULT get_output_samples(struct dmo_wrapper *filter) static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) { DMO_OUTPUT_DATA_BUFFER *buffers = filter->buffers; + HRESULT hr = S_OK, process_hr; DWORD status, i; BOOL more_data; - HRESULT hr; do { more_data = FALSE; - hr = IMediaObject_ProcessOutput(dmo, DMO_PROCESS_OUTPUT_DISCARD_WHEN_NO_BUFFER, - filter->source_count, buffers, &status); - if (hr != S_OK) + if (FAILED(process_hr = IMediaObject_ProcessOutput(dmo, DMO_PROCESS_OUTPUT_DISCARD_WHEN_NO_BUFFER, + filter->source_count, buffers, &status))) break; for (i = 0; i < filter->source_count; ++i) @@ -284,7 +283,7 @@ static HRESULT process_output(struct dmo_wrapper *filter, IMediaObject *dmo) if (IMediaSample_GetActualDataLength(sample)) { - if (FAILED(hr = IMemInputPin_Receive(filter->sources[i].pin.pMemInputPin, sample))) + if ((hr = IMemInputPin_Receive(filter->sources[i].pin.pMemInputPin, sample)) != S_OK) { WARN("Downstream sink returned %#lx.\n", hr); release_output_samples(filter); @@ -305,9 +304,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,11 +328,8 @@ 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; - if (FAILED(process_hr = process_output(filter, dmo))) - { - hr = process_hr; + if ((hr = process_output(filter, dmo)) != S_OK) goto out; - } } if (FAILED(hr = get_output_samples(filter))) @@ -357,8 +353,7 @@ static HRESULT WINAPI dmo_wrapper_sink_Receive(struct strmbase_sink *iface, IMed goto out; } - if (FAILED(process_hr = process_output(filter, dmo))) - hr = process_hr; + hr = process_output(filter, dmo); out: filter->input_buffer.sample = NULL; diff --git a/dlls/qasf/tests/dmowrapper.c b/dlls/qasf/tests/dmowrapper.c index afea0b86a4c..4ade744d105 100644 --- a/dlls/qasf/tests/dmowrapper.c +++ b/dlls/qasf/tests/dmowrapper.c @@ -1766,13 +1766,9 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, /* Test Receive if downstream Receive return S_FALSE. */ sink_receive_hr = S_FALSE; hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == S_FALSE, "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; @@ -1785,7 +1781,6 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, ok(hr == S_OK, "Receive returned %#lx.\n", hr); ok(got_ProcessInput == 1, "Got %u calls to ProcessInput().\n", got_ProcessInput); ok(got_ProcessOutput == 2, "Got %u calls to ProcessOutput().\n", got_ProcessOutput); - todo_wine ok(got_Receive == 2, "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; @@ -1793,11 +1788,8 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, /* Test Receive if ProcessOutput fails. */ process_output_hr = E_FAIL; hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == S_OK, "Receive returned %#lx.\n", hr); - todo_wine ok(got_ProcessInput == 1, "Got %u calls to ProcessInput().\n", got_ProcessInput); - todo_wine ok(got_ProcessOutput == 2, "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); @@ -1806,11 +1798,8 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, /* Test Receive if ProcessOutput needs more inputs. */ process_output_hr = MF_E_TRANSFORM_NEED_MORE_INPUT; hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == S_OK, "Receive returned %#lx.\n", hr); - todo_wine ok(got_ProcessInput == 1, "Got %u calls to ProcessInput().\n", got_ProcessInput); - todo_wine ok(got_ProcessOutput == 2, "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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6957
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=150206 Your paranoid android. === debian11b (64 bit WoW report) === user32: win.c:4070: Test failed: Expected active window 0000000005ED018C, got 0000000000000000. win.c:4071: Test failed: Expected focus window 0000000005ED018C, got 0000000000000000.
I found the game I was debugging still have issues after the previous dmo wrapper MR !6915. So I spend more time investigating it, and found the correct fix should be this MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6957#note_89673
Looks good. You don't actually need the "process_hr" variable since you don't use it after checking it, but I've approved the patch since it's fine regardless. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6957#note_89762
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6957
participants (4)
-
Elizabeth Figura (＠zfigura) -
Marvin -
Ziqing Hui
-
Ziqing Hui (＠zhui)