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 537168 - Playlist should follow/jump currently playing song
Playlist should follow/jump currently playing song
Status: RESOLVED FIXED
Product: banshee
Classification: Other
Component: general
git master
Other All
: High enhancement
: 1.x
Assigned To: Alexander Kojevnikov
Banshee Maintainers
papercut
: 522731 551648 551964 560899 560937 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-06-07 21:17 UTC by Mike McWay
Modified: 2009-07-02 09:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Follow the currently playing song (2.21 KB, patch)
2009-06-15 08:25 UTC, Alexander Kojevnikov
reviewed Details | Review
Don't jump to the new track if previous track is not visible (4.33 KB, patch)
2009-06-17 00:59 UTC, Alexander Kojevnikov
reviewed Details | Review
Alternative patch (4.48 KB, patch)
2009-06-18 03:45 UTC, Alexander Kojevnikov
none Details | Review
Keep selected tracks visible when the user searches (6.31 KB, patch)
2009-06-19 04:07 UTC, Alexander Kojevnikov
committed Details | Review

Description Mike McWay 2008-06-07 21:17:36 UTC
Please describe the problem:
earlier versions would center the playlist on currently playing track during shuffle mode, but this funcionality seems to be lost.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Stefan 2008-07-03 12:58:57 UTC
Problem isn't fixed in Banshee-1 either.

Steps:
Create a larger playlist (more than one page), start playlist, fast forward tracks 

Result:
Banshee doesn't focuses the actual playing track

Expectation:
Banshee should jump to playing track automatically (as in older versions)

Does this happen every time:
Yes; happens with and without randomizer, in freshly created playlists as well as in the main library

Other info:
Banshee 1.0.0 on OSS10.3 64bit
Comment 2 Allan Day 2008-07-20 22:55:42 UTC
I can confirm using Banshee 1.0.1 (r4220). Changing status to new.
Comment 3 Gabriel Burt 2008-09-12 20:04:34 UTC
*** Bug 551964 has been marked as a duplicate of this bug. ***
Comment 4 Gabriel Burt 2008-09-13 18:50:35 UTC
*** Bug 551648 has been marked as a duplicate of this bug. ***
Comment 5 Wouter Bolsterlee (uws) 2008-09-23 16:12:16 UTC
Ok, I think this should only be done if the currently selected song is the (previously) playing song, in order to not deselect songs selected manually by the user. So for the normal use case where you just double click a song you want to listen to, this would to The Right Thing, while for the not so common case that you select other songs without playing them directly (i.e. no double click), it would still respect the user's selection. Oh, and while I'm at it: if the currently playing song is not visible because of filters (album, artist, or search terms), the song that starts playing should be selected by default as well.
Comment 6 Bertrand Lorentz 2008-11-15 13:33:25 UTC
*** Bug 560899 has been marked as a duplicate of this bug. ***
Comment 7 Bertrand Lorentz 2008-11-15 23:28:04 UTC
*** Bug 560937 has been marked as a duplicate of this bug. ***
Comment 8 Daevid Vincent 2009-02-02 22:08:54 UTC
I have to add a "Me Too" to this one (and this also gets me on the CC list). I think there needs to be a preference box to "auto jump". I agree with Wouter too.

In my use case, I leave it on random and in the background all day while working. Sometimes a song comes on that I don't want to listen to, but I *do* want to hear another by that same artist or off that album. So i have to open up banshee, then CTRL+J, then choose my new track. Kind of annoying. 

Related:
I use the tray icon to "skip" all the time. It would be nice if there were another button next to [skip] that was [open] that would open up banshee, and do the CTRL+J automatically for me, centering the current track in the display list. Then I could just pick a new one and get on with my life. :)
Comment 9 Alexander Kojevnikov 2009-06-15 08:25:55 UTC
Created attachment 136609 [details] [review]
Follow the currently playing song

A quick patch implementing what was suggested in comment #5. Please test it.

Bug 522731 is related to this one. Actually, I fail to see why they are not duplicates. This patch should fix both of them.
Comment 10 Wouter Bolsterlee (uws) 2009-06-15 20:08:08 UTC
*** Bug 522731 has been marked as a duplicate of this bug. ***
Comment 11 Gabriel Burt 2009-06-16 19:52:52 UTC
I like this approach, Alexander - very simple.  I think we should add one more check (after your existing ones) to make sure the playing track is in view as well, and only jump to the new one if that's the case, to prevent this scenario:

You've been listening to music, and Banshee has been jumping your list to the playing song, all is well, but then you start scrolling away but haven't changed the selection yet, the song changes, and you get jumped when you didn't want to be.

If you agree, we'd probably want to add a helper method to ListView: IsRowVisible (int) (that itself could use GetRowAtY)
Comment 12 Alexander Kojevnikov 2009-06-17 00:59:32 UTC
Created attachment 136790 [details] [review]
Don't jump to the new track if previous track is not visible

Gabriel, that makes perfect sense. Updated the patch to avoid jumps (but still change the selection) when the playing track is out of view.
Comment 13 Gabriel Burt 2009-06-17 15:59:25 UTC
The ListView_Model.cs changes concern me a bit.  With the current code, you can select a song, and then as you search, assuming what is selected still matches, that code will ensure it stays in the view.   It's not done perfectly, and maybe shouldn't be done at all - test it out, tell me what you think.

We could put that code in the DatabaseSource's RateLimitedReload method.
Comment 14 Alexander Kojevnikov 2009-06-18 03:44:08 UTC
(In reply to comment #13)
> The ListView_Model.cs changes concern me a bit.  With the current code, you can
> select a song, and then as you search, assuming what is selected still matches,
> that code will ensure it stays in the view.   It's not done perfectly, and
> maybe shouldn't be done at all - test it out, tell me what you think.
> 
> We could put that code in the DatabaseSource's RateLimitedReload method.
> 

The problem is that the model is reloaded whenever the next track starts playing. The removed code in ListView should only run when the filter is changed, not every time the model is reloaded. But the ListView has no idea what triggers the model reload.

I hackish way could be checking the number of tracks in the model, if it's changed - the reload is caused by the filter.

I will attach the patch. I'm not particularly fond of how it's done, but it works and I cannot think of an elegant solution that wouldn't be too bloated.
Comment 15 Alexander Kojevnikov 2009-06-18 03:45:19 UTC
Created attachment 136887 [details] [review]
Alternative patch
Comment 16 Gabriel Burt 2009-06-18 14:11:17 UTC
Ahh, right - so just move it into the FilterQuery setter - that's what I meant to suggest (so that it would only happen when the search changed).
Comment 17 Gabriel Burt 2009-06-18 14:14:37 UTC
Ok, I guess I hadn't thought that through all the way - that's what I get for sideline programming.  :)  Comment #16 isn't plausible since the DatabaseSource doesn't have reference to the ListView(s).

Maybe you could move the code to BaseTrackList, and listen for/override the existing handler for model reloads, and when it happens, if source is DatabaseSource, see if its FilterQuery is changed?
Comment 18 Alexander Kojevnikov 2009-06-19 04:07:20 UTC
Created attachment 136968 [details] [review]
Keep selected tracks visible when the user searches

(In reply to comment #17)
> Maybe you could move the code to BaseTrackList, and listen for/override the
> existing handler for model reloads, and when it happens, if source is
> DatabaseSource, see if its FilterQuery is changed?

The model doesn't have a (public) reference to the source, so we cannot access the FilterQuery property. Instead I'm using the DatabaseTrackListModel's UserQuery property.
Comment 19 Gabriel Burt 2009-06-30 18:12:49 UTC
Almost perfect!

Trivial:  can you cast as IFilterable instead of DatabaseTrackListModel to be more flexible

A bit less trivial:
It would be nice if the album/artist/[genre etc in the future] ListViews still did the center-on-selection thing that I asked you to move to BaseTrackList.

Maybe we could add a ModelReloaded event to ListView, and then have a static helper class that can tack the center-on-selection functionality to any ListView by listening for that?  In the case of the filter ListViews (album/artist/etc) we could maybe initialize that extra functionality in the FilteredListSourceContents, such that the artist/album ListViews get centered only if the main_view's model's query changed?  Or maybe it should happen in TrackFilterListView.
Comment 20 Gabriel Burt 2009-07-01 15:52:47 UTC
I'm going to try to make the above changes.
Comment 21 Gabriel Burt 2009-07-01 16:11:06 UTC
Ok, I committed basically your patch but with the CenterOnSelection code pulled out into a protected ListView method.  Thanks a lot Alexander, this is a really nice patch that a lot of users will appreciate.
Comment 22 Wouter Bolsterlee (uws) 2009-07-01 18:28:00 UTC
Great to see my suggestons from comment 5 are implemented. Thanks a lot, guys.
Comment 23 Alexander Kojevnikov 2009-07-02 09:03:34 UTC
Thanks Gabriel!