[PATCH 0/5] MR2781: mfplat: Optimize sample copy to d3d buffer.
This is ultimately a follow up for introducing d3d manager support in h264 decoder transform. The last patch avoids unneeded GPU to CPU copy when copying a memory sample to GPU sample (in particular, when sample copier is involved). There are a couple of things to fix on the way. Patch 1 fixes the crash which otherwise happens in the test introduced in patch 2 due to memory corruption. Flipping the image instead of using absolute pitch will break the test in patch 2. Patch 3 contradicts to what MS docs says [1] for MFCreate2DMediaBuffer, fBottomUp parameter: "If TRUE, the buffer's IMF2DBuffer::ContiguousCopyTo method copies the buffer into a bottom-up format.". That doesn't match the tests in patch 2. So far I see that fBottomUp parameter only affects what is returned from Lock2D as scanline0 and pitch. Thinking of it, the behaviour of ContiguousCopyFrom makes some sense to me as Lock2D (unlike Lock which converts to contiguous buffer) is not supposed to perform any copies and flips and return the underlying representation as is. And once scanline0 and pitch for that 2d buffer indicate it should be copied bottom up additionally flipping when converting from contiguous buffer wouldn't make much sense. Then, ContiguousCopyTo could flip it actually as the docs suggest, but the test seems to clearly show that it doesn't happen (there is a separate test for clarity showing that ContiguousCopyFrom / ContiguousCopyTo yields the same data). 1. https://learn.microsoft.com/en-us/windows/win32/api/mfapi/nf-mfapi-mfcreate2... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2781
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/mfplat/buffer.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index cb1b3176a08..52d05f4ef80 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -313,8 +313,14 @@ static HRESULT WINAPI memory_1d_2d_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat hr = E_OUTOFMEMORY; if (SUCCEEDED(hr)) - copy_image(buffer, buffer->_2d.linear_buffer, buffer->_2d.width, buffer->data, buffer->_2d.pitch, + { + int pitch = buffer->_2d.pitch; + + if (pitch < 0) + pitch = -pitch; + copy_image(buffer, buffer->_2d.linear_buffer, buffer->_2d.width, buffer->data, pitch, buffer->_2d.width, buffer->_2d.height); + } } if (SUCCEEDED(hr)) @@ -342,7 +348,11 @@ static HRESULT WINAPI memory_1d_2d_buffer_Unlock(IMFMediaBuffer *iface) if (buffer->_2d.linear_buffer && !--buffer->_2d.locks) { - copy_image(buffer, buffer->data, buffer->_2d.pitch, buffer->_2d.linear_buffer, buffer->_2d.width, + int pitch = buffer->_2d.pitch; + + if (pitch < 0) + pitch = -pitch; + copy_image(buffer, buffer->data, pitch, buffer->_2d.linear_buffer, buffer->_2d.width, buffer->_2d.width, buffer->_2d.height); free(buffer->_2d.linear_buffer); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2781
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 187 +++++++++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 380850da08a..53005c73c02 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -9265,6 +9265,192 @@ static void test_MFCreatePathFromURL(void) } } +#define check_reset_data(a, b, c, d, e, f) check_reset_data_(__LINE__, a, b, c, d, e, f) +static void check_reset_data_(unsigned int line, IMF2DBuffer2 *buffer2d, const BYTE *data, BOOL bottom_up, + DWORD width, DWORD height, BOOL todo) +{ + BYTE *scanline0, *buffer_start; + DWORD length, max_length; + IMFMediaBuffer *buffer; + LONG pitch; + BYTE *lock; + HRESULT hr; + int i; + + hr = IMF2DBuffer2_QueryInterface(buffer2d, &IID_IMFMediaBuffer, (void **)&buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + hr = IMF2DBuffer2_Lock2DSize(buffer2d, MF2DBuffer_LockFlags_Read, &scanline0, &pitch, &buffer_start, &length); + ok(hr == S_OK, "got hr %#lx.\n", hr); + if (bottom_up) + { + ok(pitch < 0, "got pitch %ld.\n", pitch); + ok(buffer_start == scanline0 + pitch * (LONG)(height - 1), "buffer start mismatch.\n"); + } + else + { + ok(pitch > 0, "got pitch %ld.\n", pitch); + ok(buffer_start == scanline0, "buffer start mismatch.\n"); + } + for (i = 0; i < height; ++i) + todo_wine_if(bottom_up && todo) ok_(__FILE__,line)(!memcmp(buffer_start + abs(pitch) * i, data + width * i * 4, width * 4), + "2D Data mismatch, scaline %d.\n", i); + hr = IMF2DBuffer2_Unlock2D(buffer2d); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + hr = IMFMediaBuffer_Lock(buffer, &lock, &max_length, &length); + ok(hr == S_OK, "got hr %#lx.\n", hr); + ok_(__FILE__,line)(max_length == width * height * 4, "got max_length %lu.\n", max_length); + ok_(__FILE__,line)(length == width * height * 4, "got length %lu.\n", length); + todo_wine_if(bottom_up && todo) ok_(__FILE__,line)(!memcmp(lock, data, length), "contiguous data mismatch.\n"); + memset(lock, 0xcc, length); + hr = IMFMediaBuffer_Unlock(buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + IMFMediaBuffer_Release(buffer); +} + +static void test_2dbuffer_copy_(IMFMediaBuffer *buffer, BOOL bottom_up, DWORD width, DWORD height) +{ + static const unsigned int test_data[] = + { + 0x01010101, 0x01010101, + 0x02020202, 0x02020202, + }; + + BYTE data[sizeof(test_data)]; + IMFMediaBuffer *src_buffer; + DWORD length, max_length; + IMF2DBuffer2 *buffer2d; + IMFSample *sample; + BYTE *lock; + HRESULT hr; + ULONG ref; + + hr = IMFMediaBuffer_QueryInterface(buffer, &IID_IMF2DBuffer2, (void **)&buffer2d); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + hr = MFCreateSample(&sample); + ok(hr == S_OK, "got hr %#lx.\n", hr); + hr = MFCreateMemoryBuffer(sizeof(test_data) * 2, &src_buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + hr = IMFSample_AddBuffer(sample, src_buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + hr = IMFMediaBuffer_Lock(src_buffer, &lock, &max_length, &length); + ok(hr == S_OK, "got hr %#lx.\n", hr); + ok(max_length == sizeof(test_data) * 2, "got %lu.\n", max_length); + memcpy(lock, test_data, sizeof(test_data)); + hr = IMFMediaBuffer_Unlock(src_buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + hr = IMFMediaBuffer_Lock(buffer, &lock, &max_length, &length); + ok(hr == S_OK, "got hr %#lx.\n", hr); + ok(max_length == 16, "got %lu.\n", max_length); + ok(length == 16, "got %lu.\n", length); + memset(lock, 0xcc, length); + hr = IMFMediaBuffer_Unlock(buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + hr = IMFMediaBuffer_SetCurrentLength(src_buffer, 1); + ok(hr == S_OK, "got hr %#lx.\n", hr); + hr = IMFSample_CopyToBuffer(sample, buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + memset(data, 0xcc, sizeof(data)); + data[0] = ((BYTE *)test_data)[0]; + check_reset_data(buffer2d, data, bottom_up, width, height, FALSE); + + hr = IMF2DBuffer2_ContiguousCopyFrom(buffer2d, (BYTE *)test_data, sizeof(test_data)); + ok(hr == S_OK, "got hr %#lx.\n", hr); + hr = IMF2DBuffer2_ContiguousCopyTo(buffer2d, data, sizeof(data)); + ok(hr == S_OK, "got hr %#lx.\n", hr); + ok(!memcmp(data, test_data, sizeof(data)), "data mismatch.\n"); + + check_reset_data(buffer2d, (const BYTE *)test_data, bottom_up, width, height, TRUE); + + hr = IMFMediaBuffer_SetCurrentLength(src_buffer, sizeof(test_data) + 1); + ok(hr == S_OK, "got hr %#lx.\n", hr); + hr = IMFSample_CopyToBuffer(sample, buffer); + ok(hr == MF_E_BUFFERTOOSMALL, "got hr %#lx.\n", hr); + + hr = IMFMediaBuffer_SetCurrentLength(src_buffer, sizeof(test_data)); + ok(hr == S_OK, "got hr %#lx.\n", hr); + hr = IMFSample_CopyToBuffer(sample, buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + check_reset_data(buffer2d, (const BYTE *)test_data, bottom_up, width, height, FALSE); + + IMF2DBuffer2_Release(buffer2d); + ref = IMFSample_Release(sample); + ok(!ref, "got %lu.\n", ref); + ref = IMFMediaBuffer_Release(src_buffer); + ok(!ref, "got %lu.\n", ref); +} + +static void test_2dbuffer_copy(void) +{ + D3D11_TEXTURE2D_DESC desc; + ID3D11Texture2D *texture; + IMFMediaBuffer *buffer; + ID3D11Device *device; + HRESULT hr; + ULONG ref; + + if (!pMFCreate2DMediaBuffer) + { + win_skip("MFCreate2DMediaBuffer() is not available.\n"); + return; + } + + winetest_push_context("top down"); + hr = pMFCreate2DMediaBuffer(2, 2, D3DFMT_A8R8G8B8, FALSE, &buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + test_2dbuffer_copy_(buffer, FALSE, 2, 2); + ref = IMFMediaBuffer_Release(buffer); + ok(!ref, "got %lu.\n", ref); + winetest_pop_context(); + + winetest_push_context("bottom up"); + hr = pMFCreate2DMediaBuffer(2, 2, D3DFMT_A8R8G8B8, TRUE, &buffer); + ok(hr == S_OK, "got hr %#lx.\n", hr); + test_2dbuffer_copy_(buffer, TRUE, 2, 2); + ref = IMFMediaBuffer_Release(buffer); + ok(!ref, "got %lu.\n", ref); + winetest_pop_context(); + + if (!pMFCreateDXGISurfaceBuffer) + { + win_skip("MFCreateDXGISurfaceBuffer() is not available.\n"); + return; + } + + if (!(device = create_d3d11_device())) + { + skip("Failed to create a D3D11 device, skipping tests.\n"); + return; + } + + memset(&desc, 0, sizeof(desc)); + desc.Width = 2; + desc.Height = 2; + desc.ArraySize = 1; + desc.Format = DXGI_FORMAT_B8G8R8A8_UNORM; + desc.SampleDesc.Count = 1; + desc.SampleDesc.Quality = 0; + hr = ID3D11Device_CreateTexture2D(device, &desc, NULL, &texture); + ok(hr == S_OK, "Failed to create a texture, hr %#lx.\n", hr); + + hr = pMFCreateDXGISurfaceBuffer(&IID_ID3D11Texture2D, (IUnknown *)texture, 0, FALSE, &buffer); + ok(hr == S_OK, "Failed to create a buffer, hr %#lx.\n", hr); + test_2dbuffer_copy_(buffer, FALSE, 2, 2); + + ID3D11Texture2D_Release(texture); + ref = IMFMediaBuffer_Release(buffer); + ok(!ref, "got %lu.\n", ref); + ID3D11Device_Release(device); +} + START_TEST(mfplat) { char **argv; @@ -9349,6 +9535,7 @@ START_TEST(mfplat) test_MFInitMediaTypeFromVideoInfoHeader(); test_MFInitMediaTypeFromAMMediaType(); test_MFCreatePathFromURL(); + test_2dbuffer_copy(); CoUninitialize(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2781
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/mfplat/buffer.c | 10 ++++++++-- dlls/mfplat/tests/mfplat.c | 14 +++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c index 52d05f4ef80..3850efa594c 100644 --- a/dlls/mfplat/buffer.c +++ b/dlls/mfplat/buffer.c @@ -626,7 +626,10 @@ static HRESULT WINAPI memory_2d_buffer_ContiguousCopyTo(IMF2DBuffer2 *iface, BYT if (SUCCEEDED(hr)) { - copy_image(buffer, dest_buffer, buffer->_2d.width, src_scanline0, src_pitch, buffer->_2d.width, buffer->_2d.height); + if (src_pitch < 0) + src_pitch = -src_pitch; + copy_image(buffer, dest_buffer, buffer->_2d.width, src_buffer_start, src_pitch, + buffer->_2d.width, buffer->_2d.height); if (FAILED(IMF2DBuffer2_Unlock2D(iface))) WARN("Couldn't unlock source buffer %p, hr %#lx.\n", iface, hr); @@ -652,7 +655,10 @@ static HRESULT WINAPI memory_2d_buffer_ContiguousCopyFrom(IMF2DBuffer2 *iface, c if (SUCCEEDED(hr)) { - copy_image(buffer, dst_scanline0, dst_pitch, src_buffer, buffer->_2d.width, buffer->_2d.width, buffer->_2d.height); + if (dst_pitch < 0) + dst_pitch = -dst_pitch; + copy_image(buffer, dst_buffer_start, dst_pitch, src_buffer, buffer->_2d.width, + buffer->_2d.width, buffer->_2d.height); if (FAILED(IMF2DBuffer2_Unlock2D(iface))) WARN("Couldn't unlock destination buffer %p, hr %#lx.\n", iface, hr); diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 53005c73c02..3f360a53c6c 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -9265,9 +9265,9 @@ static void test_MFCreatePathFromURL(void) } } -#define check_reset_data(a, b, c, d, e, f) check_reset_data_(__LINE__, a, b, c, d, e, f) +#define check_reset_data(a, b, c, d, e) check_reset_data_(__LINE__, a, b, c, d, e) static void check_reset_data_(unsigned int line, IMF2DBuffer2 *buffer2d, const BYTE *data, BOOL bottom_up, - DWORD width, DWORD height, BOOL todo) + DWORD width, DWORD height) { BYTE *scanline0, *buffer_start; DWORD length, max_length; @@ -9293,7 +9293,7 @@ static void check_reset_data_(unsigned int line, IMF2DBuffer2 *buffer2d, const B ok(buffer_start == scanline0, "buffer start mismatch.\n"); } for (i = 0; i < height; ++i) - todo_wine_if(bottom_up && todo) ok_(__FILE__,line)(!memcmp(buffer_start + abs(pitch) * i, data + width * i * 4, width * 4), + ok_(__FILE__,line)(!memcmp(buffer_start + abs(pitch) * i, data + width * i * 4, width * 4), "2D Data mismatch, scaline %d.\n", i); hr = IMF2DBuffer2_Unlock2D(buffer2d); ok(hr == S_OK, "got hr %#lx.\n", hr); @@ -9302,7 +9302,7 @@ static void check_reset_data_(unsigned int line, IMF2DBuffer2 *buffer2d, const B ok(hr == S_OK, "got hr %#lx.\n", hr); ok_(__FILE__,line)(max_length == width * height * 4, "got max_length %lu.\n", max_length); ok_(__FILE__,line)(length == width * height * 4, "got length %lu.\n", length); - todo_wine_if(bottom_up && todo) ok_(__FILE__,line)(!memcmp(lock, data, length), "contiguous data mismatch.\n"); + ok_(__FILE__,line)(!memcmp(lock, data, length), "contiguous data mismatch.\n"); memset(lock, 0xcc, length); hr = IMFMediaBuffer_Unlock(buffer); ok(hr == S_OK, "got hr %#lx.\n", hr); @@ -9359,7 +9359,7 @@ static void test_2dbuffer_copy_(IMFMediaBuffer *buffer, BOOL bottom_up, DWORD wi memset(data, 0xcc, sizeof(data)); data[0] = ((BYTE *)test_data)[0]; - check_reset_data(buffer2d, data, bottom_up, width, height, FALSE); + check_reset_data(buffer2d, data, bottom_up, width, height); hr = IMF2DBuffer2_ContiguousCopyFrom(buffer2d, (BYTE *)test_data, sizeof(test_data)); ok(hr == S_OK, "got hr %#lx.\n", hr); @@ -9367,7 +9367,7 @@ static void test_2dbuffer_copy_(IMFMediaBuffer *buffer, BOOL bottom_up, DWORD wi ok(hr == S_OK, "got hr %#lx.\n", hr); ok(!memcmp(data, test_data, sizeof(data)), "data mismatch.\n"); - check_reset_data(buffer2d, (const BYTE *)test_data, bottom_up, width, height, TRUE); + check_reset_data(buffer2d, (const BYTE *)test_data, bottom_up, width, height); hr = IMFMediaBuffer_SetCurrentLength(src_buffer, sizeof(test_data) + 1); ok(hr == S_OK, "got hr %#lx.\n", hr); @@ -9379,7 +9379,7 @@ static void test_2dbuffer_copy_(IMFMediaBuffer *buffer, BOOL bottom_up, DWORD wi hr = IMFSample_CopyToBuffer(sample, buffer); ok(hr == S_OK, "got hr %#lx.\n", hr); - check_reset_data(buffer2d, (const BYTE *)test_data, bottom_up, width, height, FALSE); + check_reset_data(buffer2d, (const BYTE *)test_data, bottom_up, width, height); IMF2DBuffer2_Release(buffer2d); ref = IMFSample_Release(sample); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2781
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/mfplat/sample.c | 52 ++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/dlls/mfplat/sample.c b/dlls/mfplat/sample.c index 687ada1a477..3099f903779 100644 --- a/dlls/mfplat/sample.c +++ b/dlls/mfplat/sample.c @@ -810,36 +810,42 @@ static HRESULT WINAPI sample_CopyToBuffer(IMFSample *iface, IMFMediaBuffer *buff dst_ptr = NULL; dst_length = current_length = 0; locked = SUCCEEDED(hr = IMFMediaBuffer_Lock(buffer, &dst_ptr, &dst_length, ¤t_length)); - if (locked) + if (!locked) + goto done; + + if (dst_length < total_length) { - if (dst_length < total_length) - hr = MF_E_BUFFERTOOSMALL; - else if (dst_ptr) + hr = MF_E_BUFFERTOOSMALL; + goto done; + } + + if (!dst_ptr) + goto done; + + for (i = 0; i < sample->buffer_count && SUCCEEDED(hr); ++i) + { + src_ptr = NULL; + src_max_length = current_length = 0; + + if (FAILED(hr = IMFMediaBuffer_Lock(sample->buffers[i], &src_ptr, &src_max_length, ¤t_length))) + continue; + + if (src_ptr) { - for (i = 0; i < sample->buffer_count && SUCCEEDED(hr); ++i) + if (current_length > dst_length) + hr = MF_E_BUFFERTOOSMALL; + else if (current_length) { - src_ptr = NULL; - src_max_length = current_length = 0; - if (SUCCEEDED(hr = IMFMediaBuffer_Lock(sample->buffers[i], &src_ptr, &src_max_length, ¤t_length))) - { - if (src_ptr) - { - if (current_length > dst_length) - hr = MF_E_BUFFERTOOSMALL; - else if (current_length) - { - memcpy(dst_ptr, src_ptr, current_length); - dst_length -= current_length; - dst_current_length += current_length; - dst_ptr += current_length; - } - } - IMFMediaBuffer_Unlock(sample->buffers[i]); - } + memcpy(dst_ptr, src_ptr, current_length); + dst_length -= current_length; + dst_current_length += current_length; + dst_ptr += current_length; } } + IMFMediaBuffer_Unlock(sample->buffers[i]); } +done: IMFMediaBuffer_SetCurrentLength(buffer, dst_current_length); if (locked) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2781
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/mfplat/sample.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/dlls/mfplat/sample.c b/dlls/mfplat/sample.c index 3099f903779..a50f81c93d4 100644 --- a/dlls/mfplat/sample.c +++ b/dlls/mfplat/sample.c @@ -796,8 +796,9 @@ static HRESULT WINAPI sample_CopyToBuffer(IMFSample *iface, IMFMediaBuffer *buff struct sample *sample = impl_from_IMFSample(iface); DWORD total_length, dst_length, dst_current_length, src_max_length, current_length; BYTE *src_ptr, *dst_ptr; - BOOL locked; - HRESULT hr; + IMF2DBuffer *buffer2d; + BOOL locked = FALSE; + HRESULT hr = E_FAIL; size_t i; TRACE("%p, %p.\n", iface, buffer); @@ -807,6 +808,25 @@ static HRESULT WINAPI sample_CopyToBuffer(IMFSample *iface, IMFMediaBuffer *buff total_length = sample_get_total_length(sample); dst_current_length = 0; + if (sample->buffer_count == 1 + && SUCCEEDED(IMFMediaBuffer_QueryInterface(buffer, &IID_IMF2DBuffer, (void **)&buffer2d))) + { + if (SUCCEEDED(IMFMediaBuffer_GetCurrentLength(sample->buffers[0], ¤t_length)) + && SUCCEEDED(IMF2DBuffer_GetContiguousLength(buffer2d, &dst_length)) + && current_length == dst_length + && SUCCEEDED(IMFMediaBuffer_Lock(sample->buffers[0], &src_ptr, &src_max_length, ¤t_length))) + { + hr = IMF2DBuffer_ContiguousCopyFrom(buffer2d, src_ptr, current_length); + IMFMediaBuffer_Unlock(sample->buffers[0]); + } + IMF2DBuffer_Release(buffer2d); + if (SUCCEEDED(hr)) + { + dst_current_length = current_length; + goto done; + } + } + dst_ptr = NULL; dst_length = current_length = 0; locked = SUCCEEDED(hr = IMFMediaBuffer_Lock(buffer, &dst_ptr, &dst_length, ¤t_length)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2781
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)