From: Nikolay Sivov Subject: Re: [PATCH] mfplat: Copy all image planes in buffer copies. Message-Id: Date: Thu, 3 Jun 2021 10:37:08 +0300 In-Reply-To: <20210602211856.429704-1-dlesho@codeweavers.com> References: <20210602211856.429704-1-dlesho@codeweavers.com> 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=, ptr=0x1825918, size=) [Z:\ssd\data\wine\wine-git\include\wine\list.h:100] in ntdll (0x006cfa68)   1 0x7bc279a4 HEAP_MakeInUseBlockFree+0xe3(subheap=, pArena=) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:665] in ntdll (0x006cfaa8)   2 0x7bc2822c HEAP_IsRealArena+0x73b(heapPtr=, flags=, block=) [Z:\ssd\data\wine\wine-git\dlls\ntdll\heap.c:1767] in ntdll (0x006cfb08)   3 0x7bc2954a RtlCreateHeap+0x139(flags=, addr=, totalSize=, commitSize=, unknown=, definition=) [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. diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index 5830b1af4ff..c4835bddb5d 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -32,33 +32,7 @@ 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; - } -} +typedef void (*p_copy_image_func)(BYTE *dest, LONG dest_stride, const BYTE *src, LONG src_stride, DWORD width, DWORD lines); struct buffer { @@ -81,10 +55,8 @@ struct buffer unsigned int width; unsigned int height; int pitch; - - unsigned int fourcc; - unsigned int locks; + p_copy_image_func copy_image; } _2d; struct { @@ -103,6 +75,35 @@ struct buffer CRITICAL_SECTION cs; }; +static void copy_image(const struct buffer *buffer, BYTE *dest, LONG dest_stride, const BYTE *src, + LONG src_stride, DWORD width, DWORD lines) +{ + MFCopyImage(dest, dest_stride, src, src_stride, width, lines); + + if (buffer->_2d.copy_image) + { + dest += dest_stride * lines; + src += src_stride * lines; + buffer->_2d.copy_image(dest, dest_stride, src, src_stride, width, lines); + } +} + +static void copy_image_nv12(BYTE *dest, LONG dest_stride, const BYTE *src, LONG src_stride, DWORD width, DWORD lines) +{ + //MFCopyImage(dest, dest_stride, src, src_stride, width, lines / 2); +} + +static void copy_image_imc1(BYTE *dest, LONG dest_stride, const BYTE *src, LONG src_stride, DWORD width, DWORD lines) +{ + MFCopyImage(dest, dest_stride, src, src_stride, width / 2, lines); +} + +static void copy_image_imc2(BYTE *dest, LONG dest_stride, const BYTE *src, LONG src_stride, DWORD width, DWORD lines) +{ + 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); +} + static inline struct buffer *impl_from_IMFMediaBuffer(IMFMediaBuffer *iface) { return CONTAINING_RECORD(iface, struct buffer, IMFMediaBuffer_iface); @@ -337,8 +338,8 @@ static HRESULT WINAPI memory_1d_2d_buffer_Unlock(IMFMediaBuffer *iface) if (buffer->_2d.linear_buffer && !--buffer->_2d.locks) { - copy_multiplane_image(buffer->data, buffer->_2d.pitch, buffer->_2d.linear_buffer, buffer->_2d.width, - buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc); + copy_image(buffer, buffer->data, buffer->_2d.pitch, buffer->_2d.linear_buffer, buffer->_2d.width, + buffer->_2d.width, buffer->_2d.height); free(buffer->_2d.linear_buffer); buffer->_2d.linear_buffer = NULL; @@ -387,8 +388,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)) { - copy_multiplane_image(buffer->_2d.linear_buffer, buffer->_2d.width, rect.pBits, rect.Pitch, - buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc); + copy_image(buffer, buffer->_2d.linear_buffer, buffer->_2d.width, rect.pBits, rect.Pitch, + buffer->_2d.width, buffer->_2d.height); IDirect3DSurface9_UnlockRect(buffer->d3d9_surface.surface); } } @@ -426,8 +427,8 @@ static HRESULT WINAPI d3d9_surface_buffer_Unlock(IMFMediaBuffer *iface) if (SUCCEEDED(hr = IDirect3DSurface9_LockRect(buffer->d3d9_surface.surface, &rect, NULL, 0))) { - copy_multiplane_image(rect.pBits, rect.Pitch, buffer->_2d.linear_buffer, buffer->_2d.width, - buffer->_2d.width, buffer->_2d.height, buffer->_2d.fourcc); + copy_image(buffer, rect.pBits, rect.Pitch, buffer->_2d.linear_buffer, buffer->_2d.width, + buffer->_2d.width, buffer->_2d.height); IDirect3DSurface9_UnlockRect(buffer->d3d9_surface.surface); } @@ -966,8 +967,8 @@ static HRESULT WINAPI dxgi_surface_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat hr = dxgi_surface_buffer_map(buffer); if (SUCCEEDED(hr)) { - 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); + copy_image(buffer, 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); } } } @@ -1000,8 +1001,8 @@ static HRESULT WINAPI dxgi_surface_buffer_Unlock(IMFMediaBuffer *iface) hr = HRESULT_FROM_WIN32(ERROR_WAS_UNLOCKED); else if (!--buffer->_2d.locks) { - 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); + copy_image(buffer, 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); dxgi_surface_buffer_unmap(buffer); free(buffer->_2d.linear_buffer); @@ -1292,6 +1293,17 @@ static HRESULT create_1d_buffer(DWORD max_length, DWORD alignment, IMFMediaBuffe return S_OK; } +static p_copy_image_func get_2d_buffer_copy_func(DWORD fourcc) +{ + if (fourcc == MAKEFOURCC('N','V','1','2')) + return copy_image_nv12; + if (fourcc == MAKEFOURCC('I','M','C','1') || fourcc == MAKEFOURCC('I','M','C','3')) + return copy_image_imc1; + if (fourcc == MAKEFOURCC('I','M','C','2') || fourcc == MAKEFOURCC('I','M','C','4')) + return copy_image_imc2; + return NULL; +} + static HRESULT create_2d_buffer(DWORD width, DWORD height, DWORD fourcc, BOOL bottom_up, IMFMediaBuffer **buffer) { unsigned int stride, max_length, plane_size; @@ -1364,7 +1376,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; + object->_2d.copy_image = get_2d_buffer_copy_func(fourcc); *buffer = &object->IMFMediaBuffer_iface; @@ -1403,8 +1415,7 @@ static HRESULT create_d3d9_surface_buffer(IUnknown *surface, BOOL bottom_up, IMF object->_2d.width = stride; object->_2d.height = desc.Height; object->max_length = object->_2d.plane_size; - - object->_2d.fourcc = desc.Format; + object->_2d.copy_image = get_2d_buffer_copy_func(desc.Format); *buffer = &object->IMFMediaBuffer_iface; @@ -1459,8 +1470,7 @@ static HRESULT create_dxgi_surface_buffer(IUnknown *surface, unsigned int sub_re object->_2d.width = stride; object->_2d.height = desc.Height; object->max_length = object->_2d.plane_size; - - object->_2d.fourcc = desc.Format; + object->_2d.copy_image = get_2d_buffer_copy_func(desc.Format); if (FAILED(hr = init_attributes_object(&object->dxgi_surface.attributes, 0))) {