Now resolve the preferred artist artworks during scan steps

src/libs/database/impl/Artist.cpp

@@ -21,9 +21,9 @@
 #include <Wt/Dbo/WtSqlTraits.h>
 
 #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<Image> Artist::getImage() const
+    ObjectPtr<Artwork> Artist::getPreferredArtwork() const
     {
-        return ObjectPtr<Image>{ _image };
+        return ObjectPtr<Artwork>{ _preferredArtwork };
     }
 
-    ImageId Artist::getImageId() const
+    ArtworkId Artist::getPreferredArtworkId() const
     {
-        return _image.id();
+        return _preferredArtwork.id();
     }
 
     RangeResults<ArtistId> Artist::findSimilarArtistIds(core::EnumSet<TrackArtistLinkType> artistLinkTypes, std::optional<Range> 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> image)
+    void Artist::setPreferredArtwork(ObjectPtr<Artwork> artwork)
     {
-        _image = getDboPtr(image);
+        _preferredArtwork = getDboPtr(artwork);
     }
 } // namespace lms::db

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");
     }

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)");

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<core::UUID> getMBID() const;
         bool hasMBID() const;
-        ObjectPtr<Image> getImage() const;
+        ObjectPtr<Artwork> getPreferredArtwork() const;
-        ImageId getImageId() const;
+        ArtworkId getPreferredArtworkId() const;
         void visitLinks(std::function<void(const ObjectPtr<TrackArtistLink>& 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<core::UUID>& mbid) { _mbid = mbid ? mbid->getAsString() : ""; }
         void setSortName(std::string_view sortName);
-        void setImage(ObjectPtr<Image> image);
+        void setPreferredArtwork(ObjectPtr<Artwork> artwork);
 
         template<class Action>
         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> _image;
+        Wt::Dbo::ptr<Artwork> _preferredArtwork;
         Wt::Dbo::collection<Wt::Dbo::ptr<TrackArtistLink>> _trackArtistLinks; // Tracks involving this artist
         Wt::Dbo::collection<Wt::Dbo::ptr<StarredArtist>> _starredArtists;     // starred entries for this artist
     };

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<Wt::Dbo::ptr<Artist>> _artists;
         Wt::Dbo::ptr<Directory> _directory;
     };
 } // namespace lms::db

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<db::Artwork>;
     using ScopedImage = ScopedEntity<db::Image>;
 
     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());
+            auto artistArtwork(artist.get()->getPreferredArtwork());
-            ASSERT_TRUE(artistImage);
+            ASSERT_TRUE(artistArtwork);
-            EXPECT_EQ(artistImage->getId(), image.getId());
+            EXPECT_EQ(artistArtwork->getId(), artwork.getId());
         }
     }
 

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) })
+        const db::Artist::pointer artist{ db::Artist::find(session, artistId) };
-        {
+        if (!artist)
-            if (const db::ImageId imageId{ artist->getImageId() }; imageId.isValid())
+            return res;
-                res = imageId;
 
-            // TODO fallback on embedded Band/LeadArtist/Artist?
+        const db::Artwork::pointer artwork{ artist->getPreferredArtwork() };
-            // TODO fallback on first release?
+        if (artwork && artwork->getImageId().isValid())
-        }
+            res = artwork->getImageId();
+        else if (artwork && artwork->getTrackEmbeddedImageId().isValid())
+            res = artwork->getTrackEmbeddedImageId();
 
         return res;
     }

src/libs/services/scanner/impl/ScannerService.cpp

@@ -462,9 +462,9 @@ namespace lms::scanner
         _scanSteps.emplace_back(std::make_unique<ScanStepArtistReconciliation>(params));
         _scanSteps.emplace_back(std::make_unique<ScanStepAssociatePlayListTracks>(params));
         _scanSteps.emplace_back(std::make_unique<ScanStepUpdateLibraryFields>(params));
-        _scanSteps.emplace_back(std::make_unique<ScanStepAssociateArtistImages>(params));
         _scanSteps.emplace_back(std::make_unique<ScanStepAssociateReleaseImages>(params));
-        _scanSteps.emplace_back(std::make_unique<ScanStepAssociateTrackImages>(params)); // must come after ScanStepAssociateReleaseImages
+        _scanSteps.emplace_back(std::make_unique<ScanStepAssociateArtistImages>(params)); // must come after ScanStepAssociateReleaseImages
+        _scanSteps.emplace_back(std::make_unique<ScanStepAssociateTrackImages>(params));  // must come after ScanStepAssociateReleaseImages
         _scanSteps.emplace_back(std::make_unique<ScanStepAssociateExternalLyrics>(params));
         _scanSteps.emplace_back(std::make_unique<ScanStepRemoveOrphanedDbEntries>(params));
         _scanSteps.emplace_back(std::make_unique<ScanStepCompact>(params));

src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp

@@ -24,6 +24,7 @@
 #include <deque>
 #include <set>
 #include <span>
+#include <variant>
 
 #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 };
+        using ArtistArtwork = std::variant<std::monostate, db::ImageId>; // TODO handle embedded images in tracks?
-        constexpr std::size_t writeBatchSize{ 20 };
+        bool isSameArtwork(ArtistArtwork preferredArtwork, const db::ObjectPtr<db::Artwork>& artwork)
-
-        struct ArtistImageAssociation
         {
-            db::ArtistId artistId;
+            if (std::holds_alternative<std::monostate>(preferredArtwork))
-            db::ImageId imageId;
+                return !artwork;
+
+            if (const db::ImageId* imageId = std::get_if<db::ImageId>(&preferredArtwork))
+                return artwork && *imageId == artwork->getImageId();
+
+            return false;
+        }
+
+        struct ArtistArtworkAssociation
+        {
+            db::Artist::pointer artist;
+            ArtistArtwork preferredArtwork;
         };
-        using ArtistImageAssociationContainer = std::deque<ArtistImageAssociation>;
+        using ArtistArtworkAssociationContainer = std::deque<ArtistArtworkAssociation>;
 
         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)
+            if (!imageId.isValid())
-                image = searchImageInArtistInfoDirectory(searchContext, artist->getId());
+                imageId = searchImageInArtistInfoDirectory(searchContext, artist->getId());
 
-            if (!image)
+            if (!imageId.isValid())
-                image = searchImageInDirectories(searchContext, artist->getId());
+                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() : "<none>") << "'");
-                        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) };
+            db::Artist::pointer artist{ ArtistArtworkAssociation.artist };
-            assert(artist);
 
-            db::Image::pointer image;
+            db::Artwork::pointer artwork;
-            if (artistImageAssociation.imageId.isValid())
+            if (const db::ImageId * imageId{ std::get_if<db::ImageId>(&ArtistArtworkAssociation.preferredArtwork) })
-                image = db::Image::find(session, artistImageAssociation.imageId);
+                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 context.stats.nbChanges() > 0;
-            return true;
-
-        return false;
     }
 
     void ScanStepAssociateArtistImages::process(ScanContext& context)
@@ -279,13 +293,13 @@ namespace lms::scanner
             .artistFileNames = _artistFileNames,
         };
 
-        ArtistImageAssociationContainer artistImageAssociations;
+        ArtistArtworkAssociationContainer ArtistArtworkAssociations;
-        while (fetchNextArtistImagesToUpdate(searchContext, artistImageAssociations))
+        while (fetchNextArtistArtworksToUpdate(searchContext, ArtistArtworkAssociations))
         {
             if (_abortScan)
                 return;
 
-            updateArtistImages(session, artistImageAssociations);
+            updateArtistArtworks(session, ArtistArtworkAssociations);
             context.currentStepStats.processedElems = searchContext.processedArtistCount;
             _progressCallback(context.currentStepStats);
         }

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())
             {