Rename the existing "Shuffle by album" mode to "Shuffle tracks in this album", and add a new "Shuffle albums" mode that plays all the tracks in each album sequentially, but then jumps to a different random album afterwards.

Fixes issue 1152

src/core/song.cpp

@@ -53,6 +53,7 @@
 
 #include <QFile>
 #include <QFileInfo>
+#include <QLatin1Literal>
 #include <QSqlQuery>
 #include <QtConcurrentRun>
 #include <QTextCodec>
@@ -1359,6 +1360,13 @@ bool Song::IsOnSameAlbum(const Song& other) const {
   return album() == other.album() && artist() == other.artist();
 }
 
+QString Song::AlbumKey() const {
+  return QString("%1|%2|%3").arg(
+        is_compilation() ? "_compilation" : artist(),
+        has_cue() ? cue_path() : "",
+        album());
+}
+
 void Song::ToXesam(QVariantMap* map) const {
   using mpris::AddMetadata;
   using mpris::AddMetadataAsList;

src/core/song.h

@@ -292,6 +292,11 @@ class Song {
 
   bool operator==(const Song& other) const;
 
+  // Two songs that are on the same album will have the same AlbumKey.  It is
+  // more efficient to use IsOnSameAlbum, but this function can be used when
+  // you need to hash the key to do fast lookups.
+  QString AlbumKey() const;
+
  private:
   void GuessFileType(TagLib::FileRef* fileref);
   static bool Save(const Song& song);

src/playlist/playlist.cpp

@@ -375,7 +375,7 @@ int Playlist::NextVirtualIndex(int i) const {
   PlaylistSequence::RepeatMode repeat_mode = playlist_sequence_->repeat_mode();
   PlaylistSequence::ShuffleMode shuffle_mode = playlist_sequence_->shuffle_mode();
   bool album_only = repeat_mode == PlaylistSequence::Repeat_Album ||
-                    shuffle_mode == PlaylistSequence::Shuffle_Album;
+                    shuffle_mode == PlaylistSequence::Shuffle_InsideAlbum;
 
   // This one's easy - if we have to repeat the current track then just return i
   if (repeat_mode == PlaylistSequence::Repeat_Track) {
@@ -415,7 +415,7 @@ int Playlist::PreviousVirtualIndex(int i) const {
   PlaylistSequence::RepeatMode repeat_mode = playlist_sequence_->repeat_mode();
   PlaylistSequence::ShuffleMode shuffle_mode = playlist_sequence_->shuffle_mode();
   bool album_only = repeat_mode == PlaylistSequence::Repeat_Album ||
-                    shuffle_mode == PlaylistSequence::Shuffle_Album;
+                    shuffle_mode == PlaylistSequence::Shuffle_InsideAlbum;
 
   // This one's easy - if we have to repeat the current track then just return i
   if (repeat_mode == PlaylistSequence::Repeat_Track) {
@@ -1617,17 +1617,84 @@ void Playlist::Shuffle() {
   Save();
 }
 
+namespace {
+bool AlbumShuffleComparator(const QMap<QString, int>& album_key_positions,
+                            const QMap<int, QString>& album_keys,
+                            int left, int right) {
+  const int left_pos = album_key_positions[album_keys[left]];
+  const int right_pos = album_key_positions[album_keys[right]];
+
+  if (left_pos == right_pos)
+    return left < right;
+  return left_pos < right_pos;
+}
+}
+
 void Playlist::ReshuffleIndices() {
-  if (!is_shuffled_) {
+  if (playlist_sequence_->shuffle_mode() == PlaylistSequence::Shuffle_Off) {
+    // No shuffling - sort the virtual item list normally.
     std::sort(virtual_items_.begin(), virtual_items_.end());
     if (current_row() != -1)
       current_virtual_index_ = virtual_items_.indexOf(current_row());
-  } else {
-    QList<int>::iterator begin = virtual_items_.begin();
-    if (current_virtual_index_ != -1)
-      std::advance(begin, current_virtual_index_ + 1);
+    return;
+  }
 
-    std::random_shuffle(begin, virtual_items_.end());
+  // If the user is already playing a song, advance the begin iterator to
+  // only shuffle items that haven't been played yet.
+  QList<int>::iterator begin = virtual_items_.begin();
+  QList<int>::iterator end = virtual_items_.end();
+  if (current_virtual_index_ != -1)
+    std::advance(begin, current_virtual_index_ + 1);
+
+  switch (playlist_sequence_->shuffle_mode()) {
+  case PlaylistSequence::Shuffle_Off:
+    // Handled above.
+    break;
+
+  case PlaylistSequence::Shuffle_All:
+  case PlaylistSequence::Shuffle_InsideAlbum:
+    std::random_shuffle(begin, end);
+    break;
+
+  case PlaylistSequence::Shuffle_Albums: {
+    QMap<int, QString> album_keys; // real index -> key
+    QSet<QString> album_key_set;   // unique keys
+
+    // Find all the unique albums in the playlist
+    for (QList<int>::iterator it = begin ; it != end ; ++it) {
+      const int index = *it;
+      const QString key = items_[index]->Metadata().AlbumKey();
+      album_keys[index] = key;
+      album_key_set << key;
+    }
+
+    // Shuffle them
+    QStringList shuffled_album_keys = album_key_set.toList();
+    std::random_shuffle(shuffled_album_keys.begin(),
+                        shuffled_album_keys.end());
+
+    // If the user is currently playing a song, force its album to be first.
+    if (current_virtual_index_ != -1) {
+      const QString key = items_[current_row()]->Metadata().AlbumKey();
+      const int pos = shuffled_album_keys.indexOf(key);
+      if (pos >= 1) {
+        std::swap(shuffled_album_keys[0], shuffled_album_keys[pos]);
+      }
+    }
+
+    // Create album key -> position mapping for fast lookup
+    QMap<QString, int> album_key_positions;
+    for (int i=0 ; i<shuffled_album_keys.count() ; ++i) {
+      album_key_positions[shuffled_album_keys[i]] = i;
+    }
+
+    // Sort the virtual items
+    std::stable_sort(begin, end,
+                     boost::bind(AlbumShuffleComparator, album_key_positions,
+                                 album_keys, _1, _2));
+
+    break;
+  }
   }
 }
 

src/playlist/playlistsequence.cpp

@@ -57,7 +57,8 @@ PlaylistSequence::PlaylistSequence(QWidget *parent, SettingsProvider *settings)
   QActionGroup* shuffle_group = new QActionGroup(this);
   shuffle_group->addAction(ui_->action_shuffle_off);
   shuffle_group->addAction(ui_->action_shuffle_all);
-  shuffle_group->addAction(ui_->action_shuffle_album);
+  shuffle_group->addAction(ui_->action_shuffle_inside_album);
+  shuffle_group->addAction(ui_->action_shuffle_albums);
   shuffle_menu_->addActions(shuffle_group->actions());
   ui_->shuffle->setMenu(shuffle_menu_);
 
@@ -120,8 +121,9 @@ void PlaylistSequence::RepeatActionTriggered(QAction* action) {
 
 void PlaylistSequence::ShuffleActionTriggered(QAction* action) {
   ShuffleMode mode = Shuffle_Off;
-  if (action == ui_->action_shuffle_all)   mode = Shuffle_All;
-  if (action == ui_->action_shuffle_album) mode = Shuffle_Album;
+  if (action == ui_->action_shuffle_all)          mode = Shuffle_All;
+  if (action == ui_->action_shuffle_inside_album) mode = Shuffle_InsideAlbum;
+  if (action == ui_->action_shuffle_albums)       mode = Shuffle_Albums;
 
   SetShuffleMode(mode);
 }
@@ -148,9 +150,10 @@ void PlaylistSequence::SetShuffleMode(ShuffleMode mode) {
   ui_->shuffle->setChecked(mode != Shuffle_Off);
 
   switch (mode) {
-    case Shuffle_Off:   ui_->action_shuffle_off->setChecked(true);   break;
-    case Shuffle_All:   ui_->action_shuffle_all->setChecked(true);   break;
-    case Shuffle_Album: ui_->action_shuffle_album->setChecked(true); break;
+    case Shuffle_Off:         ui_->action_shuffle_off->setChecked(true);          break;
+    case Shuffle_All:         ui_->action_shuffle_all->setChecked(true);          break;
+    case Shuffle_InsideAlbum: ui_->action_shuffle_inside_album->setChecked(true); break;
+    case Shuffle_Albums:      ui_->action_shuffle_albums->setChecked(true);       break;
   }
 
 
@@ -184,9 +187,10 @@ void PlaylistSequence::CycleShuffleMode() {
   ShuffleMode mode = Shuffle_Off;
   //we cycle through the shuffle modes
   switch (shuffle_mode()) {
-    case Shuffle_Off:   mode = Shuffle_All;   break;
-    case Shuffle_All:   mode = Shuffle_Album;   break;
-    case Shuffle_Album: break;
+    case Shuffle_Off:         mode = Shuffle_All;           break;
+    case Shuffle_All:         mode = Shuffle_InsideAlbum;   break;
+    case Shuffle_InsideAlbum: mode = Shuffle_Albums;        break;
+    case Shuffle_Albums: break;
   }
 
   SetShuffleMode(mode);

src/playlist/playlistsequence.h

@@ -44,7 +44,8 @@ class PlaylistSequence : public QWidget {
   enum ShuffleMode {
     Shuffle_Off = 0,
     Shuffle_All = 1,
-    Shuffle_Album = 2,
+    Shuffle_InsideAlbum = 2,
+    Shuffle_Albums = 3,
   };
 
   static const char* kSettingsGroup;

src/playlist/playlistsequence.ui

@@ -109,12 +109,12 @@
     <string>Don't shuffle</string>
    </property>
   </action>
-  <action name="action_shuffle_album">
+  <action name="action_shuffle_inside_album">
    <property name="checkable">
     <bool>true</bool>
    </property>
    <property name="text">
-    <string>Shuffle by album</string>
+    <string>Shuffle tracks in this album</string>
    </property>
   </action>
   <action name="action_shuffle_all">
@@ -125,6 +125,14 @@
     <string>Shuffle all</string>
    </property>
   </action>
+  <action name="action_shuffle_albums">
+   <property name="checkable">
+    <bool>true</bool>
+   </property>
+   <property name="text">
+    <string>Shuffle albums</string>
+   </property>
+  </action>
  </widget>
  <resources/>
  <connections/>

src/widgets/osd.cpp

@@ -291,9 +291,10 @@ void OSD::ShuffleModeChanged(PlaylistSequence::ShuffleMode mode) {
   if (show_on_play_mode_change_) {
     QString current_mode = QString();
     switch (mode) {
-      case PlaylistSequence::Shuffle_Off:   current_mode = tr("Don't shuffle");   break;
-      case PlaylistSequence::Shuffle_All:   current_mode = tr("Shuffle all");   break;
-      case PlaylistSequence::Shuffle_Album: current_mode = tr("Shuffle by album"); break;
+      case PlaylistSequence::Shuffle_Off:         current_mode = tr("Don't shuffle");   break;
+      case PlaylistSequence::Shuffle_All:         current_mode = tr("Shuffle all");     break;
+      case PlaylistSequence::Shuffle_InsideAlbum: current_mode = tr("Shuffle tracks in this album"); break;
+      case PlaylistSequence::Shuffle_Albums:      current_mode = tr("Shuffle albums");  break;
     }
     ShowMessage(QCoreApplication::applicationName(), current_mode);
   }