diff --git a/src/libs/database/impl/Artist.cpp b/src/libs/database/impl/Artist.cpp index d7906600..57ff3c50 100644 --- a/src/libs/database/impl/Artist.cpp +++ b/src/libs/database/impl/Artist.cpp @@ -21,9 +21,9 @@ #include #include "core/ILogger.hpp" +#include "database/Artwork.hpp" #include "database/Cluster.hpp" #include "database/Directory.hpp" -#include "database/Image.hpp" #include "database/Release.hpp" #include "database/Session.hpp" #include "database/Track.hpp" @@ -315,14 +315,14 @@ AND NOT EXISTS ( return getMBID().has_value(); } - ObjectPtr Artist::getImage() const + ObjectPtr Artist::getPreferredArtwork() const { - return ObjectPtr{ _image }; + return ObjectPtr{ _preferredArtwork }; } - ImageId Artist::getImageId() const + ArtworkId Artist::getPreferredArtworkId() const { - return _image.id(); + return _preferredArtwork.id(); } RangeResults Artist::findSimilarArtistIds(core::EnumSet artistLinkTypes, std::optional range) const @@ -421,8 +421,8 @@ AND NOT EXISTS ( LMS_LOG(DB, WARNING, "Artist sort name too long, truncated to '" << _sortName << "'"); } - void Artist::setImage(ObjectPtr image) + void Artist::setPreferredArtwork(ObjectPtr artwork) { - _image = getDboPtr(image); + _preferredArtwork = getDboPtr(artwork); } } // namespace lms::db diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp index 1f0e7e6b..63a67cc6 100644 --- a/src/libs/database/impl/Migration.cpp +++ b/src/libs/database/impl/Migration.cpp @@ -1245,7 +1245,6 @@ FROM tracklist)"); "comment" text not null, "preferred_artwork_id" bigint, constraint "fk_release_preferred_artwork" foreign key ("preferred_artwork_id") references "artwork" ("id") on delete set null deferrable initially deferred))"); - // Migrate data, with the new preferred_artwork_id field set to null utils::executeCommand(*session.getDboSession(), R"(INSERT INTO release_backup SELECT @@ -1310,7 +1309,6 @@ FROM release)"); constraint "fk_track_preferred_artwork" foreign key ("preferred_artwork_id") references "artwork" ("id") on delete set null deferrable initially deferred, constraint "fk_track_preferred_media_artwork" foreign key ("preferred_media_artwork_id") references "artwork" ("id") on delete set null deferrable initially deferred ))"); - // Migrate data, with the new preferred_artwork_id and preferred_media_artwork_id fields set to null utils::executeCommand(*session.getDboSession(), R"(INSERT INTO track_backup SELECT @@ -1354,6 +1352,30 @@ FROM track)"); utils::executeCommand(*session.getDboSession(), "DROP TABLE track"); utils::executeCommand(*session.getDboSession(), "ALTER TABLE track_backup RENAME TO track"); + // Replaced image by artwork for artist + utils::executeCommand(*session.getDboSession(), R"(CREATE TABLE IF NOT EXISTS "artist_backup" ( + "id" integer primary key autoincrement, + "version" integer not null, + "name" text not null, + "sort_name" text not null, + "mbid" text not null, + "preferred_artwork_id" bigint, + constraint "fk_artist_preferred_artwork" foreign key ("preferred_artwork_id") references "artwork" ("id") on delete set null deferrable initially deferred + ))"); + // Migrate data, with the new preferred_artwork_id field set to null + utils::executeCommand(*session.getDboSession(), R"(INSERT INTO artist_backup +SELECT + id, + version, + name, + sort_name, + mbid, + NULL as preferred_artwork_id +FROM artist)"); + + utils::executeCommand(*session.getDboSession(), "DROP TABLE artist"); + utils::executeCommand(*session.getDboSession(), "ALTER TABLE artist_backup RENAME TO artist"); + // Just increment the scan version of the settings to make the next scan rescan everything utils::executeCommand(*session.getDboSession(), "UPDATE scan_settings SET audio_scan_version = audio_scan_version + 1"); } diff --git a/src/libs/database/impl/Session.cpp b/src/libs/database/impl/Session.cpp index cc6f9dad..deac39c9 100644 --- a/src/libs/database/impl/Session.cpp +++ b/src/libs/database/impl/Session.cpp @@ -198,7 +198,6 @@ namespace lms::db { auto transaction{ createWriteTransaction() }; utils::executeCommand(_session, "CREATE INDEX IF NOT EXISTS artist_id_idx ON artist(id)"); - utils::executeCommand(_session, "CREATE INDEX IF NOT EXISTS artist_image_idx ON artist(image_id)"); utils::executeCommand(_session, "CREATE INDEX IF NOT EXISTS artist_name_mbid_idx ON artist(name, mbid)"); utils::executeCommand(_session, "CREATE INDEX IF NOT EXISTS artist_sort_name_nocase_idx ON artist(sort_name COLLATE NOCASE)"); utils::executeCommand(_session, "CREATE INDEX IF NOT EXISTS artist_mbid_idx ON artist(mbid)"); diff --git a/src/libs/database/include/database/Artist.hpp b/src/libs/database/include/database/Artist.hpp index 39ee300d..86972a75 100644 --- a/src/libs/database/include/database/Artist.hpp +++ b/src/libs/database/include/database/Artist.hpp @@ -31,9 +31,9 @@ #include "core/EnumSet.hpp" #include "core/UUID.hpp" #include "database/ArtistId.hpp" +#include "database/ArtworkId.hpp" #include "database/ClusterId.hpp" #include "database/Filters.hpp" -#include "database/ImageId.hpp" #include "database/MediaLibraryId.hpp" #include "database/Object.hpp" #include "database/ReleaseId.hpp" @@ -43,10 +43,9 @@ namespace lms::db { - + class Artwork; class Cluster; class ClusterType; - class Image; class Release; class Session; class StarredArtist; @@ -137,8 +136,8 @@ namespace lms::db const std::string& getSortName() const { return _sortName; } std::optional getMBID() const; bool hasMBID() const; - ObjectPtr getImage() const; - ImageId getImageId() const; + ObjectPtr getPreferredArtwork() const; + ArtworkId getPreferredArtworkId() const; void visitLinks(std::function& link)> visitor) const; // No artistLinkTypes means get them all @@ -152,7 +151,7 @@ namespace lms::db void setName(std::string_view name); void setMBID(const std::optional& mbid) { _mbid = mbid ? mbid->getAsString() : ""; } void setSortName(std::string_view sortName); - void setImage(ObjectPtr image); + void setPreferredArtwork(ObjectPtr artwork); template void persist(Action& a) @@ -161,7 +160,7 @@ namespace lms::db Wt::Dbo::field(a, _sortName, "sort_name"); Wt::Dbo::field(a, _mbid, "mbid"); - Wt::Dbo::belongsTo(a, _image, "image", Wt::Dbo::OnDeleteSetNull); + Wt::Dbo::belongsTo(a, _preferredArtwork, "preferred_artwork", Wt::Dbo::OnDeleteSetNull); Wt::Dbo::hasMany(a, _trackArtistLinks, Wt::Dbo::ManyToOne, "artist"); Wt::Dbo::hasMany(a, _starredArtists, Wt::Dbo::ManyToMany, "user_starred_artists", "", Wt::Dbo::OnDeleteCascade); } @@ -178,7 +177,7 @@ namespace lms::db std::string _sortName; std::string _mbid; // Musicbrainz Identifier - Wt::Dbo::ptr _image; + Wt::Dbo::ptr _preferredArtwork; Wt::Dbo::collection> _trackArtistLinks; // Tracks involving this artist Wt::Dbo::collection> _starredArtists; // starred entries for this artist }; diff --git a/src/libs/database/include/database/Image.hpp b/src/libs/database/include/database/Image.hpp index 816b4785..5361fabc 100644 --- a/src/libs/database/include/database/Image.hpp +++ b/src/libs/database/include/database/Image.hpp @@ -32,7 +32,6 @@ namespace lms::db { - class Artist; class Directory; class Session; @@ -99,7 +98,6 @@ namespace lms::db Wt::Dbo::field(a, _width, "width"); Wt::Dbo::field(a, _height, "height"); - Wt::Dbo::hasMany(a, _artists, Wt::Dbo::ManyToOne, "image"); Wt::Dbo::belongsTo(a, _directory, "directory", Wt::Dbo::OnDeleteCascade); } @@ -115,7 +113,6 @@ namespace lms::db int _width{}; int _height{}; - Wt::Dbo::collection> _artists; Wt::Dbo::ptr _directory; }; } // namespace lms::db diff --git a/src/libs/database/test/Artist.cpp b/src/libs/database/test/Artist.cpp index f7228502..1b82997b 100644 --- a/src/libs/database/test/Artist.cpp +++ b/src/libs/database/test/Artist.cpp @@ -19,10 +19,12 @@ #include "Common.hpp" +#include "database/Artwork.hpp" #include "database/Image.hpp" namespace lms::db::tests { + using ScopedArtwork = ScopedEntity; using ScopedImage = ScopedEntity; TEST_F(DatabaseFixture, Artist) @@ -711,27 +713,28 @@ namespace lms::db::tests } } - TEST_F(DatabaseFixture, Artist_image) + TEST_F(DatabaseFixture, Artist_artwork) { - ScopedArtist release{ session, "MyArtist" }; + ScopedImage image{ session, "/image1.jpg" }; + ScopedArtwork artwork{ session, image.lockAndGet() }; + + ScopedArtist artist{ session, "MyArtist" }; { auto transaction{ session.createReadTransaction() }; - EXPECT_FALSE(release.get()->getImage()); + EXPECT_FALSE(artist.get()->getPreferredArtwork()); } - ScopedImage image{ session, "/myImage" }; - { auto transaction{ session.createWriteTransaction() }; - release.get().modify()->setImage(image.get()); + artist.get().modify()->setPreferredArtwork(artwork.get()); } { auto transaction{ session.createReadTransaction() }; - auto artistImage(release.get()->getImage()); - ASSERT_TRUE(artistImage); - EXPECT_EQ(artistImage->getId(), image.getId()); + auto artistArtwork(artist.get()->getPreferredArtwork()); + ASSERT_TRUE(artistArtwork); + EXPECT_EQ(artistArtwork->getId(), artwork.getId()); } } diff --git a/src/libs/services/artwork/impl/ArtworkService.cpp b/src/libs/services/artwork/impl/ArtworkService.cpp index c39573d6..1ac708a9 100644 --- a/src/libs/services/artwork/impl/ArtworkService.cpp +++ b/src/libs/services/artwork/impl/ArtworkService.cpp @@ -143,14 +143,15 @@ namespace lms::artwork ImageFindResult res; - if (const db::Artist::pointer artist{ db::Artist::find(session, artistId) }) - { - if (const db::ImageId imageId{ artist->getImageId() }; imageId.isValid()) - res = imageId; + const db::Artist::pointer artist{ db::Artist::find(session, artistId) }; + if (!artist) + return res; - // TODO fallback on embedded Band/LeadArtist/Artist? - // TODO fallback on first release? - } + const db::Artwork::pointer artwork{ artist->getPreferredArtwork() }; + if (artwork && artwork->getImageId().isValid()) + res = artwork->getImageId(); + else if (artwork && artwork->getTrackEmbeddedImageId().isValid()) + res = artwork->getTrackEmbeddedImageId(); return res; } diff --git a/src/libs/services/scanner/impl/ScannerService.cpp b/src/libs/services/scanner/impl/ScannerService.cpp index 654cad04..251da22f 100644 --- a/src/libs/services/scanner/impl/ScannerService.cpp +++ b/src/libs/services/scanner/impl/ScannerService.cpp @@ -462,9 +462,9 @@ namespace lms::scanner _scanSteps.emplace_back(std::make_unique(params)); _scanSteps.emplace_back(std::make_unique(params)); _scanSteps.emplace_back(std::make_unique(params)); - _scanSteps.emplace_back(std::make_unique(params)); _scanSteps.emplace_back(std::make_unique(params)); - _scanSteps.emplace_back(std::make_unique(params)); // must come after ScanStepAssociateReleaseImages + _scanSteps.emplace_back(std::make_unique(params)); // must come after ScanStepAssociateReleaseImages + _scanSteps.emplace_back(std::make_unique(params)); // must come after ScanStepAssociateReleaseImages _scanSteps.emplace_back(std::make_unique(params)); _scanSteps.emplace_back(std::make_unique(params)); _scanSteps.emplace_back(std::make_unique(params)); diff --git a/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp b/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp index 96c16532..f190cb92 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include "core/IConfig.hpp" #include "core/ILogger.hpp" @@ -31,27 +32,38 @@ #include "core/String.hpp" #include "database/Artist.hpp" #include "database/ArtistInfo.hpp" +#include "database/Artwork.hpp" #include "database/Db.hpp" #include "database/Directory.hpp" #include "database/Image.hpp" #include "database/Session.hpp" #include "database/Track.hpp" +#include "ArtworkUtils.hpp" #include "ScanContext.hpp" namespace lms::scanner { namespace { - constexpr std::size_t readBatchSize{ 100 }; - constexpr std::size_t writeBatchSize{ 20 }; - - struct ArtistImageAssociation + using ArtistArtwork = std::variant; // TODO handle embedded images in tracks? + bool isSameArtwork(ArtistArtwork preferredArtwork, const db::ObjectPtr& artwork) { - db::ArtistId artistId; - db::ImageId imageId; + if (std::holds_alternative(preferredArtwork)) + return !artwork; + + if (const db::ImageId* imageId = std::get_if(&preferredArtwork)) + return artwork && *imageId == artwork->getImageId(); + + return false; + } + + struct ArtistArtworkAssociation + { + db::Artist::pointer artist; + ArtistArtwork preferredArtwork; }; - using ArtistImageAssociationContainer = std::deque; + using ArtistArtworkAssociationContainer = std::deque; struct SearchArtistImageContext { @@ -87,7 +99,7 @@ namespace lms::scanner return image; } - db::Image::pointer getImageFromMbid(SearchArtistImageContext& searchContext, const core::UUID& mbid) + db::ImageId getImageFromMbid(SearchArtistImageContext& searchContext, const core::UUID& mbid) { db::Image::pointer image; @@ -97,10 +109,10 @@ namespace lms::scanner image = foundImg; }); - return image; + return image ? image->getId() : db::ImageId{}; } - db::Image::pointer searchImageInArtistInfoDirectory(SearchArtistImageContext& searchContext, db::ArtistId artistId) + db::ImageId searchImageInArtistInfoDirectory(SearchArtistImageContext& searchContext, db::ArtistId artistId) { db::Image::pointer image; @@ -115,10 +127,10 @@ namespace lms::scanner if (fileInfoPaths.size() > 1) LMS_LOG(DBUPDATER, DEBUG, "Found " << fileInfoPaths.size() << " artist info files for same artist: " << core::stringUtils::joinStrings(fileInfoPaths, ", ")); - return image; + return image ? image->getId() : db::ImageId{}; } - db::Image::pointer searchImageInDirectories(SearchArtistImageContext& searchContext, db::ArtistId artistId) + db::ImageId searchImageInDirectories(SearchArtistImageContext& searchContext, db::ArtistId artistId) { db::Image::pointer image; @@ -147,7 +159,7 @@ namespace lms::scanner { image = findImageInDirectory(searchContext, directoryToInspect, searchContext.artistFileNames); if (image) - return image; + return image->getId(); std::filesystem::path parentPath{ directoryToInspect.parent_path() }; if (parentPath == directoryToInspect) @@ -164,44 +176,44 @@ namespace lms::scanner { image = findImageInDirectory(searchContext, releasePath, searchContext.artistFileNames); if (image) - return image; + return image->getId(); } } - return image; + return image ? image->getId() : db::ImageId{}; } - db::Image::pointer computeBestArtistImage(SearchArtistImageContext& searchContext, const db::Artist::pointer& artist) + ArtistArtwork computePreferredArtwork(SearchArtistImageContext& searchContext, const db::Artist::pointer& artist) { - db::Image::pointer image; + db::ImageId imageId; if (const auto mbid{ artist->getMBID() }) - image = getImageFromMbid(searchContext, *mbid); + imageId = getImageFromMbid(searchContext, *mbid); - if (!image) - image = searchImageInArtistInfoDirectory(searchContext, artist->getId()); + if (!imageId.isValid()) + imageId = searchImageInArtistInfoDirectory(searchContext, artist->getId()); - if (!image) - image = searchImageInDirectories(searchContext, artist->getId()); + if (!imageId.isValid()) + imageId = searchImageInDirectories(searchContext, artist->getId()); - return image; + return imageId.isValid() ? ArtistArtwork{ imageId } : ArtistArtwork{}; } - bool fetchNextArtistImagesToUpdate(SearchArtistImageContext& searchContext, ArtistImageAssociationContainer& artistImageAssociations) + bool fetchNextArtistArtworksToUpdate(SearchArtistImageContext& searchContext, ArtistArtworkAssociationContainer& ArtistArtworkAssociations) { const db::ArtistId artistId{ searchContext.lastRetrievedArtistId }; { + constexpr std::size_t readBatchSize{ 100 }; + auto transaction{ searchContext.session.createReadTransaction() }; db::Artist::find(searchContext.session, searchContext.lastRetrievedArtistId, readBatchSize, [&](const db::Artist::pointer& artist) { - db::Image::pointer image{ computeBestArtistImage(searchContext, artist) }; + ArtistArtwork preferredArtwork{ computePreferredArtwork(searchContext, artist) }; + + if (!isSameArtwork(preferredArtwork, artist->getPreferredArtwork())) + ArtistArtworkAssociations.push_back(ArtistArtworkAssociation{ artist, preferredArtwork }); - if (image != artist->getImage()) - { - LMS_LOG(DBUPDATER, DEBUG, "Updating artist image for artist '" << artist->getName() << "', using '" << (image ? image->getAbsoluteFilePath().c_str() : "") << "'"); - artistImageAssociations.push_back(ArtistImageAssociation{ artist->getId(), image ? image->getId() : db::ImageId{} }); - } searchContext.processedArtistCount++; }); } @@ -209,27 +221,32 @@ namespace lms::scanner return artistId != searchContext.lastRetrievedArtistId; } - void updateArtistImage(db::Session& session, const ArtistImageAssociation& artistImageAssociation) + void updateArtistPreferredArtwork(db::Session& session, const ArtistArtworkAssociation& ArtistArtworkAssociation) { - db::Artist::pointer artist{ db::Artist::find(session, artistImageAssociation.artistId) }; - assert(artist); + db::Artist::pointer artist{ ArtistArtworkAssociation.artist }; - db::Image::pointer image; - if (artistImageAssociation.imageId.isValid()) - image = db::Image::find(session, artistImageAssociation.imageId); + db::Artwork::pointer artwork; + if (const db::ImageId * imageId{ std::get_if(&ArtistArtworkAssociation.preferredArtwork) }) + artwork = utils::getOrCreateArtworkFromImage(session, *imageId); - artist.modify()->setImage(image); + artist.modify()->setPreferredArtwork(artwork); + if (artwork) + LMS_LOG(DBUPDATER, DEBUG, "Updated preferred artwork for artist '" << artist->getName() << "' with image in " << utils::toPath(session, artwork->getId())); + else + LMS_LOG(DBUPDATER, DEBUG, "Removed preferred artwork from artist '" << artist->getName() << "'"); } - void updateArtistImages(db::Session& session, ArtistImageAssociationContainer& imageAssociations) + void updateArtistArtworks(db::Session& session, ArtistArtworkAssociationContainer& imageAssociations) { + constexpr std::size_t writeBatchSize{ 50 }; + while (!imageAssociations.empty()) { auto transaction{ session.createWriteTransaction() }; for (std::size_t i{}; !imageAssociations.empty() && i < writeBatchSize; ++i) { - updateArtistImage(session, imageAssociations.front()); + updateArtistPreferredArtwork(session, imageAssociations.front()); imageAssociations.pop_front(); } } @@ -258,10 +275,7 @@ namespace lms::scanner bool ScanStepAssociateArtistImages::needProcess(const ScanContext& context) const { - if (context.stats.nbChanges() > 0) - return true; - - return false; + return context.stats.nbChanges() > 0; } void ScanStepAssociateArtistImages::process(ScanContext& context) @@ -279,13 +293,13 @@ namespace lms::scanner .artistFileNames = _artistFileNames, }; - ArtistImageAssociationContainer artistImageAssociations; - while (fetchNextArtistImagesToUpdate(searchContext, artistImageAssociations)) + ArtistArtworkAssociationContainer ArtistArtworkAssociations; + while (fetchNextArtistArtworksToUpdate(searchContext, ArtistArtworkAssociations)) { if (_abortScan) return; - updateArtistImages(session, artistImageAssociations); + updateArtistArtworks(session, ArtistArtworkAssociations); context.currentStepStats.processedElems = searchContext.processedArtistCount; _progressCallback(context.currentStepStats); } diff --git a/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp b/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp index b76c9611..3c4f1c33 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp @@ -231,7 +231,7 @@ namespace lms::scanner void updateReleaseImages(db::Session& session, ReleaseImageAssociationContainer& imageAssociations) { - constexpr std::size_t writeBatchSize{ 20 }; + constexpr std::size_t writeBatchSize{ 50 }; while (!imageAssociations.empty()) {