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

[Zhiyi Zhang (＠zhiyi)]

--
  v4: mfmediaengine: Use a mftime_to_seconds() helper to convert time.
      mfmediaengine/tests: Test IMFMediaEngine::GetDuration().

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

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

[Zhiyi Zhang]

From: Zhiyi Zhang

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

diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index aa9de414fcb..18c86e3af43 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,40 @@ done:
     IMFMediaEngineNotify_Release(&notify->IMFMediaEngineNotify_iface);
 }

+static void test_GetDuration(void)
+{
+    static const double allowed_error = 0.000001;
+    struct test_transfer_notify *notify;
+    IMFMediaEngineEx *media_engine;
+    IMFByteStream *stream;
+    double duration;
+    HRESULT hr;
+    DWORD res;
+    BSTR url;
+
+    notify = create_transfer_notify();
+    media_engine = create_media_engine_ex(&notify->IMFMediaEngineNotify_iface, NULL, 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(&notify->IMFMediaEngineNotify_iface);
+}
+
 START_TEST(mfmediaengine)
 {
     HRESULT hr;
@@ -1957,6 +1997,7 @@ START_TEST(mfmediaengine)
     test_audio_configuration();
     test_TransferVideoFrame();
     test_effect();
+    test_GetDuration();

IMFMediaEngineClassFactory_Release(factory);

[PATCH v4 2/2] mfmediaengine: Use a mftime_to_seconds() helper to convert time.

[Zhiyi Zhang]

From: Zhiyi Zhang

---
 dlls/mfmediaengine/main.c                | 13 ++++++++-----
 dlls/mfmediaengine/tests/mfmediaengine.c |  1 -
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c
index e664db81d1d..7b0db3d229a 100644
--- a/dlls/mfmediaengine/main.c
+++ b/dlls/mfmediaengine/main.c
@@ -63,6 +63,12 @@ static BOOL mf_array_reserve(void **elements, size_t *capacity, size_t count, si
     return TRUE;
 }

+/* Convert 100ns to seconds */
+static double mftime_to_seconds(MFTIME time)
+{
+    return (double)time / 10000000.0;
+}
+
 enum media_engine_mode
 {
     MEDIA_ENGINE_INVALID,
@@ -1248,10 +1254,7 @@ static HRESULT media_engine_create_topology(struct media_engine *engine, IMFMedi

/* Assume live source if duration was not provided. */
     if (SUCCEEDED(IMFPresentationDescriptor_GetUINT64(pd, &MF_PD_DURATION, &duration)))
-    {
-        /* Convert 100ns to seconds. */
-        engine->duration = duration / 10000000;
-    }
+        engine->duration = mftime_to_seconds(duration);
     else
         engine->duration = INFINITY;

@@ -1749,7 +1752,7 @@ static double WINAPI media_engine_GetCurrentTime(IMFMediaEngineEx *iface)
     }
     else if (SUCCEEDED(IMFPresentationClock_GetTime(engine->clock, &clocktime)))
     {
-        ret = (double)clocktime / 10000000.0;
+        ret = mftime_to_seconds(clocktime);
     }
     LeaveCriticalSection(&engine->cs);

diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index 18c86e3af43..30c428ab9ac 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -1955,7 +1955,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);

[PATCH v4 2/2] mfmediaengine: Use a mftime_to_seconds() helper to convert time.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=135764

Your paranoid android.

=== w1064_tsign (64 bit report) ===

mfmediaengine:
mfmediaengine.c:1912: Test failed: Unexpected ref 4.
mfmediaengine.c:1917: Test failed: Unexpected ref 4.
mfmediaengine.c:1923: Test failed: Unexpected ref 1.

=== debian11b (64 bit WoW report) ===

Report validation errors:
xinput1_3:xinput crashed (c0000005)

[Nikolay Sivov (＠nsivov)]

We'll see if missing IMFMediaEngineEx is a problem, I don't see such skips on a test page.

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

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.