Subsonic API: added artist images in getCoverArt endpoint, fixes #392

conf/lms.conf

@@ -84,7 +84,10 @@ cover-max-cache-size = 30;
 cover-jpeg-quality = 75;
 
 # Preferred file names for covers (order is important)
-cover-preferred-file-names = ("cover", "front" );
+cover-preferred-file-names = ("cover", "front");
+
+# File names for artist images (order is important)
+artist-image-file-names = ("artist");
 
 # Playqueue max entry count
 playqueue-max-entry-count = 1000;

src/libs/services/cover/impl/CoverService.cpp

@@ -19,6 +19,8 @@
 
 #include "CoverService.hpp"
 
+#include <set>
+
 #include "av/IAudioFile.hpp"
 
 #include "database/Db.hpp"
@@ -30,13 +32,13 @@
 #include "image/IRawImage.hpp"
 #include "utils/IConfig.hpp"
 #include "utils/ILogger.hpp"
+#include "utils/Path.hpp"
 #include "utils/Random.hpp"
 #include "utils/String.hpp"
 #include "utils/Utils.hpp"
 
 namespace Cover
 {
-
     namespace
     {
         struct TrackInfo
@@ -85,6 +87,19 @@ namespace Cover
             return res;
         }
 
+        std::vector<std::string> constructArtistFileNames()
+        {
+            std::vector<std::string> res;
+
+            Service<IConfig>::get()->visitStrings("artist-image-file-names",
+                [&res](std::string_view fileName)
+                {
+                    res.emplace_back(fileName);
+                }, { "artist" });
+
+            return res;
+        }
+
         bool isFileSupported(const std::filesystem::path& file, const std::vector<std::filesystem::path>& extensions)
         {
             return (std::find(std::cbegin(extensions), std::cend(extensions), file.extension()) != std::cend(extensions));
@@ -106,7 +121,7 @@ namespace Cover
         , _maxCacheSize{ Service<IConfig>::get()->getULong("cover-max-cache-size", 30) * 1000 * 1000 }
         , _maxFileSize{ Service<IConfig>::get()->getULong("cover-max-file-size", 10) * 1000 * 1000 }
         , _preferredFileNames{ constructPreferredFileNames() }
-
+        , _artistFileNames{ constructArtistFileNames() }
     {
         setJpegQuality(Service<IConfig>::get()->getULong("cover-jpeg-quality", 75));
 
@@ -196,7 +211,7 @@ namespace Cover
         }
     }
 
-    std::unique_ptr<IEncodedImage> CoverService::getFromDirectory(const std::filesystem::path& directory, ImageSize width) const
+    std::unique_ptr<IEncodedImage> CoverService::getFromDirectory(const std::filesystem::path& directory, ImageSize width, const std::vector<std::string>& preferredFileNames, bool allowPickRandom) const
     {
         const std::multimap<std::string, std::filesystem::path> coverPaths{ getCoverPaths(directory) };
 
@@ -216,19 +231,21 @@ namespace Cover
 
         std::unique_ptr<IEncodedImage> image;
 
-        for (std::string_view filename : _preferredFileNames)
+        for (std::string_view filename : preferredFileNames)
         {
             image = tryLoadImageFromFilename(filename);
             if (image)
                 return image;
         }
 
-        // Just pick one
+        if (allowPickRandom)
-        for (const auto& [filename, coverPath] : coverPaths)
         {
-            image = getFromCoverFile(coverPath, width);
+            for (const auto& [filename, coverPath] : coverPaths)
-            if (image)
+            {
-                return image;
+                image = getFromCoverFile(coverPath, width);
+                if (image)
+                    return image;
+            }
         }
 
         return image;
@@ -269,7 +286,7 @@ namespace Cover
 
         if (std::filesystem::file_size(filePath, ec) > _maxFileSize && !ec)
         {
-            LMS_LOG(COVER, INFO, "Cover file '" << filePath.string() << " is too big (" << std::filesystem::file_size(filePath, ec) << "), limit is " << _maxFileSize);
+            LMS_LOG(COVER, INFO, "Image file '" << filePath.string() << " is too big (" << std::filesystem::file_size(filePath, ec) << "), limit is " << _maxFileSize);
             return false;
         }
 
@@ -341,7 +358,7 @@ namespace Cover
             if (!cover && trackInfo->isMultiDisc)
             {
                 if (trackInfo->trackPath.parent_path().has_parent_path())
-                    cover = getFromDirectory(trackInfo->trackPath.parent_path().parent_path(), width);
+                    cover = getFromDirectory(trackInfo->trackPath.parent_path().parent_path(), width, _preferredFileNames, true);
             }
         }
 
@@ -389,7 +406,7 @@ namespace Cover
 
         if (const std::optional<ReleaseInfo> releaseInfo{ getReleaseInfo() })
         {
-            cover = getFromDirectory(releaseInfo->releaseDirectory, width);
+            cover = getFromDirectory(releaseInfo->releaseDirectory, width, _preferredFileNames, true);
             if (!cover)
                 cover = getFromTrack(session, releaseInfo->firstTrackId, width, false /* no release fallback */);
         }
@@ -400,6 +417,50 @@ namespace Cover
         return cover;
     }
 
+    std::shared_ptr<IEncodedImage> CoverService::getFromArtist(Database::ArtistId artistId, ImageSize width)
+    {
+        using namespace Database;
+        const CacheEntryDesc cacheEntryDesc{ artistId, width };
+
+        std::shared_ptr<IEncodedImage> artistImage{ loadFromCache(cacheEntryDesc) };
+        if (artistImage)
+            return artistImage;
+
+        std::set<std::filesystem::path> parentPaths;
+        {
+            Session& session{ _db.getTLSSession() };
+            auto transaction{ session.createReadTransaction() };
+
+            Track::find(session, Track::FindParameters{}.setArtist(artistId), [&](const Track::pointer& track)
+                {
+                    parentPaths.insert(track->getPath().parent_path());
+                });
+        }
+
+        if (parentPaths.size() == 1)
+            artistImage = getFromDirectory(parentPaths.begin()->parent_path(), width, _artistFileNames, false);
+        else if (parentPaths.size() > 1)
+        {
+            const std::filesystem::path longestCommonPath{ PathUtils::getLongestCommonPath(std::cbegin(parentPaths), std::cend(parentPaths)) };
+            artistImage = getFromDirectory(longestCommonPath, width, _artistFileNames, false);
+        }
+
+        if (!artistImage)
+        {
+            for (const std::filesystem::path& parentPath : parentPaths)
+            {
+                artistImage = getFromDirectory(parentPath, width, _artistFileNames, false);
+                if (artistImage)
+                    break;
+            }
+        }
+
+        if (artistImage)
+            saveToCache(cacheEntryDesc, artistImage);
+
+        return artistImage;
+    }
+
     void CoverService::flushCache()
     {
         std::unique_lock lock{ _cacheMutex };

src/libs/services/cover/impl/CoverService.hpp

@@ -47,7 +47,7 @@ namespace Cover
 {
     struct CacheEntryDesc
     {
-        std::variant<Database::TrackId, Database::ReleaseId> id;
+        std::variant<Database::ArtistId, Database::ReleaseId, Database::TrackId> id;
         std::size_t			size;
 
         bool operator==(const CacheEntryDesc& other) const
@@ -92,6 +92,7 @@ namespace Cover
     private:
         std::shared_ptr<Image::IEncodedImage>   getFromTrack(Database::TrackId trackId, Image::ImageSize width) override;
         std::shared_ptr<Image::IEncodedImage>   getFromRelease(Database::ReleaseId releaseId, Image::ImageSize width) override;
+        std::shared_ptr<Image::IEncodedImage>   getFromArtist(Database::ArtistId artistId, Image::ImageSize width) override;
         std::shared_ptr<Image::IEncodedImage>   getDefault(Image::ImageSize width) override;
         void                                    flushCache() override;
         void                                    setJpegQuality(unsigned quality) override;
@@ -102,7 +103,7 @@ namespace Cover
 
         std::unique_ptr<Image::IEncodedImage>   getFromTrack(const std::filesystem::path& path, Image::ImageSize width) const;
         std::multimap<std::string, std::filesystem::path>   getCoverPaths(const std::filesystem::path& directoryPath) const;
-        std::unique_ptr<Image::IEncodedImage>   getFromDirectory(const std::filesystem::path& directory, Image::ImageSize width) const;
+        std::unique_ptr<Image::IEncodedImage>   getFromDirectory(const std::filesystem::path& directory, Image::ImageSize width, const std::vector<std::string>& preferredFileNames, bool allowPickRandom) const;
         std::unique_ptr<Image::IEncodedImage>   getFromSameNamedFile(const std::filesystem::path& filePath, Image::ImageSize width) const;
 
         bool                                    checkCoverFile(const std::filesystem::path& directoryPath) const;
@@ -124,6 +125,7 @@ namespace Cover
         static inline const std::vector<std::filesystem::path> _fileExtensions{ ".jpg", ".jpeg", ".png", ".bmp" }; // TODO parametrize
         const std::size_t _maxFileSize;
         const std::vector<std::string> _preferredFileNames;
+        const std::vector<std::string> _artistFileNames;
         unsigned _jpegQuality;
     };
 

src/libs/services/cover/include/services/cover/ICoverService.hpp

@@ -22,6 +22,7 @@
 #include <filesystem>
 #include <memory>
 
+#include "database/ArtistId.hpp"
 #include "database/ReleaseId.hpp"
 #include "database/TrackId.hpp"
 #include "image/IEncodedImage.hpp"
@@ -40,6 +41,7 @@ namespace Cover
 
         virtual std::shared_ptr<Image::IEncodedImage> getFromTrack(Database::TrackId trackId, Image::ImageSize width) = 0;
         virtual std::shared_ptr<Image::IEncodedImage> getFromRelease(Database::ReleaseId releaseId, Image::ImageSize width) = 0;
+        virtual std::shared_ptr<Image::IEncodedImage> getFromArtist(Database::ArtistId artistId, Image::ImageSize width) = 0;
 
         virtual std::shared_ptr<Image::IEncodedImage> getDefault(Image::ImageSize width) = 0;
 

src/libs/subsonic/impl/entrypoints/MediaRetrieval.cpp

@@ -269,13 +269,9 @@ namespace API::Subsonic
         else if (releaseId)
             cover = Service<Cover::ICoverService>::get()->getFromRelease(*releaseId, size);
         else if (artistId)
-        {
+            cover = Service<Cover::ICoverService>::get()->getFromArtist(*artistId, size);
-            // TODO handle a placeholder for artists
-            response.setStatus(404);
-            return;
-        }
 
-        if (!cover && context.enableDefaultCover)
+        if (!cover && context.enableDefaultCover && !artistId)
             cover = Service<Cover::ICoverService>::get()->getDefault(size);
 
         if (!cover)

src/libs/subsonic/impl/responses/Artist.cpp

@@ -79,6 +79,7 @@ namespace API::Subsonic
 
         artistNode.setAttribute("id", idToString(artist->getId()));
         artistNode.setAttribute("name", artist->getName());
+        artistNode.setAttribute("coverArt", idToString(artist->getId()));
 
         if (id3)
         {

src/libs/utils/impl/Path.cpp

@@ -162,4 +162,21 @@ namespace PathUtils
 
         return false;
     }
+
+    std::filesystem::path getLongestCommonPath(const std::filesystem::path& path1, const std::filesystem::path& path2)
+    {
+        std::filesystem::path longestCommonPath;
+
+        auto it1{ path1.begin() };
+        auto it2{ path2.begin() };
+
+        while (it1 != std::cend(path1) && it2 != std::cend(path2) && *it1 == *it2)
+        {
+            longestCommonPath /= *it1;
+            ++it1;
+            ++it2;
+        }
+
+        return longestCommonPath;
+    }
 } // ns PathUtils

src/libs/utils/include/utils/Path.hpp

@@ -28,23 +28,39 @@
 
 namespace PathUtils
 {
-	std::uint32_t computeCrc32(const std::filesystem::path& p);
+    std::uint32_t computeCrc32(const std::filesystem::path& p);
 
-	// Make sure the given path is a directory
+    // Make sure the given path is a directory
-	// Create it if needed
+    // Create it if needed
-	bool ensureDirectory(const std::filesystem::path& dir);
+    bool ensureDirectory(const std::filesystem::path& dir);
 
-	// Get the last write time since Epoch
+    // Get the last write time since Epoch
-	Wt::WDateTime getLastWriteTime(const std::filesystem::path& dir);
+    Wt::WDateTime getLastWriteTime(const std::filesystem::path& dir);
 
-	// returns false if aborted by user
+    // returns false if aborted by user
-	bool exploreFilesRecursive(const std::filesystem::path& directory, std::function<bool(std::error_code, const std::filesystem::path&)> cb, const std::filesystem::path* excludeDirFileName = {});
+    bool exploreFilesRecursive(const std::filesystem::path& directory, std::function<bool(std::error_code, const std::filesystem::path&)> cb, const std::filesystem::path* excludeDirFileName = {});
 
-	// Check if file's extension is one of provided extensions
+    // Check if file's extension is one of provided extensions
-	bool hasFileAnyExtension(const std::filesystem::path& file, const std::vector<std::filesystem::path>& extensions);
+    bool hasFileAnyExtension(const std::filesystem::path& file, const std::vector<std::filesystem::path>& extensions);
 
-	// Check if a path is within a directory (excludeDirFileName is a relative can be used to exclude a whole directory and its subdirectory, must not have parent_path)
+    // Check if a path is within a directory (excludeDirFileName is a relative can be used to exclude a whole directory and its subdirectory, must not have parent_path)
-	bool isPathInRootPath(const std::filesystem::path& path, const std::filesystem::path& rootPath, const std::filesystem::path* excludeDirFileName = {});
+    bool isPathInRootPath(const std::filesystem::path& path, const std::filesystem::path& rootPath, const std::filesystem::path* excludeDirFileName = {});
 
+    std::filesystem::path getLongestCommonPath(const std::filesystem::path& path1, const std::filesystem::path& path2);
 
+    template <typename Iterator>
+    std::filesystem::path getLongestCommonPath(Iterator first, Iterator last)
+    {
+        std::filesystem::path longestCommonPath;
+
+        if (first == last)
+            return longestCommonPath;
+
+        longestCommonPath = *first++;
+
+        while (first != last)
+            longestCommonPath = PathUtils::getLongestCommonPath(*first++, longestCommonPath);
+
+        return longestCommonPath;
+    }
 }

src/libs/utils/test/CMakeLists.txt

@@ -2,6 +2,7 @@ include(GoogleTest)
 
 add_executable(test-utils
 	EnumSet.cpp
+	Path.cpp
 	RecursiveSharedMutex.cpp
 	String.cpp
 	Utils.cpp

src/libs/utils/test/Path.cpp

@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2019 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * LMS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <gtest/gtest.h>
+
+#include "utils/Path.hpp"
+
+TEST(Path, getLongestCommonPath)
+{
+    using namespace PathUtils;
+
+    struct TestCase
+    {
+        std::filesystem::path path1;
+        std::filesystem::path path2;
+        std::filesystem::path expectedCommonPath;
+    };
+
+    TestCase tests[]
+    {
+        {"foo.txt", "/foo/foo.txt", ""},
+        {"/", "/file.txt", "/"},
+        {"/foo/bar/file1.txt", "/foo/bar/file2.txt", "/foo/bar"},
+        {"/foo/bar/file.txt", "/foo/bar/file.txt", "/foo/bar/file.txt"},
+        {"/dir1/file.txt", "/dir2/file.txt", "/"},
+        {"/prefix/folder/file.txt", "/prefix/folder/subfolder/file.txt", "/prefix/folder"},
+    };
+
+    for (const TestCase& test : tests)
+    {
+        EXPECT_EQ(PathUtils::getLongestCommonPath(test.path1, test.path2), test.expectedCommonPath);
+    }
+}
+
+
+TEST(Path, getLongestCommonPathIterator)
+{
+    using namespace PathUtils;
+
+    struct TestCase
+    {
+        std::vector<std::filesystem::path> paths;
+        std::filesystem::path expectedCommonPath;
+    };
+
+    TestCase tests[]
+    {
+        {{}, ""},
+        {{"/"}, "/"},
+        {{"/foo", "/bar"}, "/"},
+        {{"/foo/bar/file1.txt", "/foo/bar/file2.txt"}, "/foo/bar"},
+        {{"/foo", "/foo/"}, "/foo"},
+        {{"/foo/", "/foo/"}, "/foo/"},
+        {{"/foo/", "/foo/", "/bar"}, "/"},
+        {{"/foo/", "/foo/", "/foo/bar"}, "/foo"},
+    };
+
+    for (const TestCase& test : tests)
+    {
+        EXPECT_EQ(PathUtils::getLongestCommonPath(std::cbegin(test.paths), std::cend(test.paths)), test.expectedCommonPath);
+    }
+}

src/lms/main.cpp

@@ -292,7 +292,7 @@ int main(int argc, char* argv[])
         scannerService->getEvents().scanComplete.connect([&]
             {
                 // Flush cover cache even if no changes:
-                // covers may be external files that changed and we don't keep track of them
+                // covers may be external files that changed and we don't keep track of them for now (but we should)
                 coverService->flushCache();
             });