Getting a cover for a track will more efficiently fallback to album

src/libs/cover/impl/CoverArtGrabber.cpp

@@ -112,7 +112,7 @@ Grabber::getFromAvMediaFile(const Av::MediaFile& input, ImageSize width) const
 }
 
 std::unique_ptr<IEncodedImage>
-Grabber::getFromFile(const std::filesystem::path& p, ImageSize width) const
+Grabber::getFromCoverFile(const std::filesystem::path& p, ImageSize width) const
 {
 	std::unique_ptr<IEncodedImage> image;
 
@@ -146,7 +146,7 @@ Grabber::getDefault(ImageSize width)
 		if (auto it {_defaultCoverCache.find(width)}; it != std::cend(_defaultCoverCache))
 			return it->second;
 
-		std::shared_ptr<IEncodedImage> image {getFromFile(_defaultCoverPath, width)};
+		std::shared_ptr<IEncodedImage> image {getFromCoverFile(_defaultCoverPath, width)};
 		_defaultCoverCache[width] = image;
 		LMS_LOG(COVER, DEBUG) << "Default cache entries = " << _defaultCoverCache.size();
 
@@ -155,9 +155,9 @@ Grabber::getDefault(ImageSize width)
 }
 
 std::unique_ptr<IEncodedImage>
-Grabber::getFromDirectory(const std::filesystem::path& p, std::string_view preferredFileName, ImageSize width) const
+Grabber::getFromDirectory(const std::filesystem::path& directory, ImageSize width) const
 {
-	const std::multimap<std::string, std::filesystem::path> coverPaths {getCoverPaths(p)};
+	const std::multimap<std::string, std::filesystem::path> coverPaths {getCoverPaths(directory)};
 
 	auto tryLoadImageFromFilename = [&](std::string_view fileName)
 	{
@@ -166,7 +166,7 @@ Grabber::getFromDirectory(const std::filesystem::path& p, std::string_view prefe
 		auto range {coverPaths.equal_range(std::string {fileName})};
 		for (auto it {range.first}; it != range.second; ++it)
 		{
-			image = getFromFile(it->second, width);
+			image = getFromCoverFile(it->second, width);
 			if (image)
 				break;
 		}
@@ -175,13 +175,6 @@ Grabber::getFromDirectory(const std::filesystem::path& p, std::string_view prefe
 
 	std::unique_ptr<IEncodedImage> image;
 
-	if (!preferredFileName.empty())
-	{
-		image = tryLoadImageFromFilename(preferredFileName);
-		if (image)
-			return image;
-	}
-
 	for (std::string_view filename : _preferredFileNames)
 	{
 		image = tryLoadImageFromFilename(filename);
@@ -192,7 +185,7 @@ Grabber::getFromDirectory(const std::filesystem::path& p, std::string_view prefe
 	// Just pick one
 	for (const auto& [filename, coverPath] : coverPaths)
 	{
-		image = getFromFile(coverPath, width);
+		image = getFromCoverFile(coverPath, width);
 		if (image)
 			return image;
 	}
@@ -200,6 +193,50 @@ Grabber::getFromDirectory(const std::filesystem::path& p, std::string_view prefe
 	return image;
 }
 
+std::unique_ptr<IEncodedImage>
+Grabber::getFromSameNamedFile(const std::filesystem::path& filePath, ImageSize width) const
+{
+	std::unique_ptr<IEncodedImage> res;
+
+	std::filesystem::path coverPath {filePath};
+	for (const std::filesystem::path& extension : _fileExtensions)
+	{
+		coverPath.replace_extension(extension);
+
+		if (!checkCoverFile(coverPath))
+			continue;
+
+		res = getFromCoverFile(coverPath, width);
+		if (res)
+			break;
+	}
+
+	return res;
+}
+
+bool
+Grabber::checkCoverFile(const std::filesystem::path& filePath) const
+{
+	std::error_code ec;
+
+	if (!isFileSupported(filePath, _fileExtensions))
+		return false;
+
+	if (!std::filesystem::exists(filePath, ec))
+		return false;
+
+	if (!std::filesystem::is_regular_file(filePath, ec))
+		return false;
+
+	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;
+		return false;
+	}
+
+	return true;
+}
+
 std::multimap<std::string, std::filesystem::path>
 Grabber::getCoverPaths(const std::filesystem::path& directoryPath) const
 {
@@ -210,22 +247,12 @@ Grabber::getCoverPaths(const std::filesystem::path& directoryPath) const
 	std::filesystem::directory_iterator itEnd;
 	while (!ec && itPath != itEnd)
 	{
-		const std::filesystem::path path {*itPath};
+		const std::filesystem::path& path {*itPath};
-		itPath.increment(ec);
-
-		if (!std::filesystem::is_regular_file(path))
-			continue;
-
-		if (!isFileSupported(path, _fileExtensions))
-			continue;
-
-		if (std::filesystem::file_size(path) > _maxFileSize)
-		{
-			LMS_LOG(COVER, INFO) << "Cover file '" << path.string() << " is too big (" << std::filesystem::file_size(path) << "), limit is " << _maxFileSize;
-			continue;
-		}
 
+		if (checkCoverFile(path))
 			res.emplace(std::filesystem::path{ path }.filename().replace_extension("").string(), path);
+
+		itPath.increment(ec);
 	}
 
 	return res;
@@ -238,8 +265,7 @@ Grabber::getFromTrack(const std::filesystem::path& p, ImageSize width) const
 
 	try
 	{
-		Av::MediaFile input {p};
+		const Av::MediaFile input {p};
-
 		image = getFromAvMediaFile(input, width);
 	}
 	catch (Av::AvException& e)
@@ -252,6 +278,12 @@ Grabber::getFromTrack(const std::filesystem::path& p, ImageSize width) const
 
 std::shared_ptr<IEncodedImage>
 Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, ImageSize width)
+{
+	return getFromTrack(dbSession, trackId, width, true /* allow release fallback*/);
+}
+
+std::shared_ptr<IEncodedImage>
+Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, ImageSize width, bool allowReleaseFallback)
 {
 	using namespace Database;
 
@@ -261,35 +293,55 @@ Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, Im
 	if (cover)
 		return cover;
 
+	struct TrackInfo
+	{
 		bool hasCover {};
 		bool isMultiDisc {};
 		std::filesystem::path trackPath;
+		std::optional<Database::IdType> releaseId;
+	};
 
+	auto getTrackInfo {[&]
 	{
+		std::optional<TrackInfo> res;
+
 		auto transaction {dbSession.createSharedTransaction()};
 
 		const Track::pointer track {Track::getById(dbSession, trackId)};
-		if (track)
+		if (!track)
+			return res;
+
+		res = TrackInfo {};
+
+		res->hasCover = track->hasCover();
+		res->trackPath = track->getPath();
+
+		if (const Release::pointer& release {track->getRelease()})
 		{
-			hasCover = track->hasCover();
+			res->releaseId = release.id();
-			trackPath = track->getPath();
+			if (release->getTotalDisc() > 1)
-
+				res->isMultiDisc = true;
-			auto release {track->getRelease()};
-			if (release && release->getTotalDisc() > 1)
-				isMultiDisc = true;
-		}
 		}
 
-	if (hasCover)
+		return res;
-		cover = getFromTrack(trackPath, width);
+	}};
+
+	if (const std::optional<TrackInfo> trackInfo {getTrackInfo()})
+	{
+		if (trackInfo->hasCover)
+			cover = getFromTrack(trackInfo->trackPath, width);
 
 		if (!cover)
-		cover = getFromDirectory(trackPath.parent_path(), trackPath.filename().replace_extension("").string(), width);
+			cover = getFromSameNamedFile(trackInfo->trackPath, width);
 
-	if (!cover && isMultiDisc)
+		if (!cover && trackInfo->releaseId && allowReleaseFallback)
+			cover = getFromRelease(dbSession, *trackInfo->releaseId, width);
+
+		if (!cover && trackInfo->isMultiDisc)
 		{
-		if (trackPath.parent_path().has_parent_path())
+			if (trackInfo->trackPath.parent_path().has_parent_path())
-			cover = getFromDirectory(trackPath.parent_path().parent_path(), {}, width);
+				cover = getFromDirectory(trackInfo->trackPath.parent_path().parent_path(), width);
+		}
 	}
 
 	if (!cover)
@@ -310,22 +362,39 @@ Grabber::getFromRelease(Database::Session& session, Database::IdType releaseId,
 	if (cover)
 		return cover;
 
-	std::optional<Database::IdType> trackId;
+	struct ReleaseInfo
 	{
+		Database::IdType firstTrackId;
+		std::filesystem::path releaseDirectory;
+	};
+
+	auto getReleaseInfo {[&]
+	{
+		std::optional<ReleaseInfo> res;
+
 		auto transaction {session.createSharedTransaction()};
 
-		const auto release {Database::Release::getById(session, releaseId)};
+		if (const Database::Release::pointer release {Database::Release::getById(session, releaseId)})
-		if (release)
 		{
-			const auto tracks {release->getTracks()};
+			if (const auto firstTrack {release->getFirstTrack()})
-			if (!tracks.empty())
+			{
-				trackId = tracks.front().id();
+				res = ReleaseInfo {};
+				res->firstTrackId = firstTrack.id();
+				res->releaseDirectory = firstTrack->getPath().parent_path();
 			}
 		}
 
-	if (trackId)
+		return res;
-		cover = getFromTrack(session, *trackId, width);
+	}};
-	else
+
+	if (const std::optional<ReleaseInfo> releaseInfo {getReleaseInfo()})
+	{
+		cover = getFromDirectory(releaseInfo->releaseDirectory, width);
+		if (!cover)
+			cover = getFromTrack(session, releaseInfo->firstTrackId, width, false /* no release fallback */);
+	}
+
+	if (!cover)
 		cover = getDefault(width);
 
 	if (cover)

src/libs/cover/impl/CoverArtGrabber.hpp

@@ -105,14 +105,18 @@ namespace CoverArt
 			std::shared_ptr<IEncodedImage>	getFromRelease(Database::Session& dbSession, Database::IdType releaseId, ImageSize width) override;
 			void							flushCache() override;
 
+			std::shared_ptr<IEncodedImage>	getFromTrack(Database::Session& dbSession, Database::IdType trackId, ImageSize width, bool allowReleaseFallback);
 			std::unique_ptr<IEncodedImage>	getFromAvMediaFile(const Av::MediaFile& input, ImageSize width) const;
-			std::unique_ptr<IEncodedImage>	getFromFile(const std::filesystem::path& p, ImageSize width) const;
+			std::unique_ptr<IEncodedImage>	getFromCoverFile(const std::filesystem::path& p, ImageSize width) const;
 
 			std::unique_ptr<IEncodedImage>	getFromTrack(const std::filesystem::path& path, ImageSize width) const;
 			std::multimap<std::string, std::filesystem::path>	getCoverPaths(const std::filesystem::path& directoryPath) const;
-			std::unique_ptr<IEncodedImage>	getFromDirectory(const std::filesystem::path& path, std::string_view preferredFileName, ImageSize width) const;
+			std::unique_ptr<IEncodedImage>	getFromDirectory(const std::filesystem::path& directory, ImageSize width) const;
+			std::unique_ptr<IEncodedImage>	getFromSameNamedFile(const std::filesystem::path& filePath, ImageSize width) const;
 			std::shared_ptr<IEncodedImage>	getDefault(ImageSize width);
 
+			bool							checkCoverFile(const std::filesystem::path& directoryPath) const;
+
 			std::shared_mutex _cacheMutex;
 			std::unordered_map<CacheEntryDesc, std::shared_ptr<IEncodedImage>> _cache;
 			std::unordered_map<ImageSize, std::shared_ptr<IEncodedImage>> _defaultCoverCache;

src/libs/database/impl/Release.cpp

@@ -232,15 +232,15 @@ Release::getLastWritten(Session& session,
 }
 
 std::vector<Release::pointer>
-Release::getByYear(Session& session, int yearFrom, int yearTo, std::optional<std::size_t> offset, std::optional<std::size_t> limit)
+Release::getByYear(Session& session, int yearFrom, int yearTo, std::optional<Range> range)
 {
 	Wt::Dbo::collection<Release::pointer> res = session.getDboSession().query<Release::pointer>
 		("SELECT DISTINCT r from release r INNER JOIN track t ON r.id = t.release_id")
 		.where("t.year >= ?").bind(yearFrom)
 		.where("t.year <= ?").bind(yearTo)
 		.orderBy("t.year, r.name COLLATE NOCASE")
-		.offset(offset ? static_cast<int>(*offset) : -1)
+		.offset(range ? static_cast<int>(range->offset) : -1)
-		.limit(limit ? static_cast<int>(*limit) : -1);
+		.limit(range ? static_cast<int>(range->limit) : -1);
 
 	return std::vector<pointer>(res.begin(), res.end());
 }
@@ -532,6 +532,20 @@ Release::getTracksCount() const
 	return _tracks.size();
 }
 
+Wt::Dbo::ptr<Track>
+Release::getFirstTrack() const
+{
+	assert(self());
+	assert(self()->id() != Wt::Dbo::dbo_traits<Artist>::invalidId());
+	assert(session());
+
+	return session()->query<Track::pointer>("SELECT t from track t")
+		.join("release r ON t.release_id = r.id")
+		.where("r.id = ?").bind(self()->id())
+		.orderBy("t.disc_number,t.track_number")
+		.limit(1);
+}
+
 std::chrono::milliseconds
 Release::getDuration() const
 {

src/libs/database/include/database/Release.hpp

@@ -58,7 +58,7 @@ class Release : public Wt::Dbo::Dbo<Release>
 		static std::vector<pointer>	getAllRandom(Session& session, const std::set<IdType>& clusters, std::optional<std::size_t> size = {});
 		static std::vector<IdType>	getAllIdsRandom(Session& session, const std::set<IdType>& clusters, std::optional<std::size_t> size = {});
 		static std::vector<pointer>	getLastWritten(Session& session, std::optional<Wt::WDateTime> after, const std::set<IdType>& clusters, std::optional<Range> range, bool& moreResults);
-		static std::vector<pointer>	getByYear(Session& session, int yearFrom, int yearTo, std::optional<std::size_t> offset = {}, std::optional<std::size_t> size = {});
+		static std::vector<pointer>	getByYear(Session& session, int yearFrom, int yearTo, std::optional<Range> range = std::nullopt);
 		static std::vector<pointer> getStarred(Session& session, Wt::Dbo::ptr<User> user, const std::set<IdType>& clusters, std::optional<Range> range, bool& moreResults);
 
 		static std::vector<pointer>	getByClusters(Session& session, const std::set<IdType>& clusters);
@@ -71,6 +71,7 @@ class Release : public Wt::Dbo::Dbo<Release>
 
 		std::vector<Wt::Dbo::ptr<Track>> getTracks(const std::set<IdType>& clusters = std::set<IdType>()) const;
 		std::size_t			getTracksCount() const;
+		Wt::Dbo::ptr<Track>			getFirstTrack() const;
 
 		// Get the cluster of the tracks that belong to this release
 		// Each clusters are grouped by cluster type, sorted by the number of occurence (max to min)

src/libs/utils/impl/FileResourceHandler.cpp

@@ -60,7 +60,7 @@ FileResourceHandler::processRequest(const Wt::Http::Request& request, Wt::Http::
 		const ::uint64_t fileSize {static_cast<::uint64_t>(ifs.tellg())};
 		ifs.seekg(0, std::ios::beg);
 
-		LMS_LOG(UTILS, DEBUG) << "fileSize = " << fileSize;
+		LMS_LOG(UTILS, DEBUG) << "File '" << _path.string() << "', fileSize = " << fileSize;
 
 		const Wt::Http::Request::ByteRangeSpecifier ranges {request.getRanges(fileSize)};
 		if (!ranges.isSatisfiable())

src/lms/ui/resource/AudioFileResource.hpp

@@ -19,7 +19,6 @@
 
 #pragma once
 
-#include <filesystem>
 #include <Wt/WResource.h>
 
 #include "database/Types.hpp"

src/lms/ui/resource/ImageResource.hpp

@@ -17,17 +17,13 @@
  * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef COVER_RESOURCE_HPP_
+#pragma once
-#define COVER_RESOURCE_HPP_
-
-#include <mutex>
 
 #include <Wt/WResource.h>
-
 #include "database/Types.hpp"
 
-namespace UserInterface {
+namespace UserInterface
-
+{
 
 	class ImageResource : public Wt::WResource
 	{
@@ -52,4 +48,3 @@ class ImageResource : public Wt::WResource
 
 } // namespace UserInterface
 
-#endif

src/test/database/DatabaseTest.cpp

@@ -586,6 +586,53 @@ testMultiTracksSingleReleaseTotalDiscTrack(Session& session)
 	}
 }
 
+static
+void
+testMultiTracksSingleReleaseFirstTrack(Session& session)
+{
+	ScopedRelease release1 {session, "MyRelease1"};
+	ScopedRelease release2 {session, "MyRelease2"};
+
+	ScopedTrack track1A {session, "MyTrack1A"};
+	ScopedTrack track1B {session, "MyTrack1B"};
+	ScopedTrack track2A {session, "MyTrack2A"};
+	ScopedTrack track2B {session, "MyTrack2B"};
+
+	{
+		auto transaction {session.createSharedTransaction()};
+
+		CHECK(!release1->getFirstTrack());
+		CHECK(!release2->getFirstTrack());
+	}
+
+	{
+		auto transaction {session.createUniqueTransaction()};
+
+		track1A.get().modify()->setRelease(release1.get());
+		track1B.get().modify()->setRelease(release1.get());
+		track2A.get().modify()->setRelease(release2.get());
+		track2B.get().modify()->setRelease(release2.get());
+
+		track1A.get().modify()->setTrackNumber(1);
+		track1B.get().modify()->setTrackNumber(2);
+
+		track2A.get().modify()->setDiscNumber(2);
+		track2A.get().modify()->setTrackNumber(1);
+		track2B.get().modify()->setTrackNumber(2);
+		track2B.get().modify()->setDiscNumber(1);
+	}
+
+	{
+		auto transaction {session.createSharedTransaction()};
+
+		CHECK(release1->getFirstTrack());
+		CHECK(release2->getFirstTrack());
+
+		CHECK(release1->getFirstTrack().id() == track1A.getId());
+		CHECK(release2->getFirstTrack().id() == track2B.getId());
+	}
+}
+
 static
 void
 testSingleTrackSingleCluster(Session& session)
@@ -1930,6 +1977,7 @@ int main()
 
 		RUN_TEST(testSingleTrackSingleRelease);
 		RUN_TEST(testMultiTracksSingleReleaseTotalDiscTrack);
+		RUN_TEST(testMultiTracksSingleReleaseFirstTrack);
 
 		RUN_TEST(testSingleTrackSingleCluster);
 		RUN_TEST(testMultipleTracksSingleCluster);