From 1e2c1caeed95e0aeed78f820413970f5a672daf2 Mon Sep 17 00:00:00 2001 From: emeric Date: Thu, 13 Feb 2020 16:10:43 +0100 Subject: [PATCH] Isolated ImageMagick++ in liblms --- src/liblms/CMakeLists.txt | 16 ++++-- .../impl/api/subsonic/SubsonicResource.cpp | 4 +- src/liblms/impl/cover/CoverArtGrabber.cpp | 55 +++++++++++-------- src/liblms/impl/cover/CoverArtGrabber.hpp | 21 ++++--- src/liblms/impl/cover/Image.cpp | 36 +++++++----- src/liblms/impl/cover/Image.hpp | 50 +++++++---------- src/liblms/include/cover/ICoverArtGrabber.hpp | 3 +- src/lms/main.cpp | 6 +- src/lms/ui/LmsApplication.cpp | 2 +- src/lms/ui/resource/ImageResource.cpp | 11 ++-- src/lms/ui/resource/ImageResource.hpp | 2 - 11 files changed, 107 insertions(+), 99 deletions(-) diff --git a/src/liblms/CMakeLists.txt b/src/liblms/CMakeLists.txt index 37ba7cfd..2c1644ff 100644 --- a/src/liblms/CMakeLists.txt +++ b/src/liblms/CMakeLists.txt @@ -10,6 +10,7 @@ add_library(liblms SHARED impl/av/AvTranscoder.cpp impl/av/AvTypes.cpp impl/cover/CoverArtGrabber.cpp + impl/cover/Image.cpp impl/database/Artist.cpp impl/database/Cluster.cpp impl/database/Db.cpp @@ -43,18 +44,21 @@ add_library(liblms SHARED impl/utils/WtLogger.cpp ) -include_directories(liblms include/) - target_include_directories(liblms INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/include ) -# TODO make this private -target_include_directories(liblms PUBLIC ${IMAGEMAGICKXX_INCLUDE_DIRS}) -target_link_libraries(liblms PUBLIC ${IMAGEMAGICKXX_LIBRARIES}) -target_compile_options(liblms PUBLIC ${IMAGEMAGICKXX_CFLAGS_OTHER}) +target_include_directories(liblms PRIVATE + include/ + ${IMAGEMAGICKXX_INCLUDE_DIRS} + ) + +target_compile_options(liblms PRIVATE + ${IMAGEMAGICKXX_CFLAGS_OTHER} + ) target_link_libraries(liblms PRIVATE + ${IMAGEMAGICKXX_LIBRARIES} avformat avutil config++ diff --git a/src/liblms/impl/api/subsonic/SubsonicResource.cpp b/src/liblms/impl/api/subsonic/SubsonicResource.cpp index c2e72223..3a4db863 100644 --- a/src/liblms/impl/api/subsonic/SubsonicResource.cpp +++ b/src/liblms/impl/api/subsonic/SubsonicResource.cpp @@ -1895,10 +1895,10 @@ handleGetCoverArt(RequestContext& context, Wt::Http::ResponseContinuation*) switch (id.type) { case Id::Type::Track: - res.data = ServiceProvider::get()->getFromTrack(context.dbSession, id.value, CoverArt::Format::JPEG, {size, size}); + res.data = ServiceProvider::get()->getFromTrack(context.dbSession, id.value, CoverArt::Format::JPEG, size); break; case Id::Type::Release: - res.data = ServiceProvider::get()->getFromRelease(context.dbSession, id.value, CoverArt::Format::JPEG, {size, size}); + res.data = ServiceProvider::get()->getFromRelease(context.dbSession, id.value, CoverArt::Format::JPEG, size); break; default: throw BadParameterGenericError {"id"}; diff --git a/src/liblms/impl/cover/CoverArtGrabber.cpp b/src/liblms/impl/cover/CoverArtGrabber.cpp index 110e9ddd..110b80d1 100644 --- a/src/liblms/impl/cover/CoverArtGrabber.cpp +++ b/src/liblms/impl/cover/CoverArtGrabber.cpp @@ -39,8 +39,19 @@ isFileSupported(const std::filesystem::path& file, const std::vector createGrabber(const std::filesystem::path& execPath) { + return std::make_unique(execPath); +} + +Grabber::Grabber(const std::filesystem::path& execPath) +{ + init(execPath); +} + +Grabber::~Grabber() +{ + deinit(); } void @@ -50,7 +61,7 @@ Grabber::setDefaultCover(const std::filesystem::path& p) throw LmsException("Cannot read default cover file '" + p.string() + "'"); } -Image::Image +Image Grabber::getDefaultCover(std::size_t size) { LMS_LOG(COVER, DEBUG) << "Getting a default cover using size = " << size; @@ -59,12 +70,12 @@ Grabber::getDefaultCover(std::size_t size) auto it = _defaultCovers.find(size); if (it == _defaultCovers.end()) { - Image::Image cover = _defaultCover; + 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(Image::Geometry{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); @@ -74,14 +85,14 @@ Grabber::getDefaultCover(std::size_t size) return it->second; } -static std::optional +static std::optional getFromAvMediaFile(const Av::MediaFile& input) { - std::vector res; + std::vector res; for (auto& picture : input.getAttachedPictures(2)) { - Image::Image image; + Image image; if (image.load(picture.data)) return image; @@ -93,12 +104,12 @@ getFromAvMediaFile(const Av::MediaFile& input) return std::nullopt; } -std::optional +std::optional Grabber::getFromDirectory(const std::filesystem::path& p) const { for (auto coverPath : getCoverPaths(p)) { - Image::Image image; + Image image; if (image.load(coverPath)) return image; @@ -143,7 +154,7 @@ Grabber::getCoverPaths(const std::filesystem::path& directoryPath) const return res; } -std::optional +std::optional Grabber::getFromTrack(const std::filesystem::path& p) const { try @@ -159,12 +170,12 @@ Grabber::getFromTrack(const std::filesystem::path& p) const } } -Image::Image +Image Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, std::size_t size) { using namespace Database; - std::optional cover; + std::optional cover; bool hasCover {}; bool isMultiDisc {}; @@ -200,16 +211,16 @@ Grabber::getFromTrack(Database::Session& dbSession, Database::IdType trackId, st if (!cover) cover = getDefaultCover(size); else - cover->scale(Image::Geometry {size, size}); + cover->scale(Geometry {size, size}); return *cover; } -Image::Image +Image Grabber::getFromRelease(Database::Session& session, Database::IdType releaseId, std::size_t size) { - std::optional cover; + std::optional cover; std::optional trackId; { @@ -230,26 +241,26 @@ Grabber::getFromRelease(Database::Session& session, Database::IdType releaseId, if (!cover) cover = getDefaultCover(size); else - cover->scale(Image::Geometry {size, size}); + cover->scale(Geometry {size, size}); return *cover; } std::vector -Grabber::getFromTrack(Database::Session& session, Database::IdType trackId, Format format, std::size_t size) +Grabber::getFromTrack(Database::Session& session, Database::IdType trackId, Format format, std::size_t width) { - const Image::Image cover {getFromTrack(session, trackId, size)}; + const Image cover {getFromTrack(session, trackId, width)}; - assert(format == Image::Format::JPEG); + assert(format == Format::JPEG); return cover.save(format); } std::vector -Grabber::getFromRelease(Database::Session& session, Database::IdType releaseId, Format format, std::size_t size) +Grabber::getFromRelease(Database::Session& session, Database::IdType releaseId, Format format, std::size_t width) { - const Image::Image cover {getFromRelease(session, releaseId, size)}; + const Image cover {getFromRelease(session, releaseId, width)}; - assert(format == Image::Format::JPEG); + assert(format == Format::JPEG); return cover.save(format); } diff --git a/src/liblms/impl/cover/CoverArtGrabber.hpp b/src/liblms/impl/cover/CoverArtGrabber.hpp index 3911ad43..bc921355 100644 --- a/src/liblms/impl/cover/CoverArtGrabber.hpp +++ b/src/liblms/impl/cover/CoverArtGrabber.hpp @@ -40,7 +40,9 @@ namespace CoverArt class Grabber : public IGrabber { public: - Grabber(); + Grabber(const std::filesystem::path& execPath); + ~Grabber(); + Grabber(const Grabber&) = delete; Grabber& operator=(const Grabber&) = delete; Grabber(Grabber&&) = delete; @@ -53,24 +55,21 @@ namespace CoverArt private: - Image::Image getFromTrack(Database::Session& dbSession, Database::IdType trackId, std::size_t size); - Image::Image getFromRelease(Database::Session& dbSession, Database::IdType releaseId, std::size_t size); + Image getFromTrack(Database::Session& dbSession, Database::IdType trackId, std::size_t size); + Image getFromRelease(Database::Session& dbSession, Database::IdType releaseId, std::size_t size); - std::optional getFromTrack(const std::filesystem::path& path) const; + std::optional getFromTrack(const std::filesystem::path& path) const; std::vector getCoverPaths(const std::filesystem::path& directoryPath) const; - std::optional getFromDirectory(const std::filesystem::path& path) const; + std::optional getFromDirectory(const std::filesystem::path& path) const; + Image getDefaultCover(std::size_t size); - Image::Image getDefaultCover(std::size_t size); - - Image::Image _defaultCover; + Image _defaultCover; std::mutex _mutex; - std::map _defaultCovers; + std::map _defaultCovers; static inline const std::vector _fileExtensions {".jpg", ".jpeg", ".png", ".bmp"}; // TODO parametrize - static inline const std::size_t _maxFileSize {10000000}; - static inline const std::vector _preferredFileNames {"cover", "front"}; // TODO parametrize }; diff --git a/src/liblms/impl/cover/Image.cpp b/src/liblms/impl/cover/Image.cpp index 06532eca..a39c0061 100644 --- a/src/liblms/impl/cover/Image.cpp +++ b/src/liblms/impl/cover/Image.cpp @@ -17,14 +17,27 @@ * along with LMS. If not, see . */ -#include "image/Image.hpp" +#include "Image.hpp" #include "utils/Logger.hpp" -namespace Image { +namespace CoverArt { + +void +init(const std::filesystem::path& path) +{ + Magick::InitializeMagick(path.string().c_str()); +} + +void +deinit() +{ + MagickCore::MagickCoreTerminus(); +} static -std::string format_to_magick(Format format) +std::string +formatToMagick(Format format) { switch (format) { @@ -34,7 +47,8 @@ std::string format_to_magick(Format format) return "JPEG"; } -std::string format_to_mimeType(Format format) +std::string +formatToMimeType(Format format) { switch (format) { @@ -44,18 +58,13 @@ std::string format_to_mimeType(Format format) return "application/octet-stream"; } -void -init(const char *path) -{ - Magick::InitializeMagick(path); -} bool Image::load(const std::vector& rawData) { try { - Magick::Blob blob(&rawData[0], rawData.size()); + Magick::Blob blob {&rawData[0], rawData.size()}; _image.read(blob); return true; @@ -116,9 +125,9 @@ Image::save(Format format) const try { - Magick::Image outputImage(_image); + Magick::Image outputImage {_image}; - outputImage.magick( format_to_magick(format)); + outputImage.magick(formatToMagick(format)); Magick::Blob blob; outputImage.write(&blob); @@ -135,4 +144,5 @@ Image::save(Format format) const } } -} // namespace Image +} // namespace CoverArt + diff --git a/src/liblms/impl/cover/Image.hpp b/src/liblms/impl/cover/Image.hpp index 40bc2d6b..6932825f 100644 --- a/src/liblms/impl/cover/Image.hpp +++ b/src/liblms/impl/cover/Image.hpp @@ -24,44 +24,34 @@ #include -namespace Image +#include "cover/CoverArt.hpp" + +namespace CoverArt { -enum class Format -{ - JPEG, -}; + void init(const std::filesystem::path& path); + void deinit(); -std::string format_to_mimeType(Format format); + class Image + { + public: -void init(const char *path); + // input + bool load(const std::vector& rawData); + bool load(const std::filesystem::path& p); -struct Geometry -{ - std::size_t width; - std::size_t height; -}; + Geometry getSize() const; -class Image -{ - public: + // Operations + bool scale(Geometry geometry); - // input - bool load(const std::vector& rawData); - bool load(const std::filesystem::path& p); + // output + std::vector save(Format format) const; - Geometry getSize() const; - - // Operations - bool scale(Geometry geometry); - - // output - std::vector save(Format format) const; - - private: - Magick::Image _image; -}; + private: + Magick::Image _image; + }; -} // namespace Image +} // namespace CoverArt diff --git a/src/liblms/include/cover/ICoverArtGrabber.hpp b/src/liblms/include/cover/ICoverArtGrabber.hpp index e018521a..c62e3955 100644 --- a/src/liblms/include/cover/ICoverArtGrabber.hpp +++ b/src/liblms/include/cover/ICoverArtGrabber.hpp @@ -40,8 +40,9 @@ class IGrabber virtual std::vector getFromTrack(Database::Session& dbSession, Database::IdType trackId, Format format, std::size_t width) = 0; virtual std::vector getFromRelease(Database::Session& dbSession, Database::IdType releaseId, Format format, std::size_t width) = 0; - }; +std::unique_ptr createGrabber(const std::filesystem::path& execPath); + } // namespace CoverArt diff --git a/src/lms/main.cpp b/src/lms/main.cpp index b51512d0..5f5bf313 100644 --- a/src/lms/main.cpp +++ b/src/lms/main.cpp @@ -27,9 +27,8 @@ #include "av/AvTranscoder.hpp" #include "auth/IAuthTokenService.hpp" #include "auth/IPasswordService.hpp" -#include "cover/CoverArtGrabber.hpp" +#include "cover/ICoverArtGrabber.hpp" #include "database/Db.hpp" -#include "image/Image.hpp" #include "scanner/MediaScanner.hpp" #include "recommendation/FeaturesRecommendationProviderCreator.hpp" #include "recommendation/IEngine.hpp" @@ -130,7 +129,6 @@ int main(int argc, char* argv[]) server.setServerConfiguration (wtServerArgs.size(), const_cast(&wtArgv[0])); // lib init - Image::init(argv[0]); Av::Transcoder::init(); // Initializing a connection pool to the database that will be shared along services @@ -151,7 +149,7 @@ int main(int argc, char* argv[]) recommendationEngine.addProvider(Recommendation::createFeaturesRecommendationProvider(mediaScanner), 0); recommendationEngine.addProvider(Recommendation::createClustersRecommendationProvider(), 1); - CoverArt::Grabber& coverArtGrabber {ServiceProvider::create()}; + CoverArt::IGrabber& coverArtGrabber {ServiceProvider::assign(CoverArt::createGrabber(argv[0]))}; coverArtGrabber.setDefaultCover(server.appRoot() + "/images/unknown-cover.jpg"); API::Subsonic::SubsonicResource subsonicResource {database}; diff --git a/src/lms/ui/LmsApplication.cpp b/src/lms/ui/LmsApplication.cpp index 669417e2..56480d94 100644 --- a/src/lms/ui/LmsApplication.cpp +++ b/src/lms/ui/LmsApplication.cpp @@ -29,7 +29,7 @@ #include #include -#include "cover/CoverArtGrabber.hpp" +#include "cover/ICoverArtGrabber.hpp" #include "database/Artist.hpp" #include "database/Cluster.hpp" #include "database/Db.hpp" diff --git a/src/lms/ui/resource/ImageResource.cpp b/src/lms/ui/resource/ImageResource.cpp index f0bade0c..6abd1d42 100644 --- a/src/lms/ui/resource/ImageResource.cpp +++ b/src/lms/ui/resource/ImageResource.cpp @@ -22,7 +22,7 @@ #include #include -#include "cover/CoverArtGrabber.hpp" +#include "cover/ICoverArtGrabber.hpp" #include "database/Track.hpp" #include "utils/Exception.hpp" #include "utils/Logger.hpp" @@ -33,9 +33,6 @@ namespace UserInterface { -static const std::string unknownCoverPath = "/images/unknown-cover.jpg"; -static const std::string unknownArtistImagePath = "/images/unknown-artist.jpg"; - ImageResource::~ImageResource() { beingDeleted(); @@ -80,7 +77,7 @@ ImageResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons // DbSession are not thread safe { Wt::WApplication::UpdateLock lock {LmsApp}; - cover = ServiceProvider::get()->getFromTrack(LmsApp->getDbSession(), *trackId, Image::Format::JPEG, *size); + cover = ServiceProvider::get()->getFromTrack(LmsApp->getDbSession(), *trackId, CoverArt::Format::JPEG, *size); } } else if (releaseIdStr) @@ -92,7 +89,7 @@ ImageResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons // DbSession are not thread safe { Wt::WApplication::UpdateLock lock {LmsApp}; - cover = ServiceProvider::get()->getFromRelease(LmsApp->getDbSession(), *releaseId, Image::Format::JPEG, *size); + cover = ServiceProvider::get()->getFromRelease(LmsApp->getDbSession(), *releaseId, CoverArt::Format::JPEG, *size); } } else @@ -106,7 +103,7 @@ ImageResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons std::string ImageResource::getMimeType() { - return Image::format_to_mimeType(Image::Format::JPEG); + return CoverArt::formatToMimeType(CoverArt::Format::JPEG); } } // namespace UserInterface diff --git a/src/lms/ui/resource/ImageResource.hpp b/src/lms/ui/resource/ImageResource.hpp index 2f372b7d..29b5a595 100644 --- a/src/lms/ui/resource/ImageResource.hpp +++ b/src/lms/ui/resource/ImageResource.hpp @@ -26,8 +26,6 @@ #include "database/Types.hpp" -#include "image/Image.hpp" - namespace UserInterface {