[PATCH 0/5] MR8042: amstream: Change ddraw stream private allocator properties after changing format.
This should fix the crash reported in https://bugs.winehq.org/show_bug.cgi?id=58171 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8042
From: Santino Mazza <smazza(a)codeweavers.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8042
From: Santino Mazza <smazza(a)codeweavers.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8042
From: Santino Mazza <smazza(a)codeweavers.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8042
From: Santino Mazza <smazza(a)codeweavers.com> --- 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8042
From: Santino Mazza <smazza(a)codeweavers.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8042
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? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8042#note_103916
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8042#note_103981
This merge request was closed by Santino Mazza. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8042
participants (3)
-
Elizabeth Figura (＠zfigura) -
Santino Mazza
-
Santino Mazza (＠tati)