Add skippable segments (Skip Intro/Recap/Preview/Credits) + Up Next integration - #90
Merged
Merged
Conversation
…ntegration Adds support for Jellyfin's Media Segments feature: a floating "Skip Intro"/"Skip Recap"/"Skip Preview"/"Skip Commercial"/"Skip Credits" button appears while the playhead is inside one of an item's tagged segments and jumps to the segment's end when tapped. For episodic content, the item's end-credits segment (the last `.outro` segment chronologically, since Jellyfin has no separate "opening credits" vs. "closing credits" segment type) integrates with the existing "Up Next" feature instead of showing its own Skip Credits button, whenever Up Next is enabled: the Up Next card appears the instant that segment starts, with a countdown fixed to 10 seconds (capped to however much real duration remains, if less), overriding the normal duration-relative trigger and the user's configured countdown-length preference for that item. Also fixes two related bugs found during testing: - Tapping Skip used to reveal the main transport chrome (via the seek's own `.seeking`/`.buffering` state change) before immediately hiding it again once the seek landed; now the button's own slot swaps to a small spinner instead, and the skipped segment's button hides immediately on tap rather than waiting for `currentTime` to catch up to the seek target. - Scrubbing straight past where the end-credits countdown's trigger point would already have elapsed used to compute an instantly clamped-to-0 `remaining` and silently auto-advance to the next episode with no countdown UI ever shown; the countdown is now timed from an anchor that's reset by every explicit seek, so a scrub landing anywhere inside the segment always gets a fresh countdown from wherever it lands. Along the way, fixed `nextUpSecondsRemaining` rounding a fractional remaining time up rather than truncating it, which read one second higher than the scrubber's own truncating "time remaining" label. DionysusPlayer/Core/Playback/PlaybackSegment.swift and DionysusPlayer/Features/Player/SkipSegmentOverlay.swift are new; JellyfinAPIClient/JellyfinModels gain the MediaSegments DTOs and endpoint; PlayerViewModel/PlayerView carry the bulk of the new behavior. 16 new tests in PlayerViewModelTests.swift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`endCreditsSegment` was a computed property re-deriving the "last
.outro segment" answer via .filter{}.max{} on every single read.
That's read from `updateNextUpCountdownAnchor()` on every engine
time-update tick (~10x/sec, for the whole session, not just near the
credits), plus once or twice more per render from
nextUpSecondsRemaining/nextUpTotalCountdownSeconds/currentSkipSegment
— up to 4+ redundant passes over `mediaSegments` per render, none of
which could ever produce a different answer, since `mediaSegments` is
set exactly once (in loadMediaSegments(for:)'s fire-and-forget fetch)
and never mutated again afterward.
Reviewed the whole skippable-segments branch for memory/CPU/network
optimization opportunities before opening a PR; this was the one
genuine (if small in absolute terms) redundant-work issue found —
network and memory were already effectively optimal. See the plan
notes for what else was considered and why it wasn't worth changing.
Computes and caches it once, at the same point `mediaSegments` itself
is set, eliminating the redundant passes with no staleness risk (it's
the only point the answer can ever change) and no observable behavior
difference — full test suite (464 tests) unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…segments # Conflicts: # TESTING.md
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.
What
Adds support for Jellyfin's Media Segments feature: a floating "Skip Intro"/"Skip Recap"/"Skip Preview"/"Skip Commercial"/"Skip Credits" button appears while the playhead is inside one of an item's tagged segments and jumps to the segment's end when tapped.
For episodic content, the item's end-credits segment (the last
.outrosegment chronologically — Jellyfin has no separate "opening credits" vs. "closing credits" segment type) integrates with the existing "Up Next" feature instead of showing its own Skip Credits button, whenever Up Next is enabled: the Up Next card appears the instant that segment starts, with a countdown fixed to 10 seconds (capped to however much real duration remains, if less), overriding the normal duration-relative trigger and the user's configured countdown-length preference for that item.New files
DionysusPlayer/Core/Playback/PlaybackSegment.swift— app-facing model for a JellyfinMediaSegmentDtoDionysusPlayer/Features/Player/SkipSegmentOverlay.swift— the floating skip button / buffering spinnerAlso fixes, found during testing
.seeking/.bufferingstate change) before immediately hiding it again once the seek landed. Now the button's own slot swaps to a small spinner instead, and the tapped segment's button hides immediately rather than waiting forcurrentTimeto catch up to the seek target.0remainingand silently auto-advance to the next episode with zero warning. The countdown is now timed from an anchor reset by every explicit seek, so a scrub landing anywhere inside the segment always gets a fresh countdown from wherever it lands (capped to however much real duration remains, per the same rule above).nextUpSecondsRemainingrounded a fractional remaining time up, reading one second higher than the scrubber's own truncating "time remaining" label — fixed to truncate, matching that label.endCreditsSegmentwas recomputed via.filter{}.max{}on every read (including every ~10Hz engine tick, for the whole session) despitemediaSegmentsonly ever being set once — now cached at that single point instead.Testing
16 new tests in
PlayerViewModelTests.swiftcovering segment loading, the end-credits/Up-Next override (including both directions of the scrub-timing fix and the truncation fix), and the skip-button interactions. Full suite (469 tests) passes.Manually verified on a physical device and the Simulator across a couple of episodes with real Intro/Outro segments — see PR #89 for a separate, unrelated freeze found during that testing (an AetherEngine backward-seek bug, not caused by this change).
🤖 Generated with Claude Code