Fix delayed playback info updates in the audio navigator

Sources/Navigator/Audiobook/AudioNavigator.swift

@@ -147,6 +147,7 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo
 
     isolated deinit {
         playTask?.cancel()
+        durationLoadTask?.cancel()
         notificationTask?.cancel()
         if let token = audioSessionToken {
             audioSession.end(with: token)
@@ -171,18 +172,26 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo
     /// Index of the current resource in the reading order.
     private var resourceIndex: Int = 0
 
+    /// Exact duration reported by the current player item's asset, once
+    /// loaded.
+    private var loadedAssetDuration: Double?
+
+    /// Task loading `loadedAssetDuration`, which can be awaited to get the
+    /// exact duration of the current item.
+    private var durationLoadTask: Task<Double?, Never>?
+
+    /// A newly loaded asset duration is reported to the delegate only when it
+    /// differs from the previous one by more than this many seconds.
+    private static let assetDurationTolerance: Double = 0.1
+
     /// Starting time of the current resource, in the reading order.
     private var resourceStartingTime: Double? {
         durations[..<resourceIndex].reduce(0, +)
     }
 
     /// Duration in seconds in the current resource.
     private var resourceDuration: Double? {
-        if let duration = player.currentItem?.duration, duration.isNumeric {
-            return duration.secondsOrZero
-        } else {
-            return publication.readingOrder[resourceIndex].duration
-        }
+        loadedAssetDuration ?? publication.readingOrder[resourceIndex].duration
     }
 
     /// Total duration in the publication.
@@ -271,10 +280,10 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo
                 preferredTimescale: 1000
             ),
             queue: .main
-        ) { [weak self] time in
+        ) { [weak self] _ in
             MainActor.assumeIsolated {
                 guard let self = self else { return }
-                self.playbackDidChange(time.secondsOrZero)
+                self.locationDidChange()
             }
         }
         timeObserverToken = TimeObserverToken(player: player, observer: periodicObserver)
@@ -319,14 +328,9 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo
                     continue
                 }
 
-                self.shouldPlayNextResource { playNext in
-                    if playNext {
-                        Task { @MainActor [weak self] in
-                            guard let self = self else { return }
-                            if await self.goForward() {
-                                self.play()
-                            }
-                        }
+                if self.shouldPlayNextResource() {
+                    if await self.goForward() {
+                        self.play()
                     }
                 }
             }
@@ -335,53 +339,27 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo
         return player
     }
 
-    private func shouldPlayNextResource(completion: @escaping @MainActor @Sendable (Bool) -> Void) {
+    private func shouldPlayNextResource() -> Bool {
         guard let delegate = delegate else {
-            completion(true)
-            return
+            return true
         }
 
-        makePlaybackInfo { info in
-            completion(delegate.navigator(self, shouldPlayNextResource: info))
-        }
+        return delegate.navigator(self, shouldPlayNextResource: playbackInfo)
     }
 
-    private func playbackDidChange(_ time: Double? = nil) {
-        if let time = time {
-            let locator = makeLocator(forTime: time)
-            currentLocation = locator
-            delegate?.navigator(self, locationDidChange: locator)
-        }
-
-        makePlaybackInfo(forTime: time) { [weak self] info in
-            guard let self = self else { return }
-            self.delegate?.navigator(self, playbackDidChange: info)
-        }
+    /// Notifies the delegate that the player state changed.
+    private func playbackDidChange() {
+        delegate?.navigator(self, playbackDidChange: playbackInfo)
     }
 
-    private func makePlaybackInfo(forTime time: Double? = nil, completion: @escaping @MainActor @Sendable (MediaPlaybackInfo) -> Void) {
-        let resourceIndex = resourceIndex
-        let state = state
-        let time = time ?? currentTime
-        let defaultDuration = publication.readingOrder[resourceIndex].duration
-        let currentItem = player.currentItem
-
-        Task {
-            // A deadlock can occur when loading HTTP assets and creating the playback info from the main thread.
-            // To fix this, we load the duration asynchronously.
-            var duration = defaultDuration
-            if let currentItem = currentItem, let seconds = try? await currentItem.asset.load(.duration).seconds, seconds.isFinite {
-                duration = seconds
-            }
+    /// Refreshes `currentLocation` and notifies the delegate, after the
+    /// playback position or the resource duration changed.
+    private func locationDidChange() {
+        let locator = makeLocator(forTime: currentTime)
+        currentLocation = locator
+        delegate?.navigator(self, locationDidChange: locator)
 
-            let info = MediaPlaybackInfo(
-                resourceIndex: resourceIndex,
-                state: state,
-                time: time,
-                duration: duration
-            )
-            completion(info)
-        }
+        playbackDidChange()
     }
 
     private func makeLocator(forTime time: Double) -> Locator {
@@ -409,6 +387,45 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo
         )
     }
 
+    /// Loads in the background the exact duration reported by the current
+    /// asset, caching it in `loadedAssetDuration`.
+    ///
+    /// Loading it eagerly keeps `resourceDuration` synchronous, as awaiting the
+    /// asset can take several seconds over HTTP.
+    private func loadAssetDuration() {
+        loadedAssetDuration = nil
+        durationLoadTask?.cancel()
+        durationLoadTask = nil
+
+        guard let currentItem = player.currentItem else {
+            return
+        }
+
+        durationLoadTask = Task { [weak self] in
+            let seconds = try? await currentItem.asset.load(.duration).seconds
+
+            // The item we loaded the duration for must still be the one being
+            // played, otherwise `go(to:)` moved on while we were loading.
+            guard
+                !Task.isCancelled,
+                let self = self,
+                currentItem == self.player.currentItem,
+                let seconds = seconds, seconds.isFinite
+            else {
+                return nil
+            }
+
+            let previousDuration = resourceDuration
+            loadedAssetDuration = seconds
+
+            if previousDuration.map({ abs(seconds - $0) > Self.assetDurationTolerance }) ?? true {
+                locationDidChange()
+            }
+
+            return seconds
+        }
+    }
+
     // MARK: - Loaded Time Ranges
 
     private var lastLoadedTimeRanges: [Range<Double>] = []
@@ -458,12 +475,31 @@ public final class AudioNavigator: Navigator, Configurable, AudioSessionUser, Lo
                 let asset = try mediaLoader.makeAsset(for: link)
                 player.replaceCurrentItem(with: AVPlayerItem(asset: asset))
                 resourceIndex = newResourceIndex
+                loadAssetDuration()
                 loadedTimeRangesTimer.fire()
                 delegate?.navigator(self, loadedTimeRangesDidChange: [])
             }
 
             // Seeks to time
-            let time = locator.locations.time?.begin ?? ((resourceDuration ?? 0) * (locator.locations.progression ?? 0))
+            let time: Double
+            if let begin = locator.locations.time?.begin {
+                time = begin
+            } else if let progression = locator.locations.progression, progression > 0 {
+                // `Link.duration` is only a hint, so converting a progression
+                // into a time requires the exact duration reported by the
+                // asset. We wait for it if it is not loaded yet.
+                var assetDuration = loadedAssetDuration
+                if assetDuration == nil, let durationLoadTask = durationLoadTask {
+                    assetDuration = await durationLoadTask.value
+                }
+                if assetDuration == nil {
+                    log(.warning, "Asset duration unavailable, falling back on the approximate `Link.duration` to convert the progression")
+                }
+
+                time = (assetDuration ?? link.duration ?? 0) * progression
+            } else {
+                time = 0
+            }
 
             let finished = await player.seek(to: CMTime(seconds: time, preferredTimescale: 1000))
             if finished {