Recycled the default cover cache to cache any cover (cache is still small to fit on devices that have small RAM constraints)

src/libs/cover/impl/CoverArtGrabber.cpp

@@ -26,6 +26,7 @@
 #include "database/Track.hpp"
 
 #include "utils/Logger.hpp"
+#include "utils/Random.hpp"
 
 namespace {
 
@@ -52,37 +53,11 @@ Grabber::Grabber(const std::filesystem::path& execPath)
 void
 Grabber::setDefaultCover(const std::filesystem::path& p)
 {
-	std::unique_lock lock {_mutex};
-
 	_defaultCover = std::make_unique<Image>();
 	if (!_defaultCover->load(p))
 		throw LmsException("Cannot read default cover file '" + p.string() + "'");
 }
 
-Image
-Grabber::getDefaultCover(std::size_t size)
-{
-	LMS_LOG(COVER, DEBUG) << "Getting a default cover using size = " << size;
-	std::unique_lock lock {_mutex};
-
-	auto it = _defaultCovers.find(size);
-	if (it == _defaultCovers.end())
-	{
-		Image cover = *_defaultCover;
-
-		LMS_LOG(COVER, DEBUG) << "default cover size = " << cover.getSize().width << " x " << cover.getSize().height;
-
-		LMS_LOG(COVER, DEBUG) << "Scaling cover to size = " << size;
-		cover.scale(Geometry{size, size});
-		LMS_LOG(COVER, DEBUG) << "Scaling DONE";
-		auto res = _defaultCovers.insert(std::make_pair(size, cover));
-		assert(res.second);
-		it = res.first;
-	}
-
-	return it->second;
-}
-
 static std::optional<Image>
 getFromAvMediaFile(const Av::MediaFile& input)
 {
@@ -173,7 +148,11 @@ Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, st
 {
 	using namespace Database;
 
-	std::optional<Image> cover;
+	const CacheEntryDesc cacheEntryDesc {CacheEntryDesc::Type::Track, trackId, size};
+	std::optional<Image> cover {loadFromCache(cacheEntryDesc)};
+
+	if (cover)
+		return *cover;
 
 	bool hasCover {};
 	bool isMultiDisc {};
@@ -182,7 +161,7 @@ Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, st
 	{
 		auto transaction {dbSession.createSharedTransaction()};
 
-		Track::pointer track = Track::getById(dbSession, trackId);
+		const Track::pointer track {Track::getById(dbSession, trackId)};
 		if (track)
 		{
 			hasCover = track->hasCover();
@@ -207,9 +186,11 @@ Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, st
 	}
 
 	if (!cover)
-		cover = getDefaultCover(size);
+		cover = *_defaultCover;
-	else
+
-		cover->scale(Geometry {size, size});
+	cover->scale(Geometry {size, size});
+
+	saveToCache(cacheEntryDesc, *cover);
 
 	return *cover;
 }
@@ -218,32 +199,53 @@ Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, st
 Image
 Grabber::getFromRelease(Database::Session& session, Database::IdType releaseId, std::size_t size)
 {
-	std::optional<Image> cover;
+	const CacheEntryDesc cacheEntryDesc {CacheEntryDesc::Type::Release, releaseId, size};
+	std::optional<Image> cover {loadFromCache(cacheEntryDesc)};
+
+	if (cover)
+		return *cover;
 
 	std::optional<Database::IdType> trackId;
 	{
 		auto transaction {session.createSharedTransaction()};
 
-		auto release {Database::Release::getById(session, releaseId)};
+		const auto release {Database::Release::getById(session, releaseId)};
 		if (release)
 		{
-			auto tracks {release->getTracks()};
+			const auto tracks {release->getTracks()};
 			if (!tracks.empty())
 				trackId = tracks.front().id();
 		}
 	}
 
 	if (trackId)
-		return getFromTrack(session, *trackId, size);
+	{
-
+		cover = getFromTrack(session, *trackId, size);
-	if (!cover)
+	}
-		cover = getDefaultCover(size);
 	else
+	{
+		if (!cover)
+			cover = *_defaultCover;
+
 		cover->scale(Geometry {size, size});
+	}
+
+	saveToCache(cacheEntryDesc, *cover);
 
 	return *cover;
 }
 
+void
+Grabber::flushCache()
+{
+	std::unique_lock lock {_cacheMutex};
+
+	LMS_LOG(COVER, DEBUG) << "Cache stats: hits = " << _cacheHits << ", misses = " << _cacheMisses;
+	_cacheHits = 0;
+	_cacheMisses = 0;
+	_cache.clear();
+}
+
 std::vector<uint8_t>
 Grabber::getFromTrack(Database::Session& session, Database::IdType trackId, Format format, std::size_t width)
 {
@@ -262,5 +264,32 @@ Grabber::getFromRelease(Database::Session& session, Database::IdType releaseId,
 	return cover.save(format);
 }
 
+void
+Grabber::saveToCache(const CacheEntryDesc& entryDesc, const Image& image)
+{
+	std::unique_lock lock {_cacheMutex};
+
+	if (_cache.size() >= _maxCacheEntries)
+		_cache.erase(Random::pickRandom(_cache));
+
+	_cache[entryDesc] = image;
+}
+
+std::optional<Image>
+Grabber::loadFromCache(const CacheEntryDesc& entryDesc)
+{
+	std::shared_lock lock {_cacheMutex};
+
+	auto it {_cache.find(entryDesc)};
+	if (it == std::cend(_cache))
+	{
+		++_cacheMisses;
+		return std::nullopt;
+	}
+
+	++_cacheHits;
+	return it->second;
+}
+
 } // namespace CoverArt
 

src/libs/cover/impl/CoverArtGrabber.hpp

@@ -20,10 +20,10 @@
 #pragma once
 
 #include <filesystem>
-#include <map>
+#include <shared_mutex>
-#include <mutex>
 #include <optional>
 #include <vector>
+#include <unordered_map>
 
 #include "cover/ICoverArtGrabber.hpp"
 #include "database/Types.hpp"
@@ -36,7 +36,48 @@ namespace Database
 
 namespace CoverArt
 {
+	struct CacheEntryDesc
+	{
+		enum class Type
+		{
+			Track,
+			Release,
+		};
 
+		Type			type;
+		Database::IdType	id;
+		std::size_t		size;
+
+		bool operator==(const CacheEntryDesc& other) const
+		{
+			return type == other.type
+				&& id == other.id
+				&& size == other.size;
+		}
+	};
+
+} // ns CoverArt
+
+namespace std
+{
+
+	template<>
+	class hash<CoverArt::CacheEntryDesc>
+	{
+		public:
+			size_t operator()(const CoverArt::CacheEntryDesc& e) const
+			{
+				size_t h = std::hash<int>()(static_cast<int>(e.type));
+				h ^= std::hash<Database::IdType>()(e.id) << 1;
+				h ^= std::hash<std::size_t>()(e.size) << 1;
+				return h;
+			}
+	};
+
+} // ns std
+
+namespace CoverArt
+{
 	class Grabber : public IGrabber
 	{
 		public:
@@ -47,12 +88,14 @@ namespace CoverArt
 			Grabber(Grabber&&) = delete;
 			Grabber& operator=(Grabber&&) = delete;
 
+		private:
+
 			void			setDefaultCover(const std::filesystem::path& defaultCoverPath) override;
 
 			std::vector<uint8_t>	getFromTrack(Database::Session& dbSession, Database::IdType trackId, Format format, std::size_t width) override;
 			std::vector<uint8_t>	getFromRelease(Database::Session& dbSession, Database::IdType releaseId, Format format, std::size_t width) override;
+			void			flushCache() override;
 
-		private:
 
 			Image					getFromTrack(Database::Session& dbSession, Database::IdType trackId, std::size_t size);
 			Image					getFromRelease(Database::Session& dbSession, Database::IdType releaseId, std::size_t size);
@@ -60,14 +103,20 @@ namespace CoverArt
 			std::optional<Image>			getFromTrack(const std::filesystem::path& path) const;
 			std::vector<std::filesystem::path>	getCoverPaths(const std::filesystem::path& directoryPath) const;
 			std::optional<Image>			getFromDirectory(const std::filesystem::path& path) const;
-			Image					getDefaultCover(std::size_t size);
 
-			std::mutex _mutex;
+			std::unique_ptr<Image> _defaultCover;	// unique_ptr to defer initializing
-			std::unique_ptr<Image> _defaultCover;
-			std::map<std::size_t /* size */, Image> _defaultCovers;
 
+			std::shared_mutex _cacheMutex;
+			std::unordered_map<CacheEntryDesc, Image> _cache;
+			std::size_t _cacheMisses {};
+			std::size_t _cacheHits {};
+
+			void saveToCache(const CacheEntryDesc& entryDesc, const Image& image);
+			std::optional<Image> loadFromCache(const CacheEntryDesc& entryDesc);
+
+			static inline constexpr std::size_t _maxCacheEntries {300};
 			static inline const std::vector<std::filesystem::path> _fileExtensions {".jpg", ".jpeg", ".png", ".bmp"}; // TODO parametrize
-			static inline const std::size_t _maxFileSize {10000000};
+			static inline constexpr std::size_t _maxFileSize {10000000};
 			static inline const std::vector<std::filesystem::path> _preferredFileNames {"cover", "front"}; // TODO parametrize
 	};
 

src/libs/cover/include/cover/ICoverArtGrabber.hpp

@@ -40,6 +40,8 @@ class IGrabber
 
 		virtual std::vector<uint8_t>	getFromTrack(Database::Session& dbSession, Database::IdType trackId, Format format, std::size_t width) = 0;
 		virtual std::vector<uint8_t>	getFromRelease(Database::Session& dbSession, Database::IdType releaseId, Format format, std::size_t width) = 0;
+
+		virtual void flushCache() = 0;
 };
 
 std::unique_ptr<IGrabber> createGrabber(const std::filesystem::path& execPath);

src/lms/main.cpp

@@ -148,6 +148,9 @@ int main(int argc, char* argv[])
 		Scanner::IMediaScanner& mediaScanner {ServiceProvider<Scanner::IMediaScanner>::assign(Scanner::createMediaScanner(database))};
 
 		Recommendation::IEngine& recommendationEngine {ServiceProvider<Recommendation::IEngine>::assign(Recommendation::createEngine(database))};
+		CoverArt::IGrabber& coverArtGrabber {ServiceProvider<CoverArt::IGrabber>::assign(CoverArt::createGrabber(argv[0]))};
+		coverArtGrabber.setDefaultCover(server.appRoot() + "/images/unknown-cover.jpg");
+
 		mediaScanner.scanComplete().connect([&]()
 		{
 			auto status = mediaScanner.getStatus();
@@ -161,11 +164,11 @@ int main(int argc, char* argv[])
 			{
 				LMS_LOG(MAIN, INFO) << "Scanner did not change files, not reloading the recommendation engine...";
 			}
+			// Flush cover cache even if no changes:
+			// covers may be external files that changed and we don't keep track of them
+			coverArtGrabber.flushCache();
 		});
 
-		CoverArt::IGrabber& coverArtGrabber {ServiceProvider<CoverArt::IGrabber>::assign(CoverArt::createGrabber(argv[0]))};
-		coverArtGrabber.setDefaultCover(server.appRoot() + "/images/unknown-cover.jpg");
-
 		API::Subsonic::SubsonicResource subsonicResource {database};
 
 		// bind API resources