[PR #127] [MERGED] Improved next/prev handling for queued tracks. (v2) #796

New issue

Closed

opened 2026-02-27 19:59:59 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 19:59:59 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/127
Author: @kingosticks
Created: 2/8/2018
Status: ✅ Merged
Merged: 2/10/2018
Merged by: @plietar

Base: master ← Head: fix/skipping-with-queued-tracks

📝 Commits (1)

d05fa10 Improved next/prev handling for queued tracks.

📊 Changes

1 file changed (+40 additions, -14 deletions)

View changed files

📝 src/spirc.rs (+40 -14)

📄 Description

My original PR at https://github.com/librespot-org/librespot/pull/119 got all screwed up when I changed my fork from plietar/librespot to librespot-org/librespot. So here it is again.

A queued track is removed once it has become the current track.
Note that the track doesn't need to actually play i.e. it could
have been immediately skipped over with next(). This is
implemented in consume_queued_track().
Queued tracks are always positioned immediately after the current
track. 1) ensures this is true for next() but prev() requires
all the queued tracks are actually moved for this to remain the
case.

Also fixed the case where prev() on the first track would incorrectly
wrap back around to the last track even when repeat was disabled. The
correct behaviour is to remain on the first track and just seek to the
start.

For example, with the following tracks and repeat enabled:

TrackA, TrackB, TrackC-Q, TrackD-Q, TrackE
        ^^^^^^

Here, the result of prev changes the current track from TrackB to
TrackA and the queued tracks (TrackC, TrackD) move to the position
immediately after TrackA:

TrackA, TrackC-Q, TrackD-Q, TrackB, TrackE
^^^^^^

Calling prev again results in the current track wrapping back around
to TrackE and the queued tracks moving after that same track:

TrackA, TrackB, TrackE, TrackC-Q, TrackD-Q
                ^^^^^^

After looking again at what @plietar suggested at https://github.com/librespot-org/librespot/pull/119#discussion_r166569148 I agree it does make things neater. So I refactored the code a bit to make full use of that. And added some more comments.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/librespot-org/librespot/pull/127 **Author:** [@kingosticks](https://github.com/kingosticks) **Created:** 2/8/2018 **Status:** ✅ Merged **Merged:** 2/10/2018 **Merged by:** [@plietar](https://github.com/plietar) **Base:** `master` ← **Head:** `fix/skipping-with-queued-tracks` --- ### 📝 Commits (1) - [`d05fa10`](https://github.com/librespot-org/librespot/commit/d05fa10067247af8543061145d1888c6c8bbe9d1) Improved next/prev handling for queued tracks. ### 📊 Changes **1 file changed** (+40 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/spirc.rs` (+40 -14) </details> ### 📄 Description My original PR at https://github.com/librespot-org/librespot/pull/119 got all screwed up when I changed my fork from plietar/librespot to librespot-org/librespot. So here it is again. 1) A queued track is removed once it has become the current track. Note that the track doesn't need to actually play i.e. it could have been immediately skipped over with `next()`. This is implemented in `consume_queued_track()`. 2) Queued tracks are always positioned immediately after the current track. 1) ensures this is true for `next()` but `prev()` requires all the queued tracks are actually moved for this to remain the case. Also fixed the case where `prev()` on the first track would incorrectly wrap back around to the last track even when repeat was disabled. The correct behaviour is to remain on the first track and just seek to the start. For example, with the following tracks and repeat enabled: ``` TrackA, TrackB, TrackC-Q, TrackD-Q, TrackE ^^^^^^ ``` Here, the result of `prev` changes the current track from TrackB to TrackA and the queued tracks (TrackC, TrackD) move to the position immediately after TrackA: ``` TrackA, TrackC-Q, TrackD-Q, TrackB, TrackE ^^^^^^ ``` Calling `prev` again results in the current track wrapping back around to TrackE and the queued tracks moving after that same track: ``` TrackA, TrackB, TrackE, TrackC-Q, TrackD-Q ^^^^^^ ``` After looking again at what @plietar suggested at https://github.com/librespot-org/librespot/pull/119#discussion_r166569148 I agree it does make things neater. So I refactored the code a bit to make full use of that. And added some more comments. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>