Repository navigation
Feat: Player - #23
Draft
AlexAxthelm wants to merge 33 commits into
Draft
Feat: Player#23AlexAxthelm wants to merge 33 commits into
AlexAxthelm wants to merge 33 commits into
Conversation
Core (Rust): - new Player capability; playback policy in shared/src/player.rs - stream undownloaded episodes while downloading, then swap to the local file at the playhead; fall back to streaming if the file is gone - persist position every ~10s and on pause/seek/background/end - played once within 15s of the end; 3s rewind on resume - skip 30/15 hard-coded in defaults.rs until Settings exists - restore the active episode on cold start (v2_play_context table) Shell (Swift): - PlaybackManager (AVPlayer, audio session, interruptions, route changes) - NowPlayingController (lock-screen card and remote commands) - MiniPlayerBar and full-screen PlayerScreen - wire the episode row and detail Play buttons - enable the audio background mode No auto-advance yet; that arrives with playlists. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- fold the play_context table into the v1_initial baseline instead of a v2 migration, per the schema baseline policy - restore the saved play context in Started (a metadata-only read), and leave the download resume on activation - flush playback position from Core.appEnteredBackground - combine UIBackgroundModes into [audio, fetch] - use the renamed NoticeBanner in the player views
A save in the last 15s (backgrounding, the 10s checkpoint) marked the episode played and reset its position while leaving the player active and the context saved. A relaunch then restored an already-played episode at 0:00, and listening on with the screen locked could be cut short. - played is decided only on natural end, an explicit pause in the tail, or starting another episode from the tail; checkpoints just record in-progress and a position - seeking to the very end finishes the episode (the engine only reports the end while playing) - don't restore an episode that is already played; clear its context - add Interrupted for system pauses (calls, unplugged headphones): keep the place and never finish, so playback can resume to the end - tag every load with a session id and ignore engine events from any other session, replacing the episode-id check - tail tolerance is capped at a quarter of the episode; an unknown duration never finishes by pausing Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Playback now remembers what it was started from as an EpisodeSource (the
internal abstraction from DATA_MODEL.md). Only Subscription exists today,
so playlists can later be added as a variant without a schema or event
change.
- add the EpisodeSource enum; ActivePlayback, the saved play context and
the player view carry it instead of a subscription id
- play_context stores source_kind + source_id (in the v1_initial
baseline); SavePlayContext takes the source and LoadPlayContext returns
a PlayContext { episode, source } result
- the "From:" row names and navigates to the source; the episode's own
feed stays separate (the lock-screen artist)
- rename PlayerSource (stream or local file) to MediaSource, and the
engine's Load field to media, to avoid confusion with the new type
- document EpisodeSource and PlayContext storage in DATA_MODEL.md and
player.md
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Replace the "Show notes coming soon" placeholder with the real show notes, rendered as rich text with the same renderer as the episode detail page, in a scrollable page. - PlayerView carries the episode description (raw HTML) plus a plain-text fallback for the frame before the HTML is parsed; a blank description counts as none - the page and its dots are left out when the episode has no show notes, and the selection snaps back to the artwork if the next episode has none - update player.md, player-show-notes.md and the roadmap, which described the page as an MVP placeholder; timestamp detection is still unbuilt Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- ArtworkView takes size: nil to fill the offered width as the largest square that fits; the player's artwork page uses it. Fixed-size callers are unchanged - play/pause and the skip buttons in the player, and the mini-player's play/pause, use the theme accent; the secondary row (hide, output routing, options) stays in the text color - note both, and the possibility of a configurable layout, in player.md Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t play The core is also created when iOS wakes the app in the background to refresh feeds, and that launch should stay metadata-only. PlaybackManager and NowPlayingController were built in Core.init, so every such launch registered remote-command handlers it never needed. - create both in startPlaybackIfNeeded(), on the first player operation, and publish the current Now Playing state straight away - a background-refresh launch still restores the saved episode for the mini-player (a DB read) but touches neither audio nor the lock screen - a restored, paused episode now appears on the lock screen only once it has been played - add wiring tests: restoring does not create the engine; the first play request does Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ment - PLAN.md: mark round 2 items 1-4 done with what was built, keep 5 (lock screen) and 6 (swap gap) open, and record both open questions' answers; the original plan text is unchanged - capabilities/player.rs: the Load doc still said `source` after the rename to `media`
Add a user story and notes on the gap and repeated second when playback moves onto a freshly downloaded file, the options for fixing it (continue from the engine's exact time; cue just ahead and hand over via AVQueuePlayer; swap at a pause or seek), and the caveats (the engine conversion, judging it by ear on a device, and dynamic ad insertion).
iOS reports "should resume" after any interruption, including one that
found playback already paused, so a call arriving while the listener had
paused would restart audio when it ended.
- the core records whether an interruption is what paused playback and
resumes only then; Interrupted gains `resumable` and a new
InterruptionEnded { should_resume } event replaces the shell's
unconditional .play
- unplugging headphones is never resumable, so a stale flag can't be
triggered by a later, unrelated call
- playing or scrubbing by hand during an interruption cancels the
auto-resume
- document the rule in player.md
A failed load (unparseable URL, missing file, audio session error) returned before touching the AVPlayer, so the previous episode kept playing while the core showed the new one paused with an error; the old item's ticks were dropped as stale and the controls acted on the wrong episode. - PlaybackManager.load unloads the current item and clears the session on any failure, leaving the audio session active so a fallback load doesn't flicker other apps' audio - add engine tests with a generated silent WAV: a failed load leaves nothing loaded, a good load still works after one Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A paused load (a source swap finishing after the listener paused, or a scrub of a restored episode) activated the non-mixable playback session, cutting off whatever the listener had switched to. - PlaybackManager claims the audio session only for a load that will autoplay, and in the Play operation (which also covers resuming after a paused load or an interruption) - a refused session fails an autoplaying load (and unloads) or a play (keeping the item), instead of throwing it away - make the activation injectable, with the real one as the default, and test when the session is and isn't claimed Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Every error on a downloaded episode was treated as a missing file, so a transient failure (another app holding the audio session, say) reset the download to not-downloaded and fetched the same file again, orphaning the good copy on disk. - add PlayerResult::MediaUnusable and Event::PlayerMediaUnusable; the shell reports a missing, unparseable or undecodable item with them and everything else (a refused audio session, a failure mid-play) as before - the core falls back to streaming and re-downloads only for an unusable local file; any other failure pauses with the error and keeps the file, so playing again retries it - a failed load unloads the current item whichever way it failed - document the rule in player.md Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
NowPlayingController rebuilt the whole card on every render: each tick, download progress update and refresh. Rewriting it resets the system's clock for the card, so its scrubber jittered, and it was needless work. - add NowPlayingPolicy: rewrite when the title, feed, artwork, duration, play state or skip intervals change, or when the playhead leaves the card's own clock by more than 2s (a seek, a skip, a stall); ordinary playback, paused time and error text never rewrite it - set the skip intervals only when they change, and clear the card only when it has content - when artwork arrives, republish from the latest state instead of patching the card in place with a stale elapsed time, which set the card's clock back - test the policy, including 600 ticks of steady playback Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Everything that watched Core was re-evaluated whenever it published, so the once-a-second position tick made SwiftUI re-evaluate the whole episode list behind the player (about 5.4% CPU with a 375-episode list open, against about 1.0% with ticks not rendering). - keep the player in its own PlayerState object; Core.view no longer carries it (view.player is always nil) - republish Core.view only when something other than the player changed - the mini-player and full player observe PlayerState through small host views, and the root view subscribes to it for dismissal instead of observing it, so a tick wakes only the views that show the player - test that ticks republish the player but not the list view, that a real list change still does, and that identical players aren't republished Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Record what was measured (payload, Rust and Swift cost, app CPU with the episode list open), what the player/list split changed, and what is not yet known: all numbers are from debug simulator builds. List the follow-ups in order (profile a release build on a device; shell-side clock for the position; compute previews once and ship raw HTML on demand) and how to re-measure.
Both recipes pipe xcodebuild into xcbeautify, so the exit status was the formatter's and a failed build or test still exited 0. - set -o pipefail in both recipes - run recipes under bash, since make 3.81 (macOS) has no .SHELLFLAGS to set it globally and not every sh supports pipefail - verified: passing suite exits 0; a broken assertion and a compile error each exit 2 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PlayerFormatting.spoken allocated a DateComponentsFormatter on every call, and the scrubber's accessibility value asks for it on every tick and every drag step. Hold one in a static let, as EpisodeFormatting does. Measured saving is modest (about 39 us to 35 us per call in a debug build; the formatting itself dominates). - test each unit appears when present, that the result is never empty, and that repeated calls on the shared formatter are stable Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The Application Support directory was looked up inline in DownloadManager.init and DatabaseManager.init, with a third copy as a static on DownloadManager for the player. They must agree, or playback would read a different root than downloads and the database. - add StorageRoot.applicationSupport() and use it in all three; each caller keeps its own failure behavior - remove DownloadManager.defaultStorageRoot(); a neutral helper avoids the database depending on the download manager - test that it is the Application Support directory and that stored relative paths resolve under it Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Post real interruption and route-change notifications at the PlaybackManager and check the core events it sends: interruption began is resumable, ended carries shouldResume, unplugging headphones is never resumable, other route changes and foreign senders are ignored. Also check that engine news carries the session of the load that produced it and that a load replaced before it is ready reports nothing for the old session. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
enqueue_download only searched the episodes of the feed on screen, which is empty after a cold start. A restored episode whose file went missing fell back to streaming but never re-downloaded, and resuming one whose download had failed never retried it. enqueue_download now also finds the active episode, set_download_state keeps the active episode's download status in step, and play() starts or retries the download when the episode is streaming with no file coming. A download already queued or underway is left alone. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The play context a relaunch restores was written once, when playback started, and its result was ignored. A failed write left the next launch restoring the wrong episode or none, with nothing to retry it (position writes recover at the next checkpoint; this did not). The save now resolves through its own event, and the active playback records whether storage confirmed it. While it is unconfirmed every checkpoint re-sends it. Answers about an episode that has since been replaced are ignored, and an episode restored from the saved context counts as already saved. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Append a round 3 section covering the ten-point branch review and the cross-check: what was fixed, what is partly done (render cost needs a device; shell notification tests), and what stays open (lock-screen controls, the swap gap, device-only checks). Correct the round 2 header to say item 6 is deferred rather than needing a device. Record two decisions in player.md that the plan points to: downloads for the active episode do not depend on its feed being open, and a failed play-context save is retried at every checkpoint. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Two cases found in review: - An interruption pauses playback and arms an automatic resume for its end. A listener's pause, or headphones being unplugged, during that interruption returned early without disarming it, so the call ending restarted audio the listener had stopped (or sent it to the speaker). Both now disarm it. - The player keeps its own copy of its episode, so a feed refresh that renewed or corrected the audio URL never reached it and retrying a failed stream loaded the old address. A reloaded episode list now updates the active episode's title, description, artwork, duration, date and stream URL, leaving position, status, download state and the loaded engine item alone. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A downloaded file the engine reports unusable was replaced by a fresh download, which was then swapped in on completion. If the fresh file was unusable too (an error page served with a 200, a corrupt or undecodable file), this repeated for as long as playback went on: stream, download, swap, fail, stream, with the same large file fetched each time. Playback now remembers that a local file was found unusable. The first time it is replaced once, as before; if the replacement is unusable too the file itself is bad, so its download is marked failed (with the reason beside Retry), playback carries on from the stream, and pressing play no longer fetches it again. A local file that loads clears the memory. The failed-download shape is now a shared helper used by the download error handler as well. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Playback downloads what it streams, but it did so unconditionally: after the listener cancelled an episode's download, resuming it, starting it again, or recovering from a lost file queued the download once more, and a file they had just deleted came back through the missing-file fallback. The core now remembers, for the session, which episodes' downloads were cancelled or deleted. Every download playback starts on its own (starting an episode, retrying on play, replacing a lost file) goes through one helper that skips those; only an explicit Download request lifts it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The lock-screen scrub converted the system's position straight to UInt32, which traps on NaN or a value above UInt32.max, so a misbehaving car or Bluetooth device could crash the app. The engine's tick and duration reports had the same conversion, guarded only against NaN and infinity. All three now go through PlayerFormatting.wholeSeconds, which refuses non-finite and out-of-range values and clamps negatives to zero. A bad scrub is answered with commandFailed rather than clamped, since clamping a huge value would seek past the end and mark the episode played. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A failed play-context save was retried at every checkpoint (about every 10s of playback, plus every pause and backgrounding) for as long as the episode played, with no limit. A write that can never succeed, such as a constraint failure or a persistent I/O error, would repeat indefinitely. Failed saves are now counted per playback, and checkpoints retry only until CONTEXT_SAVE_ATTEMPTS (5, counting the write made when playback starts). The next episode starts afresh and a success still stops the retries. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PlayerScreen's header called the second page a show-notes placeholder, but it renders the episode's real show notes. NowPlayingController's header said the Now Playing card is rebuilt on every render, but it is rewritten only when out of date (NowPlayingPolicy), which is the point of that policy. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Add seams to PlaybackManager (the loaded item, the pending-seek count, and reportTick taking an explicit playing flag) and a suite that checks what the engine tells the core: ticks are held back while a seek is pending, when not playing, with nothing loaded, and for positions that are not times; the end of the file and failures carry their session and message; a file the engine cannot open is reported unusable. The tests found that unloading an item left its end and failure observers registered, so a removed item could still report under a session the core might consider current. Unloading now removes them. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Create the audio engine and the now-playing controller independently, so a missing engine can't cause the controller (and its remote-command handlers) to be built again on every later request. Replace a vacuous assertion in an unload test with an explicit check that no end event is reported for the unloaded item's session. Bring PLAN.md's review section up to date with the later fixes, the engine tests, and the two limits still open. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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
1 participant
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.
No description provided.