Bug 254404 (UMS_copy_order)

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.

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...

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?

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.

I think good order can be defined as "The display order of the unit (Library, playlist, etc) which initiates the file copy operation".

Created attachment 75889 [details]
Auto scaling dot

Comment on attachment 75889 [details]
Auto scaling dot

I'v tried to solve the problem in this way

(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?

Riccardo: you probably meant bug 253802, didn't you?

I'd like to work on this bug.

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?

(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 #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.

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.
>

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?

(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.

(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.

I'm sorry, I was busy with my exams. Can I take this bug up now?
@mayank: Are you working on this bug?

(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).

Kk, I'll start work on it.
Also modified my profile to display my real name.
Thanks :)

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

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.

(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.

(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.

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

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.
>

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.
>>
>
>

Without the levelSort(), it will copy the items in the order of selection I believe. Would that be okay?

I think I might have another solution, I'll work on it

(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!)

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?

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.
>

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()

(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.

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.
>

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?

is anyone working on this bug? I'd like to work on it

I don't think anyone is working. If you want, we can work on it together?

sure .the comments above suggests that the bug could be in fileview ,collectiontreeview etc .comment 12 has provided a lot of  clues.