Playlist is sorted before appending

Submitted by Jimorie on Fri, 2009-06-12 20:50

Forums:

Hey!

I noticed something strange. When I use 'mocp -a /path/to/playlist.m3u' from the command line, the playlist items are sorted before added to the mocp playlist. But if I add it from the mocp user interface (hitting enter or A on the file), then the playlist items are added in the correct order.

Anyone want to comment on this?

^^

Jim

Jimorie

Fri, 2009-06-12 20:57

Hmm. I also notice 'mocp --seek 5' results in a segmentation fault. I know that used to work... It is probably my build of mocp that is simply broken. Sigh.

deep42thought

Thu, 2015-02-05 17:23

I notice the same. Currently I'm at revision 2758 without any modifications.
The bug is quite annoying for me, since I control moc from console (via apache and php) and am listening to a lot of music withou shuffle.

greetings,
deep42thought

jcf

Thu, 2015-02-05 21:47

Looking at the code, it appears that MOC explicitly sorts the playlist when loaded from the command line but not when loaded from the client interface. I'm not sure why it does that, but the next step in the first case removes any duplicate entries and it could be that the assumption was that the function which does that removal required a sorted playlist... it doesn't.

If we remove that sort (and I haven't checked for any gotchas at this stage, so it may not be possible) it would be a change in MOC's behaviour upon which others may be relying (though it seems unlikely). In my view, the two methods of loading a playlist should be equivalent, but there are differences (-a appends to the server's playlist while 'A' effectively replaces the client's playlist, for example).

I do plan to overhaul playlists at some point and this problem will disappear, but what do we do in the meantime? I can send you a patch to try out and, if it works as expected, a new option could be added to control sorting. However, I am aware that the way MOC currently stores playlists internally may become an issue in implementing this.

And thanks for reporting your MOC revision; that is so useful to me in knowing where to start looking.

deepthought

Fri, 2015-12-04 12:40

Sry for the long break between my posts (but my login didn't work, since the password had too many characters).
Thanks for the offer of a patch, but I simply commented out the line for sorting, and it works in my case as I like to. Of course, this will prevent any ordering, so
mocp -a path/to/music/*.flac will play the files in the order, the shell expands "*", and not (necessarily) alphabetical.

Cheers,
deep42thought

jcf

Fri, 2015-12-04 19:52


Of course, this will prevent any ordering, so mocp -a path/to/music/*.flac will play the files in the order, the shell expands "*", and not (necessarily) alphabetical.

I would have thought they would be alphabetical by filename (but not necessarily by title tag). The BASH manual says (in section 3.5.8 Filename Expansion):


If one of these characters appears, then the word is regarded as a PATTERN, and replaced with an alphabetically sorted list of file names matching the pattern.

It also says it's controlled by the nocaseglob option, so it might be that you have enabled that. But if you're still not getting what you want (and it's filename ordering) then this might help: mocp -a $(ls path/to/music/*.flac).

deep42thought

Mon, 2015-12-07 20:07

jcf, you got me wrong, maybe I should rephrase it:

Right now I have, what I want - with the following patch


diff --git a/interface.c b/interface.c
index 7475be8..d36df78 100644
--- a/interface.c
+++ b/interface.c
@@ -3807,7 +3807,7 @@ void interface_cmdline_append (int server_sock, lists_t_strs *args)

if (recv_server_plist(&clients_plist)) {
add_recursively (&new, args);
- plist_sort_fname (&new);
+// plist_sort_fname (&new);

send_int_to_srv (CMD_LOCK);

@@ -3831,7 +3831,7 @@ void interface_cmdline_append (int server_sock, lists_t_strs *args)
create_file_name (PLAYLIST_FILE),
cwd, 1);
add_recursively (&new, args);
- plist_sort_fname (&new);
+// plist_sort_fname (&new);

send_int_to_srv (CMD_LOCK);
plist_remove_common_items (&new, &saved_plist);

I just tried to guess a reason, why there was a separate function to sort the elements in the first place. And the only reason I could come up with (which therefore would make my patch useless for general users) was, that play list entries need to be sorted internally, because they might be entered non-alphabetically.

Sry for the confusion,
deep42thought

diomekes

Mon, 2016-12-26 17:20

Is it planned to implement this change? It has been bothering me trying to figure out what the problem was, I'm glad to see it's an easy fix.