Add Playlist Wait/Advance Modes with Real-Time Controls#17
Open
Kryxzort wants to merge 25 commits into
Open
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds "wait for video end" timing to timed playlists, persists and restores timed-state (including waiting/advance flags), extends PlayerHelper with IPC query/control and playlist-observer logic, exposes public mpv controls, and updates ViewModel and UI to configure and sync timed vs advance-on-end playback. ChangesVideo-End Wait, Playback Controls & Playlist Sync
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/livepaper/Helpers/PlayerHelper.cs`:
- Around line 606-623: SwitchFromTimedToAdvanceOnEnd uses
_history[_historyIndex] as the current file but when _waitingForVideoEnd is true
the _historyIndex points to the prefetched next item; update the logic in
SwitchFromTimedToAdvanceOnEnd to use the actual playing item (e.g.
_history[_historyIndex - 1]) when _waitingForVideoEnd is true, with proper
bounds checks against _history and _historyIndex, before building rest and
appending files; mirror the same playingHistIdx adjustment approach used in
RestoreTimedPlaylist() so the queue is rebuilt around the real currently playing
path.
In `@src/livepaper/ViewModels/MainWindowViewModel.cs`:
- Around line 391-419: The code incorrectly uses the editor setting
PlaylistShuffle instead of the active session's shuffle flag s.Shuffle when
switching modes and when writing _settings.LastSession; change calls to
PlayerHelper.SwitchFromTimedToAdvanceOnEnd and SwitchFromAdvanceOnEndToTimed to
pass s.Shuffle (not PlaylistShuffle), use s.Shuffle when constructing playPaths
(i.e., only randomize if s.Shuffle is true), and set the LastSession.Shuffle
property to s.Shuffle when updating _settings.LastSession in both branches.
- Around line 493-494: The status model never updates because the viewmodel
never subscribes to PlayerHelper.OnWallpaperChanged; subscribe to
PlayerHelper.OnWallpaperChanged in the MainWindowViewModel constructor and
implement a handler that sets _currentlyPlayingCard to the new
WallpaperCardViewModel (or null) and calls RefreshPlayingStatus(); also
unsubscribe from PlayerHelper.OnWallpaperChanged in the viewmodel's
cleanup/dispose/OnClosed path (the same place handling other event
unsubscriptions around the Dispose/OnClosed logic referenced near the block at
lines ~558-570) so you don't leak handlers.
- Line 331: The current live-sync code (e.g., OnPlaylistShuffleChanged calling
SavePlaylistStateDebounced, ApplyShuffleOrderIfRunning, RefreshPlayingStatus)
unconditionally updates any active LastSession with IsTimedPlaylist/IsPlaylist
true, causing PlayLibrary() sessions to adopt the custom-playlist's s.Paths and
reorder the live player; fix by adding an explicit guard that only
syncs/reorders when the running session is the same custom playlist being
edited. Concretely, in ApplyShuffleOrderIfRunning (and the other live-sync paths
you noted) check that LastSession references the exact playlist being modified
(compare a stable identifier—e.g., Session.PlaylistId or Session.Id—to the
current viewmodel's playlist Id, or add a helper
IsRunningCustomPlaylistSession(LastSession, currentPlaylist) that ensures
session is not the library session), and only mutate s.Paths or reapply shuffle
when that predicate is true; do not treat any IsPlaylist/IsTimedPlaylist flag
alone as sufficient.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f97f93fb-7053-43e7-8c43-9ad7852a885f
📒 Files selected for processing (5)
src/livepaper/Helpers/PlayerHelper.cssrc/livepaper/Models/AppSettings.cssrc/livepaper/Models/LastSession.cssrc/livepaper/ViewModels/MainWindowViewModel.cssrc/livepaper/Views/MainWindow.axaml
Contributor
Author
|
@coderabbitai review Both actionable findings from the previous review have been addressed in commits pushed after the review was run:
The The |
|
Tip For best results, initiate chat on the files or code changes.
[review] |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.claude/rules/session.md (1)
61-63: ⚡ Quick winRemove duplicated guard rule text
This guard behavior is documented twice with the same meaning. Keep a single canonical statement to avoid future drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/rules/session.md around lines 61 - 63, Remove the duplicated guard-rule sentence describing that ApplyEffectivePlaylistSettingsIfRunning and ApplyShuffleOrderIfRunning are guarded by IsRunningCustomPlaylist(s.Paths) so library (Play All) sessions are not mutated; keep a single canonical statement (referencing ApplyEffectivePlaylistSettingsIfRunning, ApplyShuffleOrderIfRunning, and IsRunningCustomPlaylist) and delete the redundant paragraph to avoid future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/player.md:
- Around line 12-14: The fenced code block containing the command "mpvpaper -o
\"<mpv-options>\" '*' /path/to/wallpaper.mp4" is missing a language identifier;
update that fenced block to include the bash language tag (i.e., change the
opening ``` to ```bash) so markdown linters (MD040) accept it and the snippet is
properly highlighted.
In @.claude/rules/sources.md:
- Around line 10-12: The fenced code block containing the user-agent string is
missing a language/info tag and triggers MD040; edit the fenced block (the
triple-backtick block that contains "Mozilla/5.0 (X11; Linux x86_64; rv:130.0)
Gecko/20100101 Firefox/130.0") and add an info string such as text (i.e., change
the opening ``` to ```text) so the block is explicitly tagged for markdownlint.
---
Nitpick comments:
In @.claude/rules/session.md:
- Around line 61-63: Remove the duplicated guard-rule sentence describing that
ApplyEffectivePlaylistSettingsIfRunning and ApplyShuffleOrderIfRunning are
guarded by IsRunningCustomPlaylist(s.Paths) so library (Play All) sessions are
not mutated; keep a single canonical statement (referencing
ApplyEffectivePlaylistSettingsIfRunning, ApplyShuffleOrderIfRunning, and
IsRunningCustomPlaylist) and delete the redundant paragraph to avoid future
drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3389fded-7761-407d-8700-40a8ae6b6df4
📒 Files selected for processing (9)
.claude/rules/distribution.md.claude/rules/library.md.claude/rules/player.md.claude/rules/session.md.claude/rules/sources.md.claude/rules/ui.mdCLAUDE.mdsrc/livepaper/Helpers/PlayerHelper.cssrc/livepaper/ViewModels/MainWindowViewModel.cs
✅ Files skipped from review due to trivial changes (3)
- .claude/rules/library.md
- .claude/rules/distribution.md
- .claude/rules/ui.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/livepaper/ViewModels/MainWindowViewModel.cs
- src/livepaper/Helpers/PlayerHelper.cs
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Playlist settings popup now has a "Wait for video to end before switching" checkbox. When enabled, the timed playlist queries mpv's remaining time via IPC at the switch point, waits that duration, then does a seamless in-process loadfile replace — no process kill, no flash. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
"Wait for video to end before switching" is now a global toggle in Settings → PLAYLIST, directly under "Switch when video ends". The playlist popup shows the effective value (global when override is off, per-playlist when on), matching the pattern of the other rotation controls. Also right-aligns the Thumbnail aspect and Card size dropdowns in the APPEARANCE section so they flush with the Theme dropdown edge. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Video→video: disable loop-file and append next to mpv's playlist so the transition happens inside mpv with no blank gap. Sleep remaining + 2s then restore loop and fire UI update. Eliminates the loop-race that caused the first frames of the looped video to show before switching. Video→scene: sleep remaining - SceneTransitionDelayMs before calling SwitchToFile, so LWE launches with exactly SceneTransitionDelayMs of video still playing — matching normal transition overlap behaviour. Scene→*: switch immediately (no mpv socket to query remaining time from). CancelVideoEndWait now restores loop-file=inf immediately so the current video does not advance into the queued append before TryIpcSwitchToFile takes over. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Status bar now reflects live playback state (title, wallpaper count, interval, shuffle, volume, loop, speed) and updates on any change - Playlist settings (shuffle, interval, wait-for-video-end, advance-on- end) apply instantly to the running session without restarting mpvpaper - Toggling advance-on-end switches modes via IPC (loop-file / loadfile append / playlist-clear) with no mpvpaper restart - Loop toggle sends IPC command in single-video mode without restart - Shuffle toggle reorders the active playlist immediately: sequential picks up from current wallpaper's position in the original order; shuffle randomises the remaining tail - Adding, removing, or reordering playlist items syncs to the running player in real time (100ms debounce); add-only appends to mpv's existing queue to preserve playback order; removes and reorders do a full rebuild from the current position; removing the currently playing wallpaper maps its old index into the new list so the positionally equivalent item plays next instead of restarting from the beginning Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
IPC socket isn't ready when RefreshPlayingStatus runs immediately after ApplyPlaylist/ApplyTimedPlaylist, so IsPlaying returned false and the bar stayed blank. Poll until the socket exists before refreshing. ObservePathAsync never called OnWallpaperChanged, so the status bar never updated when mpv advanced to the next video in video-end mode.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds two new fields to the LastSession model and updates all construction sites to populate them correctly. Fixes the scene-mixed branch to update the running timed playlist via IPC instead of restarting mpvpaper. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Syncs LastSession when playlist/rotation settings change while nothing is playing, so --restore picks up the new interval and advance mode even if the user hasn't started a new session yet. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes references to IsLweRunning, IsScenePath, and SceneTransitionDelayMs that were auto-applied during cherry-pick but belong to PR 10 (scene support). Also fixes RearmAdvanceOnVideoEnd to use the correct DoVideoEndWait signature introduced by 1e96c82, and adds missing using System.Threading.Tasks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When switching from advance-on-end to timed interval mode, query mpv's time-pos via IPC to calculate how much of the current video has already played. If elapsed < interval, start the countdown from the remainder so the first switch fires at the right time. If elapsed >= interval, start fresh with the full interval. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Interval NumericUpDown controls and the AdvanceOnVideoEnd IsEnabled guard were bound directly to the per-playlist properties instead of the Display* proxies, so toggling Override off didn't update the shown values to global. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SwitchFromAdvanceOnEndToTimed: when elapsed time >= interval, advance immediately instead of waiting a full interval. Cancels the stale DoVideoEndWait feel when switching from a long-running advance-on-end session. UpdateTimedSettings: disabling WaitForVideoEnd while DoVideoEndWait is in progress now cancels the wait and advances immediately, rather than leaving the playlist stuck until the video naturally ends. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
RefreshPlayingStatus read s.Paths.Count which is only updated after a 100ms debounce, causing the displayed count to lag or show the wrong number. Switched to PlaylistItems.Count which reflects the live strip. Also added RefreshPlayingStatus to PlaylistItems.CollectionChanged so the status bar updates immediately when items are added or removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When _waitingForVideoEnd is true, _historyIndex points to the prefetched next item. SwitchFromTimedToAdvanceOnEnd was using _history[_historyIndex] as the current path, rebuilding the queue around the wrong item. Apply the same playingHistIdx correction already used in RestoreTimedPlaylist. TogglePause and SignalTimerStop used File.WriteAllText (non-atomic) on timed_state.json. GUI/daemon overlap could truncate the file. Switch to write-to-tmp then File.Move (atomic rename). Also add lock(_lock) to SignalTimerStop to match TogglePause.
ApplyEffectivePlaylistSettingsIfRunning and ApplyShuffleOrderIfRunning only checked IsTimedPlaylist/IsPlaylist, so edits to the custom playlist while PlayLibrary() was running would mutate the wrong active session. Add IsRunningCustomPlaylist() helper that matches session paths against the current PlaylistItems (same pattern as matchesStrip in RefreshLastSessionFromSettingsIfIdle). Guard both methods with it.
Kryxzort
force-pushed
the
pr/playlist-wait-advance
branch
from
0042a74 to
5eeefe9
Compare
… change Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/livepaper/Views/MainWindow.axaml`:
- Around line 506-511: The WaitForVideoEnd checkboxes should be disabled when
their paired AdvanceOnVideoEnd is active; update the CheckBox with Content="Wait
for video to end after interval fires before switching" (bound to
DisplayWaitForVideoEnd) to include the same guard used elsewhere by setting its
IsEnabled to the negation of DisplayAdvanceOnVideoEnd (i.e. match the existing
pattern used on the interval StackPanel such as !DisplayAdvanceOnVideoEnd). Do
the same for the settings PlayList checkbox bound to GlobalWaitForVideoEnd: set
IsEnabled to the negation of GlobalAdvanceOnVideoEnd so the WaitForVideoEnd
controls are visually disabled when the corresponding AdvanceOnVideoEnd is
active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efb70a8b-3453-47f9-8ed4-61fae3cfbd2d
📒 Files selected for processing (7)
.claude/rules/session.md.claude/rules/ui.mdsrc/livepaper/Helpers/PlayerHelper.cssrc/livepaper/Models/AppSettings.cssrc/livepaper/Models/LastSession.cssrc/livepaper/ViewModels/MainWindowViewModel.cssrc/livepaper/Views/MainWindow.axaml
✅ Files skipped from review due to trivial changes (2)
- .claude/rules/session.md
- .claude/rules/ui.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/livepaper/Models/AppSettings.cs
- src/livepaper/Models/LastSession.cs
- src/livepaper/ViewModels/MainWindowViewModel.cs
- src/livepaper/Helpers/PlayerHelper.cs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Kryxzort
added a commit
to Kryxzort/livepaper-pro
that referenced
this pull request
May 22, 2026
session.md: kept VideoScale and AutoMuteOnlyIfMprisActive (PR sunwoo101#10/PR sunwoo101#16); added GlobalWaitForVideoEnd to global rotation settings (PR sunwoo101#17). ui.md: kept GIF hover (PR sunwoo101#13), right-click menus (PR sunwoo101#15), Speed in Playback (PR sunwoo101#10); added Wait for video to end to Playlist settings (PR sunwoo101#17). PlayerHelper: dropped duplicate _observerCts field (ghost from previous merge); added _waitForVideoEnd/_advanceOnVideoEnd/_waitingForVideoEnd/_waitCts (PR sunwoo101#17); fixed ApplyPlaylist to use shuffled paths array and not double-shuffle via mpv flag; added SetLoop and SetPlaylistShuffle IPC helpers (PR sunwoo101#17); resolved duplicate StartPlaylistObserver/StopPlaylistObserver/ObservePathAsync/ApplyOverridesForPath by keeping PR sunwoo101#17 insertions with HEAD's full ApplyOverridesForPath (volume+speed overrides) and OnWallpaperChanged invoke; removed duplicate HEAD methods. MainWindowViewModel: kept _autoMuteOnlyIfMprisActive (PR sunwoo101#16); added _speed=1.0 default (PR sunwoo101#17) to existing declaration; added _playlistSyncCts (PR sunwoo101#17); dropped duplicate _currentlyPlayingCard; adopted PR sunwoo101#17 concise OnWallpaperChanged + RefreshPlayingStatus; kept OnVolumeChanged/OnSpeedChanged with per-card override logic (PR sunwoo101#10); kept LastSession with PR sunwoo101#17 new fields. MainWindow.axaml: kept Appearance section (thumbnail/card/gif settings from PR sunwoo101#12/PR sunwoo101#13) that PR sunwoo101#17 was unaware of.
Kryxzort
added a commit
to Kryxzort/livepaper-pro
that referenced
this pull request
May 22, 2026
… Controls (via merge-fix branch) Merges sunwoo101#17. Implements playlist wait-for-video-end modes with real-time controls, status bar showing live playback state, playlist item sync via IPC, and mode switching without restart. Conflicts resolved in merge-fix/pr-17-playlist-wait-advance.
Kryxzort
added a commit
to Kryxzort/livepaper-pro
that referenced
this pull request
May 22, 2026
library.md: updated heading to include scene; added volume/speed sidecars (PR sunwoo101#10) alongside scene-specific sidecars (PR sunwoo101#18); kept all sections. session.md: merged WE settings — AllowScenes, LWE config (PR sunwoo101#18) alongside ThumbnailAspect/CardSize/LibrarySortIndex (PR sunwoo101#12/PR sunwoo101#13). ui.md: Browse tab kept GIF hover + auto-search + sort + SCENE badge; Library tab kept full ItemsRepeater grid + all features + SCENE/crash badges; Settings kept Video scale + WE scene support + Appearance section. README: added both playerctl and linux-wallpaperengine to optional deps. LibraryItem: added CopiedSceneDir (PR sunwoo101#18) + kept VolumeOverride/SpeedOverride. AppSettings: kept Speed with clamping; kept PlaylistWaitForVideoEnd; dropped duplicate LibrarySortIndex from PR sunwoo101#18. WallpaperResult: kept AddedAt (PR sunwoo101#14). livepaper.csproj: kept ItemsRepeater + AnimatedImage packages. AudioMonitor: kept async IsAnyMprisPlayerActiveAsync + better playerctl args. DownloadHelper: kept smart thumbnail extension detection. LibraryService: merged scene-copy Trash/Restore/Delete + kept volume/speed. WallpaperEngineScraper: merged all, kept AnimatedThumbnailUrl + AddedAt. WallpaperEngineService: kept SortIndex/SupportsSorting. PlayerHelper: merged 29 conflicts — kept wait/advance/playlist observer from PR sunwoo101#17 + added full scene/LWE support from PR sunwoo101#18; removed duplicates. MainWindowViewModel: merged 25 conflicts — kept all playlist wait/advance proxies from PR sunwoo101#17 + added scene-aware methods from PR sunwoo101#18. WallpaperCardViewModel: kept full GIF+volume/speed code + added scene-aware fields and LoadDurationAsync from PR sunwoo101#18. MainWindow.axaml: kept GIF animation, volume/speed sliders; added SCENE badge and crash warning panel (adjusting grid row offsets). MainWindow.axaml.cs: kept HEAD's existing TextBox guard structure.
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.
Summary
DisplayInterval*,DisplayAdvanceOnVideoEnd,DisplayWaitForVideoEnd): playlist popup shows global values when Override is off, per-playlist values when on — spinners and toggles always reflect effective settingsAdvanceOnVideoEndandWaitForVideoEndsaved toLastSessionandTimedStateso--restoreand daemon hand-off replay the correct modetime-posIPC query — if elapsed < interval, countdown starts from the remainder; if elapsed ≥ interval, advances immediatelyNotes
DoVideoEndWait(IsScenePath,IsLweRunning,SceneTransitionDelayMs) stripped — depend on the scene support PRApplyOverridesForPathsimplified (noReadVolumeOverride/ReadSpeedOverride) — depend on the per-video overrides PROnWallpaperChanged/_currentlyPlayingCard/Speedadded as stubs for future PRsTest plan
--restorereplays correct mode (timed or advance-on-end) after GUI close🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements