June 2022 - Wine-Devel - list.winehq.org

[PATCH 2/7] d3dx10/tests: Add D3DX10GetImageInfoFromMemory HRESULT argument tests.

by Piotr Caban

From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/d3dx10_43/tests/d3dx10.c | 66 ++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/dlls/d3dx10_43/tests/d3dx10.c b/dlls/d3dx10_43/tests/d3dx10.c index a817f346676..fe7a118cec9 100644 --- a/dlls/d3dx10_43/tests/d3dx10.c +++ b/dlls/d3dx10_43/tests/d3dx10.c @@ -1922,53 +1922,73 @@ static void test_get_image_info(void) WCHAR path[MAX_PATH]; unsigned int i; DWORD dword; - HRESULT hr; + HRESULT hr, hr2; CoInitialize(NULL); - hr = D3DX10GetImageInfoFromMemory(test_image[0].data, 0, NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromMemory(test_image[0].data, 0, NULL, &image_info, &hr2); ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromMemory(NULL, test_image[0].size, NULL, &image_info, NULL); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromMemory(NULL, test_image[0].size, NULL, &image_info, &hr2); ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromMemory(&dword, sizeof(dword), NULL, &image_info, NULL); + ok(hr2 == 0xdeadbeef, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromMemory(&dword, sizeof(dword), NULL, &image_info, &hr2); ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); for (i = 0; i < ARRAY_SIZE(test_image); ++i) { winetest_push_context("Test %u", i); - hr = D3DX10GetImageInfoFromMemory(test_image[i].data, test_image[i].size, NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromMemory(test_image[i].data, test_image[i].size, NULL, &image_info, &hr2); ok(hr == S_OK || broken(hr == E_FAIL && test_image[i].expected_info.ImageFileFormat == D3DX10_IFF_WMP), "Got unexpected hr %#x.\n", hr); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); if (hr == S_OK) check_image_info(&image_info, test_image + i, __LINE__); winetest_pop_context(); } - hr = D3DX10GetImageInfoFromFileW(NULL, NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromFileW(NULL, NULL, &image_info, &hr2); ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromFileW(L"deadbeaf", NULL, &image_info, NULL); + ok(hr2 == 0xdeadbeef, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromFileW(L"deadbeaf", NULL, &image_info, &hr2); ok(hr == D3D10_ERROR_FILE_NOT_FOUND, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromFileA(NULL, NULL, &image_info, NULL); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromFileA(NULL, NULL, &image_info, &hr2); ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromFileA("deadbeaf", NULL, &image_info, NULL); + ok(hr2 == 0xdeadbeef, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromFileA("deadbeaf", NULL, &image_info, &hr2); ok(hr == D3D10_ERROR_FILE_NOT_FOUND, "Got unexpected hr %#x.\n", hr); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); for (i = 0; i < ARRAY_SIZE(test_image); ++i) { winetest_push_context("Test %u", i); create_file(test_filename, test_image[i].data, test_image[i].size, path); - hr = D3DX10GetImageInfoFromFileW(path, NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromFileW(path, NULL, &image_info, &hr2); ok(hr == S_OK || broken(hr == E_FAIL && test_image[i].expected_info.ImageFileFormat == D3DX10_IFF_WMP), "Got unexpected hr %#x.\n", hr); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); if (hr == S_OK) check_image_info(&image_info, test_image + i, __LINE__); - hr = D3DX10GetImageInfoFromFileA(get_str_a(path), NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromFileA(get_str_a(path), NULL, &image_info, &hr2); ok(hr == S_OK || broken(hr == E_FAIL && test_image[i].expected_info.ImageFileFormat == D3DX10_IFF_WMP), "Got unexpected hr %#x.\n", hr); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); if (hr == S_OK) check_image_info(&image_info, test_image + i, __LINE__); @@ -1979,31 +1999,43 @@ static void test_get_image_info(void) /* D3DX10GetImageInfoFromResource tests */ - hr = D3DX10GetImageInfoFromResourceW(NULL, NULL, NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromResourceW(NULL, NULL, NULL, &image_info, &hr2); ok(hr == D3DX10_ERR_INVALID_DATA, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromResourceW(NULL, L"deadbeaf", NULL, &image_info, NULL); + ok(hr2 == 0xdeadbeef, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromResourceW(NULL, L"deadbeaf", NULL, &image_info, &hr2); ok(hr == D3DX10_ERR_INVALID_DATA, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromResourceA(NULL, NULL, NULL, &image_info, NULL); + ok(hr2 == 0xdeadbeef, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromResourceA(NULL, NULL, NULL, &image_info, &hr2); ok(hr == D3DX10_ERR_INVALID_DATA, "Got unexpected hr %#x.\n", hr); - hr = D3DX10GetImageInfoFromResourceA(NULL, "deadbeaf", NULL, &image_info, NULL); + ok(hr2 == 0xdeadbeef, "Got unexpected hr2 %#x.\n", hr2); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromResourceA(NULL, "deadbeaf", NULL, &image_info, &hr2); ok(hr == D3DX10_ERR_INVALID_DATA, "Got unexpected hr %#x.\n", hr); + ok(hr2 == 0xdeadbeef, "Got unexpected hr2 %#x.\n", hr2); for (i = 0; i < ARRAY_SIZE(test_image); ++i) { winetest_push_context("Test %u", i); resource_module = create_resource_module(test_resource_name, test_image[i].data, test_image[i].size); - hr = D3DX10GetImageInfoFromResourceW(resource_module, test_resource_name, NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromResourceW(resource_module, test_resource_name, NULL, &image_info, &hr2); ok(hr == S_OK || broken(hr == E_FAIL && test_image[i].expected_info.ImageFileFormat == D3DX10_IFF_WMP) || broken(hr == D3DX10_ERR_INVALID_DATA) /* Vista */, "Got unexpected hr %#x.\n", hr); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); if (hr == S_OK) check_image_info(&image_info, test_image + i, __LINE__); - hr = D3DX10GetImageInfoFromResourceA(resource_module, get_str_a(test_resource_name), NULL, &image_info, NULL); + hr2 = 0xdeadbeef; + hr = D3DX10GetImageInfoFromResourceA(resource_module, get_str_a(test_resource_name), NULL, &image_info, &hr2); ok(hr == S_OK || broken(hr == E_FAIL && test_image[i].expected_info.ImageFileFormat == D3DX10_IFF_WMP) || broken(hr == D3DX10_ERR_INVALID_DATA) /* Vista */, "Got unexpected hr %#x.\n", hr); + ok(hr == hr2, "Got unexpected hr2 %#x.\n", hr2); if (hr == S_OK) check_image_info(&image_info, test_image + i, __LINE__); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/205

June 8, 2022

[PATCH 1/7] d3dx10: Don't ignore HRESULT parameter in D3DX10GetImageInfoFrom* functions.

by Piotr Caban

From: Piotr Caban <piotr(a)codeweavers.com> Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> --- dlls/d3dx10_43/texture.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/dlls/d3dx10_43/texture.c b/dlls/d3dx10_43/texture.c index 3930df3677f..c018487ae7a 100644 --- a/dlls/d3dx10_43/texture.c +++ b/dlls/d3dx10_43/texture.c @@ -301,7 +301,7 @@ HRESULT WINAPI D3DX10GetImageInfoFromFileA(const char *src_file, ID3DX10ThreadPu TRACE("src_file %s, pump %p, info %p, result %p.\n", debugstr_a(src_file), pump, info, result); - if (!src_file || !info) + if (!src_file) return E_FAIL; str_len = MultiByteToWideChar(CP_ACP, 0, src_file, -1, NULL, 0); @@ -329,11 +329,15 @@ HRESULT WINAPI D3DX10GetImageInfoFromFileW(const WCHAR *src_file, ID3DX10ThreadP TRACE("src_file %s, pump %p, info %p, result %p.\n", debugstr_w(src_file), pump, info, result); - if (!src_file || !info) + if (!src_file) return E_FAIL; if (FAILED((hr = load_file(src_file, &buffer, &size)))) + { + if (result) + *result = hr; return hr; + } hr = D3DX10GetImageInfoFromMemory(buffer, size, pump, info, result); @@ -352,11 +356,7 @@ HRESULT WINAPI D3DX10GetImageInfoFromResourceA(HMODULE module, const char *resou TRACE("module %p, resource %s, pump %p, info %p, result %p.\n", module, debugstr_a(resource), pump, info, result); - if (!resource || !info) - return D3DX10_ERR_INVALID_DATA; - - hr = load_resourceA(module, resource, &buffer, &size); - if (FAILED(hr)) + if (FAILED((hr = load_resourceA(module, resource, &buffer, &size)))) return hr; return D3DX10GetImageInfoFromMemory(buffer, size, pump, info, result); @@ -372,11 +372,7 @@ HRESULT WINAPI D3DX10GetImageInfoFromResourceW(HMODULE module, const WCHAR *reso TRACE("module %p, resource %s, pump %p, info %p, result %p.\n", module, debugstr_w(resource), pump, info, result); - if (!resource || !info) - return D3DX10_ERR_INVALID_DATA; - - hr = load_resourceW(module, resource, &buffer, &size); - if (FAILED(hr)) + if (FAILED((hr = load_resourceW(module, resource, &buffer, &size)))) return hr; return D3DX10GetImageInfoFromMemory(buffer, size, pump, info, result); @@ -480,6 +476,8 @@ end: HRESULT WINAPI D3DX10GetImageInfoFromMemory(const void *src_data, SIZE_T src_data_size, ID3DX10ThreadPump *pump, D3DX10_IMAGE_INFO *img_info, HRESULT *result) { + HRESULT hr; + TRACE("src_data %p, src_data_size %Iu, pump %p, img_info %p, hresult %p.\n", src_data, src_data_size, pump, img_info, result); @@ -488,7 +486,10 @@ HRESULT WINAPI D3DX10GetImageInfoFromMemory(const void *src_data, SIZE_T src_dat if (pump) FIXME("Thread pump is not supported yet.\n"); - return get_image_info(src_data, src_data_size, img_info); + hr = get_image_info(src_data, src_data_size, img_info); + if (result) + *result = hr; + return hr; } HRESULT WINAPI D3DX10CreateTextureFromFileA(ID3D10Device *device, const char *src_file, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/205

June 8, 2022

[PATCH 0/7] MR205: Add support for HRESULT and ID3DX10ThreadPump parameters in D3DX10GetImageInfo functions

by Piotr Caban (＠piotr)

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/205

June 8, 2022

Re: [PATCH 0/5] MR204: win32u: Avoid calling PE code with user lock - approved

by Huw Davies (＠huw)

This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/204

June 8, 2022

Re: [PATCH 0/1] MR196: winegstreamer: Return void from align_video_info_planes. - approved

by Zebediah Figura (＠zfigura)

This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/196

June 8, 2022

Re: [PATCH 5/5] winegstreamer: Support zero-copy in wg_transform_push_data.

by Zebediah Figura

On 6/7/22 03:43, Rémi Bernon wrote: > +static void wg_sample_queue_append(struct wg_sample_queue *queue, struct wg_sample *wg_sample) > +{ > + struct mf_sample *mf_sample = CONTAINING_RECORD(wg_sample, struct mf_sample, wg_sample); > + > + /* make sure a concurrent wg_sample_queue_release call won't release the sample until we're done */ > + InterlockedIncrement(&wg_sample->refcount); > + mf_sample->wg_sample.flags |= WG_SAMPLE_FLAG_HAS_REFCOUNT; > + > + EnterCriticalSection(&queue->cs); > + list_add_tail(&queue->samples, &mf_sample->entry); > + LeaveCriticalSection(&queue->cs); > +} > + > +void wg_sample_queue_release(struct wg_sample_queue *queue, struct wg_sample *wg_sample, bool all) > { > - struct mf_sample *mf_sample = CONTAINING_RECORD(sample, struct mf_sample, wg_sample); > + struct mf_sample *mf_sample, *next; > + > + /* release temporary ref taken in wg_sample_queue_append */ > + if (wg_sample) > + InterlockedDecrement(&wg_sample->refcount); > + > + EnterCriticalSection(&queue->cs); > + > + LIST_FOR_EACH_ENTRY_SAFE(mf_sample, next, &queue->samples, struct mf_sample, entry) > + { > + if (!InterlockedOr(&mf_sample->wg_sample.refcount, 0) || all) > + { > + list_remove(&mf_sample->entry); > + wg_sample_release(&mf_sample->wg_sample); > + } > + } > + > + LeaveCriticalSection(&queue->cs); > +} This feels awkward, partly because it does two things at once, partly because of the lack of symmetry. But the lack of symmetry is kind of necessary... (Maybe bring the incref/decref into the caller?) I'd at least suggest to make most of that a separate helper like "wg_sample_queue_flush()" which doesn't take the specific sample argument, and have wg_sample_queue_release() decref the sample and then call wg_sample_queue_flush().

June 8, 2022

Re: [PATCH 4/5] winegstreamer: Support zero-copy output using the allocator.

by Zebediah Figura

The patch looks pretty good overall; I do have some comments inlined below. On 6/7/22 03:43, Rémi Bernon wrote: > @@ -116,8 +160,15 @@ static GstMemory *wg_allocator_alloc(GstAllocator *gst_allocator, gsize size, > memory->unix_memory = gst_allocator_alloc(NULL, size, params); > gst_memory_map(memory->unix_memory, &memory->unix_map_info, GST_MAP_WRITE); > > - GST_INFO("Allocated memory %p, unix_memory %p, data %p", memory, memory->unix_memory, > - memory->unix_map_info.data); > + pthread_mutex_lock(&allocator->mutex); > + > + memory->sample = allocator->request_sample(size, allocator->request_sample_context); > + list_add_tail(&allocator->memory_list, &memory->entry); > + > + pthread_mutex_unlock(&allocator->mutex); > + > + GST_INFO("Allocated memory %p, sample %p, unix_memory %p, data %p", memory, > + memory->sample, memory->unix_memory, memory->unix_map_info.data); > return (GstMemory *)memory; > } > Could we do this without the callback? Just thinking out loud, maybe something like void wg_allocator_add_sample(WgAllocator *allocator, struct wg_sample *sample); void wg_allocator_remove_sample(WgAllocator *allocator, struct wg_sample *sample); which manipulate an internal wg_sample pointer, instead of having them access the wg_transform's sample pointer. Then wg_allocator_remove_sample() would also end up calling wg_allocator_release_sample(). I suspect I'm missing something, though, given my below comment... > if (!(sample->flags & WG_SAMPLE_FLAG_INCOMPLETE)) > { > + /* Taint the buffer memory to make sure it cannot be reused by the buffer pool, > + * for the pool to always requests new memory from the allocator, and so we can > + * then always provide output sample memory to achieve zero-copy. > + * > + * However, some decoder keep a reference on the buffer they passed downstream, > + * to re-use it later. In this case, it will not be possible to do zero-copy, > + * and we should copy the data back to the buffer and leave it unchanged. > + * > + * Some other plugins make assumptions that the returned buffer will always have > + * at least one memory attached, we cannot just remove it and need to replace the > + * memory instead. > + */ > + if ((discard_data = gst_buffer_is_writable(output_buffer))) > + gst_buffer_replace_all_memory(output_buffer, gst_allocator_alloc(NULL, 0, NULL)); > + > gst_sample_unref(transform->output_sample); > transform->output_sample = NULL; > } Ah, took me a minute to understand this. To make sure I've got it right: this isn't a correctness issue (i.e. the patch would be fine without it, if less efficient), but the point is that if we keep our newly allocated GstMemory—which no longer has a wg_sample attached—in circulation, the buffer pool will continue to use it instead of requesting a new sample, which means that samples after the first won't be zero-copied, and to avoid this, we fill the GstBuffer with a useless empty GstMemory object. Is that right? Had to look this up, but apparently the default GstBufferPool will throw away buffers if they're modified. I thought the "empty" buffers would pile up and never be freed, but evidently not. I've probably had this question answered before, but can we replace the buffer pool itself, instead of just the allocator? > > params->result = S_OK; > + wg_allocator_release_sample(transform->allocator, sample, discard_data); > return STATUS_SUCCESS; > } Doesn't this discard data in the WG_SAMPLE_FLAG_INCOMPLETE case? Not just the data we've already copied, but the data we haven't copied as well...

June 8, 2022

Re: [PATCH 5/5] user32: Remove no longer needed helpers.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=116429 Your paranoid android. === debian11 (32 bit report) === user32: menu.c:2324: Test failed: test 25

June 8, 2022

Re: [PATCH 4/5] user32: Use a critical section in RegisterUserApiHook.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=116428 Your paranoid android. === debian11 (32 bit Chinese:China report) === user32: win.c:11787: Test failed: 011D00CE: expected NOT topmost win.c:11606: Test succeeded inside todo block: 5: hwnd 013F0054 is still topmost === debian11 (64 bit WoW report) === user32: msg.c:6881: Test failed: SetFocus(hwnd) on a button: 4: the msg 0x0007 was expected, but got msg 0x0005 instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 5: the msg 0x0138 was expected, but got msg 0x030f instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 6: the msg 0x0111 was expected, but got msg 0x001c instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 8: the msg 0x8000 was expected, but got msg 0x0086 instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 9: the msg sequence is not complete: expected 0000 - actual 0006

June 8, 2022

[PATCH 5/5] user32: Remove no longer needed helpers.

by Jacek Caban

From: Jacek Caban <jacek(a)codeweavers.com> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- dlls/user32/user_main.c | 18 ------------------ dlls/user32/win.c | 2 +- dlls/user32/win.h | 6 +----- 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/dlls/user32/user_main.c b/dlls/user32/user_main.c index eeef0d57f62..d0c1cc616a7 100644 --- a/dlls/user32/user_main.c +++ b/dlls/user32/user_main.c @@ -42,24 +42,6 @@ static DWORD exiting_thread_id; extern void WDML_NotifyThreadDetach(void); -/*********************************************************************** - * USER_Lock - */ -void USER_Lock(void) -{ - NtUserCallOneParam( 0, NtUserLock ); -} - - -/*********************************************************************** - * USER_Unlock - */ -void USER_Unlock(void) -{ - NtUserCallOneParam( 1, NtUserLock ); -} - - /*********************************************************************** * USER_CheckNotLock * diff --git a/dlls/user32/win.c b/dlls/user32/win.c index 9b9dde82561..bac75063f7d 100644 --- a/dlls/user32/win.c +++ b/dlls/user32/win.c @@ -54,7 +54,7 @@ void *get_user_handle_ptr( HANDLE handle, unsigned int type ) void release_user_handle_ptr( void *ptr ) { assert( ptr && ptr != OBJ_OTHER_PROCESS ); - USER_Unlock(); + NtUserCallOneParam( 1, NtUserLock ); } diff --git a/dlls/user32/win.h b/dlls/user32/win.h index dfa5fdf3283..9d42aef6645 100644 --- a/dlls/user32/win.h +++ b/dlls/user32/win.h @@ -48,14 +48,10 @@ extern HWND *WIN_ListChildren( HWND hwnd ) DECLSPEC_HIDDEN; extern void MDI_CalcDefaultChildPos( HWND hwndClient, INT total, LPPOINT lpPos, INT delta, UINT *id ) DECLSPEC_HIDDEN; extern HDESK open_winstation_desktop( HWINSTA hwinsta, LPCWSTR name, DWORD flags, BOOL inherit, ACCESS_MASK access ) DECLSPEC_HIDDEN; -/* user lock */ -extern void USER_Lock(void) DECLSPEC_HIDDEN; -extern void USER_Unlock(void) DECLSPEC_HIDDEN; - /* to release pointers retrieved by WIN_GetPtr */ static inline void WIN_ReleasePtr( WND *ptr ) { - USER_Unlock(); + release_user_handle_ptr( ptr ); } extern LRESULT HOOK_CallHooks( INT id, INT code, WPARAM wparam, LPARAM lparam, BOOL unicode ) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/204

June 8, 2022