Bug 111157 - sort by multiple columns in amorok (patch included)
Summary: sort by multiple columns in amorok (patch included)
Status: RESOLVED INTENTIONAL
Alias: None
Product: amarok
Classification: Applications
Component: Playlist (show other bugs)
Version: 1.3
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 106349 135693 135973 138944 145998 148489 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-08-20 16:18 UTC by bonne
Modified: 2009-06-23 20:45 UTC (History)
7 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch as described (3.79 KB, patch)
2005-08-20 16:19 UTC, bonne
Details
Update to sort length column properly (3.81 KB, patch)
2005-08-21 05:18 UTC, bonne
Details
New patch with a menu (51.73 KB, patch)
2005-08-23 05:33 UTC, bonne
Details
Patch as above without saving bug (51.73 KB, patch)
2005-08-23 05:36 UTC, bonne
Details
Heaps of bugfixes (26.63 KB, patch)
2005-08-23 11:38 UTC, bonne
Details
Patch to make the sort stable (1.58 KB, text/x-diff)
2005-09-19 12:05 UTC, Stefan Siegel
Details
Patch to stabilize the sort for 1.4.x. Slightly faster. (2.36 KB, patch)
2006-05-22 02:34 UTC, Jim Bowlin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description bonne 2005-08-20 16:18:58 UTC
Version:           1.3 (using KDE 3.4.1, Gentoo)
Compiler:          gcc version 3.4.3 20041125 (Gentoo 3.4.3-r1, ssp-3.4.3-0, pie-8.7.7)
OS:                Linux (x86_64) release 2.6.12-gentoo-r4

Many people like to sort their music in different ways. I like to sort artist,title some people like artist,album,title others like artist,album,track number. 

This patch allows for any combination of columns, sorted in any order you like. 

It's a basic patch, which could easily be adapted for use with a menu (much like the collection manager "group by" menu). 

To use it, patch amarok-1.3, compile and use it as follows:

Click the column title to sort by that column (click again to reverse order) as usual (choose the most specific column first). Then click progressively less specific columns to get the sort order that you want.

For example if you want to sort by artist, reverse album, length you would click:
length,
album twice
artist.

It stores up to 5 columns of heirarchical sorting (although this is totally arbitrary). 

Ideally this could be supplemented with a small menu so that the user can more easily tell what order they are sorting in (I just don' know enough about the gui to do this just yet ;-))

I hope you like it..

Bonne
Comment 1 bonne 2005-08-20 16:19:23 UTC
Created attachment 12282 [details]
Patch as described
Comment 2 bonne 2005-08-20 16:20:21 UTC
Just to qualify:

This is for the playlist

Comment 3 bonne 2005-08-21 05:18:37 UTC
Created attachment 12293 [details]
Update to sort length column properly
Comment 4 bonne 2005-08-23 05:33:14 UTC
Created attachment 12336 [details]
New patch with a menu

Ok this new patch is much much better. 
It includes a nice three level menu (a la Collection Browser), saves it's state
on exit using the config file, and also changes the menu when the user clicks
on the column titles and resorts. 

No known bugs thusfar but please test and tell me what you think.
Comment 5 bonne 2005-08-23 05:36:37 UTC
Created attachment 12337 [details]
Patch as above without saving bug

Made a typo in the config saving stuff... Fixed now
Comment 6 bonne 2005-08-23 11:38:56 UTC
Created attachment 12341 [details]
Heaps of bugfixes

Now I've done some more thorough testing, I think I've eliminated a bunch of
the problems with the last one. 

This should be quite stable and good enough to use.
Comment 7 Seb Ruiz 2005-09-18 15:36:52 UTC
Thanks for this bonne. I'll have a look and test it after we release 1.3.2, should be tomorrow.
Comment 8 bonne 2005-09-18 16:40:40 UTC
Yeah I've been mucking with it a bit at home too. There are a few things I know aren't in the patch, which should be: 
Handling the case of NONE sorting (i.e. when the user manually changes the sort order) at the top level
There's a typo of the word descending
It could be better implemented to only store the sort columns in one place, but I was still understanding the infrastructure when I was writing it :-)


I've been thinking of ways to use SQL to sort the playlist (to get a speed improvement) and after a few tests I've found that the only way that speed could be improved is if a playlist table is kept that included the URL of the playlist items and an identifier of those items (which could be used to then sort them). I think the best identifier I could think of would be a pointer to their memory location (I couldn't find a way to uniquely identify playlistitems - their order in the iterator changes as the playlist is shifted around). The problem with this approach is that after killing amarok and firing it back up again we'd have to repopulate the table. A better way would be to keep a unique identifier for each item, chuck that into a table which is updated on each delete, add, and then use SQL to sort that list for us (should be many times faster) which we then use to reorder the list. Should be trivial for smart playlists!

Comment 9 Seb Ruiz 2005-09-18 16:58:05 UTC
> I've been thinking of ways to use SQL to sort the playlist (to get a speed
> improvement) and after a few tests I've found that the only way that speed

SQL is substantially slower infact (in my experience), especially since most 
users are using sqlite, not mysql/postgres

> could be improved is if a playlist table is kept that included the URL of
> the playlist items and an identifier of those items (which could be used to
> then sort them). I think the best identifier I could think of would be a
> pointer to their memory location (I couldn't find a way to uniquely
> identify playlistitems - their order in the iterator changes as the
> playlist is shifted around). The problem with this approach is that after
> killing amarok and firing it back up again we'd have to repopulate the
> table. A better way would be to keep a unique identifier for each item,
> chuck that into a table which is updated on each delete, add, and then use
> SQL to sort that list for us (should be many times faster) which we then
> use to reorder the list. Should be trivial for smart playlists!

Honestly, sounds a little dirty, but will have a closer look during the week - 
i have an algorithms exam tomorrow and not really prepared... :)

Seb
Comment 10 Stefan Siegel 2005-09-19 12:05:27 UTC
The problem why sorting by multiple columns is currently impossible is 
the unstable Heapsort algorithm Qt uses. Therefore I would like to 
suggest an alternative patch which simply stabilizes the sort by 
adding an additional comparison based on the current order in the 
list.

This patch doesn't add a menu.


Created an attachment (id=12627)
Patch to make the sort stable
Comment 11 Seb Ruiz 2005-09-19 13:47:30 UTC
SVN commit 461949 by seb:

Allow users to sort by multiple columns, by making Heapsort stable.
Patch by Stefan Siegel
BUG:111157
CCMAIL:kde@sdas.de


 M  +11 -18    playlistitem.cpp  


--- trunk/extragear/multimedia/amarok/src/playlistitem.cpp #461948:461949
@@ -360,26 +360,19 @@
             b = b.rightJustify( a.length(), '0' ); //so simply left-padding is sufficient
             break;
 
-        case Year:
-            if( a == b )
-                return this->compare( i, Artist, ascending );
-            break;
+         default:;
+     }
 
-        case Artist:
-            if( a == b ) //if same artist, try to sort by album
-                return this->compare( i, Album, ascending );
-            break;
+    /*!
+     * Qt sorts our items using Heapsort, which is not a stable sort algorithm.
+     * The simplest way to let the user sort by multiple colums is to use a stable
+     * sort algorithm, so we stabilize it: If a and b are equal we don't return 0 but
+     * a value based on the current order in the list. The localeAwareCompare value
+     * is multiplied by 2 so it always dominates the current order if it is non-zero.
+     */
 
-        case Album:
-            if( a == b ) //if same album, try to sort by track
-                //TODO only sort in ascending order?
-                return this->compare( i, Track, true ) * (ascending ? 1 : -1);
-            break;
-
-        default:;
-    }
-
-    return QString::localeAwareCompare( a, b );
+    return QString::localeAwareCompare( a, b ) * 2
+            + ( ( itemPos() < i->itemPos() ) == ascending ? -1 : 1 );
 }
 
 void PlaylistItem::paintCell( QPainter *p, const QColorGroup &cg, int column, int width, int align )
Comment 12 Seb Ruiz 2005-09-19 13:49:42 UTC
Bonne: I hope you realise why I am using stefan's patch - its a net loss in code :)  We also like to avoid options wherever possible.

Thanks for the effort though!
Comment 13 bonne 2005-09-19 15:13:34 UTC
> Bonne: I hope you realise why I am using stefan's patch - its a net loss in code :)
Nah Stefan's patch is heaps better!! So simple. I only really added the menu so that people could see what's going on but it's totally unnecessary.

> We also like to avoid options wherever possible. 
I never understood the concept of reducing user options; my favourite programs are the ones where I can configure everything I want to (saves having to hack the source!) but hey I'm not in charge. 
Comment 14 Seb Ruiz 2005-09-20 10:57:49 UTC
This has been reverted due to performance issues with large playlists, hence re-opening the bug report.
Comment 15 Seb Ruiz 2005-09-23 09:18:20 UTC
*** Bug 106349 has been marked as a duplicate of this bug. ***
Comment 16 Robert Glastra 2005-12-30 18:26:35 UTC
Seb, would it be possible to make this feature optional (by, yes, a gui option)?
I personally feel it's one feature amaroK is currently lacking, as I'd like all my numbers sorted by artist and then album, which currently seems to be not possible.

If it's not possible I'll try and patch it to a clean installation for myself. :)
Comment 17 Seb Ruiz 2005-12-30 23:22:05 UTC
No, sorry Robert, but we have a strong policy of minimal gui options.
Comment 18 bonne 2006-02-17 13:53:58 UTC
We could use the method I used in my previous patch, but leave out the menu. It's basically very simple, you just save the columns previously sorted on, then iteratively compare if two items are the same. It works at the same speed as the current method. 
Comment 19 Jim Bowlin 2006-05-22 02:34:09 UTC
Created attachment 16213 [details]
Patch to stabilize the sort for 1.4.x.  Slightly faster.

Here is a patch for 1.4.x that implements Stefan Siegel's sort stabilization.
In theory, his patch should not have significantly slowed up the sorting but in
practice, I noticed a slowdown of almost a factor of 2 for long playlists (2500
songs, my entire collection).

I made a slight modification and now it sorts in almost the same time as the
non-modified version. 

This should solve performance issues with large playlists.  For a playlist of
2500 items, this sort (on a 1.6 GHz P4) typically takes between 5 and 6 seconds
which is comparable to the speed of the unmodified version.

Personally, I think that even a factor of 2 increase in the time it takes to
sort large playlists would be acceptable in order to have multi-column sorting.

What good is shaving some seconds off of the sort if the end result is not what
you want?
Comment 20 Seb Ruiz 2006-05-22 02:50:41 UTC
Looks quite good - i'll do some testing on it later.  Possibly over the next weekend at the amarok conference! :)
Comment 21 Alexandre Oliveira 2006-10-15 19:27:07 UTC
*** Bug 135693 has been marked as a duplicate of this bug. ***
Comment 22 Seb Ruiz 2006-10-20 14:39:45 UTC
*** Bug 135973 has been marked as a duplicate of this bug. ***
Comment 23 Alexandre Oliveira 2006-12-18 23:45:00 UTC
*** Bug 138944 has been marked as a duplicate of this bug. ***
Comment 24 Nael Masood 2006-12-19 01:08:48 UTC
Having been pointed here through my filing of bug #138944, I'm wondering - has the patch that Jim Bowling submitted been applied to the program?
Comment 25 Mark Kretschmann 2006-12-19 09:26:33 UTC
Sorry, I can no longer get to apply this cleanly to SVN. In fact I can't even figure out which patch here depends on which.
Comment 26 Seb Ruiz 2007-05-28 02:11:53 UTC
*** Bug 145998 has been marked as a duplicate of this bug. ***
Comment 27 Seb Ruiz 2007-08-04 03:07:43 UTC
*** Bug 148489 has been marked as a duplicate of this bug. ***
Comment 28 Scott Howard 2008-01-16 02:07:56 UTC
Has this functionality been added to the mainstream release yet?

This is the one thing about amarok that frustrates me. It's by far my favourite audio player, but this still bothers me.
Comment 29 Matt Stevens 2008-01-16 04:40:05 UTC
or at least a patch that a deb or rpm can fix? Not all of us have much programming skill.
Comment 30 Mark Kretschmann 2008-01-16 07:51:50 UTC
Amarok1 is completely feature frozen at this point (only bugfixes allowed), and Amarok2 uses an entirely rewritten playlist without columns, so the patch is no longer applicable.
Comment 31 Myriam Schweingruber 2009-06-23 20:45:34 UTC
There are no columns anymore in Amarok 2