[PATCH v3 0/2] MR9249: winegstreamer: Return NS_E_NO_MORE_SAMPLES from WMSyncReader if sample fails to read.

[Yuxuan Shui (＠yshui)]

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

[PATCH v3 1/2] wmvcore/tests: Check what happens to a WMSyncReader when its allocator fails.

[Yuxuan Shui]

From: Yuxuan Shui

---
 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();

[PATCH v3 2/2] winegstreamer: Return NS_E_NO_MORE_SAMPLES from WMSyncReader if sample fails to read.

[Yuxuan Shui]

From: Yuxuan Shui

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;

[Yuxuan Shui (＠yshui)]

sorry, had to amend some small issues.

[Elizabeth Figura (＠zfigura)]

```
+
+        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().