[PATCH v3 0/2] MR9249: winegstreamer: Return NS_E_NO_MORE_SAMPLES from WMSyncReader if sample fails to read.
If allocator fails, native returns NS_E_NO_MORE_SAMPLES whereas we propagate the allocator's failure code. Some games checks this return value and will raise an error/exception if anything other than NS_E_NO_MORE_SAMPLES is returned, for example any game using the KiriKiri movie player [0] will do this. [0]: https://github.com/krkrz/krkrz/blob/fd5c4baa6a2ef5978db1bd043634351f48667daf... -- v3: winegstreamer: Return NS_E_NO_MORE_SAMPLES from WMSyncReader if sample fails to read. https://gitlab.winehq.org/wine/wine/-/merge_requests/9249
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/wmvcore/tests/wmvcore.c | 97 ++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index 462916edfa8..8fe56968b2b 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -4170,6 +4170,102 @@ static void test_sync_reader_allocator(void) callback_cleanup(&callback); } +static struct IWMReaderAllocatorEx failing_allocator; + +static HRESULT WINAPI failing_allocator_QueryInterface(IWMReaderAllocatorEx *this, REFIID riid, void **out) +{ + if (IsEqualGUID(riid, &IID_IWMReaderAllocatorEx)) + { + *out = &failing_allocator; + return S_OK; + } + return E_NOINTERFACE; +} + +static ULONG WINAPI failing_allocator_AddRef(IWMReaderAllocatorEx *this) +{ + return 2; +} + +static ULONG WINAPI failing_allocator_Release(IWMReaderAllocatorEx *this) +{ + return 1; +} + +static HRESULT WINAPI failing_allocator_AllocateForStream(IWMReaderAllocatorEx *This, WORD wStreamNum, + DWORD cbBuffer, INSSBuffer **ppBuffer, DWORD dwFlags, QWORD cnsSampleTime, QWORD cnsSampleDuration, + void *pvContext) +{ + return E_FAIL; +} + +static HRESULT WINAPI failing_allocator_AllocateForOutput(IWMReaderAllocatorEx *This, DWORD dwOutput, + DWORD cbBuffer, INSSBuffer **ppBuffer, DWORD dwFlags, QWORD cnsSampleTime, QWORD cnsSampleDuration, + void *pvContext) +{ + return E_FAIL; +} + +static struct IWMReaderAllocatorExVtbl failing_allocator_vtbl = +{ + failing_allocator_QueryInterface, + failing_allocator_AddRef, + failing_allocator_Release, + failing_allocator_AllocateForStream, + failing_allocator_AllocateForOutput, +}; +static struct IWMReaderAllocatorEx failing_allocator = { &failing_allocator_vtbl }; + +static void test_sync_reader_allocator_failure(void) +{ + const WCHAR *filename = load_resource(L"test.wmv"); + struct teststream stream; + DWORD output_num, flags; + IWMSyncReader2 *reader; + QWORD pts, duration; + INSSBuffer *sample; + WORD stream_num; + HANDLE file; + HRESULT hr; + BOOL ret; + + hr = WMCreateSyncReader(NULL, 0, (IWMSyncReader **)&reader); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + file = CreateFileW(filename, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, 0); + ok(file != INVALID_HANDLE_VALUE, "Failed to open %s, error %lu.\n", debugstr_w(file), GetLastError()); + + teststream_init(&stream, file); + + hr = IWMSyncReader2_OpenStream(reader, &stream.IStream_iface); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(stream.refcount > 1, "Got refcount %ld.\n", stream.refcount); + + + hr = IWMSyncReader2_GetStreamNumberForOutput(reader, 0, &stream_num); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMSyncReader2_SetAllocateForStream(reader, stream_num, &failing_allocator); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMSyncReader2_SetReadStreamSamples(reader, stream_num, TRUE); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMSyncReader2_SetAllocateForOutput(reader, 1, &failing_allocator); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IWMSyncReader2_GetStreamNumberForOutput(reader, 0, &stream_num); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMSyncReader2_GetNextSample(reader, stream_num, &sample, &pts, &duration, &flags, + &output_num, &stream_num); + todo_wine ok(hr == NS_E_NO_MORE_SAMPLES, "Got hr %#lx.\n", hr); + + IWMSyncReader2_Release(reader); + + ok(stream.refcount == 1, "Got outstanding refcount %ld.\n", stream.refcount); + CloseHandle(stream.file); + ret = DeleteFileW(filename); + ok(ret, "Failed to delete %s, error %lu.\n", debugstr_w(filename), GetLastError()); +} + + START_TEST(wmvcore) { HRESULT hr; @@ -4189,6 +4285,7 @@ START_TEST(wmvcore) test_urlextension(); test_iscontentprotected(); test_sync_reader_allocator(); + test_sync_reader_allocator_failure(); test_sync_reader_settings(); test_sync_reader_streaming(); test_sync_reader_types(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9249
From: Yuxuan Shui <yshui(a)codeweavers.com> If allocator fails, native returns NS_E_NO_MORE_SAMPLES whereas we propagate the allocator's failure code. Some games checks this return value and will raise an error/exception if anything other than NS_E_NO_MORE_SAMPLES is returned, for example any game using the KiriKiri movie player [0] will do this. [0]: https://github.com/krkrz/krkrz/blob/fd5c4baa6a2ef5978db1bd043634351f48667daf... --- dlls/winegstreamer/wm_reader.c | 2 ++ dlls/wmvcore/tests/wmvcore.c | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 467662b964c..624b0c591f3 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -2063,6 +2063,8 @@ static HRESULT WINAPI reader_GetNextSample(IWMSyncReader2 *iface, if (SUCCEEDED(hr) && SUCCEEDED(hr = wm_reader_read_stream_sample(reader, &wg_buffer, sample, pts, duration, flags))) stream_number = wg_buffer.stream + 1; + + if (FAILED(hr)) hr = NS_E_NO_MORE_SAMPLES; } if (stream && hr == NS_E_NO_MORE_SAMPLES) diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index 8fe56968b2b..d16947db6bb 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -4255,7 +4255,7 @@ static void test_sync_reader_allocator_failure(void) ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IWMSyncReader2_GetNextSample(reader, stream_num, &sample, &pts, &duration, &flags, &output_num, &stream_num); - todo_wine ok(hr == NS_E_NO_MORE_SAMPLES, "Got hr %#lx.\n", hr); + ok(hr == NS_E_NO_MORE_SAMPLES, "Got hr %#lx.\n", hr); IWMSyncReader2_Release(reader); @@ -4265,7 +4265,6 @@ static void test_sync_reader_allocator_failure(void) ok(ret, "Failed to delete %s, error %lu.\n", debugstr_w(filename), GetLastError()); } - START_TEST(wmvcore) { HRESULT hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9249
sorry, had to amend some small issues. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9249#note_119294
``` + + if (FAILED(hr)) hr = NS_E_NO_MORE_SAMPLES; ``` This doesn't feel like the most idiomatic place to put this. (It also doesn't match the style of the rest of the module.) Likely better would be to replace 'return hr' with 'return NS_E_NO_MORE_SAMPLES' in wm_reader_read_stream_sample(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9249#note_119732
participants (3)
-
Elizabeth Figura (＠zfigura) -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)