[PATCH v3 0/2] MR9125: mfmediaengine: Suppport TransferVideoFrame() to IWICBitmap.
-- v3: mfmediaengine: Suppport TransferVideoFrame() to IWICBitmap. https://gitlab.winehq.org/wine/wine/-/merge_requests/9125
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfmediaengine/tests/Makefile.in | 2 +- dlls/mfmediaengine/tests/mfmediaengine.c | 113 +++++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/dlls/mfmediaengine/tests/Makefile.in b/dlls/mfmediaengine/tests/Makefile.in index e123952956d..7e050387490 100644 --- a/dlls/mfmediaengine/tests/Makefile.in +++ b/dlls/mfmediaengine/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = mfmediaengine.dll -IMPORTS = ole32 mf mfplat oleaut32 mfuuid uuid +IMPORTS = ole32 mf mfplat oleaut32 mfuuid uuid windowscodecs dxguid SOURCES = \ mfmediaengine.c \ diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c index fbaeff50710..715d44a8ef7 100644 --- a/dlls/mfmediaengine/tests/mfmediaengine.c +++ b/dlls/mfmediaengine/tests/mfmediaengine.c @@ -33,6 +33,7 @@ #include "initguid.h" #include "mmdeviceapi.h" #include "audiosessiontypes.h" +#include "wincodec.h" #include "wine/test.h" @@ -1272,9 +1273,11 @@ static void test_TransferVideoFrame(void) ID3D11Texture2D *texture = NULL, *rb_texture; D3D11_MAPPED_SUBRESOURCE map_desc; IMFMediaEngineEx *media_engine = NULL; + IWICImagingFactory *factory = NULL; IMFDXGIDeviceManager *manager; ID3D11DeviceContext *context; D3D11_TEXTURE2D_DESC desc; + IWICBitmap *bitmap = NULL; IMFByteStream *stream; ID3D11Device *device; RECT dst_rect; @@ -1317,6 +1320,13 @@ static void test_TransferVideoFrame(void) hr = ID3D11Device_CreateTexture2D(device, &desc, NULL, &texture); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER, + &IID_IWICImagingFactory, (void **)&factory); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IWICImagingFactory_CreateBitmap(factory, desc.Width, desc.Height, &GUID_WICPixelFormat32bppBGR, + WICBitmapCacheOnLoad, &bitmap); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + url = SysAllocString(L"i420-64x64.avi"); hr = IMFMediaEngineEx_SetSourceFromByteStream(media_engine, stream, url); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -1373,6 +1383,10 @@ static void test_TransferVideoFrame(void) ID3D11DeviceContext_Release(context); ID3D11Texture2D_Release(rb_texture); + hr = IMFMediaEngineEx_TransferVideoFrame(notify->media_engine, (IUnknown *)bitmap, NULL, &dst_rect, NULL); + /* not supported if a DXGI device manager was provided */ + ok(hr == E_NOINTERFACE, "Unexpected hr %#lx.\n", hr); + done: if (media_engine) { @@ -1380,6 +1394,10 @@ done: IMFMediaEngineEx_Release(media_engine); } + if (bitmap) + IWICBitmap_Release(bitmap); + if (factory) + IWICImagingFactory_Release(factory); if (texture) ID3D11Texture2D_Release(texture); if (device) @@ -1388,6 +1406,100 @@ done: IMFMediaEngineNotify_Release(¬ify->IMFMediaEngineNotify_iface); } +static void test_TransferVideoFrame_wic(void) +{ + struct test_transfer_notify *notify; + UINT lock_buffer_size, lock_buffer_stride; + IMFMediaEngineEx *media_engine = NULL; + IWICImagingFactory *factory = NULL; + IWICBitmap *bitmap = NULL; + IMFByteStream *stream; + IWICBitmapLock *lock; + WICRect wicrc = {0}; + BYTE *lock_buffer; + RECT dst_rect; + LONGLONG pts; + HRESULT hr; + DWORD res; + BSTR url; + + stream = load_resource(L"i420-64x64.avi", L"video/avi"); + + notify = create_transfer_notify(); + + create_media_engine(¬ify->IMFMediaEngineNotify_iface, NULL, DXGI_FORMAT_B8G8R8X8_UNORM, + &IID_IMFMediaEngineEx, (void **)&media_engine); + + if (!(notify->media_engine = media_engine)) + goto done; + + wicrc.Width = 64; + wicrc.Height = 64; + + hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER, + &IID_IWICImagingFactory, (void **)&factory); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IWICImagingFactory_CreateBitmap(factory, wicrc.Width, wicrc.Height, &GUID_WICPixelFormat32bppBGR, + WICBitmapCacheOnLoad, &bitmap); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + url = SysAllocString(L"i420-64x64.avi"); + hr = IMFMediaEngineEx_SetSourceFromByteStream(media_engine, stream, url); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + SysFreeString(url); + IMFByteStream_Release(stream); + + res = WaitForSingleObject(notify->frame_ready_event, 5000); + ok(!res, "Unexpected res %#lx.\n", res); + + if (FAILED(notify->error)) + { + win_skip("Media engine reported error %#lx, skipping tests.\n", notify->error); + goto done; + } + + /* FIXME: Wine first video frame is often full of garbage, wait for another update */ + res = WaitForSingleObject(notify->ready_event, 500); + /* It's also missing the MF_MEDIA_ENGINE_EVENT_TIMEUPDATE notifications */ + todo_wine + ok(!res, "Unexpected res %#lx.\n", res); + + SetRect(&dst_rect, 0, 0, wicrc.Width, wicrc.Height); + IMFMediaEngineEx_OnVideoStreamTick(notify->media_engine, &pts); + hr = IMFMediaEngineEx_TransferVideoFrame(notify->media_engine, (IUnknown *)bitmap, NULL, &dst_rect, NULL); + todo_wine + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IWICBitmap_Lock(bitmap, &wicrc, WICBitmapLockRead, &lock); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IWICBitmapLock_GetStride(lock, &lock_buffer_stride); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IWICBitmapLock_GetDataPointer(lock, &lock_buffer_size, &lock_buffer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(!!lock_buffer, "got null lock_buffer\n"); + ok(lock_buffer_size == 16384, "got lock_buffer_size %u\n", lock_buffer_size); + ok(lock_buffer_stride == wicrc.Width * 4, "got lock_buffer_stride %u\n", lock_buffer_stride); + res = check_rgb32_data(L"rgb32frame.bmp", lock_buffer, lock_buffer_stride * wicrc.Height, &dst_rect); + todo_wine + ok(res == 0, "Unexpected %lu%% diff\n", res); + + IWICBitmapLock_Release(lock); + +done: + if (media_engine) + { + IMFMediaEngineEx_Shutdown(media_engine); + IMFMediaEngineEx_Release(media_engine); + } + + if (bitmap) + IWICBitmap_Release(bitmap); + if (factory) + IWICImagingFactory_Release(factory); + + IMFMediaEngineNotify_Release(¬ify->IMFMediaEngineNotify_iface); +} + struct test_transform { IMFTransform IMFTransform_iface; @@ -2713,6 +2825,7 @@ START_TEST(mfmediaengine) test_SetSourceFromByteStream(); test_audio_configuration(); test_TransferVideoFrame(); + test_TransferVideoFrame_wic(); test_effect(); test_GetDuration(); test_GetSeekable(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9125
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- dlls/mfmediaengine/Makefile.in | 2 +- dlls/mfmediaengine/main.c | 121 +++++++++++++++++++++++ dlls/mfmediaengine/tests/mfmediaengine.c | 2 - 3 files changed, 122 insertions(+), 3 deletions(-) diff --git a/dlls/mfmediaengine/Makefile.in b/dlls/mfmediaengine/Makefile.in index 5b593814ef6..c5dc42c9e35 100644 --- a/dlls/mfmediaengine/Makefile.in +++ b/dlls/mfmediaengine/Makefile.in @@ -1,5 +1,5 @@ MODULE = mfmediaengine.dll -IMPORTS = oleaut32 ole32 mfplat mf mfuuid dxguid uuid +IMPORTS = oleaut32 ole32 mfplat mf mfuuid dxguid uuid windowscodecs EXTRADLLFLAGS = -Wb,--prefer-native diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c index aab5fd64aa2..eb738d2d0e1 100644 --- a/dlls/mfmediaengine/main.c +++ b/dlls/mfmediaengine/main.c @@ -29,6 +29,7 @@ #include "mferror.h" #include "dxgi.h" #include "d3d11.h" +#include "wincodec.h" #include "mmdeviceapi.h" #include "audiosessiontypes.h" @@ -2707,12 +2708,126 @@ done: return hr; } +static HRESULT media_engine_transfer_wic(struct media_engine *engine, IWICBitmap *bitmap, + const MFVideoNormalizedRect *src_mf_rect, const RECT *dst_rect, const MFARGB *color) +{ + UINT frame_width, frame_height, dst_width, dst_height, dst_size, src_stride, dst_stride, format_size; + RECT src_rect = {0}, dst_rect_default = {0}; + DWORD max_length, current_length; + IMFMediaBuffer *media_buffer; + WICPixelFormatGUID format; + IWICBitmapLock *lock; + IMFSample *sample; + WICRect wic_rect; + BYTE *dst, *src; + HRESULT hr; + + frame_width = engine->video_frame.size.cx; + frame_height = engine->video_frame.size.cy; + + if (src_mf_rect) + { + src_rect.left = src_mf_rect->left * frame_width + 0.5f; + src_rect.top = src_mf_rect->top * frame_height + 0.5f; + src_rect.right = src_mf_rect->right * frame_width + 0.5f; + src_rect.bottom = src_mf_rect->bottom * frame_height + 0.5f; + } + else + { + src_rect.right = frame_width; + src_rect.bottom = frame_height; + } + + if (FAILED(hr = IWICBitmap_GetPixelFormat(bitmap, &format)) + || FAILED(hr = IWICBitmap_GetSize(bitmap, &dst_width, &dst_height))) + return hr; + + if (!dst_rect) + { + dst_rect = &dst_rect_default; + dst_rect_default.right = dst_width; + dst_rect_default.bottom = dst_height; + } + + if (!video_frame_sink_get_sample(engine->presentation.frame_sink, &sample)) + return MF_E_UNEXPECTED; + hr = IMFSample_ConvertToContiguousBuffer(sample, &media_buffer); + IMFSample_Release(sample); + if (FAILED(hr)) + return hr; + + if (dst_rect->left + src_rect.right - src_rect.left > dst_width + || dst_rect->top + src_rect.bottom - src_rect.top > dst_height) + { + hr = MF_E_UNEXPECTED; + goto done; + } + if (dst_rect->right - dst_rect->left != src_rect.right - src_rect.left + || dst_rect->bottom - dst_rect->top != src_rect.bottom - src_rect.top) + { + FIXME("Scaling/letterboxing is not implemented.\n"); + goto done; + } + + if (!IsEqualGUID(&format, &GUID_WICPixelFormat32bppBGR) && !IsEqualGUID(&format, &GUID_WICPixelFormat32bppBGRA)) + { + FIXME("Unsupported format %s.\n", wine_dbgstr_guid(&format)); + goto done; + } + if (engine->video_frame.output_format != DXGI_FORMAT_B8G8R8A8_UNORM + && engine->video_frame.output_format != DXGI_FORMAT_B8G8R8X8_UNORM) + { + FIXME("Unsupported format %#x.\n", engine->video_frame.output_format); + goto done; + } + if (engine->video_frame.output_format == DXGI_FORMAT_B8G8R8A8_UNORM + && IsEqualGUID(&format, &GUID_WICPixelFormat32bppBGR)) + { + WARN("Dropping alpha channel.\n"); + } + format_size = 4; + + wic_rect.X = dst_rect->left; + wic_rect.Y = dst_rect->top; + wic_rect.Width = dst_rect->right - dst_rect->left; + wic_rect.Height = dst_rect->bottom - dst_rect->top; + if (FAILED(hr = IWICBitmap_Lock(bitmap, &wic_rect, WICBitmapLockWrite, &lock))) + goto done; + + if (FAILED(hr = IWICBitmapLock_GetStride(lock, &dst_stride)) + || FAILED(hr = IWICBitmapLock_GetDataPointer(lock, &dst_size, &dst))) + goto done_unlock_dst; + + if (FAILED(hr = IMFMediaBuffer_Lock(media_buffer, &src, &max_length, ¤t_length))) + goto done_unlock_dst; + + if (current_length < frame_width * frame_height * format_size) + { + WARN("Unexpected source length %lu.\n", current_length); + hr = MF_E_UNEXPECTED; + goto done_unlock_dst; + } + + src_stride = frame_width * format_size; + src += src_rect.top * src_stride + src_rect.left * format_size; + MFCopyImage(dst, dst_stride, src, src_stride, dst_stride, wic_rect.Height); + + IMFMediaBuffer_Unlock(media_buffer); + +done_unlock_dst: + IWICBitmapLock_Release(lock); +done: + IMFMediaBuffer_Release(media_buffer); + return hr; +} + static HRESULT WINAPI media_engine_TransferVideoFrame(IMFMediaEngineEx *iface, IUnknown *surface, const MFVideoNormalizedRect *src_rect, const RECT *dst_rect, const MFARGB *color) { struct media_engine *engine = impl_from_IMFMediaEngineEx(iface); ID3D11Texture2D *texture; HRESULT hr = E_NOINTERFACE; + IWICBitmap *bitmap; TRACE("%p, %p, %s, %s, %p.\n", iface, surface, src_rect ? wine_dbg_sprintf("(%f,%f)-(%f,%f)", src_rect->left, src_rect->top, src_rect->right, src_rect->bottom) : "(null)", @@ -2726,6 +2841,12 @@ static HRESULT WINAPI media_engine_TransferVideoFrame(IMFMediaEngineEx *iface, I hr = media_engine_transfer_to_d3d11_texture(engine, texture, src_rect, dst_rect, color); ID3D11Texture2D_Release(texture); } + /* Windows does not allow transfer to IWICBitmap if a device manager was set. */ + else if (!engine->device_manager && SUCCEEDED(IUnknown_QueryInterface(surface, &IID_IWICBitmap, (void **)&bitmap))) + { + hr = media_engine_transfer_wic(engine, bitmap, src_rect, dst_rect, color); + IWICBitmap_Release(bitmap); + } else { FIXME("Unsupported destination type.\n"); diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c index 715d44a8ef7..79b0edf8da3 100644 --- a/dlls/mfmediaengine/tests/mfmediaengine.c +++ b/dlls/mfmediaengine/tests/mfmediaengine.c @@ -1467,7 +1467,6 @@ static void test_TransferVideoFrame_wic(void) SetRect(&dst_rect, 0, 0, wicrc.Width, wicrc.Height); IMFMediaEngineEx_OnVideoStreamTick(notify->media_engine, &pts); hr = IMFMediaEngineEx_TransferVideoFrame(notify->media_engine, (IUnknown *)bitmap, NULL, &dst_rect, NULL); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IWICBitmap_Lock(bitmap, &wicrc, WICBitmapLockRead, &lock); @@ -1480,7 +1479,6 @@ static void test_TransferVideoFrame_wic(void) ok(lock_buffer_size == 16384, "got lock_buffer_size %u\n", lock_buffer_size); ok(lock_buffer_stride == wicrc.Width * 4, "got lock_buffer_stride %u\n", lock_buffer_stride); res = check_rgb32_data(L"rgb32frame.bmp", lock_buffer, lock_buffer_stride * wicrc.Height, &dst_rect); - todo_wine ok(res == 0, "Unexpected %lu%% diff\n", res); IWICBitmapLock_Release(lock); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9125
On Thu Oct 9 13:36:22 2025 +0000, Nikolay Sivov wrote:
Why does this depend on device_manager? Or what you mean is "device_manager != NULL" is a fixme case? It replicates Windows behaviour. The test added at the end of `test_TransferVideoFrame()` would fail without this check. I added a comment.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9125#note_118242
On Thu Oct 9 13:36:22 2025 +0000, Nikolay Sivov wrote:
So does this silently drop alpha channel if it's doing DXGI_FORMAT_B8G8R8A8_UNORM -> WICPixelFormat32bppBGR? Yes, I added a warning.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9125#note_118243
On Thu Oct 9 13:36:22 2025 +0000, Nikolay Sivov wrote:
Those shouldn't really fail, unless you see this error handling with user-defined IWICBitmap. If we are going to keep failure checks, we probably should use GetSize() result too? Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9125#note_118244
On Fri Oct 10 06:32:33 2025 +0000, Conor McCarthy wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/9125/diffs?diff_id=215783&start_sha=c03d6917cc7ccc8346b86aa91e479b04dcf4eb90#238bb11c24dbacb840d4252cb92aa8ea2d26e240_2813_2812) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9125#note_118245
Nikolay Sivov (@nsivov) commented about dlls/mfmediaengine/tests/Makefile.in:
TESTDLL = mfmediaengine.dll -IMPORTS = ole32 mf mfplat oleaut32 mfuuid uuid +IMPORTS = ole32 mf mfplat oleaut32 mfuuid uuid windowscodecs dxguid
Linking to windowscodecs is unnecessary both here and in the implementation. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9125#note_119335
Nikolay Sivov (@nsivov) commented about dlls/mfmediaengine/main.c:
+ WARN("Unexpected source length %lu.\n", current_length); + hr = MF_E_UNEXPECTED; + goto done_unlock_dst; + } + + src_stride = frame_width * format_size; + src += src_rect.top * src_stride + src_rect.left * format_size; + MFCopyImage(dst, dst_stride, src, src_stride, dst_stride, wic_rect.Height); + + IMFMediaBuffer_Unlock(media_buffer); + +done_unlock_dst: + IWICBitmapLock_Release(lock); +done: + IMFMediaBuffer_Release(media_buffer); + return hr; Could we replace this with a single goto, checking for null 'lock' instead?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9125#note_119336
Otherwise looks fine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9125#note_119337
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Nikolay Sivov (＠nsivov)