From 6387a01d7b892c65937b03b7faac9125f6d62d8d Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Sun, 3 Nov 2019 23:23:04 +0100 Subject: [PATCH] Fix updating compilations Fixes #288 --- src/collection/collectionbackend.cpp | 58 +++++++++++++++------------- src/collection/collectionbackend.h | 12 +++--- src/collection/collectionmodel.cpp | 1 - src/core/song.cpp | 7 +++- src/core/song.h | 7 +++- src/dialogs/edittagdialog.cpp | 4 +- 6 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/collection/collectionbackend.cpp b/src/collection/collectionbackend.cpp index 884c58e50..9c9082225 100644 --- a/src/collection/collectionbackend.cpp +++ b/src/collection/collectionbackend.cpp @@ -904,28 +904,30 @@ void CollectionBackend::UpdateCompilations() { while (q.next()) { QString artist = q.value(0).toString(); QString album = q.value(1).toString(); - QString filename = q.value(2).toString(); + QUrl url = QUrl::fromEncoded(q.value(2).toString().toUtf8()); bool compilation_detected = q.value(3).toBool(); // Ignore songs that don't have an album field set if (album.isEmpty()) continue; // Find the directory the song is in - int last_separator = filename.lastIndexOf('/'); - if (last_separator == -1) continue; + QString directory = url.toString(QUrl::PreferLocalFile|QUrl::RemoveFilename|QUrl::StripTrailingSlash); - CompilationInfo &info = compilation_info[album]; + CompilationInfo &info = compilation_info[directory + album]; + info.urls << url; + info.directory = directory; + info.album = album; info.artists.insert(artist); - info.directories.insert(filename.left(last_separator)); - if (compilation_detected) info.has_compilation_detected = true; - else info.has_not_compilation_detected = true; + if (compilation_detected) info.has_compilation_detected++; + else info.has_not_compilation_detected++; } // Now mark the songs that we think are in compilations - QSqlQuery update(db); - update.prepare(QString("UPDATE %1 SET compilation_detected = :compilation_detected, compilation_effective = ((compilation OR :compilation_detected OR compilation_on) AND NOT compilation_off) + 0 WHERE album = :album AND unavailable = 0").arg(songs_table_)); QSqlQuery find_songs(db); - find_songs.prepare(QString("SELECT ROWID, " + Song::kColumnSpec + " FROM %1 WHERE album = :album AND compilation_detected = :compilation_detected AND unavailable = 0").arg(songs_table_)); + find_songs.prepare(QString("SELECT ROWID, " + Song::kColumnSpec + " FROM %1 WHERE url = :url AND compilation_detected = :compilation_detected AND unavailable = 0").arg(songs_table_)); + + QSqlQuery update_songs(db); + update_songs.prepare(QString("UPDATE %1 SET compilation_detected = :compilation_detected, compilation_effective = ((compilation OR :compilation_detected OR compilation_on) AND NOT compilation_off) + 0 WHERE url = :url AND unavailable = 0").arg(songs_table_)); SongList deleted_songs; SongList added_songs; @@ -935,17 +937,18 @@ void CollectionBackend::UpdateCompilations() { QMap::const_iterator it = compilation_info.constBegin(); for (; it != compilation_info.constEnd(); ++it) { const CompilationInfo &info = it.value(); - QString album(it.key()); // If there were more 'effective album artists' than there were directories for this album then it's a compilation. - if (info.artists.count() > info.directories.count()) { - if (info.has_not_compilation_detected) - UpdateCompilations(find_songs, update, deleted_songs, added_songs, album, 1); - } - else { - if (info.has_compilation_detected) - UpdateCompilations(find_songs, update, deleted_songs, added_songs, album, 0); + for (const QUrl &url : info.urls) { + if (info.artists.count() > 1) { // This directory+album is a compilation. + if (info.has_not_compilation_detected > 0) // Run updates if any of the songs is not marked as compilations. + UpdateCompilations(find_songs, update_songs, deleted_songs, added_songs, url, true); + } + else { + if (info.has_compilation_detected > 0) + UpdateCompilations(find_songs, update_songs, deleted_songs, added_songs, url, false); + } } } @@ -955,27 +958,28 @@ void CollectionBackend::UpdateCompilations() { emit SongsDeleted(deleted_songs); emit SongsDiscovered(added_songs); } + } -void CollectionBackend::UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update, SongList &deleted_songs, SongList &added_songs, const QString &album, int compilation_detected) { +void CollectionBackend::UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update_songs, SongList &deleted_songs, SongList &added_songs, const QUrl &url, const bool compilation_detected) { - // Get songs that were already in that album, so we can tell the model they've been updated - find_songs.bindValue(":album", album); + // Get song, so we can tell the model its updated + find_songs.bindValue(":url", url.toString()); find_songs.bindValue(":compilation_detected", int(!compilation_detected)); find_songs.exec(); while (find_songs.next()) { Song song; song.InitFromQuery(find_songs, true); deleted_songs << song; - song.set_compilation_detected(true); + song.set_compilation_detected(compilation_detected); added_songs << song; } - // Mark this album - update.bindValue(":compilation_detected", compilation_detected); - update.bindValue(":album", album); - update.exec(); - db_->CheckErrors(update); + // Update the song + update_songs.bindValue(":compilation_detected", int(compilation_detected)); + update_songs.bindValue(":url", url); + update_songs.exec(); + db_->CheckErrors(update_songs); } diff --git a/src/collection/collectionbackend.h b/src/collection/collectionbackend.h index 33ffb81d4..2cbe7e915 100644 --- a/src/collection/collectionbackend.h +++ b/src/collection/collectionbackend.h @@ -225,16 +225,18 @@ class CollectionBackend : public CollectionBackendInterface { private: struct CompilationInfo { - CompilationInfo() : has_compilation_detected(false), has_not_compilation_detected(false) {} + CompilationInfo() : has_compilation_detected(0), has_not_compilation_detected(0) {} + QString directory; + QString album; + QList urls; QSet artists; - QSet directories; - bool has_compilation_detected; - bool has_not_compilation_detected; + int has_compilation_detected; + int has_not_compilation_detected; }; - void UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update, SongList &deleted_songs, SongList &added_songs, const QString &album, int compilation_detected); + void UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update_songs, SongList &deleted_songs, SongList &added_songs, const QUrl &url, const bool compilation_detected); AlbumList GetAlbums(const QString &artist, const QString &album_artist, bool compilation = false, const QueryOptions &opt = QueryOptions()); AlbumList GetAlbums(const QString &artist, bool compilation, const QueryOptions &opt = QueryOptions()); SubdirectoryList SubdirsInDirectory(int id, QSqlDatabase &db); diff --git a/src/collection/collectionmodel.cpp b/src/collection/collectionmodel.cpp index 5e65e5e8b..76ad5b414 100644 --- a/src/collection/collectionmodel.cpp +++ b/src/collection/collectionmodel.cpp @@ -449,7 +449,6 @@ void CollectionModel::SongsDeleted(const SongList &songs) { break; } } - beginRemoveRows(idx, node->row, node->row); node->parent->Delete(node->row); song_nodes_.remove(song.id()); diff --git a/src/core/song.cpp b/src/core/song.cpp index 24fba48ea..992373c5d 100644 --- a/src/core/song.cpp +++ b/src/core/song.cpp @@ -304,7 +304,7 @@ int Song::year() const { return d->year_; } int Song::originalyear() const { return d->originalyear_; } int Song::effective_originalyear() const { return d->originalyear_ < 0 ? d->year_ : d->originalyear_; } const QString &Song::genre() const { return d->genre_; } -bool Song::is_compilation() const { return (d->compilation_ || d->compilation_detected_ || d->compilation_on_) && ! d->compilation_off_; } +bool Song::compilation() const { return d->compilation_; } const QString &Song::composer() const { return d->composer_; } const QString &Song::performer() const { return d->performer_; } const QString &Song::grouping() const { return d->grouping_; } @@ -332,6 +332,10 @@ int Song::playcount() const { return d->playcount_; } int Song::skipcount() const { return d->skipcount_; } int Song::lastplayed() const { return d->lastplayed_; } +bool Song::compilation_detected() const { return d->compilation_detected_; } +bool Song::compilation_off() const { return d->compilation_off_; } +bool Song::compilation_on() const { return d->compilation_on_; } + const QUrl &Song::art_automatic() const { return d->art_automatic_; } const QUrl &Song::art_manual() const { return d->art_manual_; } bool Song::has_manually_unset_cover() const { return d->art_manual_.path() == kManuallyUnsetCover; } @@ -350,6 +354,7 @@ bool Song::is_collection_song() const { return d->source_ == Source_Collection; bool Song::is_metadata_good() const { return !d->title_.isEmpty() && !d->album_.isEmpty() && !d->artist_.isEmpty() && !d->url_.isEmpty() && d->end_ > 0; } bool Song::is_stream() const { return d->source_ == Source_Stream || d->source_ == Source_Tidal || d->source_ == Source_Subsonic || d->source_ == Source_Qobuz; } bool Song::is_cdda() const { return d->source_ == Source_CDDA; } +bool Song::is_compilation() const { return (d->compilation_ || d->compilation_detected_ || d->compilation_on_) && !d->compilation_off_; } bool Song::art_automatic_is_valid() const { return ( diff --git a/src/core/song.h b/src/core/song.h index db7bade45..d512c94e1 100644 --- a/src/core/song.h +++ b/src/core/song.h @@ -200,7 +200,7 @@ class Song { int year() const; int originalyear() const; const QString &genre() const; - bool is_compilation() const; + bool compilation() const; const QString &composer() const; const QString &performer() const; const QString &grouping() const; @@ -232,6 +232,10 @@ class Song { int skipcount() const; int lastplayed() const; + bool compilation_detected() const; + bool compilation_off() const; + bool compilation_on() const; + const QUrl &art_automatic() const; const QUrl &art_manual() const; @@ -249,6 +253,7 @@ class Song { bool is_metadata_good() const; bool art_automatic_is_valid() const; bool art_manual_is_valid() const; + bool is_compilation() const; // Playlist views are special because you don't want to fill in album artists automatically for compilations, but you do for normal albums: const QString &playlist_albumartist() const; diff --git a/src/dialogs/edittagdialog.cpp b/src/dialogs/edittagdialog.cpp index 7837cf0a9..afffd8023 100644 --- a/src/dialogs/edittagdialog.cpp +++ b/src/dialogs/edittagdialog.cpp @@ -914,9 +914,7 @@ void EditTagDialog::SongSaveComplete(TagReaderReply *reply, const QString &filen emit Error(message); } else if (song.directory_id() != -1) { - SongList songs; - songs << song; - app_->collection_backend()->AddOrUpdateSongs(songs); + app_->collection_backend()->AddOrUpdateSongs(SongList() << song); } if (pending_ <= 0) AcceptFinished();