Skip to content

Emit SetQueue event#1677

Merged
photovoltex merged 12 commits intolibrespot-org:devfrom
ralph:emit-set-queue-event
Feb 9, 2026
Merged

Emit SetQueue event#1677
photovoltex merged 12 commits intolibrespot-org:devfrom
ralph:emit-set-queue-event

Conversation

@ralph
Copy link
Contributor

@ralph ralph commented Jan 22, 2026

What does this do?

This emits a new universal Player::SetQueue event.

Background info: How is the Spotify Connect queue organised?

The Spotify connect queue basically has the following data structure:

context_uri: SpotifyUri
current_track: Track
prev_tracks: []Track
next_tracks: []Track

Each Track has at least a track URI and a provider which can be autoplay, context, queue or unavailable.

  • autoplay: Added by Spotify to keep playing
  • context: part of the current context identified by context_uri
  • queue: manually queued while a context was loaded
  • unavailable: self explanatory

Here you can see an example of a queue:

539394739-65d47a85-9dd3-4b21-9343-da1503151c20

Why is this event "universal"?

There are multiple actions that manipulate the queue in different ways. Examples:

  • Loading a new context: manipulates context_uri, current_track, prev_tracks, next_tracks
  • Queuing a new track/album/playlist while a context is loaded: manipulates next_tracks
  • Reordering tracks in the queue: next_tracks
  • Removing tracks from the queue: next_tracks

If a GUI application wants to adjust its state when any of these actions are performed remote or local, it just needs to listen to this one universal event. The queue displayed in the GUI is always correct and always up to date.

Why does this remove AddToQueue that was just added in 87d37c3 ?

AddToQueue is a special case of SetQueue, and SetQueue covers all cases of AddToQueue usage. Also, it's less error prone for app developers. Say a GUI developer wants to display the current queue from Spirc, and the queue looks like this:

`context` Track 1 <- currently playing
`context` Track 2
`context` Track 3
`context` Track 4

When another track is queued, it will be placed after the currently playing track:

`context` Track 1 <- currently playing
`queue` Track 1
`context` Track 2
`context` Track 3
`context` Track 4

In this case, AddToQueue will be emitted for Track 1, and the GUI developer will be responsible to add it in the right position of the app's state.

When yet another track (or collection of tracks, e.g. an album with 2 tracks) is queued, those will be placed after the currently playing track AND after all the other queue tracks:

`context` Track 1 <- currently playing
`queue` Track 1
`queue` Track 2
`queue` Track 3
`context` Track 2
`context` Track 3
`context` Track 4

In this case, AddToQueue will be emitted with Track 2, then with Track 3. Again, the developer of the consuming app is responsible for placing the new tracks in the right position.

In contrast to this, SetQueue always emits the whole queue with the tracks in the right order, so the developer of the consuming app cannot place the tracks in the wrong position.

Also, AddToQueue was never in a release, so removing it is ok imho.

Are there downsides?

As we're emitting ALL THE QUEUE FIELDS \o/ with every change, we do sometimes emit more than necessary. But we have to emit next_tracks most of the time, and the remaining queue fields are not that large compared to next_tracks. current_track and context_uri is negligible, and prev_tracks is capped at 20 tracks.

How to go from here?

I'd love to hear your input on this. Do you think a universal SetQueue event is a good thing? Or would you rather have more fine grained events emitting less data, e.g. one for prev_tracks, one for next_tracks one for current_track and current_track? Or should we keep the universal SetQueue for actions that manipulate all of next_tracks, but keep AddToTrack when adding tracks to the queue, manipulating only small parts of next_tracks?

The current approach works well for me and my reactive SwiftUI app, and makes updating the queue incredibly easy. But I'm still pretty new to this and there might be use cases that I didn't think of.

Copilot AI review requested due to automatic review settings January 22, 2026 20:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the recently added AddedToQueue player event with a new universal SetQueue event. The SetQueue event provides a comprehensive view of the queue state, including context URI, current track, next tracks, and previous tracks. This simplifies queue state management for GUI applications by emitting the complete queue state whenever it changes, rather than requiring consumers to manually track incremental changes.

Changes:

  • Replaced AddedToQueue event and command with SetQueue event and command
  • Added queue state emission when context is loaded, tracks are added to queue, or queue is set via Spotify Connect
  • Updated event handler to process the new SetQueue event structure

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
playback/src/player.rs Replaced EmitAddedToQueueEvent command and AddedToQueue event with EmitSetQueueEvent and SetQueue, including full queue state (context URI, current track, next/prev tracks)
connect/src/spirc.rs Added emit_set_queue_event() helper method and updated queue operations to emit the new SetQueue event instead of AddedToQueue
src/player_event_handler.rs Updated event handler to process SetQueue event with formatted track lists instead of single track AddedToQueue event
CHANGELOG.md Updated changelog entry to reflect the new SetQueue event

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@photovoltex
Copy link
Member

I see you iterated a bit on your previous implementation hehe. I only did read your wall of text so far, but sounds interesting in itself. I will give it a more detailed look in the coming days :)

On a side note. #1448 is related, but would be the SetQueue event for a remote device.

@photovoltex
Copy link
Member

Some things I have going through my mind right now:

  • we could probably add a separate event channel for the spirc to separate it better
    • with that we could also move some event's that are not really a player event into this event
  • cloning the entire queue (just guessing what you did) sounds pretty expensive

a handler style system, instead of the current message system would allow only borrowing the queue, but that also means the handler is called in the spirc thread and might make the connect usage less responsive.

@ralph
Copy link
Contributor Author

ralph commented Jan 23, 2026

Yes, #1448 seems related.

Cloning the entire queue sounds expensive at first, but it's only 2 small strings (uri and provider) per track. Negligible on a normal computer, much less of an impact than decoding the stream I would say, but I don't know what range of resource restricted devices librespot runs on and what implications this has.

I'm open to creating a separate Event channel for Spirc, or introducing a handler style system. If you direct me what direction you'd rather go, I'm happy to explore.

@photovoltex
Copy link
Member

Cloning the entire queue sounds expensive at first, but it's only 2 small strings (uri and provider) per track. Negligible on a normal computer, much less of an impact than decoding the stream I would say, but I don't know what range of resource restricted devices librespot runs on and what implications this has.

2 strings times 70 is still 140 strings for each queue change. So this might not be really optimal on lower end systems. We had something similar previously with PlayerEvent::PositionChanged. If we add a similar mechanism to make this addition opt-in, then we could probably add it as-is.

@ralph
Copy link
Contributor Author

ralph commented Jan 25, 2026

Thanks for your input! Will think about the opt in approach.

Copy link
Member

@photovoltex photovoltex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside my previous comment with the opt-in for the feature, and the other comment. This seems pretty good as is.

@ralph
Copy link
Contributor Author

ralph commented Jan 28, 2026

@photovoltex I'm still thinking about this:

we could probably add a separate event channel for the spirc to separate it better

I do think it's a good idea, but I did not have the time to play with it yet. Now I wonder if we should make single events optional or if we should move this to a separate Spirc channel and make the whole channel optional. 🤔

If you have any guidance, let me know.

@photovoltex
Copy link
Member

No need to rush the topic :)


hmm, I think another channel for the event's is probably a different topic then making this optional. I think it would be a good idea to move some event's to a separate spirc channel, which then is optional. But there should also be an option to disable this specific feature, as it's more resource hungry and is not always wanted, as in a standalone "just a player" use-case.

to keep it clean we should probably move the channel separation into another PR later down the line and just make this new event optional in this current context^^

Other expensive events might be added to `OptInPlayerEvents` in the
future.
@ralph ralph force-pushed the emit-set-queue-event branch from 9dd062a to 5bf1c5b Compare January 30, 2026 09:40
@ralph
Copy link
Contributor Author

ralph commented Jan 30, 2026

@photovoltex i made the new event opt-in in 5bf1c5b. OptInPlayerEvents is a struct that (as of now) only has a set_queue boolean field, but we might want to add other future or present events that are expensive. LMK what you think.

@photovoltex
Copy link
Member

The way you implemented it in 5bf1c5b only solves a clone which is done when sending the event (by the channel itself), but not the clone which is done when creating the event in the Spirc. The event is only really opt-in if you skip it before you do any logic for itself, so we need to add the skip way earlier.

So probably something like adding a parameter to the ConnectConfig, forwarding/storing it in the SpircTask and then adding a skip at the start of the emit function before any event logic is fired. Or so.

@ralph
Copy link
Contributor Author

ralph commented Feb 4, 2026

Damn, you're right, I overlooked this. 😕

I refactored this 2a5f698 to use a boolean toggle emit_set_queue_events in the ConnectConfig, which is used when initializing SpircTask, and when set to true will skip emitting before the cloning happens. Defaults to false, so this is opt-in, not opt-out.

Copy link
Member

@photovoltex photovoltex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some minor changes, after that we can get this merged. Thanks for your patience :)

ralph and others added 3 commits February 9, 2026 20:06
@ralph
Copy link
Contributor Author

ralph commented Feb 9, 2026

All requested changes implemented (I hope). Appreciate your guidance and focus on quality!

@ralph ralph requested a review from photovoltex February 9, 2026 19:32
Copy link
Member

@photovoltex photovoltex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the contribution again :)

@photovoltex photovoltex merged commit 33bf3a7 into librespot-org:dev Feb 9, 2026
12 checks passed
@ralph
Copy link
Contributor Author

ralph commented Feb 9, 2026

My pleasure! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants