Simplify the covert art resource

src/cover/CoverArtGrabber.cpp

@@ -43,12 +43,15 @@ isFileSupported(const boost::filesystem::path& file, const std::vector<boost::fi
 
 	return false;
 }
+
 } // namespace
 
 namespace CoverArt {
 
 Grabber::Grabber()
 {
+	if (!_defaultCover.load( Wt::WApplication::instance()->docRoot() + "/images/unknown-cover.jpg"))
+		throw LmsException("Cannot read default cover file");
 }
 
 Grabber&
@@ -58,48 +61,42 @@ Grabber::instance()
 	return instance;
 }
 
-static std::vector<Image::Image>
+static boost::optional<Image::Image>
-getFromAvMediaFile(const Av::MediaFile& input, std::size_t nbMaxCovers)
+getFromAvMediaFile(const Av::MediaFile& input)
 {
 	std::vector<Image::Image> res;
 
-	for (Av::Picture& picture : input.getAttachedPictures(nbMaxCovers))
+	for (auto& picture : input.getAttachedPictures(2))
 	{
 		Image::Image image;
 
 		if (image.load(picture.data))
-			res.push_back( image );
+			return image;
 		else
 			LMS_LOG(COVER, ERROR) << "Cannot load embedded cover file in '" << input.getPath().string() << "'";
 	}
 
-	return res;
+	return boost::none;
 }
 
-std::vector<Image::Image>
+boost::optional<Image::Image>
-Grabber::getFromDirectory(const boost::filesystem::path& p, std::size_t nbMaxCovers) const
+Grabber::getFromDirectory(const boost::filesystem::path& p) const
 {
-	std::vector<Image::Image> res;
+	for (auto coverPath : getCoverPaths(p))
-
-	std::vector<boost::filesystem::path> coverPathes = getCoverPaths(p, nbMaxCovers);
-	for (auto coverPath : coverPathes)
 	{
-		if (res.size() >= nbMaxCovers)
-			break;
-
 		Image::Image image;
 
 		if (image.load(coverPath))
-			res.push_back(image);
+			return image;
 		else
 			LMS_LOG(COVER, ERROR) << "Cannot load image in file '" << coverPath.string() << "'";
 	}
 
-	return res;
+	return boost::none;
 }
 
 std::vector<boost::filesystem::path>
-Grabber::getCoverPaths(const boost::filesystem::path& directoryPath, std::size_t nbMaxCovers) const
+Grabber::getCoverPaths(const boost::filesystem::path& directoryPath) const
 {
 	std::vector<boost::filesystem::path> res;
 	boost::system::error_code ec;
@@ -126,86 +123,94 @@ Grabber::getCoverPaths(const boost::filesystem::path& directoryPath, std::size_t
 		}
 
 		res.push_back(path);
-		if (res.size() >= nbMaxCovers)
-			break;
 	}
+
 	return res;
 }
 
-std::vector<Image::Image>
+boost::optional<Image::Image>
-Grabber::getFromTrack(const boost::filesystem::path& p, std::size_t nbMaxCovers) const
+Grabber::getFromTrack(const boost::filesystem::path& p) const
 {
 	try
 	{
 		Av::MediaFile input(p);
 
-		return getFromAvMediaFile(input, nbMaxCovers);
+		return getFromAvMediaFile(input);
 	}
 	catch (Av::MediaFileException& e)
 	{
 		LMS_LOG(COVER, ERROR) << "Cannot get covers from track " << p.string() << ": " << e.what();
+		return boost::none;
 	}
-
-	return std::vector<Image::Image>();
 }
 
-std::vector<Image::Image>
+Image::Image
-Grabber::getFromTrack(Wt::Dbo::Session& session, Database::IdType trackId, std::size_t nbMaxCovers) const
+Grabber::getFromTrack(Wt::Dbo::Session& session, Database::IdType trackId, std::size_t size) const
 {
 	using namespace Database;
 
-	Wt::Dbo::Transaction transaction(session);
+	boost::optional<Image::Image> cover;
 
-	Track::pointer track = Track::getById(session, trackId);
-	if (!track)
-		return std::vector<Image::Image>();
-
-	Track::CoverType coverType = track->getCoverType();
-	boost::filesystem::path trackPath = track->getPath();
-
-	transaction.commit();
-
-	switch (coverType)
 	{
-		case Track::CoverType::Embedded:
+		Wt::Dbo::Transaction transaction(session);
-			return Grabber::getFromTrack(trackPath, nbMaxCovers);
+
-		case Track::CoverType::None:
+		Track::pointer track = Track::getById(session, trackId);
-			return Grabber::getFromDirectory(trackPath.parent_path(), nbMaxCovers);
+		if (track)
+		{
+			Track::CoverType coverType = track->getCoverType();
+			boost::filesystem::path trackPath = track->getPath();
+
+			transaction.commit();
+
+			switch (coverType)
+			{
+				case Track::CoverType::Embedded:
+					cover = getFromTrack(trackPath);
+				case Track::CoverType::None:
+					cover = getFromDirectory(trackPath.parent_path());
+			}
+		}
 	}
 
-	return std::vector<Image::Image>();
+	if (!cover)
+		cover = _defaultCover;
+
+	cover->scale(size);
+
+	return *cover;
 }
 
 
-std::vector<Image::Image>
+Image::Image
-Grabber::getFromRelease(Wt::Dbo::Session& session, Database::IdType releaseId, std::size_t nbMaxCovers) const
+Grabber::getFromRelease(Wt::Dbo::Session& session, Database::IdType releaseId, std::size_t size) const
 {
 	using namespace Database;
 
-	Wt::Dbo::Transaction transaction(session);
+	boost::optional<Image::Image> cover;
 
-	// If the release does not exist, do nothing
+	{
-	Release::pointer release = Release::getById(session, releaseId);
+		Wt::Dbo::Transaction transaction(session);
-	if (!release)
-		return std::vector<Image::Image>();
 
-	std::vector<Track::pointer> tracks = release->getTracks();
+		auto release = Release::getById(session, releaseId);
-	if (tracks.empty())
+		if (release)
-		return std::vector<Image::Image>();
+		{
+			auto tracks = release->getTracks();
+			if (!tracks.empty())
+			{
+				auto trackId = tracks.front().id();
+				transaction.commit();
 
-	boost::filesystem::path firstTrackPath = tracks.front()->getPath();
+				return getFromTrack(session, trackId, size);
-	bool embeddedCover = (tracks.front()->getCoverType() == Track::CoverType::Embedded);
+			}
+		}
+	}
 
-	transaction.commit();
+	if (!cover)
+		cover = _defaultCover;
 
-	// First, try to get covers from the directory of the release
+	cover->scale(size);
-	std::vector<Image::Image> res = getFromDirectory( firstTrackPath.parent_path(), nbMaxCovers);
 
-	// Fallback on the embedded cover of the first track
+	return *cover;
-	if (res.empty() && embeddedCover)
-		res = getFromTrack( firstTrackPath, nbMaxCovers);
-
-	return res;
 }
 
 } // namespace CoverArt

src/cover/CoverArtGrabber.hpp

@@ -35,18 +35,25 @@ class Grabber
 
 		static Grabber& instance();
 
-		std::vector<boost::filesystem::path>	getCoverPaths(const boost::filesystem::path& directoryPath, std::size_t nbMaxCovers = 1) const;
+		Image::Image		getFromTrack(Wt::Dbo::Session& session, Database::IdType trackId, std::size_t size) const;
-		std::vector<Image::Image>	getFromDirectory(const boost::filesystem::path& path, std::size_t nbMaxCovers = 1) const;
+		Image::Image		getFromRelease(Wt::Dbo::Session& session, Database::IdType releaseId, std::size_t size) const;
-		std::vector<Image::Image>	getFromTrack(const boost::filesystem::path& path, std::size_t nbMaxCovers = 1) const;
-		std::vector<Image::Image>	getFromTrack(Wt::Dbo::Session& session, Database::IdType trackId, std::size_t nbMaxCovers = 1) const;
-		std::vector<Image::Image>	getFromRelease(Wt::Dbo::Session& session, Database::IdType releaseId, std::size_t nbMaxCovers = 1) const;
 
 	private:
+
 		Grabber();
 
+		boost::optional<Image::Image>		getFromTrack(const boost::filesystem::path& path) const;
+		std::vector<boost::filesystem::path>	getCoverPaths(const boost::filesystem::path& directoryPath) const;
+		boost::optional<Image::Image>		getFromDirectory(const boost::filesystem::path& path) const;
+
+
+		Image::Image _defaultCover;
+
 		std::vector<boost::filesystem::path> _fileExtensions
 			= {".jpg", ".jpeg", ".png", ".bmp"}; // TODO parametrize
+
 		std::size_t _maxFileSize = 5000000;
+
 		std::vector<boost::filesystem::path> _preferredFileNames
 			= {"cover", "front"}; // TODO parametrize
 };

src/ui/LmsApplication.cpp

@@ -328,8 +328,8 @@ LmsApplication::createHome()
 		_isAdmin = LmsApp->getUser()->isAdmin();
 	}
 
-	_imageResource = std::make_shared<ImageResource>(_db);
+	_imageResource = std::make_shared<ImageResource>();
-	_transcodeResource = std::make_shared<TranscodeResource>(_db);
+	_transcodeResource = std::make_shared<TranscodeResource>();
 
 	setConfirmCloseMessage(Wt::WString::tr("Lms.quit-confirm"));
 

src/ui/resource/ImageResource.cpp

@@ -37,61 +37,11 @@ namespace UserInterface {
 static const std::string unknownCoverPath = "/images/unknown-cover.jpg";
 static const std::string unknownArtistImagePath = "/images/unknown-artist.jpg";
 
-ImageResource::ImageResource(Database::Handler& db)
-: _db(db)
-{
-}
-
 ImageResource::~ImageResource()
 {
 	beingDeleted();
 }
 
-Image::Image
-ImageResource::getDefaultCover(std::size_t size)
-{
-	std::unique_lock<std::mutex> lock(_mutex);
-
-	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 LmsException("Cannot read default cover file");
-
-		image.scale(size);
-
-		auto res = _defaultCovers.insert(std::make_pair(size, image));
-		itCover = res.first;
-	}
-
-	return itCover->second;
-}
-
-Image::Image
-ImageResource::getDefaultArtistImage(std::size_t size)
-{
-	std::unique_lock<std::mutex> lock(_mutex);
-
-	auto itCover = _defaultArtistImages.find(size);
-	if (itCover == _defaultArtistImages.end())
-	{
-		Image::Image image;
-
-		if (!image.load( Wt::WApplication::instance()->docRoot() + unknownArtistImagePath))
-			throw LmsException("Cannot read default artist image file");
-
-		image.scale(size);
-
-		auto res = _defaultArtistImages.insert(std::make_pair(size, image));
-		itCover = res.first;
-	}
-
-	return itCover->second;
-}
-
 std::string
 ImageResource::getReleaseUrl(Database::IdType releaseId, std::size_t size) const
 {
@@ -127,29 +77,12 @@ ImageResource::putImage(Wt::Http::Response& response, Image::Image cover)
 	response.out().write(reinterpret_cast<const char *>(&data[0]), data.size());
 }
 
-void
-ImageResource::putCover(Wt::Http::Response& response, std::vector<Image::Image> covers, size_t size)
-{
-	for (Image::Image cover : covers)
-	{
-		if (cover.scale(size))
-		{
-			putImage(response, cover);
-			return;
-		}
-	}
-
-	// If no cover found, just send default one
-	putImage(response, getDefaultCover(size));
-}
-
 void
 ImageResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Response& response)
 {
 	// Retrieve parameters
 	const std::string *trackIdStr = request.getParameter("trackid");
 	const std::string *releaseIdStr = request.getParameter("releaseid");
-	const std::string *artistIdStr = request.getParameter("artistid");
 	const std::string *sizeStr = request.getParameter("size");
 
 	// Mandatory parameter size
@@ -160,48 +93,19 @@ ImageResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
 	if (!size || *size > maxSize)
 		return;
 
+	Image::Image cover;
+
 	if (trackIdStr)
 	{
 		auto trackId = readAs<Database::IdType>(*trackIdStr);
 		if (!trackId)
 			return;
 
-		boost::filesystem::path path;
+		// transactions are not thread safe
-		Database::Track::CoverType coverType = Database::Track::CoverType::None;
 		{
-			// transactions are not thread safe
+			Wt::WApplication::UpdateLock lock(LmsApp);
-			Wt::WApplication::UpdateLock lock(LmsApplication::instance());
+			cover = CoverArt::Grabber::instance().getFromTrack(LmsApp->getDboSession(), *trackId, *size);
-
-			Wt::Dbo::Transaction transaction(_db.getSession());
-
-			Database::Track::pointer track = Database::Track::getById(_db.getSession(), *trackId);
-			if (track)
-			{
-				coverType = track->getCoverType();
-				path = track->getPath();
-			}
 		}
-
-		if (!path.empty())
-		{
-			std::vector<Image::Image> covers;
-
-			switch (coverType)
-			{
-				case Database::Track::CoverType::Embedded:
-					covers = CoverArt::Grabber::instance().getFromTrack(path);
-					break;
-
-				case Database::Track::CoverType::None:
-					covers = CoverArt::Grabber::instance().getFromDirectory(path.parent_path());
-					break;
-			}
-
-			putCover(response, covers, *size);
-			return;
-		}
-
-		putImage(response, getDefaultCover(*size));
 	}
 	else if (releaseIdStr)
 	{
@@ -209,24 +113,16 @@ ImageResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
 		if (!releaseId)
 			return;
 
-		std::vector<Image::Image> covers;
-
 		// transactions are not thread safe
 		{
-			Wt::WApplication::UpdateLock lock(LmsApplication::instance());
+			Wt::WApplication::UpdateLock lock(LmsApp);
-			covers = CoverArt::Grabber::instance().getFromRelease(_db.getSession(), *releaseId);
+			cover = CoverArt::Grabber::instance().getFromRelease(LmsApp->getDboSession(), *releaseId, *size);
 		}
-
-		putCover(response, covers, *size);
-	}
-	else if (artistIdStr)
-	{
-		putImage(response, getDefaultArtistImage(*size));
 	}
 	else
-	{
+		return;
-		putImage(response, getDefaultCover(*size));
+
-	}
+	putImage(response, cover);
 }
 
 } // namespace UserInterface

src/ui/resource/ImageResource.hpp

@@ -37,7 +37,6 @@ class ImageResource : public Wt::WResource
 	public:
 		static const std::size_t maxSize = 512;
 
-		ImageResource(Database::Handler& db);
 		~ImageResource();
 
 		std::string getReleaseUrl(Database::IdType releaseId, size_t size) const;
@@ -49,22 +48,9 @@ class ImageResource : public Wt::WResource
 
 	private:
 
-		Image::Image			getDefaultCover(std::size_t size);
-		Image::Image			getDefaultArtistImage(std::size_t size);
 		void				putImage(Wt::Http::Response& response, Image::Image image);
 		void				putCover(Wt::Http::Response& response, std::vector<Image::Image> covers, std::size_t size);
 
-		// Used to protect transactions since they are not thread safe
-		std::mutex			_mutex;
-		Database::Handler&		_db;
-
-		// Default cover for different sizes
-		std::map<std::size_t, Image::Image>	_defaultCovers;
-
-		// Default artist images for different sizes
-		std::map<std::size_t, Image::Image>	_defaultArtistImages;
-
-		// TODO construct a cache for covers?
 };
 
 } // namespace UserInterface

src/ui/resource/TranscodeResource.cpp

@@ -29,15 +29,9 @@
 
 namespace UserInterface {
 
-TranscodeResource::TranscodeResource(Database::Handler& db)
-: _db(db)
-{
-	LMS_LOG(UI, DEBUG) << "CONSTRUCTING RESOURCE";
-}
 
 TranscodeResource:: ~TranscodeResource()
 {
-	LMS_LOG(UI, DEBUG) << "DESTRUCTING RESOURCE";
 	beingDeleted();
 }
 
@@ -100,12 +94,11 @@ TranscodeResource::handleRequest(const Wt::Http::Request& request,
 
 		// transactions are not thread safe
 		{
-			Wt::WApplication::UpdateLock lock(LmsApplication::instance());
+			Wt::WApplication::UpdateLock lock(LmsApp);
 
-			Wt::Dbo::Transaction transaction(_db.getSession());
+			Wt::Dbo::Transaction transaction(LmsApp->getDboSession());
 
-			Database::User::pointer user = _db.getCurrentUser();
+			Database::Track::pointer track = Database::Track::getById(LmsApp->getDboSession(), trackId);
-			Database::Track::pointer track = Database::Track::getById(_db.getSession(), trackId);
 
 			if (!track)
 			{
@@ -113,7 +106,7 @@ TranscodeResource::handleRequest(const Wt::Http::Request& request,
 				return;
 			}
 
-			parameters.bitrate = user->getAudioBitrate();
+			parameters.bitrate = LmsApp->getUser()->getAudioBitrate();
 			transcoder = std::make_shared<Av::Transcoder>(track->getPath(), parameters);
 		}
 

src/ui/resource/TranscodeResource.hpp

@@ -34,7 +34,6 @@ namespace UserInterface {
 class TranscodeResource : public Wt::WResource
 {
 	public:
-		TranscodeResource(Database::Handler& db);
 		~TranscodeResource();
 
 		std::string getUrl(Database::IdType trackId, Av::Encoding encoding) const;
@@ -44,8 +43,6 @@ class TranscodeResource : public Wt::WResource
 
 	private:
 
-		Database::Handler&		_db;
-
 		static const std::size_t	_chunkSize = 65536*4;
 };