Fix a bug in InsertItems functions that makes Cdda tracks to not be saved/restored correctly when restarting Clementine

src/core/song.cpp

@@ -299,6 +299,9 @@ int Song::filesize() const { return d->filesize_; }
 Song::FileType Song::filetype() const { return d->filetype_; }
 bool Song::is_stream() const { return d->filetype_ == Type_Stream; }
 bool Song::is_cdda() const { return d->filetype_ == Type_Cdda; }
+bool Song::is_library_song() const {
+  return !is_cdda() && !is_stream() && id() != -1;
+}
 const QString& Song::art_automatic() const { return d->art_automatic_; }
 const QString& Song::art_manual() const { return d->art_manual_; }
 const QString& Song::etag() const { return d->etag_; }

src/core/song.h

@@ -192,6 +192,8 @@ class Song {
   FileType filetype() const;
   bool is_stream() const;
   bool is_cdda() const;
+  // Guess if it is a library song using fields set. Might not be 100% reliable.
+  bool is_library_song() const;
 
   const QString& art_automatic() const;
   const QString& art_manual() const;

src/devices/cddasongloader.cpp

@@ -182,8 +182,8 @@ void CddaSongLoader::AudioCDTagsLoaded(
       qobject_cast<MusicBrainzClient*>(sender());
   musicbrainz_client->deleteLater();
   SongList songs;
-  int track_number = 1;
   if (results.size() == 0) return;
+  int track_number = 1;
   for (const MusicBrainzClient::Result& ret : results) {
     Song song;
     song.set_artist(artist);
@@ -193,6 +193,8 @@ void CddaSongLoader::AudioCDTagsLoaded(
     song.set_track(track_number);
     song.set_year(ret.year_);
     song.set_id(track_number);
+    song.set_filetype(Song::Type_Cdda);
+    song.set_valid(true);
     // We need to set url: that's how playlist will find the correct item to
     // update
     song.set_url(GetUrlFromTrack(track_number++));

src/playlist/playlist.cpp

@@ -1082,10 +1082,11 @@ void Playlist::InsertSongsOrLibraryItems(const SongList& songs, int pos,
                                          bool play_now, bool enqueue) {
   PlaylistItemList items;
   for (const Song& song : songs) {
-    if (song.id() == -1)
-      items << PlaylistItemPtr(new SongPlaylistItem(song));
-    else
+    if (song.is_library_song()) {
       items << PlaylistItemPtr(new LibraryPlaylistItem(song));
+    } else {
+      items << PlaylistItemPtr(new SongPlaylistItem(song));
+    }
   }
   InsertItems(items, pos, play_now, enqueue);
 }
@@ -1155,11 +1156,11 @@ void Playlist::UpdateItems(const SongList& songs) {
            // And CD tracks as well (tags are loaded in a second step)
            item->Metadata().filetype() == Song::Type_Cdda)) {
         PlaylistItemPtr new_item;
-        if (song.id() == -1) {
-          new_item = PlaylistItemPtr(new SongPlaylistItem(song));
-        } else {
+        if (song.is_library_song()) {
           new_item = PlaylistItemPtr(new LibraryPlaylistItem(song));
           library_items_by_id_.insertMulti(song.id(), new_item);
+        } else {
+          new_item = PlaylistItemPtr(new SongPlaylistItem(song));
         }
         items_[i] = new_item;
         emit dataChanged(index(i, 0), index(i, ColumnCount - 1));

src/playlist/songloaderinserter.cpp

@@ -116,7 +116,7 @@ void SongLoaderInserter::AudioCDTagsLoaded(bool success) {
 }
 
 void SongLoaderInserter::InsertSongs() {
-  // Insert songs (that haven't been completelly loaded) to allow user to see
+  // Insert songs (that haven't been completely loaded) to allow user to see
   // and play them while not loaded completely
   if (destination_) {
     destination_->InsertSongsOrLibraryItems(songs_, row_, play_now_, enqueue_);