diff --git a/src/libs/cover/impl/CoverArtGrabber.cpp b/src/libs/cover/impl/CoverArtGrabber.cpp index 52cd9729..c92e3f42 100644 --- a/src/libs/cover/impl/CoverArtGrabber.cpp +++ b/src/libs/cover/impl/CoverArtGrabber.cpp @@ -112,7 +112,7 @@ Grabber::getFromAvMediaFile(const Av::MediaFile& input, ImageSize width) const } std::unique_ptr -Grabber::getFromFile(const std::filesystem::path& p, ImageSize width) const +Grabber::getFromCoverFile(const std::filesystem::path& p, ImageSize width) const { std::unique_ptr 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 image {getFromFile(_defaultCoverPath, width)}; + std::shared_ptr 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 -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 coverPaths {getCoverPaths(p)}; + const std::multimap 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 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 +Grabber::getFromSameNamedFile(const std::filesystem::path& filePath, ImageSize width) const +{ + std::unique_ptr 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 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}; + + if (checkCoverFile(path)) + res.emplace(std::filesystem::path{ path }.filename().replace_extension("").string(), path); + 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; - } - - res.emplace(std::filesystem::path{path}.filename().replace_extension("").string(), path); } 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 Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, ImageSize width) +{ + return getFromTrack(dbSession, trackId, width, true /* allow release fallback*/); +} + +std::shared_ptr +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; - bool hasCover {}; - bool isMultiDisc {}; - std::filesystem::path trackPath; - + struct TrackInfo { + bool hasCover {}; + bool isMultiDisc {}; + std::filesystem::path trackPath; + std::optional releaseId; + }; + + auto getTrackInfo {[&] + { + std::optional 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(); - trackPath = track->getPath(); - - auto release {track->getRelease()}; - if (release && release->getTotalDisc() > 1) - isMultiDisc = true; + res->releaseId = release.id(); + if (release->getTotalDisc() > 1) + res->isMultiDisc = true; } - } - if (hasCover) - cover = getFromTrack(trackPath, width); + return res; + }}; - if (!cover) - cover = getFromDirectory(trackPath.parent_path(), trackPath.filename().replace_extension("").string(), width); - - if (!cover && isMultiDisc) + if (const std::optional trackInfo {getTrackInfo()}) { - if (trackPath.parent_path().has_parent_path()) - cover = getFromDirectory(trackPath.parent_path().parent_path(), {}, width); + if (trackInfo->hasCover) + cover = getFromTrack(trackInfo->trackPath, width); + + if (!cover) + cover = getFromSameNamedFile(trackInfo->trackPath, width); + + if (!cover && trackInfo->releaseId && allowReleaseFallback) + cover = getFromRelease(dbSession, *trackInfo->releaseId, width); + + if (!cover && trackInfo->isMultiDisc) + { + if (trackInfo->trackPath.parent_path().has_parent_path()) + 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 trackId; + struct ReleaseInfo { + Database::IdType firstTrackId; + std::filesystem::path releaseDirectory; + }; + + auto getReleaseInfo {[&] + { + std::optional res; + auto transaction {session.createSharedTransaction()}; - const auto release {Database::Release::getById(session, releaseId)}; - if (release) + if (const Database::Release::pointer release {Database::Release::getById(session, releaseId)}) { - const auto tracks {release->getTracks()}; - if (!tracks.empty()) - trackId = tracks.front().id(); + if (const auto firstTrack {release->getFirstTrack()}) + { + res = ReleaseInfo {}; + res->firstTrackId = firstTrack.id(); + res->releaseDirectory = firstTrack->getPath().parent_path(); + } } + + return res; + }}; + + if (const std::optional releaseInfo {getReleaseInfo()}) + { + cover = getFromDirectory(releaseInfo->releaseDirectory, width); + if (!cover) + cover = getFromTrack(session, releaseInfo->firstTrackId, width, false /* no release fallback */); } - if (trackId) - cover = getFromTrack(session, *trackId, width); - else + if (!cover) cover = getDefault(width); if (cover) diff --git a/src/libs/cover/impl/CoverArtGrabber.hpp b/src/libs/cover/impl/CoverArtGrabber.hpp index 87fbbeb9..3ba39a41 100644 --- a/src/libs/cover/impl/CoverArtGrabber.hpp +++ b/src/libs/cover/impl/CoverArtGrabber.hpp @@ -105,14 +105,18 @@ namespace CoverArt std::shared_ptr getFromRelease(Database::Session& dbSession, Database::IdType releaseId, ImageSize width) override; void flushCache() override; + std::shared_ptr getFromTrack(Database::Session& dbSession, Database::IdType trackId, ImageSize width, bool allowReleaseFallback); std::unique_ptr getFromAvMediaFile(const Av::MediaFile& input, ImageSize width) const; - std::unique_ptr getFromFile(const std::filesystem::path& p, ImageSize width) const; + std::unique_ptr getFromCoverFile(const std::filesystem::path& p, ImageSize width) const; std::unique_ptr getFromTrack(const std::filesystem::path& path, ImageSize width) const; std::multimap getCoverPaths(const std::filesystem::path& directoryPath) const; - std::unique_ptr getFromDirectory(const std::filesystem::path& path, std::string_view preferredFileName, ImageSize width) const; + std::unique_ptr getFromDirectory(const std::filesystem::path& directory, ImageSize width) const; + std::unique_ptr getFromSameNamedFile(const std::filesystem::path& filePath, ImageSize width) const; std::shared_ptr getDefault(ImageSize width); + bool checkCoverFile(const std::filesystem::path& directoryPath) const; + std::shared_mutex _cacheMutex; std::unordered_map> _cache; std::unordered_map> _defaultCoverCache; diff --git a/src/libs/database/impl/Release.cpp b/src/libs/database/impl/Release.cpp index 64a04b31..6b476f91 100644 --- a/src/libs/database/impl/Release.cpp +++ b/src/libs/database/impl/Release.cpp @@ -232,15 +232,15 @@ Release::getLastWritten(Session& session, } std::vector -Release::getByYear(Session& session, int yearFrom, int yearTo, std::optional offset, std::optional limit) +Release::getByYear(Session& session, int yearFrom, int yearTo, std::optional range) { Wt::Dbo::collection res = session.getDboSession().query ("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(*offset) : -1) - .limit(limit ? static_cast(*limit) : -1); + .offset(range ? static_cast(range->offset) : -1) + .limit(range ? static_cast(range->limit) : -1); return std::vector(res.begin(), res.end()); } @@ -532,6 +532,20 @@ Release::getTracksCount() const return _tracks.size(); } +Wt::Dbo::ptr +Release::getFirstTrack() const +{ + assert(self()); + assert(self()->id() != Wt::Dbo::dbo_traits::invalidId()); + assert(session()); + + return session()->query("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 { diff --git a/src/libs/database/include/database/Release.hpp b/src/libs/database/include/database/Release.hpp index bfa10b18..0354ff47 100644 --- a/src/libs/database/include/database/Release.hpp +++ b/src/libs/database/include/database/Release.hpp @@ -58,7 +58,7 @@ class Release : public Wt::Dbo::Dbo static std::vector getAllRandom(Session& session, const std::set& clusters, std::optional size = {}); static std::vector getAllIdsRandom(Session& session, const std::set& clusters, std::optional size = {}); static std::vector getLastWritten(Session& session, std::optional after, const std::set& clusters, std::optional range, bool& moreResults); - static std::vector getByYear(Session& session, int yearFrom, int yearTo, std::optional offset = {}, std::optional size = {}); + static std::vector getByYear(Session& session, int yearFrom, int yearTo, std::optional range = std::nullopt); static std::vector getStarred(Session& session, Wt::Dbo::ptr user, const std::set& clusters, std::optional range, bool& moreResults); static std::vector getByClusters(Session& session, const std::set& clusters); @@ -71,6 +71,7 @@ class Release : public Wt::Dbo::Dbo std::vector> getTracks(const std::set& clusters = std::set()) const; std::size_t getTracksCount() const; + Wt::Dbo::ptr 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) diff --git a/src/libs/utils/impl/FileResourceHandler.cpp b/src/libs/utils/impl/FileResourceHandler.cpp index f031adfc..f3eb1291 100644 --- a/src/libs/utils/impl/FileResourceHandler.cpp +++ b/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()) diff --git a/src/lms/ui/resource/AudioFileResource.hpp b/src/lms/ui/resource/AudioFileResource.hpp index 9e52a077..0865376b 100644 --- a/src/lms/ui/resource/AudioFileResource.hpp +++ b/src/lms/ui/resource/AudioFileResource.hpp @@ -19,7 +19,6 @@ #pragma once -#include #include #include "database/Types.hpp" diff --git a/src/lms/ui/resource/ImageResource.hpp b/src/lms/ui/resource/ImageResource.hpp index ea6adbf1..76c574aa 100644 --- a/src/lms/ui/resource/ImageResource.hpp +++ b/src/lms/ui/resource/ImageResource.hpp @@ -17,39 +17,34 @@ * along with LMS. If not, see . */ -#ifndef COVER_RESOURCE_HPP_ -#define COVER_RESOURCE_HPP_ - -#include +#pragma once #include - #include "database/Types.hpp" -namespace UserInterface { - - -class ImageResource : public Wt::WResource +namespace UserInterface { - public: - static const std::size_t maxSize {512}; - ~ImageResource(); + class ImageResource : public Wt::WResource + { + public: + static const std::size_t maxSize {512}; - enum class Size : std::size_t - { - Small = 128, - Large = 512, - }; + ~ImageResource(); - std::string getReleaseUrl(Database::IdType releaseId, Size size) const; - std::string getTrackUrl(Database::IdType trackId, Size size) const; + enum class Size : std::size_t + { + Small = 128, + Large = 512, + }; - private: - void handleRequest(const Wt::Http::Request& request, Wt::Http::Response& response) override; + std::string getReleaseUrl(Database::IdType releaseId, Size size) const; + std::string getTrackUrl(Database::IdType trackId, Size size) const; -}; + private: + void handleRequest(const Wt::Http::Request& request, Wt::Http::Response& response) override; + + }; } // namespace UserInterface -#endif diff --git a/src/test/database/DatabaseTest.cpp b/src/test/database/DatabaseTest.cpp index 2a54fbd7..6ce4fd4e 100644 --- a/src/test/database/DatabaseTest.cpp +++ b/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);