Bug 254404 (UMS_copy_order) - JJ: Using the order in the media browser while copying files to UMS devices
Summary: JJ: Using the order in the media browser while copying files to UMS devices
Status: CONFIRMED
Alias: UMS_copy_order
Product: amarok
Classification: Applications
Component: Collections/USB mass storage and MSC (show other bugs)
Version: 2.3.2
Platform: Debian testing Linux
: HI wishlist
Target Milestone: 2.7
Assignee: Amarok Developers
URL:
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2010-10-16 20:48 UTC by Hakan Bayindir
Modified: 2016-04-08 18:21 UTC (History)
12 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Auto scaling dot (4.56 KB, image/png)
2012-12-17 19:54 UTC, Riccardo Ferrazzo
Details
This takes care of order of artists upto getKIOCopyableUrls() (10.22 KB, patch)
2013-03-12 19:37 UTC, mayank
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hakan Bayindir 2010-10-16 20:48:15 UTC
Version:           2.3.2 (using KDE 4.4.5) 
OS:                Linux

Currently Amarok uses the KDE's ordering logic for copying media to the removable devices. While which is not wrong, some devices tend to use other sorting criteria internally. An example is my micro hi-fi, a Sony CMT-HX5 which can play MP3 over USB flash disks. 

My hi-fi orders the songs according to their creation time and plays with that order (first copied song plays first). Since KDE's logic generally doesn't preserver order (unless you select them all with dolphin and copy from there), Amarok is currently is not an option while preparing USB sticks for my Hi-Fi. Instead I use dolphin and copy them manually.

My proposal is to queue copy requests with the order in the media browser so the order will be preserved and Amarok will inherently become more useful for more devices.

Reproducible: Always

Steps to Reproduce:
1- Open amarok
2- Plug your UMS device and mount it, amarok will mount it too.
3- From your local library select and album, right click to it and select copy to collection. As collection, give the UMS device.
4- Files are copied with KDE's default logic.

Actual Results:  
The files will play in seemingly random order.

Expected Results:  
Files are copied according to the order in the media browser. Every album plays in the order it should.

OS: Linux (i686) release 2.6.32-5-686-bigmem
Compiler: cc
Comment 1 Sven Eppler 2011-04-14 20:43:06 UTC
I can confirm this. Espacially vfat formatted drives will preserve the order of file creation.

You can look at the "real" order on the filesystem either by using the "find" command or "ls -f" on the console.

And many low-budget MP3-Players just trust readdir() on filesystem level when it comes to ordering.

While it's probably easy and performant to not care about the order while copying, on less "intelligent" devices this renders amarok currently unusable.

Fixes this would solve trouble for dumb usb-stick and helps more intelligent sticks not to sort everything everytime all over again. ;)

Additionally awesome would be, if the files would be copied in the order which will be created after the path has been altered through the collection-specific options.

So let's say if i store my mp3s on sisk in "Album/file.mp3" but wan't to copy them to the collection in  "Album/TRACK - TITLE.mp3" the copy order should be figured out by the destination filename, instead by the source filename.
Comment 2 kiwiiii 2012-09-22 11:36:56 UTC
I have the same issue with a cheap mp3 player.
If I copy the files with amarok then the order is random. If I copy the files with dolphin by copying directories, then the order is random. But if I copy the files with dolphin by selecting the files, then the order is OK.

To copy music to my mp3 player I had to create a bash script that copies files one by one in the good order, recursively...
Comment 3 Bart Cerneels 2012-09-26 10:19:56 UTC
I don't think there is need to copy out of order. So we should just queue them in album order or playlist. I assume this is what you want?
Comment 4 kiwiiii 2012-09-26 18:35:26 UTC
Yes it should fix the issue if files are copied one by one in the good order.

"good order" is not well defined though.
If it's a drag&drop from the library, the displayed order is what I'd want.
If it's a drag&drop from the playlist, the playlist order seems logical.
Comment 5 Hakan Bayindir 2012-10-03 21:05:18 UTC
I think good order can be defined as "The display order of the unit (Library, playlist, etc) which initiates the file copy operation".
Comment 6 Riccardo Ferrazzo 2012-12-17 19:54:45 UTC
Created attachment 75889 [details]
Auto scaling dot
Comment 7 Riccardo Ferrazzo 2012-12-17 19:55:35 UTC
Comment on attachment 75889 [details]
Auto scaling dot

I'v tried to solve the problem in this way
Comment 8 Matěj Laitl 2012-12-17 19:57:49 UTC
(In reply to comment #7)
> Comment on attachment 75889 [details]
> Auto scaling dot
> 
> I'v tried to solve the problem in this way

I don't understand, is this the right bug?
Comment 9 Myriam Schweingruber 2012-12-17 22:45:46 UTC
Riccardo: you probably meant bug 253802, didn't you?
Comment 10 Anmol Ahuja 2013-02-17 09:18:11 UTC
I'd like to work on this bug.
Comment 11 mayank 2013-02-19 07:48:12 UTC
does the bug mean this:
When we copy the list of songs room the source to the destination, the folders of albums should not be created? and the list must be copied as it is?
Comment 12 Matěj Laitl 2013-02-19 10:53:06 UTC
(In reply to comment #11)
> does the bug mean this:
> When we copy the list of songs room the source to the destination, the
> folders of albums should not be created? and the list must be copied as it
> is?

Not really. I think that fixing this bug would involve:
 * generally assuring that tracks are passed to destination CollectionLocation::prepareCopy() in the visible order - this is done at various places including: FileView, CollectionTreeView, playlist
 * assuring that various drag sources (including file browser model, collection tree item model, playlist model) return tracks in correct order in their mimeData() methods do that it works with drag & drop too
 * assuring that dropMimeData() of CollectionTreeItemModel preserves order of passed tracks
 * assuring that UmsCollectionLocation itself preserves order when tracks are copied to it

^^^ these are just hints where to look, use debugging statements to find out where the order of tracks is broken. Comment #1 has good steps to reproduce and comment #2 contains commands to check whether the tracks were copied in correct order.
Comment 13 Matěj Laitl 2013-02-19 10:55:03 UTC
> Comment #1 has good steps to reproduce and comment #2 contains commands to check
> whether the tracks were copied in correct order.

Sorry, I meant comment #0 and comment #1 respectively.
Comment 14 mayank 2013-02-19 17:10:19 UTC
it could also be that creation of album folders create disorder in the
order in which the copy must be performed? Shouldnt we completely do away
with the folder creation so that order is intact throughout?


On Tue, Feb 19, 2013 at 4:25 PM, Matěj Laitl <matej@laitl.cz> wrote:

> https://bugs.kde.org/show_bug.cgi?id=254404
>
> --- Comment #13 from Matěj Laitl <matej@laitl.cz> ---
> > Comment #1 has good steps to reproduce and comment #2 contains commands
> to check
> > whether the tracks were copied in correct order.
>
> Sorry, I meant comment #0 and comment #1 respectively.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 15 mayank 2013-02-19 18:38:38 UTC
i am new to bug fixing. Can you please elaborate how to use debugging statements to identify the location in the source which is buggy?
Comment 16 Myriam Schweingruber 2013-02-19 19:00:12 UTC
(In reply to comment #15)
> i am new to bug fixing. Can you please elaborate how to use debugging
> statements to identify the location in the source which is buggy?

have a look at the debug.h for the possibilities. Also make sure your read the relevant documentation for hacking on amarok, namely the HACKING folder in the source as well as the wiki pages linked to from http://community.kde.org/Amarok/Development/Join and it's parent page.
Comment 17 Matěj Laitl 2013-02-19 20:49:48 UTC
(In reply to comment #14)
> it could also be that creation of album folders create disorder in the
> order in which the copy must be performed?

No, I don't think so.

> Shouldnt we completely do away with the folder creation so that order is intact throughout?

No.
Comment 18 Anmol Ahuja 2013-02-28 14:56:58 UTC
I'm sorry, I was busy with my exams. Can I take this bug up now?
@mayank: Are you working on this bug?
Comment 19 Matěj Laitl 2013-02-28 18:22:25 UTC
(In reply to comment #18)
> I'm sorry, I was busy with my exams.

No need to apologize, you didn't promise anything. :-)

>Can I take this bug up now? @mayank: Are you working on this bug?

Feel free to start working on this, little duplication doesn't hurt. Anmol, please *do* modify your bugzilla profile to mention your real name as we've told you earlier. Also please configure your e-mail client to mention your real name instead of "Darth Codus" (at least when interacting with open-source projects).
Comment 20 Anmol Ahuja 2013-03-02 15:44:48 UTC
Kk, I'll start work on it.
Also modified my profile to display my real name.
Thanks :)
Comment 21 Matěj Laitl 2013-03-02 15:49:29 UTC
On 2. 3. 2013 Anmol wrote:
> https://bugs.kde.org/show_bug.cgi?id=254404
> 
> --- Comment #20 from Anmol <darthcodus@gmail.com> ---
> Also modified my profile to display my real name.

Good. Full name "Anmol Ahuja" would be even better. :-)

	Matěj
Comment 22 Anmol Ahuja 2013-03-07 14:51:16 UTC
I really need help with this bug!
Here's what I can see:

CollectionTreeView calls createMetaQueryFromItems, which does not seem to return tracks in the visible order. Do I sort the tracks myself? How?

FileView creates a directory loader, which calls qStableSort(with Meta::Track::lessThan, so they're always sorted in ascending order, regardless of the way they're displayed.

IPodPlaylistProvider, CollectionTreeItemModel::dropMimeData and mp3tunesservice use intermediate QMap, so they aren't sorted by visible order.
Comment 23 Myriam Schweingruber 2013-03-07 16:56:02 UTC
(In reply to comment #22)
> I really need help with this bug!
> Here's what I can see:
> 
> CollectionTreeView calls createMetaQueryFromItems, which does not seem to
> return tracks in the visible order. Do I sort the tracks myself? How?
> 
> FileView creates a directory loader, which calls qStableSort(with
> Meta::Track::lessThan, so they're always sorted in ascending order,
> regardless of the way they're displayed.
> 
> IPodPlaylistProvider, CollectionTreeItemModel::dropMimeData and
> mp3tunesservice use intermediate QMap, so they aren't sorted by visible
> order.

I don't think the bug report is the right place to ask such questions, please use the amarok-devel@kde.org mailing list for that.
Comment 24 Matěj Laitl 2013-03-07 16:59:25 UTC
(In reply to comment #23)
> I don't think the bug report is the right place to ask such questions,
> please use the amarok-devel@kde.org mailing list for that.

Well, it depends. If the question is tightly related to the bug (this one is), it is reasonable to add the question to the bug IMO.
Comment 25 Anmol Ahuja 2013-03-09 19:08:28 UTC
https://git.reviewboard.kde.org/r/109369/
Comment 26 mayank 2013-03-10 04:47:04 UTC
For me sorting within an album still remains an issue!



On Sun, Mar 10, 2013 at 12:38 AM, Anmol <darthcodus@gmail.com> wrote:

> https://bugs.kde.org/show_bug.cgi?id=254404
>
> --- Comment #25 from Anmol <darthcodus@gmail.com> ---
> https://git.reviewboard.kde.org/r/109369/
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 27 mayank 2013-03-10 04:54:23 UTC
Also I think instead of making a level sort function migrating every method
from QSet to QList does the trick !


On Sun, Mar 10, 2013 at 10:16 AM, Mayank Jha <mayank25080562@gmail.com>wrote:

> For me sorting within an album still remains an issue!
>
>
>
> On Sun, Mar 10, 2013 at 12:38 AM, Anmol <darthcodus@gmail.com> wrote:
>
>> https://bugs.kde.org/show_bug.cgi?id=254404
>>
>> --- Comment #25 from Anmol <darthcodus@gmail.com> ---
>> https://git.reviewboard.kde.org/r/109369/
>>
>> --
>> You are receiving this mail because:
>> You are on the CC list for the bug.
>>
>
>
Comment 28 Anmol Ahuja 2013-03-10 05:25:06 UTC
Without the levelSort(), it will copy the items in the order of selection I believe. Would that be okay?
Comment 29 Anmol Ahuja 2013-03-10 05:28:38 UTC
I think I might have another solution, I'll work on it
Comment 30 Matěj Laitl 2013-03-11 13:55:46 UTC
(In reply to comment #22)
> Here's what I can see:

Nice research!

> CollectionTreeView calls createMetaQueryFromItems, which does not seem to
> return tracks in the visible order. Do I sort the tracks myself? How?

I fear you'll have to resort to sorting the tracks yourself. Note that there's QueryMaker::orderBy(), but it is not powerful enough - you'd need to be able to sort by multiple levels (e.g. by artist, then by year, then by album name, then by disc number, then by track number ...) and complicating QueryMaker::orderBy() could be a huge burden for QueryMaker implementations (but certainly one way to achieve this). Also see src/playlist/proxymodels/SortScheme.h - this facilitates multi-level sorting (but SortAlgorithms.h seem to be rather specific to playlist and not easily reusable).

> FileView creates a directory loader, which calls qStableSort(with
> Meta::Track::lessThan, so they're always sorted in ascending order,
> regardless of the way they're displayed.

Right, you ought to change that. I think in case of DirectoryLoader, one wants to recursively (e.g. sort folders by a criteria, then sort within each folder by the criteria) sort using currently displayed criteria and order. (which may be rather hard, but you can help yourself by doing KIO::listDir() instead or KIO::listRecursive() and performing the recursion and sorting manually incrementally)

> IPodPlaylistProvider, CollectionTreeItemModel::dropMimeData and
> mp3tunesservice use intermediate QMap, so they aren't sorted by visible
> order.

I don't think IPodPlaylistProvider is involved in this bug, remember this is only about using the visible order when copying *to UmsCollection*. CollectionTreeItemModel::dropMimeData: right, instead of essentially QMultiMap<Collection *, Track>, it ought to use QHash<Collection *, QList<Track> >. Note that hash[ key ], creates default value if key doesn't exist (here empty list), which will be kinda convenient in your use case.

> https://git.reviewboard.kde.org/r/109369/

Thanks for the patch, it is said than "a patch is worth 1000 words", so it is useful even if first iteration of it may not use the most optimal way to implement it. Unfortunately I'm currently rather time-constrained, so please give me a couple of days to review it. (call for other Amarok devs, especially Bart: you should help with reviewing, too!)
Comment 31 Anmol Ahuja 2013-03-11 14:11:19 UTC
That's what I did in the patch- I made a function to sort the tracks according to the various sort levels in the collectiontreeview.

I was hoping for some feedback as to whether this was the correct approach. I'll fine tune it now, thanks :)
Should I request somebody on IRC to review it?
Comment 32 mayank 2013-03-12 12:50:44 UTC
Well what I did is changed the type of QSet<CollectionTreeItem *> &items to
QList, and that seemed to preserve the order of artists but not the
internal tracks, uptil getKIOCopyableURLS. is this useful. Will post the
patch by midnight!



On Mon, Mar 11, 2013 at 7:41 PM, Anmol <darthcodus@gmail.com> wrote:

> https://bugs.kde.org/show_bug.cgi?id=254404
>
> --- Comment #31 from Anmol <darthcodus@gmail.com> ---
> That's what I did in the patch- I made a function to sort the tracks
> according
> to the various sort levels in the collectiontreeview.
>
> I was hoping for some feedback as to whether this was the correct approach.
> I'll fine tune it now, thanks :)
> Should I request somebody on IRC to review it?
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 33 mayank 2013-03-12 19:37:31 UTC
Created attachment 77990 [details]
This takes care of order of artists upto getKIOCopyableUrls()

I guess this takes care of the order of artists selected upto getKIOCopyableUrls()
Comment 34 Myriam Schweingruber 2013-03-12 20:16:04 UTC
(In reply to comment #33)
> Created attachment 77990 [details]
> This takes care of order of artists upto getKIOCopyableUrls()
> 
> I guess this takes care of the order of artists selected upto
> getKIOCopyableUrls()

Please submit code to http://reviewboard.kde.org, not to the bug report.
Comment 35 mayank 2013-03-13 14:10:39 UTC
https://git.reviewboard.kde.org/r/109464/


On Wed, Mar 13, 2013 at 1:46 AM, Myriam Schweingruber <myriam@kde.org>wrote:

> https://bugs.kde.org/show_bug.cgi?id=254404
>
> --- Comment #34 from Myriam Schweingruber <myriam@kde.org> ---
> (In reply to comment #33)
> > Created attachment 77990 [details]
> > This takes care of order of artists upto getKIOCopyableUrls()
> >
> > I guess this takes care of the order of artists selected upto
> > getKIOCopyableUrls()
>
> Please submit code to http://reviewboard.kde.org, not to the bug report.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 36 Anmol Ahuja 2013-03-13 15:02:39 UTC
Does amarok already support copying a playlists tacks to another location? I remember using a script the last time I needed the feature. Should I add it?
Comment 37 Rishabh gupta 2015-07-30 03:45:13 UTC
is anyone working on this bug? I'd like to work on it
Comment 38 Hakan Bayindir 2015-08-02 07:40:55 UTC
I don't think anyone is working. If you want, we can work on it together?
Comment 39 Rishabh gupta 2015-08-02 17:43:29 UTC
sure .the comments above suggests that the bug could be in fileview ,collectiontreeview etc .comment 12 has provided a lot of  clues.