[PATCH 1/7] 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 f89a5202d80..0d86f041c1e 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -2248,10 +2248,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); @@ -2279,6 +2279,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.34.1
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/buffer.c | 7 +++++-- dlls/mfplat/tests/mfplat.c | 1 - 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index c45309e1c31..25b71cf79de 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" @@ -173,7 +175,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); } @@ -1256,8 +1258,9 @@ 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)))) + if (!(buffer->data = _aligned_malloc(ALIGN_SIZE(max_length, alignment), 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 0d86f041c1e..67e81ff0821 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -2279,7 +2279,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.34.1
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- This doesn't really make sense to me, but it is what I am seeing on the testbot. Nikolay, have you ever seen MFGetPlaneSize return MF_E_INVALIDMEDIATYPE? --- dlls/mfplat/mediatype.c | 5 ++++- dlls/mfplat/tests/mfplat.c | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/mfplat/mediatype.c b/dlls/mfplat/mediatype.c index 41dbda572d5..4fac13db17c 100644 --- a/dlls/mfplat/mediatype.c +++ b/dlls/mfplat/mediatype.c @@ -2758,7 +2758,10 @@ HRESULT WINAPI MFGetPlaneSize(DWORD fourcc, DWORD width, DWORD height, DWORD *si subtype.Data1 = fourcc; if (!(format = mf_get_video_format(&subtype))) - return MF_E_INVALIDMEDIATYPE; + { + *size = 0; + return S_OK; + } stride = mf_get_stride_for_format(format, width); diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 67e81ff0821..ff703d94568 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -4183,6 +4183,14 @@ static void test_MFCalculateImageSize(void) ok(hr == E_INVALIDARG || broken(hr == S_OK) /* Vista */, "Unexpected hr %#x.\n", hr); ok(size == 0, "Unexpected size %u.\n", size); + if (pMFGetPlaneSize) + { + size = 1; + hr = pMFGetPlaneSize(0xdeadbeef, 1, 1, &size); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + ok(size == 0, "Unexpected size %u.\n", size); + } + for (i = 0; i < ARRAY_SIZE(image_size_tests); ++i) { const struct image_size_test *ptr = &image_size_tests[i]; -- 2.34.1
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 ff703d94568..d391c833a7f 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5712,6 +5712,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); @@ -5824,6 +5826,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.34.1
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 25b71cf79de..9f060de5f20 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -1370,7 +1370,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 d391c833a7f..52470ad577b 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5712,7 +5712,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); @@ -5826,7 +5825,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.34.1
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 52470ad577b..8367d6a2646 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5662,6 +5662,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); @@ -5715,6 +5717,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"); @@ -5741,6 +5753,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.34.1
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 9f060de5f20..ed2ecc2558a 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -313,6 +313,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 8367d6a2646..f5608ad7883 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5757,7 +5757,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.34.1
On 2/14/22 19:07, Giovanni Mascellani wrote:
- 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); @@ -2279,6 +2279,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);
Thanks, looks like this checks out for alignment values I tried. There is one issue with using _aligned_malloc() for this - it requires power of 2 for alignment argument, understandably. Media buffer function does not have this requirement. So while we definitely want this fixed, I suggest focusing on tests first, and maybe not using _aligned_malloc(), it will require to keep additional pointer, and a single helper to allocate such buffer. This will also help by using regular calloc()/free() for everything we allocate.
Hi, Il 14/02/22 19:51, Nikolay Sivov ha scritto:
Thanks, looks like this checks out for alignment values I tried. There is one issue with using _aligned_malloc() for this - it requires power of 2 for alignment argument, understandably. Media buffer function does not have this requirement.
Ah, you're right. But do you know what MFCreateAlignedMemoryBuffer is actually doing here? I.e., I tested calling MFCreateAlignedMemoryBuffer(201, 101, &buffer), but it didn't return a buffer aligned to 102 (such that, that is, address % 102 == 0). Do you know what is supposed to happen here? I suppose we just have to mangle the alignment the proper way, without needing to use an additional pointer, and then we can just call _aligned_malloc() again. Thanks, Giovanni.
participants (2)
-
Giovanni Mascellani -
Nikolay Sivov