[PATCH v2 1/6] mfplat/tests: Check that buffers are properly aligned.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 91c9d971a3b..9470cf27a90 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -2286,10 +2286,10 @@ static void test_system_memory_buffer(void) IMFMediaBuffer_Release(buffer); /* Aligned buffer. */ - hr = MFCreateAlignedMemoryBuffer(16, MF_8_BYTE_ALIGNMENT, NULL); + hr = MFCreateAlignedMemoryBuffer(16, MF_512_BYTE_ALIGNMENT, NULL); ok(FAILED(hr), "Unexpected hr %#x.\n", hr); - hr = MFCreateAlignedMemoryBuffer(201, MF_8_BYTE_ALIGNMENT, &buffer); + hr = MFCreateAlignedMemoryBuffer(201, MF_512_BYTE_ALIGNMENT, &buffer); ok(hr == S_OK, "Failed to create memory buffer, hr %#x.\n", hr); hr = IMFMediaBuffer_GetCurrentLength(buffer, &length); @@ -2317,6 +2317,8 @@ static void test_system_memory_buffer(void) hr = IMFMediaBuffer_Lock(buffer, &data, &max, &length); ok(hr == S_OK, "Failed to lock, hr %#x.\n", hr); ok(max == 201 && length == 10, "Unexpected length.\n"); + todo_wine + ok(((uintptr_t)data & MF_512_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); hr = IMFMediaBuffer_Unlock(buffer); ok(hr == S_OK, "Failed to unlock, hr %#x.\n", hr); -- 2.35.1
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- v2: * Handle alignments that are not powers of 2 minus 1. According to my tests, this result in the alignment being increased to the smalles power of 2 minus 1 bigger than the requested alignment. I can add tests if it is wanted. --- dlls/mfplat/buffer.c | 14 ++++++++++++-- dlls/mfplat/tests/mfplat.c | 1 - 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index 774a8604ae9..a91a6a561ce 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -18,6 +18,8 @@ #define COBJMACROS +#include <malloc.h> + #include "mfplat_private.h" #include "rtworkq.h" @@ -171,7 +173,7 @@ static ULONG WINAPI memory_buffer_Release(IMFMediaBuffer *iface) } DeleteCriticalSection(&buffer->cs); free(buffer->_2d.linear_buffer); - free(buffer->data); + _aligned_free(buffer->data); free(buffer); } @@ -1254,8 +1256,16 @@ static const IMFDXGIBufferVtbl dxgi_buffer_vtbl = static HRESULT memory_buffer_init(struct buffer *buffer, DWORD max_length, DWORD alignment, const IMFMediaBufferVtbl *vtbl) { - if (!(buffer->data = calloc(1, ALIGN_SIZE(max_length, alignment)))) + unsigned i; + + /* Saturate bits to the right, so that alignment is a power of 2 + * minus 1. */ + for (i = 1; i < 8 * sizeof(alignment); i *= 2) + alignment |= alignment >> i; + + if (!(buffer->data = _aligned_malloc(ALIGN_SIZE(max_length, alignment), (size_t)alignment + 1))) return E_OUTOFMEMORY; + memset(buffer->data, 0, ALIGN_SIZE(max_length, alignment)); buffer->IMFMediaBuffer_iface.lpVtbl = vtbl; buffer->refcount = 1; diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 9470cf27a90..91f19da850d 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -2317,7 +2317,6 @@ static void test_system_memory_buffer(void) hr = IMFMediaBuffer_Lock(buffer, &data, &max, &length); ok(hr == S_OK, "Failed to lock, hr %#x.\n", hr); ok(max == 201 && length == 10, "Unexpected length.\n"); - todo_wine ok(((uintptr_t)data & MF_512_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); hr = IMFMediaBuffer_Unlock(buffer); ok(hr == S_OK, "Failed to unlock, hr %#x.\n", hr); -- 2.35.1
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=109273 Your paranoid android. === w7u_2qxl (32 bit report) === mfplat: 0114:mfplat: unhandled exception c0000005 at 6F897D1C === w7u_adm (32 bit report) === mfplat: 0874:mfplat: unhandled exception c0000005 at 6ECF7D1C
On 2/25/22 19:37, Giovanni Mascellani wrote:
v2: * Handle alignments that are not powers of 2 minus 1. According to my tests, this result in the alignment being increased to the smalles power of 2 minus 1 bigger than the requested alignment. I can add tests if it is wanted. Please do, if such tests pass reliably.
+ + /* Saturate bits to the right, so that alignment is a power of 2 + * minus 1. */ + for (i = 1; i < 8 * sizeof(alignment); i *= 2) + alignment |= alignment >> i; This looks like it would loop longer than it has it.
I think it should loop until remaining alignment is not zero, like this (untested): while (alignment) { // accumulate result here alignment = alignment >> 1; } Also it's not clear if alignment == 0 should work at all.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 91f19da850d..e216c0304a9 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5793,6 +5793,8 @@ static void test_MFCreate2DMediaBuffer(void) hr = IMF2DBuffer_Lock2D(_2dbuffer, &data, &pitch); ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); ok(!!data, "Expected data pointer.\n"); + todo_wine + ok(((uintptr_t)data & MF_64_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); ok(pitch == 64, "Unexpected pitch %d.\n", pitch); hr = IMF2DBuffer_Lock2D(_2dbuffer, &data2, &pitch); @@ -5905,6 +5907,8 @@ static void test_MFCreate2DMediaBuffer(void) length2, ptr->width, ptr->height, wine_dbgstr_an((char *)&ptr->fourcc, 4)); hr = IMF2DBuffer_Lock2D(_2dbuffer, &data, &pitch); + todo_wine + ok(((uintptr_t)data & MF_64_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); hr = IMF2DBuffer_GetScanline0AndPitch(_2dbuffer, &data2, &pitch2); -- 2.35.1
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=109274 Your paranoid android. === w7u_2qxl (32 bit report) === mfplat: 0910:mfplat: unhandled exception c0000005 at 6E987D1C === debian11 (32 bit report) === mfplat: mfplat.c:5911: Test succeeded inside todo block: Data at 00D137C0 is insufficiently aligned. mfplat.c:5911: Test succeeded inside todo block: Data at 00D137C0 is insufficiently aligned. mfplat.c:5911: Test succeeded inside todo block: Data at 00D137C0 is insufficiently aligned. === debian11 (32 bit French report) === mfplat: mfplat.c:5911: Test succeeded inside todo block: Data at 00D137C0 is insufficiently aligned. mfplat.c:5911: Test succeeded inside todo block: Data at 00D137C0 is insufficiently aligned. mfplat.c:5911: Test succeeded inside todo block: Data at 00D137C0 is insufficiently aligned.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/buffer.c | 2 +- dlls/mfplat/tests/mfplat.c | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index a91a6a561ce..c2fcdd24de1 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -1375,7 +1375,7 @@ static HRESULT create_2d_buffer(DWORD width, DWORD height, DWORD fourcc, BOOL bo max_length = pitch * height; } - if (FAILED(hr = memory_buffer_init(object, max_length, MF_1_BYTE_ALIGNMENT, &memory_1d_2d_buffer_vtbl))) + if (FAILED(hr = memory_buffer_init(object, max_length, row_alignment, &memory_1d_2d_buffer_vtbl))) { free(object); return hr; diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index e216c0304a9..137ade76b05 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5793,7 +5793,6 @@ static void test_MFCreate2DMediaBuffer(void) hr = IMF2DBuffer_Lock2D(_2dbuffer, &data, &pitch); ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); ok(!!data, "Expected data pointer.\n"); - todo_wine ok(((uintptr_t)data & MF_64_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); ok(pitch == 64, "Unexpected pitch %d.\n", pitch); @@ -5907,7 +5906,6 @@ static void test_MFCreate2DMediaBuffer(void) length2, ptr->width, ptr->height, wine_dbgstr_an((char *)&ptr->fourcc, 4)); hr = IMF2DBuffer_Lock2D(_2dbuffer, &data, &pitch); - todo_wine ok(((uintptr_t)data & MF_64_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); -- 2.35.1
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=109275 Your paranoid android. === w7u_adm (32 bit report) === mfplat: 087c:mfplat: unhandled exception c0000005 at 6EC87D1C === w7u_el (32 bit report) === mfplat: 0858:mfplat: unhandled exception c0000005 at 6EA97D1C
On 2/25/22 19:37, Giovanni Mascellani wrote:
diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index e216c0304a9..137ade76b05 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5793,7 +5793,6 @@ static void test_MFCreate2DMediaBuffer(void) hr = IMF2DBuffer_Lock2D(_2dbuffer, &data, &pitch); ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); ok(!!data, "Expected data pointer.\n"); - todo_wine ok(((uintptr_t)data & MF_64_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); ok(pitch == 64, "Unexpected pitch %d.\n", pitch);
@@ -5907,7 +5906,6 @@ static void test_MFCreate2DMediaBuffer(void) length2, ptr->width, ptr->height, wine_dbgstr_an((char *)&ptr->fourcc, 4));
hr = IMF2DBuffer_Lock2D(_2dbuffer, &data, &pitch); - todo_wine ok(((uintptr_t)data & MF_64_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr);
For this another example would be useful, to see if alignment is not unconditionally 64.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 137ade76b05..4ebf7fe6424 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5743,6 +5743,8 @@ static void test_MFCreate2DMediaBuffer(void) ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); ok(max_length == length, "Unexpected length.\n"); + memset(data, 0xaa, length); + length = 0; pMFGetPlaneSize(MAKEFOURCC('N','V','1','2'), 2, 3, &length); ok(max_length == length && length == 9, "Unexpected length %u.\n", length); @@ -5796,6 +5798,16 @@ static void test_MFCreate2DMediaBuffer(void) ok(((uintptr_t)data & MF_64_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); ok(pitch == 64, "Unexpected pitch %d.\n", pitch); + ok(data[0] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[0]); + ok(data[1] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[1]); + ok(data[64] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[64]); + ok(data[65] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[65]); + ok(data[128] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[128]); + ok(data[129] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[129]); + ok(data[192] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[192]); + ok(data[193] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[193]); + memset(data, 0xbb, 194); + hr = IMF2DBuffer_Lock2D(_2dbuffer, &data2, &pitch); ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); ok(data == data2, "Expected data pointer.\n"); @@ -5822,6 +5834,16 @@ static void test_MFCreate2DMediaBuffer(void) hr = IMF2DBuffer_Unlock2D(_2dbuffer); ok(hr == HRESULT_FROM_WIN32(ERROR_WAS_UNLOCKED), "Unexpected hr %#x.\n", hr); + hr = IMFMediaBuffer_Lock(buffer, &data, NULL, NULL); + ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); + + for (i = 0; i < 8; i++) + todo_wine + ok(data[i] == 0xbb, "Invalid data 0x%02x at position %u instead of 0xbb.\n", data[i], i); + + hr = IMFMediaBuffer_Unlock(buffer); + ok(hr == S_OK, "Failed to unlock buffer, hr %#x.\n", hr); + hr = IMFMediaBuffer_QueryInterface(buffer, &IID_IMF2DBuffer2, (void **)&_2dbuffer2); ok(hr == S_OK || broken(hr == E_NOINTERFACE), "Failed to get interface, hr %#x.\n", hr); -- 2.35.1
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=109276 Your paranoid android. === w7u_el (32 bit report) === mfplat: 0868:mfplat: unhandled exception c0000005 at 6EDD7D1C === w8 (32 bit report) === mfplat: mfplat.c:680: Test failed: timeout mfplat.c:676: Test failed: Unexpected hr 0xc00d36da. mfplat.c:680: Test failed: timeout
On 2/25/22 19:37, Giovanni Mascellani wrote:
+ ok(data[0] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[0]); + ok(data[1] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[1]); + ok(data[64] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[64]); + ok(data[65] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[65]); + ok(data[128] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[128]); + ok(data[129] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[129]); + ok(data[192] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[192]); + ok(data[193] == 0xaa, "Invalid data 0x%02x instead of 0xaa.\n", data[193]); + memset(data, 0xbb, 194); + This would be shorted with memcmp.
+ for (i = 0; i < 8; i++) + todo_wine + ok(data[i] == 0xbb, "Invalid data 0x%02x at position %u instead of 0xbb.\n", data[i], i); Same here.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/buffer.c | 4 ++++ dlls/mfplat/tests/mfplat.c | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index c2fcdd24de1..08e5d793718 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -311,6 +311,10 @@ static HRESULT WINAPI memory_1d_2d_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat { if (!(buffer->_2d.linear_buffer = malloc(ALIGN_SIZE(buffer->_2d.plane_size, MF_64_BYTE_ALIGNMENT)))) hr = E_OUTOFMEMORY; + + if (SUCCEEDED(hr)) + copy_image(buffer, buffer->_2d.linear_buffer, buffer->_2d.width, buffer->data, buffer->_2d.pitch, + buffer->_2d.width, buffer->_2d.height); } if (SUCCEEDED(hr)) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 4ebf7fe6424..0f9137ae4b2 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5838,7 +5838,6 @@ static void test_MFCreate2DMediaBuffer(void) ok(hr == S_OK, "Failed to lock buffer, hr %#x.\n", hr); for (i = 0; i < 8; i++) - todo_wine ok(data[i] == 0xbb, "Invalid data 0x%02x at position %u instead of 0xbb.\n", data[i], i); hr = IMFMediaBuffer_Unlock(buffer); -- 2.35.1
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=109277 Your paranoid android. === w7u_adm (32 bit report) === mfplat: 0878:mfplat: unhandled exception c0000005 at 6E757D1C
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=109272 Your paranoid android. === w7u_adm (32 bit report) === mfplat: 0884:mfplat: unhandled exception c0000005 at 6EA37D1C === w7u_el (32 bit report) === mfplat: 0868:mfplat: unhandled exception c0000005 at 6EFA7D1C
Hi, Il 25/02/22 18:00, Marvin ha scritto:
=== w7u_adm (32 bit report) ===
mfplat: 0884:mfplat: unhandled exception c0000005 at 6EA37D1C
=== w7u_el (32 bit report) ===
mfplat: 0868:mfplat: unhandled exception c0000005 at 6EFA7D1C
As far as I can see, this is not related to my patches. The crash seems to happen also without my patch, and patch set v1 had the same exact patch which didn't crash. Also, the crash happens in a different test than then one I am touching, and looks like it is related to the well known problems in handling media events in Windows 7. Giovanni.
On 2/25/22 19:37, Giovanni Mascellani wrote:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 91c9d971a3b..9470cf27a90 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -2286,10 +2286,10 @@ static void test_system_memory_buffer(void) IMFMediaBuffer_Release(buffer);
/* Aligned buffer. */ - hr = MFCreateAlignedMemoryBuffer(16, MF_8_BYTE_ALIGNMENT, NULL); + hr = MFCreateAlignedMemoryBuffer(16, MF_512_BYTE_ALIGNMENT, NULL); ok(FAILED(hr), "Unexpected hr %#x.\n", hr); Why was this call changed?
- hr = MFCreateAlignedMemoryBuffer(201, MF_8_BYTE_ALIGNMENT, &buffer); + hr = MFCreateAlignedMemoryBuffer(201, MF_512_BYTE_ALIGNMENT, &buffer); ok(hr == S_OK, "Failed to create memory buffer, hr %#x.\n", hr);
hr = IMFMediaBuffer_GetCurrentLength(buffer, &length); @@ -2317,6 +2317,8 @@ static void test_system_memory_buffer(void) hr = IMFMediaBuffer_Lock(buffer, &data, &max, &length); ok(hr == S_OK, "Failed to lock, hr %#x.\n", hr); ok(max == 201 && length == 10, "Unexpected length.\n"); + todo_wine + ok(((uintptr_t)data & MF_512_BYTE_ALIGNMENT) == 0, "Data at %p is insufficiently aligned.\n", data); hr = IMFMediaBuffer_Unlock(buffer); ok(hr == S_OK, "Failed to unlock, hr %#x.\n", hr);
participants (3)
-
Giovanni Mascellani -
Marvin -
Nikolay Sivov