[Image] Improved cover art handling

src/Makefile.am

@@ -4,7 +4,6 @@ lms_SOURCES = \
 	$(srcdir)/main/main.cpp					\
 	$(srcdir)/av/AvInfo.cpp					\
 	$(srcdir)/av/AvTranscoder.cpp					\
-	$(srcdir)/cover/CoverArt.cpp				\
 	$(srcdir)/cover/CoverArtGrabber.cpp			\
 	$(srcdir)/database/Artist.cpp				\
 	$(srcdir)/database/DatabaseHandler.cpp			\
@@ -18,6 +17,7 @@ lms_SOURCES = \
 	$(srcdir)/database/Video.cpp				\
 	$(srcdir)/database-updater/DatabaseUpdater.cpp		\
 	$(srcdir)/database-updater/Checksum.cpp			\
+	$(srcdir)/image/Image.cpp				\
 	$(srcdir)/logger/Logger.cpp				\
 	$(srcdir)/metadata/AvFormat.cpp				\
 	$(srcdir)/metadata/Utils.cpp				\

src/cover/CoverArtGrabber.cpp

@@ -23,7 +23,6 @@
 
 #include "CoverArtGrabber.hpp"
 
-
 namespace {
 
 bool
@@ -44,7 +43,6 @@ isFileSupported(const boost::filesystem::path& file, const std::vector<boost::fi
 namespace CoverArt {
 
 Grabber::Grabber()
-: _maxFileSize(0)
 {
 }
 
@@ -55,21 +53,28 @@ Grabber::instance()
 	return instance;
 }
 
-static std::vector<CoverArt>
+static std::vector<Image::Image>
 getFromAvMediaFile(const Av::MediaFile& input, std::size_t nbMaxCovers)
 {
-	std::vector<CoverArt> res;
+	std::vector<Image::Image> res;
 
 	for (Av::Picture& picture : input.getAttachedPictures(nbMaxCovers))
-		res.push_back( CoverArt(picture.data) );
+	{
+		Image::Image image;
+
+		if (image.load(picture.data))
+			res.push_back( image );
+		else
+			LMS_LOG(COVER, ERROR) << "Cannot load embedded cover file in '" << input.getPath() << "'";
+	}
 
 	return res;
 }
 
-std::vector<CoverArt>
+std::vector<Image::Image>
 Grabber::getFromDirectory(const boost::filesystem::path& p, std::size_t nbMaxCovers) const
 {
-	std::vector<CoverArt> res;
+	std::vector<Image::Image> res;
 
 	std::vector<boost::filesystem::path> coverPathes = getCoverPaths(p, nbMaxCovers);
 	for (auto coverPath : coverPathes)
@@ -77,13 +82,12 @@ Grabber::getFromDirectory(const boost::filesystem::path& p, std::size_t nbMaxCov
 		if (res.size() >= nbMaxCovers)
 			break;
 
-		std::vector<unsigned char> data;
+		Image::Image image;
-		std::ifstream file(coverPath.string(), std::ios::binary);
-		char c;
-		while (file.get(c))
-			data.push_back(c);
 
-		res.push_back(CoverArt(data));
+		if (image.load(coverPath))
+			res.push_back(image);
+		else
+			LMS_LOG(COVER, ERROR) << "Cannot load image in file '" << coverPath << "'";
 	}
 
 	return res;
@@ -123,7 +127,7 @@ Grabber::getCoverPaths(const boost::filesystem::path& directoryPath, std::size_t
 	return res;
 }
 
-std::vector<CoverArt>
+std::vector<Image::Image>
 Grabber::getFromTrack(const boost::filesystem::path& p, std::size_t nbMaxCovers) const
 {
 	Av::MediaFile input(p);
@@ -131,10 +135,10 @@ Grabber::getFromTrack(const boost::filesystem::path& p, std::size_t nbMaxCovers)
 	if (input.open())
 		return getFromAvMediaFile(input, nbMaxCovers);
 	else
-		return std::vector<CoverArt>();
+		return std::vector<Image::Image>();
 }
 
-std::vector<CoverArt>
+std::vector<Image::Image>
 Grabber::getFromTrack(Wt::Dbo::Session& session, Database::Track::id_type trackId, std::size_t nbMaxCovers) const
 {
 	using namespace Database;
@@ -143,7 +147,7 @@ Grabber::getFromTrack(Wt::Dbo::Session& session, Database::Track::id_type trackI
 
 	Track::pointer track = Track::getById(session, trackId);
 	if (!track)
-		return std::vector<CoverArt>();
+		return std::vector<Image::Image>();
 
 	Track::CoverType coverType = track->getCoverType();
 	boost::filesystem::path trackPath = track->getPath();
@@ -154,17 +158,15 @@ Grabber::getFromTrack(Wt::Dbo::Session& session, Database::Track::id_type trackI
 	{
 		case Track::CoverType::Embedded:
 			return Grabber::getFromTrack(trackPath, nbMaxCovers);
-		case Track::CoverType::ExternalFile:
-			return Grabber::getFromDirectory(trackPath.parent_path(), nbMaxCovers);
 		case Track::CoverType::None:
-			return std::vector<CoverArt>();
+			return Grabber::getFromDirectory(trackPath.parent_path(), nbMaxCovers);
 	}
 
-	return std::vector<CoverArt>();
+	return std::vector<Image::Image>();
 }
 
 
-std::vector<CoverArt>
+std::vector<Image::Image>
 Grabber::getFromRelease(Wt::Dbo::Session& session, Database::Release::id_type releaseId, std::size_t nbMaxCovers) const
 {
 	using namespace Database;
@@ -181,14 +183,14 @@ Grabber::getFromRelease(Wt::Dbo::Session& session, Database::Release::id_type re
 					-1, 1 /* limit result size */);
 
 		if (tracks.empty())
-			return std::vector<CoverArt>();
+			return std::vector<Image::Image>();
 
 		firstTrackPath = tracks.front()->getPath();
 		embeddedCover = (tracks.front()->getCoverType() == Track::CoverType::Embedded);
 	}
 
 	// First, try to get covers from the directory of the release
-	std::vector<CoverArt> res = getFromDirectory( firstTrackPath.parent_path(), nbMaxCovers);
+	std::vector<Image::Image> res = getFromDirectory( firstTrackPath.parent_path(), nbMaxCovers);
 
 	// Fallback on the embedded cover of the first track
 	if (res.empty() && embeddedCover)

src/cover/CoverArtGrabber.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Emeric Poupon
+ * Copyright (C) 2015 Emeric Poupon
  *
  * This file is part of LMS.
  *
@@ -17,15 +17,13 @@
  * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef COVER_ART_GRABBER_HPP
+#pragma once
-#define COVER_ART_GRABBER_HPP
 
 #include <vector>
 
 #include "database/Types.hpp"
 
-#include "CoverArt.hpp"
+#include "image/Image.hpp"
-
 
 namespace CoverArt {
 
@@ -38,24 +36,20 @@ class Grabber
 		static Grabber& instance();
 
 		std::vector<boost::filesystem::path>	getCoverPaths(const boost::filesystem::path& directoryPath, std::size_t nbMaxCovers = 1) const;
-		std::vector<CoverArt>	getFromDirectory(const boost::filesystem::path& path, std::size_t nbMaxCovers = 1) const;
+		std::vector<Image::Image>	getFromDirectory(const boost::filesystem::path& path, std::size_t nbMaxCovers = 1) const;
-		std::vector<CoverArt>	getFromTrack(const boost::filesystem::path& path, std::size_t nbMaxCovers = 1) const;
+		std::vector<Image::Image>	getFromTrack(const boost::filesystem::path& path, std::size_t nbMaxCovers = 1) const;
-		std::vector<CoverArt>	getFromTrack(Wt::Dbo::Session& session, Database::Track::id_type trackId, std::size_t nbMaxCovers = 1) const;
+		std::vector<Image::Image>	getFromTrack(Wt::Dbo::Session& session, Database::Track::id_type trackId, std::size_t nbMaxCovers = 1) const;
-		std::vector<CoverArt>	getFromRelease(Wt::Dbo::Session& session, Database::Release::id_type releaseId, std::size_t nbMaxCovers = 1) const;
+		std::vector<Image::Image>	getFromRelease(Wt::Dbo::Session& session, Database::Release::id_type releaseId, std::size_t nbMaxCovers = 1) const;
 
 	private:
 		Grabber();
 
 		std::vector<boost::filesystem::path> _fileExtensions
-			= {"jpg", "jpeg"};
+			= {".jpg", ".jpeg", ".png", ".bmp"}; // TODO parametrize
 		std::size_t _maxFileSize = 5000000;
 		std::vector<boost::filesystem::path> _preferredFileNames
-			= {"cover", "front"};
+			= {"cover", "front"}; // TODO parametrize
-
-
 };
 
 } // namespace CoverArt
 
-
-#endif

src/database-updater/DatabaseUpdater.cpp

@@ -366,27 +366,13 @@ Updater::processAudioFile( const boost::filesystem::path& file, Stats& stats)
 
 		Wt::Dbo::Transaction transaction(_db.getSession());
 
-		// Skip file if last write is the same
 		Wt::Dbo::ptr<Track> track = Track::getByPath(_db.getSession(), file);
 
-		// if the file is the same and embeds covers, no need to update
+		// Skip file if last write is the same
-		if (track && track->getLastWriteTime() == lastWriteTime
-			&& (track->getCoverType() == Database::Track::CoverType::Embedded))
-			return;
-
-		// Check for external covers
-		std::vector<boost::filesystem::path> externalCovers = CoverArt::Grabber::instance().getCoverPaths(file.parent_path());
 		if (track && track->getLastWriteTime() == lastWriteTime)
-		{
-			// no change since last time we updated
-			// Skip only if no external covers has to be set
-			if (((track->getCoverType() == Database::Track::CoverType::None) && externalCovers.empty())
-				|| ((track->getCoverType() == Database::Track::CoverType::ExternalFile) && !externalCovers.empty()))
 			return;
-		}
 
 		MetaData::Items items;
-
 		if (!_metadataParser.parse(file, items))
 			return;
 
@@ -535,12 +521,7 @@ Updater::processAudioFile( const boost::filesystem::path& file, Stats& stats)
 		{
 			bool hasCover = boost::any_cast<bool>(items[MetaData::Type::HasCover]);
 
-			if (hasCover)
+			track.modify()->setCoverType( hasCover ? Track::CoverType::Embedded : Track::CoverType::None );
-				track.modify()->setCoverType( Track::CoverType::Embedded );
-			else if (!externalCovers.empty())
-				track.modify()->setCoverType( Track::CoverType::ExternalFile );
-			else
-				track.modify()->setCoverType( Track::CoverType::None);
 		}
 
 		transaction.commit();

src/database-updater/DatabaseUpdater.hpp

@@ -97,10 +97,10 @@ class Updater
 		Database::Handler	_db;
 
 		std::vector<boost::filesystem::path>	_audioExtensions
-			= {".mp3", ".ogg", ".oga", ".aac", ".m4a", ".flac", ".wav", ".wma", ".aif", ".aiff", ".ape", ".mpc", ".shn"};
+			= {".mp3", ".ogg", ".oga", ".aac", ".m4a", ".flac", ".wav", ".wma", ".aif", ".aiff", ".ape", ".mpc", ".shn"}; // TODO parametrize
 
 		std::vector<boost::filesystem::path>	_videoExtensions
-			= {".flv", ".avi", ".mpg", ".mpeg", ".mp4", ".m4v", ".mkv", ".mov", ".wmv", ".ogv", ".divx", ".m2ts"};
+			= {".flv", ".avi", ".mpg", ".mpeg", ".mp4", ".m4v", ".mkv", ".mov", ".wmv", ".ogv", ".divx", ".m2ts"}; // TODO parametrize
 
 		MetaData::Parser&	_metadataParser;
 

src/database/Track.hpp

@@ -97,7 +97,6 @@ class Track
 		enum class CoverType
 		{
 			Embedded,	// Contains embedded cover
-			ExternalFile,	// Cover is in an external file
 			None,		// No local cover available
 		};
 

src/image/Image.cpp

@@ -18,10 +18,10 @@
  */
 #include "logger/Logger.hpp"
 
-#include "CoverArt.hpp"
+#include "Image.hpp"
 
 
-namespace CoverArt {
+namespace Image {
 
 static
 std::string format_to_magick(Format format)
@@ -45,20 +45,46 @@ std::string format_to_mimeType(Format format)
 }
 
 void
-CoverArt::init(const char *path)
+init(const char *path)
 {
 	Magick::InitializeMagick(path);
 }
 
-CoverArt::CoverArt(const std::vector<unsigned char>& rawData)
+bool
+Image::load(const std::vector<unsigned char>& rawData)
 {
-	Magick::Blob blob(&rawData[0], rawData.size());
+	try
-	_image.read(blob);
+	{
+		Magick::Blob blob(&rawData[0], rawData.size());
+		_image.read(blob);
+
+		return true;
+	}
+	catch (Magick::Exception& e)
+	{
+		LMS_LOG(COVER, ERROR) << "Caught Magick exception while loading raw image: " << e.what();
+		return false;
+	}
 }
 
+bool
+Image::load(boost::filesystem::path p)
+{
+	try
+	{
+		_image.read(p.string());
+
+		return true;
+	}
+	catch (Magick::Exception& e)
+	{
+		LMS_LOG(COVER, ERROR) << "Caught Magick exception while loading image from file '" << p << "': " << e.what();
+		return false;
+	}
+}
 
 bool
-CoverArt::scale(std::size_t size)
+Image::scale(std::size_t size)
 {
 	if (!size)
 		return false;
@@ -71,15 +97,14 @@ CoverArt::scale(std::size_t size)
 	}
 	catch (Magick::Exception& e)
 	{
-		LMS_LOG(COVER, ERROR) << "Caught exception: " << e.what();
+		LMS_LOG(COVER, ERROR) << "Caught Magick exception: " << e.what();
 		return false;
 	}
 }
 
 void
-CoverArt::getData(std::vector<unsigned char>& data, Format format) const
+Image::save(std::vector<unsigned char>& data, Format format) const
 {
-
 	Magick::Image outputImage(_image);
 
 	outputImage.magick( format_to_magick(format));
@@ -91,4 +116,4 @@ CoverArt::getData(std::vector<unsigned char>& data, Format format) const
 	data.assign( charBuf, charBuf + blob.length() );
 }
 
-} // namespace CoverArt
+} // namespace Image

src/image/Image.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Emeric Poupon
+ * Copyright (C) 2015 Emeric Poupon
  *
  * This file is part of LMS.
  *
@@ -17,15 +17,15 @@
  * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef COVER_ART_HPP
+#pragma once
-#define COVER_ART_HPP
 
 #include <vector>
-#include <string>
+
+#include <boost/filesystem/path.hpp>
 
 #include <Magick++.h>
 
-namespace CoverArt
+namespace Image
 {
 
 enum class Format
@@ -35,26 +35,26 @@ enum class Format
 
 std::string format_to_mimeType(Format format);
 
-class CoverArt
+void init(const char *path);
+
+class Image
 {
 	public:
-		static void init(const char *path);
 
-		CoverArt() {}
+		// input
-		CoverArt(const std::vector<unsigned char>& rawData);
+		bool	load(const std::vector<unsigned char>& rawData);
+		bool	load(boost::filesystem::path p);
 
-		// Operations on cover arts
+		// Operations
 		bool	scale(std::size_t size);
 
 		// output
-		void	getData(std::vector<unsigned char>& data, Format format) const;
+		void	save(std::vector<unsigned char>& rawData, Format format) const;
 
 	private:
 		Magick::Image	_image;
 };
 
 
-} // namespace CoverArt
+} // namespace Image
-
-#endif
 

src/main/main.cpp

@@ -23,7 +23,7 @@
 #include "av/AvInfo.hpp"
 #include "av/AvTranscoder.hpp"
 #include "logger/Logger.hpp"
-#include "cover/CoverArtGrabber.hpp"
+#include "image/Image.hpp"
 
 #include "ui/LmsApplication.hpp"
 
@@ -45,7 +45,6 @@ int main(int argc, char* argv[])
 	try
 	{
 
-		CoverArt::CoverArt::init(argv[0]);
 		Wt::WServer server(argv[0]);
 		server.setServerConfiguration (argc, argv);
 
@@ -54,6 +53,7 @@ int main(int argc, char* argv[])
 		Service::ServiceManager& serviceManager = Service::ServiceManager::instance();
 
 		// lib init
+		Image::init(argv[0]);
 		Av::AvInit();
 		Av::Transcoder::init();
 		Database::Handler::configureAuth();

src/ui/audio/desktop/PlayQueue.cpp

@@ -389,10 +389,7 @@ PlayQueue::addTracks(const std::vector<Database::Track::id_type>& trackIds)
 			_model->setData(dataRow, COLUMN_ID_TRACK_ID, track.id(), Wt::UserRole);
 
 			std::string coverUrl;
-			if (track->getCoverType() != Track::CoverType::None)
+			coverUrl = LmsApplication::instance()->getCoverResource()->getTrackUrl(track.id(), 64);
-				coverUrl = LmsApplication::instance()->getCoverResource()->getTrackUrl(track.id(), 64);
-			else
-				coverUrl = LmsApplication::instance()->getCoverResource()->getUnknownTrackUrl(64);
 
 			_model->setData(dataRow, COLUMN_ID_COVER, coverUrl, Wt::DecorationRole);
 			_model->setData(dataRow, COLUMN_ID_COVER, std::string("playqueue-cover"), Wt::StyleClassRole);

src/ui/resource/CoverResource.cpp

@@ -41,28 +41,21 @@ CoverResource:: ~CoverResource()
 	beingDeleted();
 }
 
-const CoverArt::CoverArt&
+const Image::Image&
 CoverResource::getDefaultCover(std::size_t size)
 {
 	auto itCover = _defaultCovers.find(size);
 	if (itCover == _defaultCovers.end())
 	{
 		// Load default cover art for this size
+		Image::Image image;
 
+		if (!image.load( Wt::WApplication::instance()->docRoot() + unknownCoverPath ))
+			throw std::runtime_error("Cannot read default cover file");
 
-		std::vector<unsigned char> data;
+		image.scale(size);
-		{
-			std::ifstream ist(Wt::WApplication::instance()->docRoot() + unknownCoverPath);
-			char c;
-			while(ist.get(c))
-				data.push_back(c);
-		}
 
-		CoverArt::CoverArt defaultCover(data);
+		auto res = _defaultCovers.insert(std::make_pair(size, image));
-
-		defaultCover.scale(size);
-
-		auto res = _defaultCovers.insert(std::make_pair(size, defaultCover));
 		itCover = res.first;
 	}
 
@@ -88,13 +81,13 @@ CoverResource::getUnknownTrackUrl(size_t size) const
 }
 
 void
-CoverResource::putCover(Wt::Http::Response& response, const CoverArt::CoverArt& cover)
+CoverResource::putCover(Wt::Http::Response& response, Image::Image cover)
 {
-	response.setMimeType( format_to_mimeType(CoverArt::Format::JPEG) );
-
 	std::vector<unsigned char> data;
-	cover.getData(data, CoverArt::Format::JPEG);
 
+	cover.save(data, Image::Format::JPEG);
+
+	response.setMimeType( Image::format_to_mimeType(Image::Format::JPEG) );
 	response.out().write(reinterpret_cast<const char *>(&data[0]), data.size());
 }
 
@@ -108,7 +101,7 @@ CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
 
 	try
 	{
-		std::vector<CoverArt::CoverArt> covers;
+		std::vector<Image::Image> covers;
 
 		// Mandatory parameter size
 		if (!sizeStr)
@@ -144,17 +137,14 @@ CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
 					covers = CoverArt::Grabber::instance().getFromTrack(path);
 					break;
 
-				case Database::Track::CoverType::ExternalFile:
-					covers = CoverArt::Grabber::instance().getFromDirectory(path.parent_path());
-					break;
-
 				case Database::Track::CoverType::None:
+					covers = CoverArt::Grabber::instance().getFromDirectory(path.parent_path());
 					break;
 			}
 		}
 		else if (releaseIdStr)
 		{
-			Database::Release::id_type releaseId = std::stol(*releaseIdStr); // TODO try catch
+			Database::Release::id_type releaseId = std::stol(*releaseIdStr);
 			// transactions are not thread safe
 			std::unique_lock<std::mutex> lock(_mutex);
 			Wt::Dbo::Transaction transaction(_db.getSession());
@@ -162,7 +152,7 @@ CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
 			covers = CoverArt::Grabber::instance().getFromRelease(_db.getSession(), releaseId);
 		}
 
-		for (CoverArt::CoverArt& cover : covers)
+		for (Image::Image& cover : covers)
 		{
 			if (cover.scale(size))
 			{

src/ui/resource/CoverResource.hpp

@@ -25,7 +25,7 @@
 #include <Wt/WResource>
 
 #include "database/DatabaseHandler.hpp"
-#include "cover/CoverArt.hpp"
+#include "image/Image.hpp"
 
 namespace UserInterface {
 
@@ -47,14 +47,14 @@ class CoverResource : public Wt::WResource
 
 	private:
 
-		const CoverArt::CoverArt&	getDefaultCover(std::size_t size);
+		const Image::Image&		getDefaultCover(std::size_t size);
-		void				putCover(Wt::Http::Response& response, const CoverArt::CoverArt& cover);
+		void				putCover(Wt::Http::Response& response, Image::Image image);
 
 		std::mutex			_mutex;
 		Database::Handler&		_db;
 
 		// Default cover for different sizes
-		std::map<std::size_t, CoverArt::CoverArt>	_defaultCovers;
+		std::map<std::size_t, Image::Image>	_defaultCovers;
 
 		// TODO construct a cache for covers?
 };