After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 745647 - shuffleHistory fixes
shuffleHistory fixes
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: 3.16
Assigned To: gnome-music-maint
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2015-03-05 00:37 UTC by Carlos Garnacho
Modified: 2015-03-05 10:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
player: Use a deque for the shuffle history (3.22 KB, patch)
2015-03-05 00:38 UTC, Carlos Garnacho
none Details | Review

Description Carlos Garnacho 2015-03-05 00:37:35 UTC
On commit 6f1cb8d448d8 I happened to break _get_previous_song() for shuffled items, _get_next_song() is since called early when playing a song, this makes the current song added early to the queue, so _get_previous_song() always pops the current one.

I'm attaching a patch that fixes this, and switches to using deque. Using a LIFO, each time gnome-music made space for the new item on overflow, it was practically only replacing the most recently added item. Using deque both handles overflow for us, and behaves as a FIFO in this case, which is just right to preserve the last shuffle items.
Comment 1 Carlos Garnacho 2015-03-05 00:38:31 UTC
Created attachment 298589 [details] [review]
player: Use a deque for the shuffle history

deque seems better for the task, as we actually want to pop elements as a
FIFO so we forget about the oldest song and not the most recent in order
to add the most recent one. This is something managed automatically by
deque when setting a maxlen.

Also, commit 6f1cb8d448d8 changed player behavior so _get_next_track() is
called early after currentTrack changes, this means that the current track
is added very early to the shuffle list, so was always mistakenly popped
as the "previous" song. Make this behave nicely and either return the
actual previous song, or "rewind" if we played more than 10s of the song.
Comment 2 Vadim Rutkovsky 2015-03-05 10:36:40 UTC
Not sure if I personally like the 10 secs change, we'll see if that works for most people.

Pushed as https://git.gnome.org/browse/gnome-music/commit/?id=067a964, thanks!