[PATCH 0/5] MR8042: amstream: Change ddraw stream private allocator properties after changing format.

[Santino Mazza (＠tati)]

This should fix the crash reported in https://bugs.winehq.org/show_bug.cgi?id=58171

[PATCH 1/5] amstream: Call QueryAccept with the new media type.

[Santino Mazza]

From: Santino Mazza

---
 dlls/amstream/ddrawstream.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
index 10e63681c9e..da7fcc4bdd6 100644
--- a/dlls/amstream/ddrawstream.c
+++ b/dlls/amstream/ddrawstream.c
@@ -677,21 +677,19 @@ static HRESULT WINAPI ddraw_IDirectDrawMediaStream_SetFormat(IDirectDrawMediaStr

if (stream->peer && !is_format_compatible(stream, old_format.width, old_format.height, &old_format.pf))
     {
-        if (stream->using_private_allocator &&
-            (IPin_QueryAccept(stream->peer, &stream->mt) == S_OK))
+        if (FAILED(hr = CopyMediaType(&old_media_type, &stream->mt)))
         {
-            set_mt_from_desc(&stream->mt, format);
+            stream->format = old_format;
+            LeaveCriticalSection(&stream->cs);
+            return hr;
         }
-        else
+
+        set_mt_from_desc(&stream->mt, format);
+
+        if (!stream->using_private_allocator ||
+            (IPin_QueryAccept(stream->peer, &stream->mt) != S_OK))
         {
             /* Reconnect. */
-            if (FAILED(hr = CopyMediaType(&old_media_type, &stream->mt)))
-            {
-                stream->format = old_format;
-                LeaveCriticalSection(&stream->cs);
-                return hr;
-            }
-
             old_peer = stream->peer;
             IPin_AddRef(old_peer);

@@ -707,8 +705,9 @@ static HRESULT WINAPI ddraw_IDirectDrawMediaStream_SetFormat(IDirectDrawMediaStr
                 return DDERR_INVALIDSURFACETYPE;
             }
             IPin_Release(old_peer);
-            FreeMediaType(&old_media_type);
         }
+
+        FreeMediaType(&old_media_type);
     }

LeaveCriticalSection(&stream->cs);

[PATCH 2/5] amstream/tests: Test how the ddraw allocator properties change.

[Santino Mazza]

From: Santino Mazza

---
 dlls/amstream/tests/amstream.c | 83 ++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index 70876ed07cf..45547b14018 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -8535,6 +8535,70 @@ static void test_ddrawstream_mem_allocator(void)
     hr = IMediaStream_QueryInterface(stream, &IID_IMemAllocator, (void **)&ddraw_allocator);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);

+    /* Check default properties. */
+    hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(props.cbBuffer == 10000, "Expected sample size to be 10000 got %ld\n", props.cbBuffer);
+    todo_wine ok(props.cBuffers == 1, "Expected 1 sample got %ld\n", props.cBuffers);
+    todo_wine ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);
+
+    /* Try changing allocator properties. */
+    props.cbAlign = 1;
+    props.cbBuffer = 1000;
+    props.cBuffers = 4;
+    props.cbPrefix = 0;
+    hr = IMemAllocator_SetProperties(ddraw_allocator, &props, &ret_props);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(ret_props.cbBuffer == 10000, "Expected sample size to be 10000 got %ld\n", ret_props.cbBuffer);
+    ok(ret_props.cBuffers == 4, "Expected 4 samples got %ld\n", ret_props.cBuffers);
+    ok(ret_props.cbAlign == 1, "Expected alignment of 1 got %ld\n", ret_props.cbAlign);
+
+    /* Check how allocator properties change when setting a new format. */
+    hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb32_format, NULL);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", props.cbBuffer);
+    ok(props.cBuffers == 4, "Expected 4 samples got %ld\n", props.cBuffers);
+    ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);
+
+    props.cbAlign = 1;
+    props.cbBuffer = 1000;
+    props.cBuffers = 0;
+    props.cbPrefix = 0;
+    hr = IMemAllocator_SetProperties(ddraw_allocator, &props, &ret_props);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(ret_props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", ret_props.cbBuffer);
+    todo_wine ok(ret_props.cBuffers == 1, "Expected 1 sample got %ld\n", ret_props.cBuffers);
+    ok(ret_props.cbAlign == 1, "Expected alignment of 1 got %ld\n", ret_props.cbAlign);
+
+    /* Try setting a larger buffer size. */
+    props.cbAlign = 1;
+    props.cbBuffer = 50000;
+    props.cBuffers = 1;
+    props.cbPrefix = 0;
+    hr = IMemAllocator_SetProperties(ddraw_allocator, &props, &ret_props);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(ret_props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", ret_props.cbBuffer);
+    ok(ret_props.cBuffers == 1, "Expected 1 sample got %ld\n", ret_props.cBuffers);
+    ok(ret_props.cbAlign == 1, "Expected alignment of 1 got %ld\n", ret_props.cbAlign);
+
+    hr = IMemAllocator_Commit(ddraw_allocator);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb555_format, NULL);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(props.cbBuffer == 20000, "Expected sample size to be 20000 got %ld\n", props.cbBuffer);
+    ok(props.cBuffers == 1, "Expected 1 samples got %ld\n", props.cBuffers);
+    ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);
+
+    hr = IMemAllocator_Decommit(ddraw_allocator);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
     mem_allocator = NULL;
     hr = IMemInputPin_GetAllocator(mem_input, &mem_allocator);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
@@ -8588,6 +8652,25 @@ static void test_ddrawstream_mem_allocator(void)
     hr = IAMMultiMediaStream_SetState(mmstream, STREAMSTATE_RUN);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);

+    props.cbAlign = 1;
+    props.cbBuffer = 50000;
+    props.cBuffers = 1;
+    props.cbPrefix = 0;
+    hr = IMemAllocator_SetProperties(ddraw_allocator, &props, &ret_props);
+    ok(hr == VFW_E_ALREADY_COMMITTED, "Got hr %#lx.\n", hr);
+
+    hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb32_format, NULL);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(props.cbBuffer == 333 * 444 * 4, "Got sample size %ld\n", props.cbBuffer);
+    ok(props.cBuffers == 2, "Expected 2 samples got %ld\n", props.cBuffers);
+    ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);
+
+    hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb555_format, NULL);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
     hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, NULL, NULL, 0, &ddraw_sample1);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
     hr = IDirectDrawStreamSample_Update(ddraw_sample1, SSUPDATE_ASYNC, NULL, NULL, 0);

[PATCH 3/5] amstream: Set initial properties for ddraw stream allocator.

[Santino Mazza]

From: Santino Mazza

---
 dlls/amstream/ddrawstream.c    | 11 +++++++++++
 dlls/amstream/tests/amstream.c |  6 +++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
index da7fcc4bdd6..80a5bd5db14 100644
--- a/dlls/amstream/ddrawstream.c
+++ b/dlls/amstream/ddrawstream.c
@@ -1621,6 +1621,7 @@ static const IMemInputPinVtbl ddraw_meminput_vtbl =
 HRESULT ddraw_stream_create(IUnknown *outer, void **out)
 {
     struct ddraw_stream *object;
+    ALLOCATOR_PROPERTIES props, ret_props;
     HRESULT hr;

if (outer)
@@ -1648,6 +1649,16 @@ HRESULT ddraw_stream_create(IUnknown *outer, void **out)
         return hr;
     }

+    props.cbBuffer = object->format.height * object->format.width;
+    props.cBuffers = 1;
+    props.cbAlign = 1;
+    props.cbPrefix = 0;
+    if (FAILED(hr = IMemAllocator_SetProperties(object->private_allocator, &props, &ret_props))) {
+        IMemAllocator_Release(object->private_allocator);
+        free(object);
+        return hr;
+    }
+
     InitializeCriticalSection(&object->cs);
     InitializeConditionVariable(&object->update_queued_cv);
     list_init(&object->update_queue);
diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index 45547b14018..b3fa59f6dab 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -8538,9 +8538,9 @@ static void test_ddrawstream_mem_allocator(void)
     /* Check default properties. */
     hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(props.cbBuffer == 10000, "Expected sample size to be 10000 got %ld\n", props.cbBuffer);
-    todo_wine ok(props.cBuffers == 1, "Expected 1 sample got %ld\n", props.cBuffers);
-    todo_wine ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);
+    ok(props.cbBuffer == 10000, "Expected sample size to be 10000 got %ld\n", props.cbBuffer);
+    ok(props.cBuffers == 1, "Expected 1 sample got %ld\n", props.cBuffers);
+    ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);

/* Try changing allocator properties. */
     props.cbAlign = 1;

[PATCH 4/5] amstream: Set allocator properties when changing format.

[Santino Mazza]

From: Santino Mazza

---
 dlls/amstream/ddrawstream.c    | 19 ++++++++++++++++++-
 dlls/amstream/tests/amstream.c | 14 +++++++-------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
index 80a5bd5db14..c7eba9b4892 100644
--- a/dlls/amstream/ddrawstream.c
+++ b/dlls/amstream/ddrawstream.c
@@ -592,6 +592,7 @@ static HRESULT WINAPI ddraw_IDirectDrawMediaStream_SetFormat(IDirectDrawMediaStr
         const DDSURFACEDESC *format, IDirectDrawPalette *palette)
 {
     struct ddraw_stream *stream = impl_from_IDirectDrawMediaStream(iface);
+    ALLOCATOR_PROPERTIES new_props, ret_props;
     AM_MEDIA_TYPE old_media_type;
     struct format old_format;
     IPin *old_peer;
@@ -710,9 +711,20 @@ static HRESULT WINAPI ddraw_IDirectDrawMediaStream_SetFormat(IDirectDrawMediaStr
         FreeMediaType(&old_media_type);
     }

+    /* Set private allocator sample size */
+    IMemAllocator_GetProperties(stream->private_allocator, &new_props);
+    new_props.cbBuffer = stream->format.width * stream->format.height * (stream->format.pf.dwRGBBitCount / 8);
+    hr = IMemAllocator_SetProperties(stream->private_allocator, &new_props, &ret_props);
+    if (hr == VFW_E_ALREADY_COMMITTED)
+    {
+        IMemAllocator_Decommit(stream->private_allocator);
+        hr = IMemAllocator_SetProperties(stream->private_allocator, &new_props, &ret_props);
+        IMemAllocator_Commit(stream->private_allocator);
+    }
+
     LeaveCriticalSection(&stream->cs);

-    return S_OK;
+    return hr;
 }

static HRESULT WINAPI ddraw_IDirectDrawMediaStream_GetDirectDraw(IDirectDrawMediaStream *iface,
@@ -1362,10 +1374,15 @@ static ULONG WINAPI ddraw_mem_allocator_Release(IMemAllocator *iface)
 static HRESULT WINAPI ddraw_mem_allocator_SetProperties(IMemAllocator *iface,
         ALLOCATOR_PROPERTIES *req_props, ALLOCATOR_PROPERTIES *ret_props)
 {
+    ALLOCATOR_PROPERTIES orig_props;
     struct ddraw_stream *stream = impl_from_IMemAllocator(iface);

TRACE("stream %p, req_props %p, ret_props %p.\n", stream, req_props, ret_props);

+    IMemAllocator_GetProperties(stream->private_allocator, &orig_props);
+    req_props->cbBuffer = orig_props.cbBuffer;
+    req_props->cBuffers = req_props->cBuffers > 0 ? req_props->cBuffers : 1;
+
     return IMemAllocator_SetProperties(stream->private_allocator, req_props, ret_props);
 }

diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index b3fa59f6dab..2c05bcffd92 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -8549,7 +8549,7 @@ static void test_ddrawstream_mem_allocator(void)
     props.cbPrefix = 0;
     hr = IMemAllocator_SetProperties(ddraw_allocator, &props, &ret_props);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(ret_props.cbBuffer == 10000, "Expected sample size to be 10000 got %ld\n", ret_props.cbBuffer);
+    ok(ret_props.cbBuffer == 10000, "Expected sample size to be 10000 got %ld\n", ret_props.cbBuffer);
     ok(ret_props.cBuffers == 4, "Expected 4 samples got %ld\n", ret_props.cBuffers);
     ok(ret_props.cbAlign == 1, "Expected alignment of 1 got %ld\n", ret_props.cbAlign);

@@ -8559,7 +8559,7 @@ static void test_ddrawstream_mem_allocator(void)

hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", props.cbBuffer);
+    ok(props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", props.cbBuffer);
     ok(props.cBuffers == 4, "Expected 4 samples got %ld\n", props.cBuffers);
     ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);

@@ -8569,8 +8569,8 @@ static void test_ddrawstream_mem_allocator(void)
     props.cbPrefix = 0;
     hr = IMemAllocator_SetProperties(ddraw_allocator, &props, &ret_props);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(ret_props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", ret_props.cbBuffer);
-    todo_wine ok(ret_props.cBuffers == 1, "Expected 1 sample got %ld\n", ret_props.cBuffers);
+    ok(ret_props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", ret_props.cbBuffer);
+    ok(ret_props.cBuffers == 1, "Expected 1 sample got %ld\n", ret_props.cBuffers);
     ok(ret_props.cbAlign == 1, "Expected alignment of 1 got %ld\n", ret_props.cbAlign);

/* Try setting a larger buffer size. */
@@ -8580,7 +8580,7 @@ static void test_ddrawstream_mem_allocator(void)
     props.cbPrefix = 0;
     hr = IMemAllocator_SetProperties(ddraw_allocator, &props, &ret_props);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(ret_props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", ret_props.cbBuffer);
+    ok(ret_props.cbBuffer == 40000, "Expected sample size to be 40000 got %ld\n", ret_props.cbBuffer);
     ok(ret_props.cBuffers == 1, "Expected 1 sample got %ld\n", ret_props.cBuffers);
     ok(ret_props.cbAlign == 1, "Expected alignment of 1 got %ld\n", ret_props.cbAlign);

@@ -8592,7 +8592,7 @@ static void test_ddrawstream_mem_allocator(void)

hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(props.cbBuffer == 20000, "Expected sample size to be 20000 got %ld\n", props.cbBuffer);
+    ok(props.cbBuffer == 20000, "Expected sample size to be 20000 got %ld\n", props.cbBuffer);
     ok(props.cBuffers == 1, "Expected 1 samples got %ld\n", props.cBuffers);
     ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);

@@ -8664,7 +8664,7 @@ static void test_ddrawstream_mem_allocator(void)

hr = IMemAllocator_GetProperties(ddraw_allocator, &props);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(props.cbBuffer == 333 * 444 * 4, "Got sample size %ld\n", props.cbBuffer);
+    ok(props.cbBuffer == 333 * 444 * 4, "Got sample size %ld\n", props.cbBuffer);
     ok(props.cBuffers == 2, "Expected 2 samples got %ld\n", props.cBuffers);
     ok(props.cbAlign == 1, "Expected alignment of 1 got %ld\n", props.cbAlign);

[PATCH 5/5] amstream: Return error when calling SetFormat with allocated samples.

[Santino Mazza]

From: Santino Mazza

---
 dlls/amstream/ddrawstream.c    | 7 +++++++
 dlls/amstream/tests/amstream.c | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
index c7eba9b4892..03d0919c784 100644
--- a/dlls/amstream/ddrawstream.c
+++ b/dlls/amstream/ddrawstream.c
@@ -678,6 +678,13 @@ static HRESULT WINAPI ddraw_IDirectDrawMediaStream_SetFormat(IDirectDrawMediaStr

if (stream->peer && !is_format_compatible(stream, old_format.width, old_format.height, &old_format.pf))
     {
+        if (stream->sample_refs > 0)
+        {
+            stream->format = old_format;
+            LeaveCriticalSection(&stream->cs);
+            return MS_E_SAMPLEALLOC;
+        }
+
         if (FAILED(hr = CopyMediaType(&old_media_type, &stream->mt)))
         {
             stream->format = old_format;
diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index 2c05bcffd92..89adb41996a 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -8673,6 +8673,10 @@ static void test_ddrawstream_mem_allocator(void)

hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, NULL, NULL, 0, &ddraw_sample1);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb32_format, NULL);
+    ok(hr == MS_E_SAMPLEALLOC, "Got hr %#lx.\n", hr);
+
     hr = IDirectDrawStreamSample_Update(ddraw_sample1, SSUPDATE_ASYNC, NULL, NULL, 0);
     ok(hr == MS_S_PENDING, "Got hr %#lx.\n", hr);
     hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, NULL, NULL, 0, &ddraw_sample2);

[Elizabeth Figura (＠zfigura)]

I think we actually want to implement the real allocator rather than faking it, and my efforts to do research into this eventually led to me implementing the entire thing in 8090. Does this work for you?

[Santino Mazza (＠tati)]

On Tue May 20 14:14:55 2025 +0000, Elizabeth Figura wrote:
> I think we actually want to implement the real allocator rather than
> faking it, and my efforts to do research into this eventually led to me
> implementing the entire thing in 8090. Does this work for you?

Yes! It makes a lot more sense with your changes.

[PATCH 0/5] MR8042: amstream: Change ddraw stream private allocator properties after changing format. - closed

[Santino Mazza (＠tati)]

This merge request was closed by Santino Mazza.