[PATCH] mfmediaengine: Merge media_engine_attributes into media_engine struct.

[Jactry Zeng]

Signed-off-by: Jactry Zeng <jzeng(a)codeweavers.com>
---
 dlls/mfmediaengine/main.c                |  86 ++++++++---------
 dlls/mfmediaengine/tests/mfmediaengine.c | 117 ++++++++---------------
 2 files changed, 79 insertions(+), 124 deletions(-)

diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c
index 9e33b973d4..25ed4dbeae 100644
--- a/dlls/mfmediaengine/main.c
+++ b/dlls/mfmediaengine/main.c
@@ -48,27 +48,22 @@ BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, LPVOID reserved)
 
 enum media_engine_mode
 {
-    MEDIA_ENGINE_INVIAL,
+    MEDIA_ENGINE_INVALID,
     MEDIA_ENGINE_AUDIO_MODE,
     MEDIA_ENGINE_RENDERING_MODE,
     MEDIA_ENGINE_FRAME_SERVER_MODE,
 };
 
-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;
+    IMFMediaEngineNotify *callback;
+    UINT64 playback_hwnd;
+    DXGI_FORMAT output_format;
+    IMFDXGIDeviceManager *dxgi_manager;
+    enum media_engine_mode mode;
 };
 
 static inline struct media_engine *impl_from_IMFMediaEngine(IMFMediaEngine *iface)
@@ -103,12 +98,13 @@ static ULONG WINAPI media_engine_AddRef(IMFMediaEngine *iface)
     return refcount;
 }
 
-static void free_media_engine_attributes(struct media_engine_attributes engine_attr)
+static void free_media_engine(struct media_engine *engine)
 {
-    if (engine_attr.callback)
-        IMFMediaEngineNotify_Release(engine_attr.callback);
-    if (engine_attr.dxgi_manager)
-        IMFDXGIDeviceManager_Release(engine_attr.dxgi_manager);
+    if (engine->callback)
+        IMFMediaEngineNotify_Release(engine->callback);
+    if (engine->dxgi_manager)
+        IMFDXGIDeviceManager_Release(engine->dxgi_manager);
+    heap_free(engine);
 }
 
 static ULONG WINAPI media_engine_Release(IMFMediaEngine *iface)
@@ -119,10 +115,7 @@ static ULONG WINAPI media_engine_Release(IMFMediaEngine *iface)
     TRACE("(%p) ref=%u.\n", iface, refcount);
 
     if (!refcount)
-    {
-        free_media_engine_attributes(engine->attributes);
-        HeapFree(GetProcessHeap(), 0, engine);
-    }
+        free_media_engine(engine);
 
     return refcount;
 }
@@ -222,7 +215,7 @@ static double WINAPI media_engine_GetCurrentTime(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return 0;
+    return 0.0;
 }
 
 static HRESULT WINAPI media_engine_SetCurrentTime(IMFMediaEngine *iface, double time)
@@ -236,28 +229,28 @@ static double WINAPI media_engine_GetStartTime(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return 0;
+    return 0.0;
 }
 
 static double WINAPI media_engine_GetDuration(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return 0;
+    return 0.0;
 }
 
 static BOOL WINAPI media_engine_IsPaused(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return 0;
+    return FALSE;
 }
 
 static double WINAPI media_engine_GetDefaultPlaybackRate(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return 0;
+    return 0.0;
 }
 
 static HRESULT WINAPI media_engine_SetDefaultPlaybackRate(IMFMediaEngine *iface, double rate)
@@ -271,7 +264,7 @@ static double WINAPI media_engine_GetPlaybackRate(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return 0;
+    return 0.0;
 }
 
 static HRESULT WINAPI media_engine_SetPlaybackRate(IMFMediaEngine *iface, double rate)
@@ -299,14 +292,14 @@ static BOOL WINAPI media_engine_IsEnded(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return E_NOTIMPL;
+    return FALSE;
 }
 
 static BOOL WINAPI media_engine_GetAutoPlay(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return E_NOTIMPL;
+    return FALSE;
 }
 
 static HRESULT WINAPI media_engine_SetAutoPlay(IMFMediaEngine *iface, BOOL autoplay)
@@ -362,7 +355,7 @@ static double WINAPI media_engine_GetVolume(IMFMediaEngine *iface)
 {
     FIXME("(%p): stub.\n", iface);
 
-    return 0;
+    return 0.0;
 }
 
 static HRESULT WINAPI media_engine_SetVolume(IMFMediaEngine *iface, double volume)
@@ -497,57 +490,54 @@ 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)
+static void init_media_engine(IMFAttributes *attributes, struct media_engine *engine)
 {
     HRESULT hr;
 
-    memset(engine_attr, 0, sizeof(*engine_attr));
     hr = IMFAttributes_GetUnknown(attributes, &MF_MEDIA_ENGINE_CALLBACK, &IID_IMFMediaEngineNotify,
-                                  (void **)&engine_attr->callback);
+                                  (void **)&engine->callback);
     if (FAILED(hr))
         return;
 
-    IMFAttributes_GetUINT64(attributes, &MF_MEDIA_ENGINE_PLAYBACK_HWND, (UINT64 *)&engine_attr->playback_hwnd);
+    IMFAttributes_GetUINT64(attributes, &MF_MEDIA_ENGINE_PLAYBACK_HWND, &engine->playback_hwnd);
     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;
+                             (void **)&engine->dxgi_manager);
+    hr = IMFAttributes_GetUINT32(attributes, &MF_MEDIA_ENGINE_VIDEO_OUTPUT_FORMAT, &engine->output_format);
+    if (engine->playback_hwnd) /* FIXME: handle MF_MEDIA_ENGINE_PLAYBACK_VISUAL */
+        engine->mode = MEDIA_ENGINE_RENDERING_MODE;
     else
     {
         if (SUCCEEDED(hr))
-            engine_attr->mode = MEDIA_ENGINE_FRAME_SERVER_MODE;
+            engine->mode = MEDIA_ENGINE_FRAME_SERVER_MODE;
         else
-            engine_attr->mode = MEDIA_ENGINE_AUDIO_MODE;
+            engine->mode = MEDIA_ENGINE_AUDIO_MODE;
     }
 }
 
 static HRESULT WINAPI media_engine_factory_CreateInstance(IMFMediaEngineClassFactory *iface, DWORD flags,
                                                           IMFAttributes *attributes, IMFMediaEngine **engine)
 {
-    struct media_engine_attributes engine_attr;
     struct media_engine *object;
 
-    FIXME("(%p, %#x, %p, %p): semi-stub.\n", iface, flags, attributes, engine);
+    TRACE("(%p, %#x, %p, %p).\n", iface, flags, attributes, engine);
 
     if (!attributes || !engine)
         return E_POINTER;
 
-    init_media_engine_attributes(attributes, &engine_attr);
-    if (!engine_attr.mode)
-        return MF_E_ATTRIBUTENOTFOUND;
-
     object = heap_alloc_zero(sizeof(*object));
     if (!object)
-    {
-        free_media_engine_attributes(engine_attr);
         return E_OUTOFMEMORY;
+
+    init_media_engine(attributes, object);
+    if (!object->mode)
+    {
+        free_media_engine(object);
+        return MF_E_ATTRIBUTENOTFOUND;
     }
 
     object->IMFMediaEngine_iface.lpVtbl = &media_engine_vtbl;
     object->refcount = 1;
     object->flags = flags;
-    object->attributes = engine_attr;
 
     *engine = &object->IMFMediaEngine_iface;
 
diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index a264d0d033..2bbc79fb6c 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -105,7 +105,13 @@ static IMFMediaEngineNotifyVtbl media_engine_notify_vtbl =
 
 static void test_factory(void)
 {
+    struct media_engine_notify notify_impl = {{&media_engine_notify_vtbl}, 1};
+    IMFMediaEngineNotify *notify = ¬ify_impl.IMFMediaEngineNotify_iface;
     IMFMediaEngineClassFactory *factory, *factory2;
+    IMFDXGIDeviceManager *manager;
+    IMFMediaEngine *media_engine;
+    IMFAttributes *attributes;
+    UINT token;
     HRESULT hr;
 
     CoInitialize(NULL);
@@ -120,84 +126,43 @@ static void test_factory(void)
     ok(hr == CLASS_E_NOAGGREGATION || broken(hr == REGDB_E_CLASSNOTREG) /* pre-win8 */,
        "Unexpected hr %#x.\n", hr);
 
-    if (factory)
+    if (!factory)
     {
-        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);
-        }
-
-        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);
-
-        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);
-        }
-
-        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);
-        }
-
-        IMFMediaEngine_Release(media_engine);
-        IMFMediaEngine_Release(media_engine2);
-        IMFAttributes_DeleteAllItems(attributes);
-        IMFAttributes_Release(attributes);
-        IMFDXGIDeviceManager_Release(manager);
-        IMFMediaEngineClassFactory_Release(factory);
-    }
-    else
         win_skip("Not IMFMediaEngineClassFactory support.\n");
+        CoUninitialize();
+        return;
+    }
+
+    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);
+
+    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);
+
+    IMFAttributes_DeleteAllItems(attributes);
+    hr = IMFAttributes_SetUnknown(attributes, &MF_MEDIA_ENGINE_CALLBACK, (IUnknown *)notify);
+    ok(hr == S_OK, "IMFAttributes_SetUnknown failed: %#x.\n", hr);
+    hr = IMFAttributes_SetUINT32(attributes, &MF_MEDIA_ENGINE_VIDEO_OUTPUT_FORMAT, DXGI_FORMAT_UNKNOWN);
+    ok(hr == S_OK, "IMFAttributes_SetUINT32 failed: %#x.\n", hr);
+    EXPECT_REF(factory, 1);
+    hr = IMFMediaEngineClassFactory_CreateInstance(factory, MF_MEDIA_ENGINE_WAITFORSTABLE_STATE,
+                                                   attributes, &media_engine);
+    ok(hr == S_OK, "IMFMediaEngineClassFactory_CreateInstance failed: %#x.\n", hr);
+    EXPECT_REF(factory, 1);
+
+    IMFMediaEngine_Release(media_engine);
+    IMFAttributes_Release(attributes);
+    IMFDXGIDeviceManager_Release(manager);
+    IMFMediaEngineClassFactory_Release(factory);
 
     CoUninitialize();
 }
-- 
2.23.0.rc1

[Nikolay Sivov]

On 9/5/19 1:25 PM, Jactry Zeng wrote:
> +    init_media_engine(attributes, object);
> +    if (!object->mode)
> +    {
> +        free_media_engine(object);
> +        return MF_E_ATTRIBUTENOTFOUND;
>       }
> It would be better to have init helper return correct error code. 
> Otherwise looks good.