Fix delayed playback info updates in the audio navigator - #848
Merged
Conversation
Contributor
Author
|
I reverted the changes for PublicationOpener for now. If we use |
stevenzeck
marked this pull request as ready for review
July 31, 2026 19:25
There was a problem hiding this comment.
Pull request overview
This PR adjusts AudioNavigator’s makePlaybackInfo to avoid delaying the completion handler while waiting for an exact media duration (notably improving responsiveness for HTTP streaming), and updates the GitHub Actions workflow so CI runs when a PR is moved from draft to “Ready for review”.
Changes:
- Return
MediaPlaybackInfoimmediately using the best available duration, then (optionally) refresh with the exact duration once loaded. - Introduce a duration cache reset when switching to a new
AVPlayerItem. - Trigger
Checksworkflow onpull_requestready_for_reviewevents.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Sources/Navigator/Audiobook/AudioNavigator.swift | Avoids blocking the initial playback-info callback on exact duration load; adds duration caching/reset behavior. |
| .github/workflows/checks.yml | Ensures PR checks run when a draft PR is marked ready for review. |
Suppressed comments (2)
Sources/Navigator/Audiobook/AudioNavigator.swift:384
- The async duration load can start multiple times and the comparison
seconds != bestAvailableDurationdoes not type-check becausebestAvailableDurationis aDouble?. Also, if you introduce an in-flight task (recommended), ensure it’s set/cleared so repeated calls don’t launch redundant loads.
if loadDurationAsync, let currentItem = currentItem, exactDurationCache == nil {
Task {
if let seconds = try? await currentItem.asset.load(.duration).seconds, seconds.isFinite {
guard resourceIndex == self.resourceIndex, currentItem == self.player.currentItem else {
return
Sources/Navigator/Audiobook/AudioNavigator.swift:478
- If you track an in-flight duration load task, it should be cancelled/cleared when swapping the current player item; otherwise an old load could keep running unnecessarily and may block starting a new one until it finishes (depending on the gating logic).
log(.info, "Starts playing \(link.href)")
let asset = try mediaLoader.makeAsset(for: link)
player.replaceCurrentItem(with: AVPlayerItem(asset: asset))
resourceIndex = newResourceIndex
exactDurationCache = nil
loadedTimeRangesTimer.fire()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mickael-menu
force-pushed
the
swift6-fixes
branch
from
August 14, 2026 15:08
4eb215f to
1d9ea8e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm leaving this in draft as more issues might creep up. Here is what is changed and why:
AudioNavigator,makePlaybackInfowas wrapping its completion handler in aTaskthat awaitedcurrentItem.asset.load. Because this fetch suspends the task until the exact duration is returned, the completion handler was delayed. While not an impact for local files, this caused a several second delay for HTTP streaming where the UI would freeze/fail to update.types: [opened, synchronize, reopened, ready_for_review]to the pull_request check so moving from draft to ready for review triggers checks.Relates to #758.