[PATCH] mfmediaengine: Add IMFMediaEngine stub.
Nikolay Sivov
nsivov at codeweavers.com
Wed Sep 4 01:26:40 CDT 2019
On 9/3/19 4:38 PM, Jactry Zeng wrote:
> Signed-off-by: Jactry Zeng <jzeng at 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.
More information about the wine-devel
mailing list