Signed-off-by: Derek Lesho dlesho@codeweavers.com --- I decided to put the branching in the copy functions, as it there were too many variables involved in how to copy the secondary planes. --- dlls/mfplat/buffer.c | 55 +++++++++++++++++++++++++++++++------- dlls/mfplat/tests/mfplat.c | 42 ++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 11 deletions(-)
diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index e04cc1a9251..5830b1af4ff 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -32,6 +32,34 @@ WINE_DEFAULT_DEBUG_CHANNEL(mfplat);
#define ALIGN_SIZE(size, alignment) (((size) + (alignment)) & ~((alignment)))
+static void copy_multiplane_image(BYTE *dest, LONG dest_stride, const BYTE *src, LONG src_stride, DWORD width, DWORD lines, unsigned int fourcc) +{ + MFCopyImage(dest, dest_stride, src, src_stride, width, lines); + dest += dest_stride * lines; + src += src_stride * lines; + + switch (fourcc) + { + case MAKEFOURCC('N','V','1','2'): + MFCopyImage(dest, dest_stride, src, src_stride, width, lines / 2); + break; + + case MAKEFOURCC('I','M','C','1'): + case MAKEFOURCC('I','M','C','3'): + MFCopyImage(dest, dest_stride, src, src_stride, width / 2, lines); + break; + + case MAKEFOURCC('I','M','C','2'): + case MAKEFOURCC('I','M','C','4'): + MFCopyImage(dest, dest_stride, src, src_stride, width / 2, lines / 2); + MFCopyImage(dest + dest_stride / 2, dest_stride, src + src_stride / 2, src_stride, width / 2, lines / 2); + break; + + default: + break; + } +} + struct buffer { IMFMediaBuffer IMFMediaBuffer_iface; @@ -54,6 +82,8 @@ struct buffer unsigned int height; int pitch;
+ unsigned int fourcc; + unsigned int locks; } _2d; struct @@ -307,8 +337,8 @@ static HRESULT WINAPI memory_1d_2d_buffer_Unlock(IMFMediaBuffer *iface)
if (buffer->_2d.linear_buffer && !--buffer->_2d.locks) { - MFCopyImage(buffer->data, buffer->_2d.pitch, buffer->_2d.linear_buffer, buffer->_2d.width, - buffer->_2d.width, buffer->_2d.height); + copy_multiplane_image(buffer->data, buffer->_2d.pitch, buffer->_2d.linear_buffer, buffer->_2d.width, + buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc);
free(buffer->_2d.linear_buffer); buffer->_2d.linear_buffer = NULL; @@ -357,8 +387,8 @@ static HRESULT WINAPI d3d9_surface_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat hr = IDirect3DSurface9_LockRect(buffer->d3d9_surface.surface, &rect, NULL, 0); if (SUCCEEDED(hr)) { - MFCopyImage(buffer->_2d.linear_buffer, buffer->_2d.width, rect.pBits, rect.Pitch, - buffer->_2d.width, buffer->_2d.height); + copy_multiplane_image(buffer->_2d.linear_buffer, buffer->_2d.width, rect.pBits, rect.Pitch, + buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc); IDirect3DSurface9_UnlockRect(buffer->d3d9_surface.surface); } } @@ -396,8 +426,8 @@ static HRESULT WINAPI d3d9_surface_buffer_Unlock(IMFMediaBuffer *iface)
if (SUCCEEDED(hr = IDirect3DSurface9_LockRect(buffer->d3d9_surface.surface, &rect, NULL, 0))) { - MFCopyImage(rect.pBits, rect.Pitch, buffer->_2d.linear_buffer, buffer->_2d.width, - buffer->_2d.width, buffer->_2d.height); + copy_multiplane_image(rect.pBits, rect.Pitch, buffer->_2d.linear_buffer, buffer->_2d.width, + buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc); IDirect3DSurface9_UnlockRect(buffer->d3d9_surface.surface); }
@@ -936,8 +966,8 @@ static HRESULT WINAPI dxgi_surface_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat hr = dxgi_surface_buffer_map(buffer); if (SUCCEEDED(hr)) { - MFCopyImage(buffer->_2d.linear_buffer, buffer->_2d.width, buffer->dxgi_surface.map_desc.pData, - buffer->dxgi_surface.map_desc.RowPitch, buffer->_2d.width, buffer->_2d.height); + copy_multiplane_image(buffer->_2d.linear_buffer, buffer->_2d.width, buffer->dxgi_surface.map_desc.pData, + buffer->dxgi_surface.map_desc.RowPitch, buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc); } } } @@ -970,8 +1000,8 @@ static HRESULT WINAPI dxgi_surface_buffer_Unlock(IMFMediaBuffer *iface) hr = HRESULT_FROM_WIN32(ERROR_WAS_UNLOCKED); else if (!--buffer->_2d.locks) { - MFCopyImage(buffer->dxgi_surface.map_desc.pData, buffer->dxgi_surface.map_desc.RowPitch, - buffer->_2d.linear_buffer, buffer->_2d.width, buffer->_2d.width, buffer->_2d.height); + copy_multiplane_image(buffer->dxgi_surface.map_desc.pData, buffer->dxgi_surface.map_desc.RowPitch, + buffer->_2d.linear_buffer, buffer->_2d.width, buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc); dxgi_surface_buffer_unmap(buffer);
free(buffer->_2d.linear_buffer); @@ -1334,6 +1364,7 @@ static HRESULT create_2d_buffer(DWORD width, DWORD height, DWORD fourcc, BOOL bo object->_2d.height = height; object->_2d.pitch = bottom_up ? -pitch : pitch; object->_2d.scanline0 = bottom_up ? object->data + pitch * (object->_2d.height - 1) : object->data; + object->_2d.fourcc = fourcc;
*buffer = &object->IMFMediaBuffer_iface;
@@ -1373,6 +1404,8 @@ static HRESULT create_d3d9_surface_buffer(IUnknown *surface, BOOL bottom_up, IMF object->_2d.height = desc.Height; object->max_length = object->_2d.plane_size;
+ object->_2d.fourcc = desc.Format; + *buffer = &object->IMFMediaBuffer_iface;
return S_OK; @@ -1427,6 +1460,8 @@ static HRESULT create_dxgi_surface_buffer(IUnknown *surface, unsigned int sub_re object->_2d.height = desc.Height; object->max_length = object->_2d.plane_size;
+ object->_2d.fourcc = desc.Format; + if (FAILED(hr = init_attributes_object(&object->dxgi_surface.attributes, 0))) { IMFMediaBuffer_Release(&object->IMFMediaBuffer_iface); diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 65ab72bd57b..d73641652b1 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -5433,10 +5433,10 @@ static void test_MFCreate2DMediaBuffer(void) }; unsigned int max_length, length, length2; BYTE *buffer_start, *data, *data2; + int i, j, k, pitch, pitch2, stride; IMF2DBuffer2 *_2dbuffer2; IMF2DBuffer *_2dbuffer; IMFMediaBuffer *buffer; - int i, pitch, pitch2; HRESULT hr; BOOL ret;
@@ -5631,6 +5631,8 @@ static void test_MFCreate2DMediaBuffer(void) ok(length2 == ptr->contiguous_length, "%d: unexpected linear buffer length %u for %u x %u, format %s.\n", i, length2, ptr->width, ptr->height, wine_dbgstr_an((char *)&ptr->fourcc, 4));
+ memset(data, 0xff, length2); + hr = IMFMediaBuffer_Unlock(buffer); ok(hr == S_OK, "Failed to unlock buffer, hr %#x.\n", hr);
@@ -5649,6 +5651,44 @@ static void test_MFCreate2DMediaBuffer(void) ok(data2 == data, "Unexpected data pointer.\n"); ok(pitch == pitch2, "Unexpected pitch.\n");
+ /* primary plane */ + for(j = 0; j < ptr->height; j++) + for (k = 0; k < ptr->width; k++) + ok(data[j * pitch + k] == 0xff, "Unexpected byte %02x at test %d row %d column %d.\n", data[j * pitch + k], i, j, k); + + hr = pMFGetStrideForBitmapInfoHeader(ptr->fourcc, ptr->width, &stride); + ok(hr == S_OK, "Failed to get stride, hr %#x.\n", hr); + + /* secondary planes */ + switch (ptr->fourcc) + { + case MAKEFOURCC('I','M','C','1'): + case MAKEFOURCC('I','M','C','3'): + for (j = ptr->height; j < length2 / stride; j++) + for (k = 0; k < ptr->width / 2; k++) + ok(data[j * pitch + k] == 0xff, "Unexpected byte %02x at test %d row %d column %d.\n", data[j * pitch + k], i, j, k); + break; + + case MAKEFOURCC('I','M','C','2'): + case MAKEFOURCC('I','M','C','4'): + for (j = ptr->height; j < length2 / stride; j++) + for (k = 0; k < ptr->width / 2; k++) + ok(data[j * pitch + k] == 0xff, "Unexpected byte %02x at test %d row %d column %d.\n", data[j * pitch + k], i, j, k); + for (j = ptr->height; j < length2 / stride; j++) + for (k = pitch / 2; k < pitch / 2 + ptr->width / 2; k++) + ok(data[j * pitch + k] == 0xff, "Unexpected byte %02x at test %d row %d column %d.\n", data[j * pitch + k], i, j, k); + break; + + case MAKEFOURCC('N','V','1','2'): + for (j = ptr->height; j < length2 / stride; j++) + for (k = 0; k < ptr->width; k++) + ok(data[j * pitch + k] == 0xff, "Unexpected byte %02x at test %d row %d column %d.\n", data[j * pitch + k], i, j, k); + break; + + default: + ; + } + hr = IMF2DBuffer_Unlock2D(_2dbuffer); ok(hr == S_OK, "Failed to unlock buffer, hr %#x.\n", hr);
Not sure what's happening yet, but this gives me heap corruption it seems, with crashes looking like this:
Backtrace: =>0 0x7bc26f16 HEAP_CreateFreeBlock+0x126(subheap=<is not available>, ptr=0x1825918, size=<is not available>) [Z:\ssd\data\wine\wine-git\include\wine\list.h:100] in ntdll (0x006cfa68) 1 0x7bc279a4 HEAP_MakeInUseBlockFree+0xe3(subheap=<is not available>, pArena=<is not available>) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:665] in ntdll (0x006cfaa8) 2 0x7bc2822c HEAP_IsRealArena+0x73b(heapPtr=<is not available>, flags=<is not available>, block=<is not available>) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:1767] in ntdll (0x006cfb08) 3 0x7bc2954a RtlCreateHeap+0x139(flags=<is not available>, addr=<is not available>, totalSize=<is not available>, commitSize=<is not available>, unknown=<is not available>, definition=<is not available>) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:1744] in ntdll (0x006cfb48) 4 0x1002821b EntryPoint+0x16ba() in ucrtbase (0x006cfb68) 5 0x00cc396d memory_buffer_GetMaxLength+0x10c() [Z:\ssd\data\wine\wine-git\dlls\mfplat\buffer.c:175] in mfplat (0x006cfba8) 6 0x00415b46 test_MFCreate2DMediaBuffer+0x16d5() [Z:\ssd\data\wine\build\wine32\include\mfobjects.h:809] in mfplat_test (0x006cfc58)
It seems to be about IMC2/IMC4 case, if I comment out both MFCopyImage calls it doesn't crash. For IMC1/IMC3 same strides are used, so you copy U/V in one go, including padding half-sized areas. For IMC2/IMC4 same stride is used for Y and U/V making two calls, is this correct?
I was going to suggest something like attached patch as an alternative to whole thing, but we need to fix this corruption first.
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=91672
Your paranoid android.
=== debiant2 (build log) ===
error: patch failed: dlls/mfplat/buffer.c:32 Task: Patch failed to apply
=== debiant2 (build log) ===
error: patch failed: dlls/mfplat/buffer.c:32 Task: Patch failed to apply
Sorry for the late reply, I missed your mail when it first came.
On 6/3/21 3:37 AM, Nikolay Sivov wrote:
Not sure what's happening yet, but this gives me heap corruption it seems, with crashes looking like this:
Backtrace: =>0 0x7bc26f16 HEAP_CreateFreeBlock+0x126(subheap=<is not available>, ptr=0x1825918, size=<is not available>) [Z:\ssd\data\wine\wine-git\include\wine\list.h:100] in ntdll (0x006cfa68) 1 0x7bc279a4 HEAP_MakeInUseBlockFree+0xe3(subheap=<is not available>, pArena=<is not available>) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:665] in ntdll (0x006cfaa8) 2 0x7bc2822c HEAP_IsRealArena+0x73b(heapPtr=<is not available>, flags=<is not available>, block=<is not available>) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:1767] in ntdll (0x006cfb08) 3 0x7bc2954a RtlCreateHeap+0x139(flags=<is not available>, addr=<is not available>, totalSize=<is not available>, commitSize=<is not available>, unknown=<is not available>, definition=<is not available>) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:1744] in ntdll (0x006cfb48) 4 0x1002821b EntryPoint+0x16ba() in ucrtbase (0x006cfb68) 5 0x00cc396d memory_buffer_GetMaxLength+0x10c() [Z:\ssd\data\wine\wine-git\dlls\mfplat\buffer.c:175] in mfplat (0x006cfba8) 6 0x00415b46 test_MFCreate2DMediaBuffer+0x16d5() [Z:\ssd\data\wine\build\wine32\include\mfobjects.h:809] in mfplat_test (0x006cfc58)
It seems to be about IMC2/IMC4 case, if I comment out both MFCopyImage calls it doesn't crash. For IMC1/IMC3 same strides are used, so you copy U/V in one go, including padding half-sized areas. For IMC2/IMC4 same stride is used for Y and U/V making two calls, is this correct?
In both formats (IMC1, IMC2, and derivatives), the stride is the same between all three planes. In IMC2 the only difference is that the U plane is offset by half a stride.
I was going to suggest something like attached patch as an alternative to whole thing, but we need to fix this corruption first.
Yeah, I'll look into it.
-
Derek Lesho -
Marvin -
Nikolay Sivov