I think a better solution might be to fix the "shuffle" option to make sure that
songs don't repeat. Sometimes I hear the same song twice in a row if the
playlist is sufficiently short.

I was about to file this bug again, I also has problems with the random algorithm, I liked the way that xmms does, it never repeats any song until all the songs of the playlist are played.

There are plangs to change this?

This is exactly what the shuffle play order is supposed to do.  The play order you get by selecting shuffle and repeat does not try to avoid repeats.  If you are experiencing repeats with the shuffle play order, please provide more details.

What I like about a "randomize playlist" function is that it allows the users to reorder a bit its playlist if he's not totally satisfied with what the player gave him.

Related to this is a small bit of bug 129785.

What would solve this is an explicit "Sort Playlist" action, with one of the options for sorting being "random".

For me, songs typically don't repeat with the shuffle option; however, music from the same 3-5 artists tends to dominate the shuffled play despite having up to 147 artists to choose from. I have to close rhythmbox (0.10.0) completely to reset the shuffle, but the next time it will just be a different 3-5 artists which dominate the shuffled play. I have tried sorting the list by other categories (i.e. title instead of artist) before playing but it doesn't seem to have much effect. 

Is there some way to add an option to the shuffle algorithm so that there is more "even" representation of all artists in the shuffled play?

The way I've discovered that that I like to do is to set up an automatic playlist which is set to something like "songs not heard in last 20 days" and then do shuffle mode on that playlist all the time.  Occasionally I adjust the number of days to readjust the amount of music in the playlist.

This gives me basically everything I wanted from the "randomize playlist" mode, except better since it works from the library.

(In reply to comment #3)
> This is exactly what the shuffle play order is supposed to do.  The play order
> you get by selecting shuffle and repeat does not try to avoid repeats.  If you
> are experiencing repeats with the shuffle play order, please provide more
> details.
> 

I really disagree.  If I have a playlist for an album (say 12-20 songs) and I select "repeat" and "shuffle" I expect it to play through all the songs in a random order and then repeat the entire playlist.  This is not just my personal preference, this is how most music players work including every home stereo, discman, mp3 player, and ghetto-blaster I have ever owned as well as most software music players.

The problem with deselecting "repeat" is that after all the songs have played through, it will stop playing.  I want it to keep playing, but just through *all* of the songs in a random order before repeating.  If there is an argument to have it repeat songs before playing through all (I doubt there is...), perhaps that behaviour could be turned on with an option in preferences.

Moreover, there are some serious problems with the randomization algorithm in RhythmBox.  It repeats songs very often on a short playlist like mentioned above.  More suprisingly, it often repeats songs when I am playing through my entire library (some 3000 songs)!  The other comments about repeating artists and always playing the same songs from day to day and leaving thousands unplayed are also true.

Finally, I think these problems have shown up fairly recently (last 6 months or so).  I am sorry I don't know the exact version bump where it occurred but there was a definite point in time when the randomization started sucking.  Suddenly it started repeating songs and I thought, at first, it was some strange one-in-a-million chance.  However, it happens all the time.  For example, as I typed up this comment, RhythmBox repeated one song three times and two other songs twice.  The playlist has 12 songs and only one other songs has been played.  So the play order was like this:  A A B A C B D C where each letter represents a different song.  In my opinion, this is an important bug (don't lots of people listen to albums on repeat and shuffle?  This is the main way I use music players).

I am using gentoo linux on AMD64.  Rhythmbox 0.10.0. My library is a mix of ogg and mp3 files, primarily oggs.  When a song repeats, I usually skip to the next song (wouldn't most people), I mention this because maybe this screws up the randomization algorithm further.

As far as the randomization algorithm repeating, please do a Google search for "Birthday Paradox"

Created attachment 99993 [details] [review]
Gives "shuffle playlist" functionality.

Doesn't save the playlist after it's been shuffled.
Also, the option appears on automatic playlists, where it probably doesn't work.

The diff I posted is a patch to add "shuffle playlist" functionality.  It's not ready to be committed.  I can't figure out why, but it doesn't save the playlist after it's been shuffled.  I'm hoping that someone can help me figure out why that is.  Thanks.

I think I've got a fix for the playlist not being saved in shuffled order.  I'll commit it once I've done a bit more testing to make sure it doesn't break anything else.

I don't really like the way you're passing data to _shuffle_track_cb.  It would be nicer to create a new structure type containing whatever data the function needs, create an instance of that on the stack, and pass a pointer to that.

There's no need to declare _shuffle_track_cb as taking two gpointers.  Declare it as taking whatever pointer types you want, then cast it to GFunc in the call to g_list_foreach.

The shuffle playlist action should be disabled (or made invisible) when an automatic playlist is selected.  rb_playlist_manager_set_source is probably the place to do this, and there are a few examples there of how to do it.

Alternatively, we could move the shuffling code inside RhythmDBQueryModel.  It could be implemented more efficiently there (it's probably O(N^2) or worse now), and we could also make it work for automatic playlists.

I've committed a change that fixes rb_static_playlist_move_entry, so this should work now.

2007-12-02  Jonathan Matthew  <jonathan@d14n.org>

        * sources/rb-static-playlist-source.c:
        (rb_static_playlist_source_move_entry):
        Move entries in the base query model, not the displayed query model.
        Otherwise, the order change is lost when saving the playlist.

Created attachment 100089 [details] [review]
Patch against 5470

Okay, here's a patch against svn 5470.  It has the randomize playlist feature, including the suggestions that were made:
* It passes data to _shuffle_track_cb in a more sane way.
* It ghosts the "Shuffle" option on non-static playlists.

I did not move it to the query model, since this is (so far) working for me (my playlists are never crazay big).

I can start hunting for a O(N) way of doing this, though, if we decide we'd like to.

As for putting this into smart playlists, I'm just a little concerned about conflicting with the existing sorting functions.  It may not be as much of a problem as I think.

Thanks so much.

No new comments lately...  Are we waiting on me?

Created attachment 101080 [details] [review]
Patch against 5492

This latest patch implements the remaining recommendation -- I've moved the shuffle functionality inside of RhythmDBQueryModel, so as to do the shuffle in O(n) time instead of O(n^2).

Generally looks good to me.  I haven't looked at the actual shuffling algorithm in detail, though.

+	/* Emit reorder signals. */
+	for (i=0; i<listsize; i++) {
+		rhythmdb_query_model_emit_reorder (model, i, map_old_new[i]);
+	}
+

This is fairly inefficient, though.  If you instead generated a map_new_old array that was the inverse of map_old_new, you could make a single call to gtk_tree_model_rows_reordered:

  GtkTreePath *path;
  GtkTreeIter iter;

  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (model), &iter);
  path = gtk_tree_model_get_path (GTK_TREE_MODEL (model), &iter);
  gtk_tree_model_rows_reordered (GTK_TREE_MODEL (model), path, &iter, map_new_old);
  gtk_tree_path_free (path);

Created attachment 101198 [details] [review]
Patch against 5495

Okay.  Here's a patch against 5495 that contains your changes.  I didn't know about the ability to emit all the changes in one call.  Good catch.

Anything I should be doing?  I can't think of anything.

I finally got around to testing out this version of the patch, and everything seems to work nicely.  It shuffles the 14000 in a barely noticeable amount of time, doesn't leak memory as far as I can tell.  The only remaining thing I don't entirely like is that a new rb_static_playlist_source_x function is required, but there's no real way around that..

I've fixed up a few minor code style things and committed the patch to svn.  There's a bit more to be done to make this work for the play queue too, but I'll take care of that.  Thanks for working on this.

Excellent.  This is the greatest christmas present ever.

Thank you very much.

Re making it work for the play queue:  I was wondering if the shuffle function could be made a method of rb_playlist_source, with implementations in rb-static-playlist-source, etc.  I'm not sure if that would help for the play queue, though -- does the play queue inherit from rb_playlist_source?

Anyway, that might help get rid of the rb_static_playlist_source_x call in there.  I was just too unfamiliar with this OOP-in-C idiom.

Something else I'm working on will probably let us get rid of the rb_static_playlist_source method and (in theory) make this work for any source.  I'm considering adding a 'base-query-model' source property so things like daap can get unfiltered access to playlists, and the same thing would help here.

Just for my understanding, after the work in this bug has been applied to a RB release, does it only work on playlists or will it work if I'm playing out of the "Music" "Library" as well?

Secondly, should I expect that given this patch, that my "Play Count" should start to equalize?  That is, will the fact that I have some tracks which have been played dozens of times vs. others that have never been played be taken into account when "shuffl"ing?  i.e. will the tracks at the high end of the Play Count be ignored until the ones that are at the low end have been played more times to catch up to the high end?

Thirdly this all applies to the "shuffle" play_order, yes?

Thanx in advance for your comments.

No, this just shuffles static playlists into a random order.