refactor(windows): simplify position and duration processing (#1553)

# Description Use `double` for `position` and `duration` on windows implementation ## Related Issues Closes #1547 Co-authored-by: SNNAFI <[email protected]>
bluefireteam · Jun 27, 2023 · ca63c5a · ca63c5a
1 parent 125ed96
commit ca63c5a
Show file tree

Hide file tree

Showing 10 changed files with 116 additions and 66 deletions.
diff --git a/packages/audioplayers/example/integration_test/lib_test.dart b/packages/audioplayers/example/integration_test/lib_test.dart
@@ -28,18 +28,18 @@ void main() {
     source: UrlSource(wavUrl1),
     duration: const Duration(milliseconds: 451),
   );
+  final mp3Url1TestData = LibSourceTestData(
+    source: UrlSource(mp3Url1),
+    duration: const Duration(minutes: 3, seconds: 30, milliseconds: 77),
+  );
   final audioTestDataList = [
     if (features.hasUrlSource) wavUrl1TestData,
     if (features.hasUrlSource)
       LibSourceTestData(
         source: UrlSource(wavUrl2),
         duration: const Duration(seconds: 1, milliseconds: 068),
       ),
-    if (features.hasUrlSource)
-      LibSourceTestData(
-        source: UrlSource(mp3Url1),
-        duration: const Duration(minutes: 3, seconds: 30, milliseconds: 77),
-      ),
+    if (features.hasUrlSource) mp3Url1TestData,
     if (features.hasUrlSource)
       LibSourceTestData(
         source: UrlSource(mp3Url2),
@@ -393,6 +393,56 @@ void main() {
       }
     });
 
+    testWidgets('#seek with millisecond precision', (tester) async {
+      final platform = AudioplayersPlatformInterface.instance;
+
+      final playerId = 'somePlayerId${isLinux ? linuxPlayerCount++ : ''}';
+      await platform.create(playerId);
+
+      final preparedCompleter = Completer<void>();
+      final eventStream = platform.getEventStream(playerId);
+      final onPreparedSub = eventStream
+          .where((event) => event.eventType == AudioEventType.prepared)
+          .map((event) => event.isPrepared!)
+          .listen(
+        (isPrepared) {
+          if (isPrepared) {
+            preparedCompleter.complete();
+          }
+        },
+        onError: preparedCompleter.completeError,
+      );
+      if (isLinux) {
+        // FIXME(gustl22): Linux needs additional pump (#1507)
+        await tester.pump();
+      }
+      await platform.setSourceUrl(
+        playerId,
+        (mp3Url1TestData.source as UrlSource).url,
+      );
+      await preparedCompleter.future.timeout(const Duration(seconds: 30));
+
+      final seekCompleter = Completer<void>();
+      final onSeekSub = eventStream
+          .where((event) => event.eventType == AudioEventType.seekComplete)
+          .listen(
+        (_) {
+          seekCompleter.complete();
+        },
+        onError: seekCompleter.completeError,
+      );
+      await platform.seek(playerId, const Duration(milliseconds: 21));
+      await seekCompleter.future.timeout(const Duration(seconds: 30));
+      await onSeekSub.cancel();
+      expect(await platform.getCurrentPosition(playerId), 21);
+
+      await onPreparedSub.cancel();
+      if (!isLinux) {
+        // FIXME(gustl22): Linux not disposing properly (#1507)
+        await platform.dispose(playerId);
+      }
+    });
+
     testWidgets('Set same source twice (#1520)', (tester) async {
       final platform = AudioplayersPlatformInterface.instance;
 

diff --git a/packages/audioplayers/example/windows/runner/Runner.rc b/packages/audioplayers/example/windows/runner/Runner.rc
@@ -60,14 +60,14 @@ IDI_APP_ICON            ICON                    "resources\\app_icon.ico"
 // Version
 //
 
-#ifdef FLUTTER_BUILD_NUMBER
-#define VERSION_AS_NUMBER FLUTTER_BUILD_NUMBER
+#if defined(FLUTTER_VERSION_MAJOR) && defined(FLUTTER_VERSION_MINOR) && defined(FLUTTER_VERSION_PATCH) && defined(FLUTTER_VERSION_BUILD)
+#define VERSION_AS_NUMBER FLUTTER_VERSION_MAJOR,FLUTTER_VERSION_MINOR,FLUTTER_VERSION_PATCH,FLUTTER_VERSION_BUILD
 #else
-#define VERSION_AS_NUMBER 1,0,0
+#define VERSION_AS_NUMBER 1,0,0,0
 #endif
 
-#ifdef FLUTTER_BUILD_NAME
-#define VERSION_AS_STRING #FLUTTER_BUILD_NAME
+#if defined(FLUTTER_VERSION)
+#define VERSION_AS_STRING FLUTTER_VERSION
 #else
 #define VERSION_AS_STRING "1.0.0"
 #endif

diff --git a/packages/audioplayers_windows/windows/CMakeLists.txt b/packages/audioplayers_windows/windows/CMakeLists.txt
@@ -33,6 +33,7 @@ add_library(${PLUGIN_NAME} SHARED
   "audioplayers_windows_plugin.cpp"
   "audio_player.h"
   "audio_player.cpp"
+  "audioplayers_helpers.h"
   "event_stream_handler.h"
   "MediaEngineExtension.h"
   "MediaEngineExtension.cpp"

diff --git a/packages/audioplayers_windows/windows/MediaEngineWrapper.cpp b/packages/audioplayers_windows/windows/MediaEngineWrapper.cpp
@@ -18,6 +18,7 @@
 
 #include "MediaEngineWrapper.h"
 #include "MediaFoundationHelpers.h"
+#include "audioplayers_helpers.h"
 #include <Audioclient.h>
 
 using namespace Microsoft::WRL;
@@ -143,15 +144,14 @@ void MediaEngineWrapper::Shutdown()
     });
 }
 
-void MediaEngineWrapper::StartPlayingFrom(uint64_t timeStamp)
+void MediaEngineWrapper::StartPlayingFrom(double timestampInSeconds)
 {
     RunSyncInMTA([&]()
     {
         auto lock = m_lock.lock();
         if (m_mediaEngine == nullptr) {
             return;
         }
-        const double timestampInSeconds = ConvertHnsToSeconds(timeStamp);
         THROW_IF_FAILED(m_mediaEngine->SetCurrentTime(timestampInSeconds));
         THROW_IF_FAILED(m_mediaEngine->Play());
     });
@@ -233,52 +233,52 @@ bool MediaEngineWrapper::GetLooping()
     return looping;
 }
 
-void MediaEngineWrapper::SeekTo(uint64_t timeStamp)
+void MediaEngineWrapper::SeekTo(double timestampInSeconds)
 {
     RunSyncInMTA([&]()
     {
         auto lock = m_lock.lock();
         if (m_mediaEngine == nullptr) {
             return;
         }
-        const double timestampInSeconds = ConvertHnsToSeconds(timeStamp);
         THROW_IF_FAILED(m_mediaEngine->SetCurrentTime(timestampInSeconds));
     });
 }
 
-uint64_t MediaEngineWrapper::GetMediaTime()
+// Get media time in seconds
+double MediaEngineWrapper::GetMediaTime()
 {
-    uint64_t currentTimeInHns = 0;
+    double currentTimeInSeconds = 0;
     RunSyncInMTA([&]()
     {
         auto lock = m_lock.lock();
         if (m_mediaEngine == nullptr) {
             return;
         }
-        double currentTimeInSeconds = m_mediaEngine->GetCurrentTime();
-        currentTimeInHns = ConvertSecondsToHns(currentTimeInSeconds);
+        currentTimeInSeconds = m_mediaEngine->GetCurrentTime();
     });
-    return currentTimeInHns;
+    return currentTimeInSeconds;
 }
 
-uint64_t MediaEngineWrapper::GetDuration()
+// Get duration in seconds
+double MediaEngineWrapper::GetDuration()
 {
-    uint64_t durationInHns = 0;
+    double durationInSeconds = 0;
     RunSyncInMTA([&]()
     {
         auto lock = m_lock.lock();
         if (m_mediaEngine == nullptr) {
             return;
         }
-        double durationInSeconds = m_mediaEngine->GetDuration();
-        durationInHns = ConvertSecondsToHns(durationInSeconds);
+        durationInSeconds = m_mediaEngine->GetDuration();
     });
-    return durationInHns;
+    return durationInSeconds;
 }
 
-std::vector<std::tuple<uint64_t, uint64_t>> MediaEngineWrapper::GetBufferedRanges()
+// Get buffered ranges in milliseconds
+std::vector<std::tuple<int64_t, int64_t>> MediaEngineWrapper::GetBufferedRanges()
 {
-    std::vector<std::tuple<uint64_t, uint64_t>> result;
+    std::vector<std::tuple<int64_t, int64_t>> result;
     RunSyncInMTA([&]()
     {
         auto lock = m_lock.lock();
@@ -296,7 +296,7 @@ std::vector<std::tuple<uint64_t, uint64_t>> MediaEngineWrapper::GetBufferedRange
         {
             mediaTimeRange->GetStart(i, &start);
             mediaTimeRange->GetEnd(i, &end);
-            result.push_back(std::make_tuple(ConvertSecondsToHns(start), ConvertSecondsToHns(end)));
+            result.push_back(std::make_tuple(ConvertSecondsToMs(start), ConvertSecondsToMs(end)));
         }
     });
     return result;

diff --git a/packages/audioplayers_windows/windows/MediaEngineWrapper.h b/packages/audioplayers_windows/windows/MediaEngineWrapper.h
@@ -43,21 +43,21 @@ class MediaEngineWrapper : public winrt::implements<MediaEngineWrapper, IUnknown
     void Shutdown();
 
     // Control various aspects of playback
-    void StartPlayingFrom(uint64_t timeStamp);
+    void StartPlayingFrom(double timestampInSeconds);
     void Resume();
     void SetPlaybackRate(double playbackRate);
     void SetVolume(float volume);
     void SetBalance(double balance);
     void SetLooping(bool isLooping);
-    void SeekTo(uint64_t timeStamp);
+    void SeekTo(double timeStamp);
 
     // Query the current playback position
-    uint64_t GetMediaTime();
-    uint64_t GetDuration();
+    double GetMediaTime();
+    double GetDuration();
 
     bool GetLooping();
 
-    std::vector<std::tuple<uint64_t, uint64_t>> GetBufferedRanges();
+    std::vector<std::tuple<int64_t, int64_t>> GetBufferedRanges();
 
   private:
     wil::critical_section m_lock;
@@ -80,4 +80,4 @@ class MediaEngineWrapper : public winrt::implements<MediaEngineWrapper, IUnknown
     void OnSeekCompleted();
 };
 
-} // namespace media
+} // namespace media
diff --git a/packages/audioplayers_windows/windows/MediaFoundationHelpers.h b/packages/audioplayers_windows/windows/MediaFoundationHelpers.h
@@ -143,21 +143,4 @@ inline void RunSyncInMTA(std::function<void()> callback)
     }
 }
 
-constexpr uint64_t c_hnsPerSecond = 10000000;
-
-template<typename SecondsT>
-inline uint64_t ConvertSecondsToHns(SecondsT seconds)
-{
-    if (isinf(seconds))
-        return 0;
-    return static_cast<uint64_t>(seconds * c_hnsPerSecond);
-}
-
-template<typename HnsT>
-inline double ConvertHnsToSeconds(HnsT hns)
-{
-    return static_cast<double>(hns) / c_hnsPerSecond;
-}
-
-
-} // namespace media
+} // namespace media
diff --git a/packages/audioplayers_windows/windows/audio_player.cpp b/packages/audioplayers_windows/windows/audio_player.cpp
@@ -9,6 +9,8 @@
 #include <shobjidl.h>
 #include <windows.h>
 
+#include "audioplayers_helpers.h"
+
 #undef GetCurrentTime
 
 using namespace winrt;
@@ -145,8 +147,7 @@ void AudioPlayer::OnTimeUpdate() {
                   flutter::EncodableValue("audio.onCurrentPosition")},
                  {flutter::EncodableValue("value"),
                   flutter::EncodableValue(
-                      (int64_t)m_mediaEngineWrapper->GetMediaTime() /
-                      10000)}})));
+                      ConvertSecondsToMs(m_mediaEngineWrapper->GetMediaTime()))}})));
     }
 }
 
@@ -158,8 +159,7 @@ void AudioPlayer::OnDurationUpdate() {
                   flutter::EncodableValue("audio.onDuration")},
                  {flutter::EncodableValue("value"),
                   flutter::EncodableValue(
-                      (int64_t)m_mediaEngineWrapper->GetDuration() /
-                      10000)}})));
+                      ConvertSecondsToMs(m_mediaEngineWrapper->GetDuration()))}})));
     }
 }
 
@@ -236,12 +236,12 @@ void AudioPlayer::Resume() {
     OnDurationUpdate();
 }
 
-int64_t AudioPlayer::GetPosition() {
+double AudioPlayer::GetPosition() {
     return m_mediaEngineWrapper->GetMediaTime();
 }
 
-int64_t AudioPlayer::GetDuration() {
+double AudioPlayer::GetDuration() {
     return m_mediaEngineWrapper->GetDuration();
 }
 
-void AudioPlayer::SeekTo(int64_t seek) { m_mediaEngineWrapper->SeekTo(seek); }
+void AudioPlayer::SeekTo(double seek) { m_mediaEngineWrapper->SeekTo(seek); }
diff --git a/packages/audioplayers_windows/windows/audio_player.h b/packages/audioplayers_windows/windows/audio_player.h
@@ -69,11 +69,11 @@ class AudioPlayer {
 
     bool GetLooping();
 
-    int64_t GetPosition();
+    double GetPosition();
 
-    int64_t GetDuration();
+    double GetDuration();
 
-    void SeekTo(int64_t seek);
+    void SeekTo(double seek);
 
     void SetSourceUrl(std::string url);
 

diff --git a/packages/audioplayers_windows/windows/audioplayers_helpers.h b/packages/audioplayers_windows/windows/audioplayers_helpers.h
@@ -0,0 +1,15 @@
+constexpr int64_t c_msPerSecond = 1000;
+
+template<typename SecondsT>
+inline int64_t ConvertSecondsToMs(SecondsT seconds)
+{
+    if (isinf(seconds))
+        return 0;
+    return static_cast<int64_t>(seconds * c_msPerSecond);
+}
+
+template<typename MsT>
+inline double ConvertMsToSeconds(MsT ms)
+{
+    return static_cast<double>(ms) / c_msPerSecond;
+}
diff --git a/packages/audioplayers_windows/windows/audioplayers_windows_plugin.cpp b/packages/audioplayers_windows/windows/audioplayers_windows_plugin.cpp
@@ -16,6 +16,7 @@
 #include <sstream>
 
 #include "audio_player.h"
+#include "audioplayers_helpers.h"
 
 namespace {
 
@@ -170,9 +171,9 @@ void AudioplayersWindowsPlugin::HandleMethodCall(
         player->SeekTo(0);
         result->Success(EncodableValue(1));
     } else if (method_call.method_name().compare("seek") == 0) {
-        auto position = GetArgument<int>("position", args,
-                                         (int)(player->GetPosition() / 10000));
-        player->SeekTo(static_cast<int64_t>(position * 10000.0));
+        auto positionInMs = GetArgument<int>("position", args,
+                                         (int)ConvertSecondsToMs(player->GetPosition()));
+        player->SeekTo(ConvertMsToSeconds(positionInMs));
         result->Success(EncodableValue(1));
     } else if (method_call.method_name().compare("setSourceUrl") == 0) {
         auto url = GetArgument<std::string>("url", args, std::string());
@@ -185,13 +186,13 @@ void AudioplayersWindowsPlugin::HandleMethodCall(
         std::thread(&AudioPlayer::SetSourceUrl, player, url).detach();
         result->Success(EncodableValue(1));
     } else if (method_call.method_name().compare("getDuration") == 0) {
-        result->Success(EncodableValue(player->GetDuration() / 10000));
+        result->Success(EncodableValue(ConvertSecondsToMs(player->GetDuration())));
     } else if (method_call.method_name().compare("setVolume") == 0) {
         auto volume = GetArgument<double>("volume", args, 1.0);
         player->SetVolume(volume);
         result->Success(EncodableValue(1));
     } else if (method_call.method_name().compare("getCurrentPosition") == 0) {
-        result->Success(EncodableValue(player->GetPosition() / 10000));
+        result->Success(EncodableValue(ConvertSecondsToMs(player->GetPosition())));
     } else if (method_call.method_name().compare("setPlaybackRate") == 0) {
         auto playbackRate = GetArgument<double>("playbackRate", args, 1.0);
         player->SetPlaybackSpeed(playbackRate);