GNOME Bugzilla – Bug 327042
Play queue and static playlists are not sortable
Last modified: 2018-05-24 11:05:43 UTC
Clicking a column header such as "Artist" should resort the playlist by artist, as it does in the Library. This is how iTunes behaves, and is particularly usefull for mt-daapd's dynamic playlists. Other information:
Now that we are using a filter model for static playlists, it would be fairly easy to copy the sorting stuff from automatic playlists into the static ones. We would probably want a "return to original order" control somewhere, since the user may either want to do that or not meant to have sorted the list. What to do with local static playlists is also something we need to think about. Where to insert a track that has been dropped into a sorted playlist could be complicated (maybe we should just insert at the end).
*** Bug 336080 has been marked as a duplicate of this bug. ***
Retitling summary to make include static playlists (addressed by James in comment #1).
*** Bug 338125 has been marked as a duplicate of this bug. ***
*** Bug 372946 has been marked as a duplicate of this bug. ***
This also affects play queue.
(In reply to comment #1) > What to do with local static playlists is also something we need to think > about. Where to insert a track that has been dropped into a sorted playlist > could be complicated (maybe we should just insert at the end). Regarding this issue, I think that once the playlist has been sorted it should just "forget" that it has been sorted, and insert the track whereever the user drops it. The user can then resort it later. In other words it should sort it instantaneously, rather than persistently, because you might want to refine it after the search. I think that other apps (like iTunes) do it this way, although I should check.
Created attachment 76420 [details] [review] start of a patch This trivial patch lets you sort non-local (e.g. DAAP) playlists.
(In reply to comment #8) > This trivial patch lets you sort non-local (e.g. DAAP) playlists. Works for me.
(In reply to comment #8) > Created an attachment (id=76420) [edit] > start of a patch > > This trivial patch lets you sort non-local (e.g. DAAP) playlists. > Works great for me. Thanks!
Ubuntu bug about that: https://launchpad.net/products/rhythmbox/+bug/76449
*** Bug 392524 has been marked as a duplicate of this bug. ***
Should the play queue really be sortable? What would happen to the playing track (which is always the first one)?
Hi, I believe playing queue should be sortable. Not everybody is used to have albums in the Library. Many of my friends just like to throw a MP3 folder on the player (or they do not have iPod, but other mp3 device, which is accessible as USB mass storage and they copy music to it via folders) . Or in my case I have external HDD with mp3 which is not always connected. So to play music, I start player, throw mp3, sort and then play it. And many times when I do drag and drop music is not sorted (likely due to mismatch in select order, file name or tag info). I believe sorting when playing should not affect playing song, or another mechanism for sorting can be implemented. For example select songs one is interested to sort and via right click contect menu Sort->By Name ->By Artist or any other sortable option. But still general sorting by clicking on columns should work. I can't tell my friends to change habits and this seems to be only serious bug they experience when using Rhythmbox when playing mp3's.
Created attachment 101556 [details] [review] Introduced per-playlist sorting with local/DAAP state storage This is a proof-of-concept patch to implement: a) Sorting functionality for local and DAAP playlists. b) Sorting state recollection for local and DAAP playlists. Remote state recollection is done by guessing that the combination of IP address and playlist name will be persistent. This is correct for my local media server setup and, I believe, appropriate for roaming clients (but not roaming servers) too. The state storage mechanism is unrefined. Currently state information is stored in per-playlist keys inside /apps/rhythmbox/state/sorting/ using an MD5 hash of a unique playlist name (IP + playlist name for DAAP, playlist name for local). This is probably an inappropriate use of GConf but I could not see a more elegant storage solution - I am open to ideas. Note that the performance impact will be negligible, however, as the total number of playlists a client will ever see is small. Patch only tested loosely but working well in my setup.
Forgot to comment above: the build dependencies in Rhythmbox are not quite correct. Thus, after applying the above patch you should make clean and rebuild completely. Also, the patch is diff'd against a patched Rhythmbox 0.11.2 codebase. If it's too fuzzy to resolve against HEAD just give me a shout.
I don't really like the idea of using md5(playlist name), although it should work perfectly well. I think it'd be better to escape any characters that don't work in gconf keys (probably the same as filenames). Also, I think it's probably better to use the share name rather than the IP address, as the address is likely to change in your typical dhcp or zeroconf network. You'll only see collisions if you access playlists with the same name on two different shares with the same name. Enabling sorting for local static playlists is a bit risky if there's no way to return to the original (unsorted) order. If we do, I'd rather see the sort order stored in playlists.xml as it is for automatic playlists.
Created attachment 101853 [details] [review] (Revision 2) Introduced per-playlist sorting with local/DAAP state storage OK, comments taken on board. This revision uses gconf_escape_string (didn't know that existed!) instead of MD5, the DAAP share name rather than IP and disables sorting for local playlists.
Vote +1
I haven't had a chance to test it yet, but a few comments: + case PROP_SORTING_NAME: + source->priv->sorting_name = g_value_dup_string (value); This should have a 'g_free (source->priv->sorting_name)' in there, so the string is freed if the sorting name changes. It should also be freed in the finalize method. + old_sorting_key = g_strdup (view->priv->sorting_key); g_free (view->priv->sorting_key); Duplicating and then freeing the string is somewhat pointless, and could be replaced with "old_sorting_key = view->priv->sorting_key".
Created attachment 102734 [details] [review] (Revision 3) Introduced DAAP per-playlist sorting with state storage You're right, that was a bit sloppy. I did a quick audit of the patch and caught two extra missed g_free's.
I am unable to get this patch successfully applied to SVN. I tried to update the patch but it seems a number of things have been moved around or restructured since 11.2. Here are the specific chunks that failed: Hunk #1 succeeded at 371 (offset 18 lines). patching file ./shell/rb-playlist-manager.c Hunk #1 succeeded at 937 (offset 18 lines). patching file ./sources/rb-playlist-source.c Hunk #1 FAILED at 92. Hunk #2 FAILED at 115. Hunk #3 FAILED at 126. Hunk #4 FAILED at 182. Hunk #5 FAILED at 233. Hunk #6 FAILED at 250. Hunk #7 succeeded at 436 with fuzz 2 (offset 55 lines). Hunk #8 succeeded at 481 with fuzz 2 (offset 67 lines). Hunk #9 FAILED at 959. 7 out of 9 hunks FAILED -- saving rejects to file ./sources/rb-playlist-source.c.rej patching file ./sources/rb-static-playlist-source.c Hunk #1 succeeded at 297 (offset 8 lines). Hunk #2 succeeded at 341 (offset 8 lines). patching file ./sources/rb-static-playlist-source.h patching file ./widgets/rb-entry-view.c
Created attachment 102774 [details] [review] (Revision 4) Introduced DAAP per-playlist sorting with state storage A few things have changed around on HEAD; I work on 0.11.2. Try this one. I can't test it locally (HEAD dependencies are too old) but the differences were only spatial.
Cool, thanks. Seems to be working great, but I'll keep testing for the next few days.
What is the status of having this work in a release? Is there any version target? It is linked to the Ubuntu bug https://bugs.launchpad.net/rhythmbox/+bug/76449 and has been open for over a year so I am just trying to discern progress. From the recent comments and patches it seems like this is still active so that is great. Unfortunately I assume this bug will still be present in Ubuntu Hardy as that uses 0.11.4 which exhibits this bug? Thanks for any input/insight. Is there anything I can do to help?
The patch I provided does not affect local smart playlists, only smart playlists shared over DAAP. Extending functionality to local playlists is probably quite easy. However, the Rhythmbox version I was working on printed GLib CRITICAL errors to the terminal when I tested local smart playlists. Hence I omitted them from this patch. I don't really have the time to look into the reasons for this.
*** Bug 529139 has been marked as a duplicate of this bug. ***
This looks OK and seems to work properly, so I've committed it (with a few tiny cleanups). Jay, I figure you already agreed to relicense your earlier contributions, so I'm assuming you're OK with relicensing these changes too.
Hi Jonathan, glad to see some progress on this bug and that the latest patch was committed. Is my understanding correct though that it doesn't address this bug, since it only allows for sorting of DAAP playlists but doesn't do anything for static playlists, or the play queue for that matter?
Yes, the new license is fine, John. This patch added the infrastructure to support persistent sortable playlists in any part of Rhythmbox. Only DAAP has been enabled so far. When I went to test integration with static playlists there were critical warnings from another part of Rhythmbox, so I held off making any changes at that point. They've probably been fixed since, so, if there's interest, I can provide a second small patch to enable sorting of static and automatic playlists. The play queue can be sorted with a simpler method but I'm not sure if a UI policy has been decided for that yet.
Automatic playlists are already sortable, unless they have a size/duration/song count limit set, in which case the sort order is part of the definition of the playlist.
OK, I've taken a look at extending this functionality to static playlists. It should be as simple as tying updates to the "name" property on RBStaticPlaylistSource to "sorting-name" as well, i.e. intercept in GObjectClass::set_property and set the same string on "sorting-name". However, I can't see a way to do this in the GNOME object model - the "name" property is part of RBSource and there doesn't seem to be a way to receive notification of updates to this property in the static subclass. If that's possible then the patch is just a couple of lines.
*** Bug 565565 has been marked as a duplicate of this bug. ***
The latest patch (2008-01-13) doesn't work for me. Can somebody fix this? This problem is really annoying.
*** Bug 581521 has been marked as a duplicate of this bug. ***
I'm confused on the status. I'm seeing this problem when browsing playlists on iPod. There was a patch committed on 2008-01-13, yet I still have this problem in 0.12.1. Did that patch not address iPods, or is there a different problem?
There is also a patch against 0.11 here: http://launchpadlibrarian.net/20866080/lp76449.debdiff
Created attachment 137033 [details] [review] Playlist sorting for 0.12.1 This is the patch from https://bugs.launchpad.net/rhythmbox/+bug/76449 but reformatted in a way that you as upstream maintainers can easily apply to your tree.
The patch that was committed was only for playlists on DAAP shares. This patch only addresses the most superficial aspect (actually enabling sorting). The more complicated bits are: - Figure out how the sorted playlist order interacts with the original order. How do you restore the original order? Does sorting actually change the order of the playlist, or does it just change the view of the track list? Is this different for local playlists than for playlists on ipods? - Store the selected sort order in playlists.xml for local playlists and restore it when the playlist is loaded
It's not perfect, I know, and those all are good things to address. But it's better than nothing, being a cosmetic fix at least. It does everything I want from it, and I would hope it could be committed anyhow, as a good first step.
I don't want to commit this because it makes it too easy for the user to inadvertantly destroy their own data.
I've been using it for a few days now, and haven't had any problem with it at all yet.
In 0.12.4 I do not see playlist sorting (at least not in the obvious way--clicking on the table headers). I assume it was never committed? It seems to me that either instantaneous or persistent sort would be just fine; my own choice would be for persistent (at least for the static playlist) for the following reason: I would have expected the table of tracks in the GUI to be a standard widget that has sorting built in. I'm surprised that this appears not to be the case. I don't have the first clue as to the internals of rhythmbox or indeed Gnome, or how such a standard component would break something (can anyone point out the obvious?), but I've seen this approach used in many tables in many applications... This may be partially an artifact of the way in which I'm using playlists? I feel that an explanation might help, since I'm claiming a need for a sortable entity and sortable playlists fill the need. I have ~/music, under which I have directories for music that I tend to want to listen to in a session, eg. "tango", "white noise", "blues", "symphonies", "string ensembles", "Bach", etc (genres could work but require retagging of everything in my collection as I discover where I want it, and I've got too much music that falls into multiple and unknown genres). Under xmms I could say "xmms music/tango", but rhythmbox imported everything so I can't do that. So I use playlists to see just the stuff under tango, whence I can add it to the playqueue during a party.
I'm going to preface this comment by saying I'm not saying the following to be rude. Just trying to light a fire behind a few behinds. I stumbled across this bug report because I'm having the same problem. I was taken aback when I went to click on "Artist" and nothing happened. This is a feature that should already be implemented. It's been three years since this bug report was opened and STILL hasn't been fixed. My question is why, during the last three years, hasn't this been fixed and what is going to be done to fix it now? And I mean now. Not a month from now. Or months from now. I mean now. This has been put off long enough.
Hmm, end of the month is soon, sounds like you are the one signing the pay checks, glad to meet you just in time!
In other words, people are hacking on rhythmbox in their free time on whatever feels the most important to them, if a bug seems important to you but isn't fixed, that's unfortunate, but noone can't really guarantee a timeline for a fix except if you get your hands dirty yourself (or if someone steps up to do it).
(In reply to comment #44) > I'm going to preface this comment by saying I'm not saying the following to be > rude. Just trying to light a fire behind a few behinds. You don't get to do that. Regardless of your intent, this behaviour is actually quite rude.
Jonathan is correct--it is rude to point out development process failures to volunteers. Does that mean that the failures aren't real? Is everything rude also false? Chris is correct in that the delay for fixing this simple problem is absurd, and given the ad-hoc partial solutions in this bug I'm guessing that the code wasn't very well thought out from the beginning. Yes, it's rude to say so, and the correct solution is for Chris (or me, or whomever has a problem with it) to fix the structure of the code so that it's easier to maintain. Or, if you're a developer, at least to realise that there is a problem with the codebase when trivial behaviour is difficult to implement in general. Perhaps a new bug called "Rhythmbox's code isn't pretty"? But what's really going on here? Rhythmbox seems to be one of the more refined of the iTunes clones, but in Debian Sid I see (and have tried a bunch of) abraca, amarok, aqualung, ario, audacious, banshee, bluemindo, cynthiune, decibel, elisa, gbemol, gimmix, glurp, gmpc, gmusicbrowser, intone, irmp3, jajuk, jlgui, jokosher, juk, listen, lxmusic, minirok, moosic, muine, musiclibrarian, potamus, promoe, pympd, pytone, quodlibet, sonata, xfmpc, xmms2, in addition to various servers, command-line clients, indexing tools, etc. Yes, Linux is strong because of the license that allows branches of most any component, but in the absence of good judgement the same freedom weakens us, and I think that the example of music players, while not unique, illustrates this brilliantly. This is killing Linux on the desktop. What can be done to fix it? Could development teams swallow some pride and learn to work together? What if all the developers of those 35 projects could agree on everything, or even just some things? That is, Linux as a media player is a toy, top-heavy with undisciplined hackers who are churning out great beginnings but not following through, ignoring rude but valid criticism because they are volunteers; they do not have to respond to the rude jerks. I found 35 different programs that all set out to do exactly the same thing IN A MAJOR DISTRIBUTION'S PACKAGE LIST, and that was from a quick scan. Yes, hacking music software is fun, but if the object were to actually produce something solid, there might be better ways to go about it. So if you don't like the way the Rhythmbox project is managed, fork off! ;) Just a thought. Largely off-topic as well, but I don't know where this problem should be discussed, and since I bet y'all have some really good insights, this is probably as good a spot as any to open the furnace ;)
Ben, this isn't a forum, and has nothing to do with the bug. Vent your frustration on the list, or whereever else you please, but not in Bugzilla. Consider this your first warning.
Trying to progress this bug, it really has been open way to long. If destroying user data is so important why is there a "Shuffle playlist" option available? I only used that because I assumed that sorting the play list was equally easy. But now I have a quiet large play list that I can't sort. Should the "Shuffle Playlist" option be removed till their is a more robust method of tracking play list history and allowing undo (it'd need to be persistent across reboots, as I didn't discover I'd lost this data till days later)
Maybe. I'm not sure how that counts as "progress[ing] this bug" though. Unless you do it yourself, there is not going to be a more robust method of tracking playlist history any time soon.
I was hoping that acknowledgement that currently the app supports methods of trashing that same data that shot down the submitted patch might allow that patch (or a rework of it now that I'm guessing it wont apply cleanly to the current code base) to be merged. I know that it would go a long way to easing my recreation of the order data I have lost. If that wont fly, what level of undo would be required? Just the ability to undo while Rhythm Box is still running? Or would some method of being able to track multiple orderings on disk for later use be required?
*** Bug 687048 has been marked as a duplicate of this bug. ***
I have a suggestion regarding restoring the original order for a playlist. There is a first column in the playlist which looks like sequence number starting from one. In the smart playlists, nothing happens when clicked on this column. Can this be enhanced in ordinary playlists to restore order (or based on sequence number) when clicked on the fist column? Not sure if this idea was already discussed. Didn't go through all 53 comments.
I don't think it is possible to use the sequence number - I believe that is part of how the GTK control displays the entries and not an actual rhythmbox entry field. My proposal would be to add a hidden field to the entries in the static playlist which would indicate the default ordering. This would then be saved in playlists.xml . And, of course, you'd have to update it when using drag'n'drop. To return to the default sort order, you would click a third time on the table headers. I have no idea how to implement all this though. And I don't even know if it is possible.
(In reply to comment #55) > I don't think it is possible to use the sequence number - I believe that is > part of how the GTK control displays the entries and not an actual rhythmbox > entry field. More or less correct. > My proposal would be to add a hidden field to the entries in the static > playlist which would indicate the default ordering. This would then be saved in > playlists.xml . And, of course, you'd have to update it when using drag'n'drop. RBPlaylistSource includes a hash table that records which entries are in the playlist. I'd suggest replacing this with a GSequence so it has an order, then using it for the 'playlist position' column and for saving static playlists. The playlist track number column probably shouldn't be shown in automatic playlists. Drag and drop should be disabled when the playlist is sorted by anything other than playlist position. > To return to the default sort order, you would click a third time on the table > headers. I think this should be done by clicking on the playlist position column header. > I have no idea how to implement all this though. And I don't even know if it is > possible. I'm confident that what I've outlined above is possible to implement.
(In reply to comment #56) > > To return to the default sort order, you would click a third time on the table > > headers. > > I think this should be done by clicking on the playlist position column header. There are applications that have 3 states per column (like Tomboy for example) 1. ordered (UI: arrow down) 2. ordered reverse (UI: arrow up) 3. not ordered (UI: no arrow) This way, the original order is simply when no column is "ordered" (direct or reverse) And the playlist position column can be hidden if needed.
(In reply to comment #56) > (In reply to comment #55) > > I don't think it is possible to use the sequence number - I believe that is > > part of how the GTK control displays the entries and not an actual rhythmbox > > entry field. > > More or less correct. > > > My proposal would be to add a hidden field to the entries in the static > > playlist which would indicate the default ordering. This would then be saved in > > playlists.xml . And, of course, you'd have to update it when using drag'n'drop. > > RBPlaylistSource includes a hash table that records which entries are in the > playlist. I'd suggest replacing this with a GSequence so it has an order, then > using it for the 'playlist position' column and for saving static playlists. > The playlist track number column probably shouldn't be shown in automatic > playlists. > > Drag and drop should be disabled when the playlist is sorted by anything other > than playlist position. > > > To return to the default sort order, you would click a third time on the table > > headers. > > I think this should be done by clicking on the playlist position column header. > > > I have no idea how to implement all this though. And I don't even know if it is > > possible. > > I'm confident that what I've outlined above is possible to implement. Great! Thanks. Expecting the fix soon. :)
ping.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/rhythmbox/issues/120.