[PATCH 0/2] MR3510: Cast duration to double to avoid precision loss.

[Zhiyi Zhang (＠zhiyi)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3510

[PATCH 1/2] mfmediaengine/tests: Test IMFMediaEngine::GetDuration().

[Zhiyi Zhang]

From: Zhiyi Zhang <zzhang(a)codeweavers.com>

---
 dlls/mfmediaengine/tests/mfmediaengine.c | 58 ++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index aa9de414fcb..a0d355c1ec1 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -17,6 +17,7 @@
  */
 
 #include <stdarg.h>
+#include <math.h>
 
 #define COBJMACROS
 
@@ -66,6 +67,11 @@ static void check_interface_(unsigned int line, void *iface_ptr, REFIID iid, BOO
         IUnknown_Release(unk);
 }
 
+static BOOL compare_double(double a, double b, double allowed_error)
+{
+    return fabs(a - b) <= allowed_error;
+}
+
 static DWORD compare_rgb32(const BYTE *data, DWORD *length, const RECT *rect, const BYTE *expect)
 {
     DWORD x, y, size, diff = 0, width = (rect->right + 0xf) & ~0xf, height = (rect->bottom + 0xf) & ~0xf;
@@ -1925,6 +1931,57 @@ done:
     IMFMediaEngineNotify_Release(¬ify->IMFMediaEngineNotify_iface);
 }
 
+static void test_GetDuration(void)
+{
+    static const double allowed_error = 0.000001;
+    struct test_transfer_notify *notify;
+    IMFMediaEngineEx *media_engine;
+    IMFDXGIDeviceManager *manager;
+    IMFByteStream *stream;
+    ID3D11Device *device;
+    double duration;
+    HRESULT hr;
+    UINT token;
+    DWORD res;
+    BSTR url;
+
+    if (!(device = create_d3d11_device()))
+    {
+        skip("Failed to create a D3D11 device, skipping tests.\n");
+        return;
+    }
+
+    hr = pMFCreateDXGIDeviceManager(&token, &manager);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IMFDXGIDeviceManager_ResetDevice(manager, (IUnknown *)device, token);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    notify = create_transfer_notify();
+    media_engine = create_media_engine_ex(¬ify->IMFMediaEngineNotify_iface, manager,
+            DXGI_FORMAT_B8G8R8X8_UNORM);
+    notify->media_engine = media_engine;
+    ok(!!media_engine, "create_media_engine_ex failed.\n");
+
+    stream = load_resource(L"i420-64x64.avi", L"video/avi");
+    url = SysAllocString(L"i420-64x64.avi");
+    hr = IMFMediaEngineEx_SetSourceFromByteStream(media_engine, stream, url);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    res = WaitForSingleObject(notify->ready_event, 5000);
+    ok(!res, "Unexpected res %#lx.\n", res);
+
+    duration = IMFMediaEngineEx_GetDuration(media_engine);
+    todo_wine
+    ok(compare_double(duration, 0.133467, allowed_error), "Got unexpected duration %lf.\n", duration);
+
+    SysFreeString(url);
+    IMFByteStream_Release(stream);
+    IMFMediaEngineEx_Shutdown(media_engine);
+    IMFMediaEngineEx_Release(media_engine);
+    IMFMediaEngineNotify_Release(¬ify->IMFMediaEngineNotify_iface);
+    IMFDXGIDeviceManager_Release(manager);
+    ID3D11Device_Release(device);
+}
+
 START_TEST(mfmediaengine)
 {
     HRESULT hr;
@@ -1957,6 +2014,7 @@ START_TEST(mfmediaengine)
     test_audio_configuration();
     test_TransferVideoFrame();
     test_effect();
+    test_GetDuration();
 
     IMFMediaEngineClassFactory_Release(factory);
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3510

[PATCH 2/2] mfmediaengine: Cast duration to double to avoid precision loss.

[Zhiyi Zhang]

From: Zhiyi Zhang <zzhang(a)codeweavers.com>

---
 dlls/mfmediaengine/main.c                | 2 +-
 dlls/mfmediaengine/tests/mfmediaengine.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c
index e664db81d1d..9f8a9cc8354 100644
--- a/dlls/mfmediaengine/main.c
+++ b/dlls/mfmediaengine/main.c
@@ -1250,7 +1250,7 @@ static HRESULT media_engine_create_topology(struct media_engine *engine, IMFMedi
     if (SUCCEEDED(IMFPresentationDescriptor_GetUINT64(pd, &MF_PD_DURATION, &duration)))
     {
         /* Convert 100ns to seconds. */
-        engine->duration = duration / 10000000;
+        engine->duration = (double)duration / 10000000;
     }
     else
         engine->duration = INFINITY;
diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index a0d355c1ec1..a403d1127a8 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -1970,7 +1970,6 @@ static void test_GetDuration(void)
     ok(!res, "Unexpected res %#lx.\n", res);
 
     duration = IMFMediaEngineEx_GetDuration(media_engine);
-    todo_wine
     ok(compare_double(duration, 0.133467, allowed_error), "Got unexpected duration %lf.\n", duration);
 
     SysFreeString(url);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3510

[Jeffrey Smith (＠whydoubt)]

Jeffrey Smith (@whydoubt) commented about dlls/mfmediaengine/main.c:
> if (SUCCEEDED(IMFPresentationDescriptor_GetUINT64(pd, &MF_PD_DURATION, &duration)))
>      {
>          /* Convert 100ns to seconds. */
> -        engine->duration = duration / 10000000;
> +        engine->duration = (double)duration / 10000000;

Not sure if the lead devs have a preference on this one, but `= duration / 10000000.0` compiles to the same thing.

I also prefer `1e7` over `10000000.0` because that many 0's is hard to read at a glance.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3510#note_41341

[Bernhard Kölbl]

Bernhard Kölbl (@besentv) commented about dlls/mfmediaengine/tests/mfmediaengine.c:
> +    media_engine = create_media_engine_ex(¬ify->IMFMediaEngineNotify_iface, manager,
> +            DXGI_FORMAT_B8G8R8X8_UNORM);
> +    notify->media_engine = media_engine;
> +    ok(!!media_engine, "create_media_engine_ex failed.\n");
> +
> +    stream = load_resource(L"i420-64x64.avi", L"video/avi");
> +    url = SysAllocString(L"i420-64x64.avi");
> +    hr = IMFMediaEngineEx_SetSourceFromByteStream(media_engine, stream, url);
> +    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> +    res = WaitForSingleObject(notify->ready_event, 5000);
> +    ok(!res, "Unexpected res %#lx.\n", res);
> +
> +    duration = IMFMediaEngineEx_GetDuration(media_engine);
> +    todo_wine
> +    ok(compare_double(duration, 0.133467, allowed_error), "Got unexpected duration %lf.\n", duration);
> +
> This test duplicates a bunch of code, so not sure, but maybe we could integrate this in the TransferVideoFrames test?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3510#note_41347

[Bernhard Kölbl]

On Fri Aug  4 12:35:04 2023 +0000, Jeffrey Smith wrote:
> Not sure if the lead devs have a preference on this one, but `= duration
> / 10000000.0` compiles to the same thing.
> I also prefer `1e7` over `10000000.0` because that many 0's is hard to
> read at a glance.
> I'd prefer this as well.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3510#note_41348