On 9/3/19 4:38 PM, Jactry Zeng wrote:
Signed-off-by: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/mfmediaengine/main.c | 482 ++++++++++++++++++++++- dlls/mfmediaengine/tests/Makefile.in | 2 +- dlls/mfmediaengine/tests/mfmediaengine.c | 152 +++++++ include/mfmediaengine.idl | 51 +++ 4 files changed, 684 insertions(+), 3 deletions(-)
diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c index 4bca0fe0ba..9e33b973d4 100644 --- a/dlls/mfmediaengine/main.c +++ b/dlls/mfmediaengine/main.c @@ -24,8 +24,11 @@ #include "winbase.h"
#include "mfmediaengine.h" +#include "mferror.h" +#include "dxgi.h"
#include "wine/debug.h" +#include "wine/heap.h"
WINE_DEFAULT_DEBUG_CHANNEL(mfplat);
@@ -43,6 +46,432 @@ BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, LPVOID reserved) return TRUE; }
+enum media_engine_mode +{ + MEDIA_ENGINE_INVIAL, + MEDIA_ENGINE_AUDIO_MODE, + MEDIA_ENGINE_RENDERING_MODE, + MEDIA_ENGINE_FRAME_SERVER_MODE, +}; Where does this come from? Is _INVIAL a typo? + +struct media_engine_attributes +{ + IMFMediaEngineNotify *callback; + HWND playback_hwnd; + DXGI_FORMAT output_format; + IMFDXGIDeviceManager *dxgi_manager; + enum media_engine_mode mode; +}; + +struct media_engine +{ + IMFMediaEngine IMFMediaEngine_iface; + LONG refcount; + DWORD flags; + struct media_engine_attributes attributes; +}; +
Do you need these structures separate? You can still have init_media_engine() + heap_alloc_zero() in CreateInstance().
+ +static ULONG WINAPI media_engine_Release(IMFMediaEngine *iface) +{ + struct media_engine *engine = impl_from_IMFMediaEngine(iface); + ULONG refcount = InterlockedDecrement(&engine->refcount); + + TRACE("(%p) ref=%u.\n", iface, refcount); + + if (!refcount) + { + free_media_engine_attributes(engine->attributes); + HeapFree(GetProcessHeap(), 0, engine); + } + + return refcount; +}
heap_free(), but that's not very important.
+ +static double WINAPI media_engine_GetCurrentTime(IMFMediaEngine *iface) +{ + FIXME("(%p): stub.\n", iface); + + return 0; +} This should use proper float constant. + +static BOOL WINAPI media_engine_IsPaused(IMFMediaEngine *iface) +{ + FIXME("(%p): stub.\n", iface); + + return 0; +} 0 -> FALSE. +static BOOL WINAPI media_engine_GetAutoPlay(IMFMediaEngine *iface) +{ + FIXME("(%p): stub.\n", iface); + + return E_NOTIMPL; +} Wrong return value. static HRESULT WINAPI media_engine_factory_QueryInterface(IMFMediaEngineClassFactory *iface, REFIID riid, void **obj) { if (IsEqualIID(riid, &IID_IMFMediaEngineClassFactory) || @@ -68,12 +497,61 @@ static ULONG WINAPI media_engine_factory_Release(IMFMediaEngineClassFactory *ifa return 1; }
+static void init_media_engine_attributes(IMFAttributes *attributes, struct media_engine_attributes *engine_attr) +{ + HRESULT hr; + + memset(engine_attr, 0, sizeof(*engine_attr)); + hr = IMFAttributes_GetUnknown(attributes, &MF_MEDIA_ENGINE_CALLBACK, &IID_IMFMediaEngineNotify, + (void **)&engine_attr->callback); + if (FAILED(hr)) + return; + + IMFAttributes_GetUINT64(attributes, &MF_MEDIA_ENGINE_PLAYBACK_HWND, (UINT64 *)&engine_attr->playback_hwnd); This will be writing 8 bytes to 4 bytes destination, for 32 bit build. + IMFAttributes_GetUnknown(attributes, &MF_MEDIA_ENGINE_DXGI_MANAGER, &IID_IMFDXGIDeviceManager, + (void **)&engine_attr->dxgi_manager); + hr = IMFAttributes_GetUINT32(attributes, &MF_MEDIA_ENGINE_VIDEO_OUTPUT_FORMAT, &engine_attr->output_format); + if (engine_attr->playback_hwnd) /* FIXME: handle MF_MEDIA_ENGINE_PLAYBACK_VISUAL */ + engine_attr->mode = MEDIA_ENGINE_RENDERING_MODE; + else + { + if (SUCCEEDED(hr)) + engine_attr->mode = MEDIA_ENGINE_FRAME_SERVER_MODE; + else + engine_attr->mode = MEDIA_ENGINE_AUDIO_MODE; + } +} + static HRESULT WINAPI media_engine_factory_CreateInstance(IMFMediaEngineClassFactory *iface, DWORD flags, IMFAttributes *attributes, IMFMediaEngine **engine) { - FIXME("(%p, %#x, %p, %p): stub.\n", iface, flags, attributes, engine); + struct media_engine_attributes engine_attr; + struct media_engine *object;
- return E_NOTIMPL; + FIXME("(%p, %#x, %p, %p): semi-stub.\n", iface, flags, attributes, engine); This one could as well be promoted to TRACE().
static HRESULT WINAPI media_engine_factory_CreateTimeRange(IMFMediaEngineClassFactory *iface, diff --git a/dlls/mfmediaengine/tests/Makefile.in b/dlls/mfmediaengine/tests/Makefile.in index 3f2bb5a094..d508db9f6f 100644 --- a/dlls/mfmediaengine/tests/Makefile.in +++ b/dlls/mfmediaengine/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = mfmediaengine.dll -IMPORTS = ole32 mfplat mfmediaengine mfuuid +IMPORTS = ole32 mfplat mfmediaengine mfuuid uuid
C_SRCS = \ mfmediaengine.c diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c index c7fc89f2b1..a264d0d033 100644 --- a/dlls/mfmediaengine/tests/mfmediaengine.c +++ b/dlls/mfmediaengine/tests/mfmediaengine.c @@ -24,11 +24,85 @@ #include "winbase.h"
#include "mfapi.h" +#include "mfidl.h" #include "mfmediaengine.h" +#include "mferror.h" +#include "dxgi.h" +#include "initguid.h"
#include "wine/heap.h" #include "wine/test.h"
+static HRESULT (WINAPI *pMFCreateDXGIDeviceManager)(UINT *token, IMFDXGIDeviceManager **manager); + +#define EXPECT_REF(obj,ref) _expect_ref((IUnknown*)obj, ref, __LINE__) +static void _expect_ref(IUnknown *obj, ULONG ref, int line) +{ + ULONG rc; + IUnknown_AddRef(obj); + rc = IUnknown_Release(obj); + ok_(__FILE__,line)(rc == ref, "Unexpected refcount %d, expected %d.\n", rc, ref); +} + +static void init_functions(void) +{ + HMODULE mod = GetModuleHandleA("mfplat.dll"); + +#define X(f) p##f = (void*)GetProcAddress(mod, #f) + X(MFCreateDXGIDeviceManager); +#undef X +} + +struct media_engine_notify +{ + IMFMediaEngineNotify IMFMediaEngineNotify_iface; + LONG refcount; +}; + +static inline struct media_engine_notify *impl_from_IMFMediaEngineNotify(IMFMediaEngineNotify *iface) +{ + return CONTAINING_RECORD(iface, struct media_engine_notify, IMFMediaEngineNotify_iface); +} + +static HRESULT WINAPI media_engine_notify_QueryInterface(IMFMediaEngineNotify *iface, REFIID riid, void **obj) +{ + if (IsEqualIID(riid, &IID_IMFMediaEngineNotify) || + IsEqualIID(riid, &IID_IUnknown)) + { + *obj = iface; + IMFMediaEngineNotify_AddRef(iface); + return S_OK; + } + + *obj = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI media_engine_notify_AddRef(IMFMediaEngineNotify *iface) +{ + struct media_engine_notify *notify = impl_from_IMFMediaEngineNotify(iface); + return InterlockedIncrement(¬ify->refcount); +} + +static ULONG WINAPI media_engine_notify_Release(IMFMediaEngineNotify *iface) +{ + struct media_engine_notify *notify = impl_from_IMFMediaEngineNotify(iface); + return InterlockedDecrement(¬ify->refcount); +} + +static HRESULT WINAPI media_engine_notify_EventNotify(IMFMediaEngineNotify *iface, DWORD event, DWORD_PTR param1, DWORD param2) +{ + return S_OK; +} + +static IMFMediaEngineNotifyVtbl media_engine_notify_vtbl = +{ + media_engine_notify_QueryInterface, + media_engine_notify_AddRef, + media_engine_notify_Release, + media_engine_notify_EventNotify, +}; + static void test_factory(void) { IMFMediaEngineClassFactory *factory, *factory2; @@ -47,7 +121,83 @@ static void test_factory(void) "Unexpected hr %#x.\n", hr);
if (factory) + { Let's return early on !(factory) and indent everything that follows back one step. + struct media_engine_notify notify_impl = {{&media_engine_notify_vtbl}, 1}; + IMFMediaEngineNotify *notify = ¬ify_impl.IMFMediaEngineNotify_iface; + IMFMediaEngine *media_engine, *media_engine2; + IMFDXGIDeviceManager *manager; + IMFAttributes *attributes; + UINT token; + + hr = pMFCreateDXGIDeviceManager(&token, &manager); + ok(hr == S_OK, "MFCreateDXGIDeviceManager failed: %#x.\n", hr); + hr = MFCreateAttributes(&attributes, 3); + ok(hr == S_OK, "MFCreateAttributes failed: %#x.\n", hr); + + if (0) + { + /* Crashed on Windows 8 */ + hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE, + NULL, &media_engine); + ok(hr == E_POINTER, "IMFMediaEngineClassFactory_CreateInstance got %#x.\n", hr); + + hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE, + NULL, NULL); + ok(hr == E_POINTER, "IMFMediaEngineClassFactory_CreateInstance got %#x.\n", hr); + } I think you can remove this crashing test. + + hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE, + attributes, &media_engine); + ok(hr == MF_E_ATTRIBUTENOTFOUND, "IMFMediaEngineClassFactory_CreateInstance got %#x.\n", hr); + + hr = IMFAttributes_SetUnknown(attributes, &MF_MEDIA_ENGINE_OPM_HWND, NULL); + ok(hr == S_OK, "IMFAttributes_SetUnknown failed: %#x.\n", hr); + hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE, + attributes, &media_engine); + ok(hr == MF_E_ATTRIBUTENOTFOUND, "IMFMediaEngineClassFactory_CreateInstance got %#x.\n", hr); + + hr = IMFAttributes_SetUnknown(attributes, &MF_MEDIA_ENGINE_CALLBACK, (IUnknown *)notify); + ok(hr == S_OK, "IMFAttributes_SetUnknown failed: %#x.\n", hr); + hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE, + attributes, &media_engine); + ok(hr == S_OK || broken(hr == MF_E_ATTRIBUTENOTFOUND) /* pre win10v1809 */, + "IMFMediaEngineClassFactory_CreateInstance got %#x.\n", hr); + if (SUCCEEDED(hr)) + IMFMediaEngine_Release(media_engine); Let's remove this one too and test only with format attribute. + + EXPECT_REF(factory, 1); + hr = IMFAttributes_SetUINT32(attributes, &MF_MEDIA_ENGINE_VIDEO_OUTPUT_FORMAT, DXGI_FORMAT_UNKNOWN); + ok(hr == S_OK, "IMFAttributes_SetUINT32 failed: %#x.\n", hr); + hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE, + attributes, &media_engine); + ok(hr == S_OK, "IMFMediaEngineClassFactory_CreateInstance failed: %#x.\n", hr); + if (0) + { + /* Different version of Windows has different reference count behaviour, and I didn't + see their logics. So skipping these tests on Windows. */ + EXPECT_REF(media_engine, 1); + EXPECT_REF(factory, 1); + } If it's inconsistent there is no point in having such dead statements. + + hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE, + attributes, &media_engine2); + ok(hr == S_OK, "IMFMediaEngineClassFactory_CreateInstance failed: %#x.\n", hr); + if (0) + { + EXPECT_REF(media_engine, 1); + EXPECT_REF(media_engine2, 1); + EXPECT_REF(factory, 1); + } Same here. + + IMFMediaEngine_Release(media_engine); + IMFMediaEngine_Release(media_engine2); + IMFAttributes_DeleteAllItems(attributes); + IMFAttributes_Release(attributes); You don't have to delete items by hand.