feat: sync play queue with subsonic servers - #618
Open
FormalSnake wants to merge 1 commit into
Open
Conversation
Adds savePlayQueue/getPlayQueue to the Subsonic client and a play_queue capability on the source abstraction, so kopuz interoperates with other Subsonic clients' play queue (Navidrome, Arpeggi). The queue pushes to the server on pause, track change and app quit; on startup, if the server's queue was last saved by another client, it's adopted over the local one. Verified savePlayQueue/getPlayQueue's request/response shape against a live Navidrome instance.
Contributor
📝 WalkthroughWalkthroughAdds remote play-queue persistence for Subsonic sources. The player pushes eligible queue changes, restores queues changed by another client at startup, and flushes queue state during shutdown. Other source types explicitly disable this capability. ChangesPlay-queue synchronization
Sequence Diagram(s)sequenceDiagram
participant PlayerTask
participant play_queue_sync
participant MediaSource
participant SubsonicClient
participant PlayerController
PlayerTask->>play_queue_sync: push queue on track change or pause
play_queue_sync->>MediaSource: save_play_queue(item IDs, current ID, position)
MediaSource->>SubsonicClient: save_play_queue(parameters)
PlayerController->>play_queue_sync: restore_if_changed_elsewhere at startup
play_queue_sync->>MediaSource: get_play_queue()
MediaSource->>SubsonicClient: get_play_queue()
play_queue_sync->>PlayerController: restore changed remote queue
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@crates/hooks/src/play_queue_sync.rs`:
- Around line 59-66: The playback-triggered save in the async block must use a
source-scoped single-flight, latest-wins writer so remote queue snapshots are
serialized and stale requests cannot overwrite newer state; update the
play-queue synchronization flow around save_play_queue accordingly. In
crates/kopuz/src/main.rs lines 624-630, route the close flush through that same
writer and await completion of its final snapshot. Ensure both sites share the
writer rather than issuing independent savePlayQueue requests.
- Around line 18-20: Update server_item_ids so an empty queue returns Some with
an empty Vec instead of None, allowing save_payload to call savePlayQueue
without a current item. Add a test verifying that an empty queue produces an
empty payload and reaches savePlayQueue.
In `@crates/hooks/src/use_player_task.rs`:
- Around line 1033-1051: Update the play-queue sync block in the task loop to
track a queue revision or fingerprint instead of only current_item_id, so
additions, removals, and reordering trigger a push even when the current track
is unchanged. Compare and update this queue-change value alongside the existing
track state, while preserving the just_paused trigger for position updates and
the existing service filtering.
🪄 Autofix
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 Plus
Run ID: b8017b43-31a0-4a9c-8191-1bb4163f6ede
📒 Files selected for processing (14)
crates/hooks/src/lib.rscrates/hooks/src/play_queue_sync.rscrates/hooks/src/use_player_task.rscrates/kopuz/src/main.rscrates/server/src/source.rscrates/server/src/source/jellyfin.rscrates/server/src/source/local.rscrates/server/src/source/offline.rscrates/server/src/source/soundcloud.rscrates/server/src/source/spotify.rscrates/server/src/source/subsonic.rscrates/server/src/source/types.rscrates/server/src/source/youtube_music.rscrates/server/src/subsonic.rs
Comment on lines
+18
to
+20
| fn server_item_ids(queue: &[Track]) -> Option<Vec<String>> { | ||
| if queue.is_empty() { | ||
| return None; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Allow an empty queue to clear the remote queue.
Line 20 returns None for an empty queue. save_payload then skips both normal synchronization and the close flush. A locally cleared queue leaves the prior remote queue intact. A later startup can restore that stale queue.
Return Some(Vec::new()) for an empty queue. Add a test that verifies an empty payload reaches savePlayQueue without a current item.
Proposed fix
if queue.is_empty() {
- return None;
+ return Some(Vec::new());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn server_item_ids(queue: &[Track]) -> Option<Vec<String>> { | |
| if queue.is_empty() { | |
| return None; | |
| fn server_item_ids(queue: &[Track]) -> Option<Vec<String>> { | |
| if queue.is_empty() { | |
| return Some(Vec::new()); |
🤖 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 `@crates/hooks/src/play_queue_sync.rs` around lines 18 - 20, Update
server_item_ids so an empty queue returns Some with an empty Vec instead of
None, allowing save_payload to call savePlayQueue without a current item. Add a
test verifying that an empty queue produces an empty payload and reaches
savePlayQueue.
Comment on lines
+59
to
+66
| spawn(async move { | ||
| if let Err(e) = source | ||
| .save_play_queue(&item_ids, current_id.as_deref(), position_ms) | ||
| .await | ||
| { | ||
| tracing::debug!(error = %e, "play queue push failed"); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize all remote queue snapshot writes.
savePlayQueue writes a complete queue snapshot. Independent requests can finish out of order. An older playback-triggered request can therefore overwrite a newer queue or the shutdown snapshot.
crates/hooks/src/play_queue_sync.rs#L59-L66: send snapshots through a source-scoped single-flight, latest-wins writer.crates/kopuz/src/main.rs#L624-L630: use the same writer for the close flush and wait for its final snapshot to complete.
📍 Affects 2 files
crates/hooks/src/play_queue_sync.rs#L59-L66(this comment)crates/kopuz/src/main.rs#L624-L630
🤖 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 `@crates/hooks/src/play_queue_sync.rs` around lines 59 - 66, The
playback-triggered save in the async block must use a source-scoped
single-flight, latest-wins writer so remote queue snapshots are serialized and
stale requests cannot overwrite newer state; update the play-queue
synchronization flow around save_play_queue accordingly. In
crates/kopuz/src/main.rs lines 624-630, route the close flush through that same
writer and await completion of its final snapshot. Ensure both sites share the
writer rather than issuing independent savePlayQueue requests.
Comment on lines
+1033
to
+1051
| // Play-queue sync (Subsonic savePlayQueue): push on track change | ||
| // and on pause. No-ops via capabilities() for every other backend. | ||
| { | ||
| let current_idx = *ctrl.current_queue_index.read(); | ||
| let current_item_id = ctrl.get_track_at(current_idx).and_then(|t| { | ||
| matches!( | ||
| t.id.service(), | ||
| Some(MusicService::Subsonic) | Some(MusicService::Custom) | ||
| ) | ||
| .then(|| t.id.key().into_owned()) | ||
| }); | ||
| let track_changed = last_queue_push_id != current_item_id; | ||
| let just_paused = is_playing != prev_playing && !is_playing; | ||
|
|
||
| if current_item_id.is_some() && (track_changed || just_paused) { | ||
| crate::play_queue_sync::push(ctrl); | ||
| } | ||
| last_queue_push_id = current_item_id; | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Push when the queue changes, not only when the current item changes.
Line 1044 compares only the current item ID. Adding, removing, or reordering other items leaves that ID unchanged. Line 1048 then does not save the modified queue until pause or exit.
Track a queue revision or queue fingerprint. Push when that value changes. Keep the existing pause trigger for position updates.
🤖 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 `@crates/hooks/src/use_player_task.rs` around lines 1033 - 1051, Update the
play-queue sync block in the task loop to track a queue revision or fingerprint
instead of only current_item_id, so additions, removals, and reordering trigger
a push even when the current track is unchanged. Compare and update this
queue-change value alongside the existing track state, while preserving the
just_paused trigger for position updates and the existing service filtering.
Member
|
I will kirk morph vms wallahi |
Contributor
Author
|
on wallahi idk what you just said 😭 |
'on i swear to god' يحرق بيتك |
Contributor
Author
wallahi is deadass the only thing i understood idk what he meant with kirk vms |
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
3 participants
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.
Adds support for Navidrome's queue sync that is also supported by other clients but not Kopuz yet. It's not like spotify sync, it's like I play some music on my phone, I come back to the computer after pausing on my phone, and the same song and queue is ready. Pretty cool ngl :D
AI assistance has been used, and has been tested
Sanity Checking
rules.
contribution guidelines, or this pull request did not use AI assistance.
Style and Consistency
style.
cargo fmt --all --checkorcargo fmt --allas appropriate.cargo clippy --workspace --all-targets -- -D warnings, orexplained why it could not be run.
this change depends on them.
Testing
Tested on platform(s):
x86_64-linuxaarch64-linuxx86_64-darwinaarch64-darwin