From 9be641ee87a9293603f8721ce9b3e6a413fb8289 Mon Sep 17 00:00:00 2001 From: David Sansome Date: Fri, 6 Jan 2012 21:27:02 +0000 Subject: [PATCH] The external tagreader mostly works now: * Make TagReaderClient a singleton until it's easier to pass dependencies around * Add a WaitForSignal() that uses a local event loop to wait for a signal to be emitted * Add a WaitForFinished() to _MessageReplyBase that blocks using a semaphore * Add blocking versions of all TagReaderClient methods * Use the TagReaderClient everywhere that Song::InitFromFile and friends were used before --- ext/clementine-tagreader/main.cpp | 4 ++ ext/clementine-tagreader/tagreaderworker.cpp | 10 +++ ext/libclementine-common/CMakeLists.txt | 1 + .../core/messagehandler.cpp | 12 +++- .../core/messagehandler.h | 20 ++++-- .../core/waitforsignal.cpp | 26 ++++++++ ext/libclementine-common/core/waitforsignal.h | 25 +++++++ ext/libclementine-common/core/workerpool.h | 51 ++++++++++---- src/CMakeLists.txt | 4 ++ src/core/organise.cpp | 3 +- src/core/song.h | 2 - src/core/songloader.cpp | 7 +- src/core/tagreaderclient.cpp | 66 +++++++++++++++++-- src/core/tagreaderclient.h | 14 +++- src/covers/albumcoverloader.cpp | 8 ++- src/globalsearch/spotifysearchprovider.cpp | 2 +- src/internet/spotifyserver.cpp | 4 +- src/internet/spotifyservice.cpp | 4 +- src/library/library.h | 2 +- src/library/libraryplaylistitem.cpp | 3 +- src/library/librarywatcher.cpp | 12 ++-- src/main.cpp | 14 ++-- src/playlist/playlist.cpp | 21 +++--- src/playlist/playlist.h | 3 +- src/playlist/songplaylistitem.cpp | 8 +-- src/playlistparsers/parserbase.cpp | 3 +- src/ui/edittagdialog.cpp | 6 +- src/ui/mainwindow.cpp | 33 +++++----- src/ui/mainwindow.h | 7 +- src/ui/organisedialog.cpp | 4 +- src/ui/trackselectiondialog.cpp | 3 +- 31 files changed, 293 insertions(+), 89 deletions(-) create mode 100644 ext/libclementine-common/core/waitforsignal.cpp create mode 100644 ext/libclementine-common/core/waitforsignal.h diff --git a/ext/clementine-tagreader/main.cpp b/ext/clementine-tagreader/main.cpp index 80b002e0f..5440baade 100644 --- a/ext/clementine-tagreader/main.cpp +++ b/ext/clementine-tagreader/main.cpp @@ -17,6 +17,7 @@ #include "tagreaderworker.h" #include "core/encoding.h" +#include "core/logging.h" #include #include @@ -35,6 +36,9 @@ int main(int argc, char** argv) { return 1; } + logging::Init(); + qLog(Info) << "TagReader worker connecting to" << args[1]; + // Detect technically invalid usage of non-ASCII in ID3v1 tags. UniversalEncodingHandler handler; TagLib::ID3v1::Tag::setStringHandler(&handler); diff --git a/ext/clementine-tagreader/tagreaderworker.cpp b/ext/clementine-tagreader/tagreaderworker.cpp index 1cabf3694..abc768bd7 100644 --- a/ext/clementine-tagreader/tagreaderworker.cpp +++ b/ext/clementine-tagreader/tagreaderworker.cpp @@ -18,6 +18,7 @@ #include "fmpsparser.h" #include "tagreaderworker.h" #include "core/encoding.h" +#include "core/logging.h" #include "core/timeconstants.h" #include @@ -90,6 +91,7 @@ TagLib::String QStringToTaglibString(const QString& s) { TagReaderWorker::TagReaderWorker(QIODevice* socket, QObject* parent) : AbstractMessageHandler(socket, parent), + factory_(new TagLibFileRefFactory), kEmbeddedCover("(embedded)") { } @@ -122,6 +124,8 @@ void TagReaderWorker::ReadFile(const QString& filename, const QByteArray url(QUrl::fromLocalFile(filename).toEncoded()); const QFileInfo info(filename); + qLog(Debug) << "Reading tags from" << filename; + song->set_basefilename(DataCommaSizeFromQString(info.fileName())); song->set_url(url.constData(), url.size()); song->set_filesize(info.size()); @@ -402,6 +406,8 @@ bool TagReaderWorker::SaveFile(const QString& filename, if (filename.isNull()) return false; + qLog(Debug) << "Saving tags to" << filename; + scoped_ptr fileref(factory_->GetFileRef(filename)); if (!fileref || fileref->isNull()) // The file probably doesn't exist @@ -475,6 +481,8 @@ void TagReaderWorker::SetTextFrame(const char* id, const std::string& value, } bool TagReaderWorker::IsMediaFile(const QString& filename) const { + qLog(Debug) << "Checking for valid file" << filename; + scoped_ptr fileref(factory_->GetFileRef(filename)); return !fileref->isNull() && fileref->tag(); } @@ -483,6 +491,8 @@ QByteArray TagReaderWorker::LoadEmbeddedArt(const QString& filename) const { if (filename.isEmpty()) return QByteArray(); + qLog(Debug) << "Loading art from" << filename; + #ifdef Q_OS_WIN32 TagLib::FileRef ref(filename.toStdWString().c_str()); #else diff --git a/ext/libclementine-common/CMakeLists.txt b/ext/libclementine-common/CMakeLists.txt index 1096d969a..ec0ddf2e8 100644 --- a/ext/libclementine-common/CMakeLists.txt +++ b/ext/libclementine-common/CMakeLists.txt @@ -8,6 +8,7 @@ set(SOURCES core/encoding.cpp core/logging.cpp core/messagehandler.cpp + core/waitforsignal.cpp core/workerpool.cpp ) diff --git a/ext/libclementine-common/core/messagehandler.cpp b/ext/libclementine-common/core/messagehandler.cpp index f2bb92c4b..adfc8a7c0 100644 --- a/ext/libclementine-common/core/messagehandler.cpp +++ b/ext/libclementine-common/core/messagehandler.cpp @@ -95,13 +95,21 @@ void _MessageHandlerBase::WriteMessage(const QByteArray& data) { _MessageReplyBase::_MessageReplyBase(int id, QObject* parent) : QObject(parent), id_(id), - finished_(false) + finished_(false), + success_(false) { } +bool _MessageReplyBase::WaitForFinished() { + semaphore_.acquire(); + return success_; +} + void _MessageReplyBase::Abort() { Q_ASSERT(!finished_); finished_ = true; + success_ = false; - emit Finished(false); + emit Finished(success_); + semaphore_.release(); } diff --git a/ext/libclementine-common/core/messagehandler.h b/ext/libclementine-common/core/messagehandler.h index 5124d14fe..483c9b405 100644 --- a/ext/libclementine-common/core/messagehandler.h +++ b/ext/libclementine-common/core/messagehandler.h @@ -27,6 +27,7 @@ #include #include #include +#include #include class QAbstractSocket; @@ -46,10 +47,16 @@ class _MessageReplyBase : public QObject { Q_OBJECT public: - _MessageReplyBase(int id, QObject* parent); + _MessageReplyBase(int id, QObject* parent = 0); int id() const { return id_; } bool is_finished() const { return finished_; } + bool is_successful() const { return success_; } + + // Waits for the reply to finish by waiting on a semaphore. Never call this + // from the MessageHandler's thread or it will block forever. + // Returns true if the call was successful. + bool WaitForFinished(); void Abort(); @@ -59,6 +66,9 @@ signals: protected: int id_; bool finished_; + bool success_; + + QSemaphore semaphore_; }; @@ -67,7 +77,7 @@ protected: template class MessageReply : public _MessageReplyBase { public: - MessageReply(int id, QObject* parent); + MessageReply(int id, QObject* parent = 0); const MessageType& message() const { return message_; } @@ -219,7 +229,7 @@ AbstractMessageHandler::NewReply( QMutexLocker l(&mutex_); const int id = next_id_ ++; - reply = new ReplyType(id, this); + reply = new ReplyType(id); pending_replies_[id] = reply; } @@ -260,8 +270,10 @@ void MessageReply::SetReply(const MessageType& message) { message_.MergeFrom(message); finished_ = true; + success_ = true; - emit Finished(true); + emit Finished(success_); + semaphore_.release(); } #endif // MESSAGEHANDLER_H diff --git a/ext/libclementine-common/core/waitforsignal.cpp b/ext/libclementine-common/core/waitforsignal.cpp new file mode 100644 index 000000000..62820526a --- /dev/null +++ b/ext/libclementine-common/core/waitforsignal.cpp @@ -0,0 +1,26 @@ +/* This file is part of Clementine. + Copyright 2011, David Sansome + + Clementine is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Clementine is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Clementine. If not, see . +*/ + +#include "waitforsignal.h" + +#include + +void WaitForSignal(QObject* sender, const char* signal) { + QEventLoop loop; + QObject::connect(sender, signal, &loop, SLOT(quit())); + loop.exec(); +} diff --git a/ext/libclementine-common/core/waitforsignal.h b/ext/libclementine-common/core/waitforsignal.h new file mode 100644 index 000000000..4c90c2e68 --- /dev/null +++ b/ext/libclementine-common/core/waitforsignal.h @@ -0,0 +1,25 @@ +/* This file is part of Clementine. + Copyright 2011, David Sansome + + Clementine is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Clementine is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Clementine. If not, see . +*/ + +#ifndef WAITFORSIGNAL_H +#define WAITFORSIGNAL_H + +class QObject; + +void WaitForSignal(QObject* sender, const char* signal); + +#endif // WAITFORSIGNAL_H diff --git a/ext/libclementine-common/core/workerpool.h b/ext/libclementine-common/core/workerpool.h index 8ebaee578..299fa24b8 100644 --- a/ext/libclementine-common/core/workerpool.h +++ b/ext/libclementine-common/core/workerpool.h @@ -21,11 +21,14 @@ #include #include #include +#include #include #include #include #include "core/closure.h" +#include "core/logging.h" +#include "core/waitforsignal.h" // Base class containing signals and slots - required because moc doesn't do @@ -46,6 +49,7 @@ signals: void WorkerConnected(); protected slots: + virtual void DoStart() {} virtual void NewConnection() {} virtual void ProcessError(QProcess::ProcessError) {} }; @@ -83,6 +87,7 @@ public: HandlerType* NextHandler(); protected: + void DoStart(); void NewConnection(); void ProcessError(QProcess::ProcessError error); @@ -99,15 +104,17 @@ private: template Worker* FindWorker(T Worker::*member, T value) { - foreach (Worker& worker, workers_) { - if (worker.*member == value) { - return &worker; + for (typename QList::iterator it = workers_.begin() ; + it != workers_.end() ; ++it) { + if ((*it).*member == value) { + return &(*it); } } return NULL; } - void DeleteQObjectPointerLater(QObject** p) { + template + void DeleteQObjectPointerLater(T** p) { if (*p) { (*p)->deleteLater(); *p = NULL; @@ -157,7 +164,13 @@ void WorkerPool::SetExecutableName(const QString& executable_name) template void WorkerPool::Start() { + metaObject()->invokeMethod(this, "DoStart"); +} + +template +void WorkerPool::DoStart() { Q_ASSERT(workers_.isEmpty()); + Q_ASSERT(!executable_name_.isEmpty()); // Find the executable if we can, default to searching $PATH executable_path_ = executable_name_; @@ -200,7 +213,7 @@ void WorkerPool::StartOneWorker(Worker* worker) { // Create a server, find an unused name and start listening forever { - const int unique_number = qrand() ^ reinterpret_cast(this); + const int unique_number = qrand() ^ ((int)(quint64(this) & 0xFFFFFFFF)); const QString name = QString("%1_%2").arg(local_server_name_).arg(unique_number); if (worker->local_server_->listen(name)) { @@ -208,10 +221,13 @@ void WorkerPool::StartOneWorker(Worker* worker) { } } + qLog(Debug) << "Starting worker" << executable_path_ + << worker->local_server_->fullServerName(); + // Start the process worker->process_->setProcessChannelMode(QProcess::ForwardedChannels); worker->process_->start(executable_path_, - QStringList() << worker->socket_server_->fullServerName()); + QStringList() << worker->local_server_->fullServerName()); } template @@ -223,11 +239,14 @@ void WorkerPool::NewConnection() { if (!worker) return; + qLog(Debug) << "Worker connected to" << server->fullServerName(); + // Accept the connection. QLocalSocket* socket = server->nextPendingConnection(); // We only ever accept one connection per worker, so destroy the server now. - server->deleteLater(); + socket->setParent(this); + worker->local_server_->deleteLater(); worker->local_server_ = NULL; // Create the handler. @@ -250,11 +269,13 @@ void WorkerPool::ProcessError(QProcess::ProcessError error) { // Failed to start errors are bad - it usually means the worker isn't // installed. Don't restart the process, but tell our owner, who will // probably want to do something fatal. + qLog(Error) << "Worker failed to start"; emit WorkerFailedToStart(); break; default: // On any other error we just restart the process. + qLog(Debug) << "Worker failed with error" << error << "- restarting"; StartOneWorker(worker); break; } @@ -262,15 +283,19 @@ void WorkerPool::ProcessError(QProcess::ProcessError error) { template HandlerType* WorkerPool::NextHandler() { - for (int i=0 ; i #include @@ -137,7 +138,7 @@ void Organise::ProcessSomeFiles() { // Read metadata from the file Song song; - song.InitFromFile(task.filename_, -1); + TagReaderClient::Instance()->ReadFileBlocking(task.filename_, &song); if (!song.is_valid()) continue; diff --git a/src/core/song.h b/src/core/song.h index 695963403..b23c880bb 100644 --- a/src/core/song.h +++ b/src/core/song.h @@ -208,8 +208,6 @@ class Song { // Setters bool IsEditable() const; - bool Save() const; - QFuture BackgroundSave() const; void set_id(int id); void set_valid(bool v); diff --git a/src/core/songloader.cpp b/src/core/songloader.cpp index 29d8ad975..bdb653760 100644 --- a/src/core/songloader.cpp +++ b/src/core/songloader.cpp @@ -19,13 +19,14 @@ #include "songloader.h" #include "core/logging.h" #include "core/song.h" +#include "core/tagreaderclient.h" #include "core/timeconstants.h" +#include "internet/fixlastfm.h" #include "library/librarybackend.h" #include "library/sqlrow.h" #include "playlistparsers/parserbase.h" #include "playlistparsers/cueparser.h" #include "playlistparsers/playlistparser.h" -#include "internet/fixlastfm.h" #include #include @@ -286,7 +287,7 @@ SongLoader::Result SongLoader::LoadLocal(const QString& filename, bool block, // it's a normal media file } else { Song song; - song.InitFromFile(filename, -1); + TagReaderClient::Instance()->ReadFileBlocking(filename, &song); song_list << song; @@ -316,7 +317,7 @@ void SongLoader::EffectiveSongsLoad() { } else { // it's a normal media file QString filename = song.url().toLocalFile(); - song.InitFromFile(filename, -1); + TagReaderClient::Instance()->ReadFileBlocking(filename, &song); } } } diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index 70163f717..834919d4b 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -24,11 +24,15 @@ const char* TagReaderClient::kWorkerExecutableName = "clementine-tagreader"; +TagReaderClient* TagReaderClient::sInstance = NULL; TagReaderClient::TagReaderClient(QObject* parent) : QObject(parent), worker_pool_(new WorkerPool(this)) { + sInstance = this; + + worker_pool_->SetExecutableName(kWorkerExecutableName); } void TagReaderClient::Start() { @@ -41,7 +45,7 @@ TagReaderReply* TagReaderClient::ReadFile(const QString& filename) { req->set_filename(DataCommaSizeFromQString(filename)); - return handler_->SendMessageWithReply(&message); + return worker_pool_->NextHandler()->SendMessageWithReply(&message); } TagReaderReply* TagReaderClient::SaveFile(const QString& filename, const Song& metadata) { @@ -51,7 +55,7 @@ TagReaderReply* TagReaderClient::SaveFile(const QString& filename, const Song& m req->set_filename(DataCommaSizeFromQString(filename)); metadata.ToProtobuf(req->mutable_metadata()); - return handler_->SendMessageWithReply(&message); + return worker_pool_->NextHandler()->SendMessageWithReply(&message); } TagReaderReply* TagReaderClient::IsMediaFile(const QString& filename) { @@ -60,7 +64,7 @@ TagReaderReply* TagReaderClient::IsMediaFile(const QString& filename) { req->set_filename(DataCommaSizeFromQString(filename)); - return handler_->SendMessageWithReply(&message); + return worker_pool_->NextHandler()->SendMessageWithReply(&message); } TagReaderReply* TagReaderClient::LoadEmbeddedArt(const QString& filename) { @@ -69,5 +73,59 @@ TagReaderReply* TagReaderClient::LoadEmbeddedArt(const QString& filename) { req->set_filename(DataCommaSizeFromQString(filename)); - return handler_->SendMessageWithReply(&message); + return worker_pool_->NextHandler()->SendMessageWithReply(&message); +} + +void TagReaderClient::ReadFileBlocking(const QString& filename, Song* song) { + Q_ASSERT(QThread::currentThread() != thread()); + + TagReaderReply* reply = ReadFile(filename); + if (reply->WaitForFinished()) { + song->InitFromProtobuf(reply->message().read_file_response().metadata()); + } + reply->deleteLater(); +} + +bool TagReaderClient::SaveFileBlocking(const QString& filename, const Song& metadata) { + Q_ASSERT(QThread::currentThread() != thread()); + + bool ret = false; + + TagReaderReply* reply = SaveFile(filename, metadata); + if (reply->WaitForFinished()) { + ret = reply->message().save_file_response().success(); + } + reply->deleteLater(); + + return ret; +} + +bool TagReaderClient::IsMediaFileBlocking(const QString& filename) { + Q_ASSERT(QThread::currentThread() != thread()); + + bool ret = false; + + TagReaderReply* reply = IsMediaFile(filename); + if (reply->WaitForFinished()) { + ret = reply->message().is_media_file_response().success(); + } + reply->deleteLater(); + + return ret; +} + +QImage TagReaderClient::LoadEmbeddedArtBlocking(const QString& filename) { + Q_ASSERT(QThread::currentThread() != thread()); + + QImage ret; + + TagReaderReply* reply = LoadEmbeddedArt(filename); + if (reply->WaitForFinished()) { + const std::string& data_str = + reply->message().load_embedded_art_response().data(); + ret.loadFromData(QByteArray(data_str.data(), data_str.size())); + } + reply->deleteLater(); + + return ret; } diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 9c7dca983..f6dd2af88 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -46,10 +46,20 @@ public: ReplyType* IsMediaFile(const QString& filename); ReplyType* LoadEmbeddedArt(const QString& filename); -private: - void SendOrQueue(const pb::tagreader::Message& message); + // Convenience functions that call the above functions and wait for a + // response. These block the calling thread with a semaphore, and must NOT + // be called from the TagReaderClient's thread. + void ReadFileBlocking(const QString& filename, Song* song); + bool SaveFileBlocking(const QString& filename, const Song& metadata); + bool IsMediaFileBlocking(const QString& filename); + QImage LoadEmbeddedArtBlocking(const QString& filename); + + // TODO: Make this not a singleton + static TagReaderClient* Instance() { return sInstance; } private: + static TagReaderClient* sInstance; + WorkerPool* worker_pool_; QList message_queue_; }; diff --git a/src/covers/albumcoverloader.cpp b/src/covers/albumcoverloader.cpp index 5c8a8b27f..d6fab628e 100644 --- a/src/covers/albumcoverloader.cpp +++ b/src/covers/albumcoverloader.cpp @@ -19,6 +19,7 @@ #include "config.h" #include "core/logging.h" #include "core/network.h" +#include "core/tagreaderclient.h" #include "core/utilities.h" #include "internet/internetmodel.h" @@ -137,7 +138,9 @@ AlbumCoverLoader::TryLoadResult AlbumCoverLoader::TryLoadImage( return TryLoadResult(false, true, default_); if (filename == Song::kEmbeddedCover && !task.song_filename.isEmpty()) { - QImage taglib_image = Song::LoadEmbeddedArt(task.song_filename); + const QImage taglib_image = + TagReaderClient::Instance()->LoadEmbeddedArtBlocking(task.song_filename); + if (!taglib_image.isNull()) return TryLoadResult(false, true, ScaleAndPad(taglib_image)); } @@ -263,7 +266,8 @@ QPixmap AlbumCoverLoader::TryLoadPixmap(const QString& automatic, ret.load(manual); if (ret.isNull()) { if (automatic == Song::kEmbeddedCover && !filename.isNull()) - ret = QPixmap::fromImage(Song::LoadEmbeddedArt(filename)); + ret = QPixmap::fromImage( + TagReaderClient::Instance()->LoadEmbeddedArtBlocking(filename)); else if (!automatic.isEmpty()) ret.load(automatic); } diff --git a/src/globalsearch/spotifysearchprovider.cpp b/src/globalsearch/spotifysearchprovider.cpp index 6810b039b..d876b22b8 100644 --- a/src/globalsearch/spotifysearchprovider.cpp +++ b/src/globalsearch/spotifysearchprovider.cpp @@ -15,13 +15,13 @@ along with Clementine. If not, see . */ +#include "spotifymessagehandler.h" #include "spotifysearchprovider.h" #include "core/logging.h" #include "internet/internetmodel.h" #include "internet/spotifyserver.h" #include "internet/spotifyservice.h" #include "playlist/songmimedata.h" -#include "spotifyblob/common/spotifymessagehandler.h" SpotifySearchProvider::SpotifySearchProvider(QObject* parent) : SearchProvider(parent), diff --git a/src/internet/spotifyserver.cpp b/src/internet/spotifyserver.cpp index 2aa387143..2802abd6d 100644 --- a/src/internet/spotifyserver.cpp +++ b/src/internet/spotifyserver.cpp @@ -19,8 +19,8 @@ #include "core/closure.h" #include "core/logging.h" -#include "spotifyblob/common/spotifymessages.pb.h" -#include "spotifyblob/common/spotifymessagehandler.h" +#include "spotifymessages.pb.h" +#include "spotifymessagehandler.h" #include #include diff --git a/src/internet/spotifyservice.cpp b/src/internet/spotifyservice.cpp index 5676bef4c..5a6bd3fb9 100644 --- a/src/internet/spotifyservice.cpp +++ b/src/internet/spotifyservice.cpp @@ -1,6 +1,8 @@ +#include "blobversion.h" #include "config.h" #include "internetmodel.h" #include "spotifyblobdownloader.h" +#include "spotifymessagehandler.h" #include "spotifyserver.h" #include "spotifyservice.h" #include "spotifysearchplaylisttype.h" @@ -15,8 +17,6 @@ #include "playlist/playlist.h" #include "playlist/playlistcontainer.h" #include "playlist/playlistmanager.h" -#include "spotifyblob/common/blobversion.h" -#include "spotifyblob/common/spotifymessagehandler.h" #include "widgets/didyoumean.h" #include "ui/iconloader.h" diff --git a/src/library/library.h b/src/library/library.h index e857041fb..903360015 100644 --- a/src/library/library.h +++ b/src/library/library.h @@ -34,7 +34,7 @@ class Library : public QObject { Q_OBJECT public: - Library(BackgroundThread* db_thread, TaskManager* task_manager_, + Library(BackgroundThread* db_thread, TaskManager* task_manager, QObject* parent); static const char* kSongsTable; diff --git a/src/library/libraryplaylistitem.cpp b/src/library/libraryplaylistitem.cpp index 2cdbfa3e4..625565c81 100644 --- a/src/library/libraryplaylistitem.cpp +++ b/src/library/libraryplaylistitem.cpp @@ -16,6 +16,7 @@ */ #include "libraryplaylistitem.h" +#include "core/tagreaderclient.h" #include @@ -36,7 +37,7 @@ QUrl LibraryPlaylistItem::Url() const { } void LibraryPlaylistItem::Reload() { - song_.InitFromFile(song_.url().toLocalFile(), song_.directory_id()); + TagReaderClient::Instance()->ReadFileBlocking(song_.url().toLocalFile(), &song_); } bool LibraryPlaylistItem::InitFromQuery(const SqlRow& query) { diff --git a/src/library/librarywatcher.cpp b/src/library/librarywatcher.cpp index da7e86c7a..dbacf6c4d 100644 --- a/src/library/librarywatcher.cpp +++ b/src/library/librarywatcher.cpp @@ -18,6 +18,7 @@ #include "librarywatcher.h" #include "librarybackend.h" #include "core/logging.h" +#include "core/tagreaderclient.h" #include "core/taskmanager.h" #include "playlistparsers/cueparser.h" @@ -451,7 +452,8 @@ void LibraryWatcher::UpdateNonCueAssociatedSong(const QString& file, const Song& } Song song_on_disk; - song_on_disk.InitFromFile(file, t->dir()); + song_on_disk.set_directory_id(t->dir()); + TagReaderClient::Instance()->ReadFileBlocking(file, &song_on_disk); if(song_on_disk.is_valid()) { PreserveUserSetData(file, image, matching_song, &song_on_disk, t); @@ -476,8 +478,10 @@ SongList LibraryWatcher::ScanNewFile(const QString& file, const QString& path, // media files. Playlist parser for CUEs considers every entry in sheet // valid and we don't want invalid media getting into library! foreach(const Song& cue_song, cue_parser_->Load(&cue, matching_cue, path)) { - if(cue_song.url().toLocalFile() == file && cue_song.HasProperMediaFile()) { - song_list << cue_song; + if (cue_song.url().toLocalFile() == file) { + if (TagReaderClient::Instance()->IsMediaFileBlocking(file)) { + song_list << cue_song; + } } } @@ -488,7 +492,7 @@ SongList LibraryWatcher::ScanNewFile(const QString& file, const QString& path, // it's a normal media file } else { Song song; - song.InitFromFile(file, -1); + TagReaderClient::Instance()->ReadFileBlocking(file, &song); if (song.is_valid()) { song_list << song; diff --git a/src/main.cpp b/src/main.cpp index 2e3229748..92965dcee 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -369,6 +369,14 @@ int main(int argc, char *argv[]) { CoverProviders cover_providers; cover_providers.AddProvider(new AmazonCoverProvider); + // Create the tag loader on another thread. + TagReaderClient tag_reader_client; + + QThread tag_reader_thread; + tag_reader_thread.start(); + tag_reader_client.moveToThread(&tag_reader_thread); + tag_reader_client.Start(); + // Create some key objects scoped_ptr > database( new BackgroundThreadImplementation(NULL)); @@ -408,9 +416,6 @@ int main(int argc, char *argv[]) { GlobalSearchService global_search_service(&global_search); #endif - // Tag reader client - TagReaderClient tag_reader_client; - // Window MainWindow w( database.get(), @@ -422,8 +427,7 @@ int main(int argc, char *argv[]) { &osd, &art_loader, &cover_providers, - &global_search, - &tag_reader_client); + &global_search); #ifdef HAVE_DBUS QObject::connect(&mpris, SIGNAL(RaiseMainWindow()), &w, SLOT(Raise())); #endif diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 87229e49f..b9c93379c 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -25,8 +25,10 @@ #include "songloaderinserter.h" #include "songmimedata.h" #include "songplaylistitem.h" +#include "core/closure.h" #include "core/logging.h" #include "core/modelfuturewatcher.h" +#include "core/tagreaderclient.h" #include "core/timeconstants.h" #include "internet/jamendoplaylistitem.h" #include "internet/jamendoservice.h" @@ -321,24 +323,25 @@ bool Playlist::setData(const QModelIndex &index, const QVariant &value, int) { library_->AddOrUpdateSongs(SongList() << song); emit EditingFinished(index); } else { - QFuture future = song.BackgroundSave(); - ModelFutureWatcher* watcher = new ModelFutureWatcher(index, this); - watcher->setFuture(future); - connect(watcher, SIGNAL(finished()), SLOT(SongSaveComplete())); + TagReaderReply* reply = TagReaderClient::Instance()->SaveFile( + song.url().toLocalFile(), song); + + NewClosure(reply, SIGNAL(Finished(bool)), + this, SLOT(SongSaveComplete(TagReaderReply*,QPersistentModelIndex)), + reply, QPersistentModelIndex(index)); } return true; } -void Playlist::SongSaveComplete() { - ModelFutureWatcher* watcher = static_cast*>(sender()); - watcher->deleteLater(); - const QPersistentModelIndex& index = watcher->index(); - if (index.isValid()) { +void Playlist::SongSaveComplete(TagReaderReply* reply, const QPersistentModelIndex& index) { + if (reply->is_successful() && index.isValid()) { QFuture future = item_at(index.row())->BackgroundReload(); ModelFutureWatcher* watcher = new ModelFutureWatcher(index, this); watcher->setFuture(future); connect(watcher, SIGNAL(finished()), SLOT(ItemReloadComplete())); } + + reply->deleteLater(); } void Playlist::ItemReloadComplete() { diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index dc3a4438c..d99ce766c 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -25,6 +25,7 @@ #include "playlistitem.h" #include "playlistsequence.h" +#include "core/tagreaderclient.h" #include "core/song.h" #include "smartplaylists/generator_fwd.h" @@ -328,7 +329,7 @@ class Playlist : public QAbstractListModel { void TracksDequeued(); void TracksEnqueued(const QModelIndex&, int begin, int end); void QueueLayoutChanged(); - void SongSaveComplete(); + void SongSaveComplete(TagReaderReply* reply, const QPersistentModelIndex& index); void ItemReloadComplete(); void ItemsLoaded(); void SongInsertVetoListenerDestroyed(); diff --git a/src/playlist/songplaylistitem.cpp b/src/playlist/songplaylistitem.cpp index 3903d22e1..ac0bc103b 100644 --- a/src/playlist/songplaylistitem.cpp +++ b/src/playlist/songplaylistitem.cpp @@ -17,6 +17,7 @@ #include "playlistbackend.h" #include "songplaylistitem.h" +#include "core/tagreaderclient.h" #include "library/sqlrow.h" @@ -40,8 +41,6 @@ bool SongPlaylistItem::InitFromQuery(const SqlRow& query) { if (type() == "Stream") { song_.set_filetype(Song::Type_Stream); - } else { - song_.set_directory_id(-1); } return true; @@ -54,11 +53,8 @@ QUrl SongPlaylistItem::Url() const { void SongPlaylistItem::Reload() { if (song_.url().scheme() != "file") return; - QString old_filename = song_.url().toLocalFile(); - int old_directory_id = song_.directory_id(); - song_ = Song(); - song_.InitFromFile(old_filename, old_directory_id); + TagReaderClient::Instance()->ReadFileBlocking(song_.url().toLocalFile(), &song_); } Song SongPlaylistItem::Metadata() const { diff --git a/src/playlistparsers/parserbase.cpp b/src/playlistparsers/parserbase.cpp index 613e6be57..8400f4e62 100644 --- a/src/playlistparsers/parserbase.cpp +++ b/src/playlistparsers/parserbase.cpp @@ -16,6 +16,7 @@ */ #include "parserbase.h" +#include "core/tagreaderclient.h" #include "library/librarybackend.h" #include "library/libraryquery.h" #include "library/sqlrow.h" @@ -74,7 +75,7 @@ void ParserBase::LoadSong(const QString& filename_or_url, qint64 beginning, if (library_song.is_valid()) { *song = library_song; } else { - song->InitFromFile(filename, -1); + TagReaderClient::Instance()->ReadFileBlocking(filename, song); } } diff --git a/src/ui/edittagdialog.cpp b/src/ui/edittagdialog.cpp index 5ada0a35d..e307dd918 100644 --- a/src/ui/edittagdialog.cpp +++ b/src/ui/edittagdialog.cpp @@ -20,6 +20,7 @@ #include "trackselectiondialog.h" #include "ui_edittagdialog.h" #include "core/logging.h" +#include "core/tagreaderclient.h" #include "core/utilities.h" #include "covers/albumcoverloader.h" #include "covers/coverproviders.h" @@ -195,7 +196,7 @@ QList EditTagDialog::LoadData(const SongList& songs) const if (song.IsEditable()) { // Try reloading the tags from file Song copy(song); - copy.InitFromFile(copy.url().toLocalFile(), copy.directory_id()); + TagReaderClient::Instance()->ReadFileBlocking(copy.url().toLocalFile(), ©); if (copy.is_valid()) ret << Data(copy); @@ -607,7 +608,8 @@ void EditTagDialog::SaveData(const QList& data) { if (ref.current_.IsMetadataEqual(ref.original_)) continue; - if (!ref.current_.Save()) { + if (!TagReaderClient::Instance()->SaveFileBlocking( + ref.current_.url().toLocalFile(), ref.current_)) { emit Error(tr("An error occurred writing metadata to '%1'").arg(ref.current_.url().toLocalFile())); } } diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index 817849e10..468f32c88 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -161,7 +161,6 @@ MainWindow::MainWindow( ArtLoader* art_loader, CoverProviders* cover_providers, GlobalSearch* global_search, - TagReader* tag_reader_client, QWidget* parent) : QMainWindow(parent), ui_(new Ui_MainWindow), @@ -178,7 +177,6 @@ MainWindow::MainWindow( library_(NULL), global_shortcuts_(new GlobalShortcuts(this)), global_search_(global_search), - tag_reader_client_(tag_reader_client), remote_(NULL), devices_(NULL), library_view_(new LibraryViewContainer(this)), @@ -1499,21 +1497,24 @@ void MainWindow::RenumberTracks() { if (song.IsEditable()) { song.set_track(track); - QFuture future = song.BackgroundSave(); - ModelFutureWatcher* watcher = new ModelFutureWatcher(source_index, this); - watcher->setFuture(future); - connect(watcher, SIGNAL(finished()), SLOT(SongSaveComplete())); + + TagReaderReply* reply = + TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); + + NewClosure(reply, SIGNAL(Finished(bool)), + this, SLOT(SongSaveComplete(TagReaderReply*,QPersistentModelIndex)), + reply, QPersistentModelIndex(source_index)); } track++; } } -void MainWindow::SongSaveComplete() { - ModelFutureWatcher* watcher = static_cast*>(sender()); - watcher->deleteLater(); - if (watcher->index().isValid()) { - playlists_->current()->ReloadItems(QList() << watcher->index().row()); +void MainWindow::SongSaveComplete(TagReaderReply* reply, + const QPersistentModelIndex& index) { + if (reply->is_successful() && index.isValid()) { + playlists_->current()->ReloadItems(QList() << index.row()); } + reply->deleteLater(); } void MainWindow::SelectionSetValue() { @@ -1531,10 +1532,12 @@ void MainWindow::SelectionSetValue() { Song song = playlists_->current()->item_at(row)->Metadata(); if (Playlist::set_column_value(song, column, column_value)) { - QFuture future = song.BackgroundSave(); - ModelFutureWatcher* watcher = new ModelFutureWatcher(source_index, this); - watcher->setFuture(future); - connect(watcher, SIGNAL(finished()), SLOT(SongSaveComplete())); + TagReaderReply* reply = + TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); + + NewClosure(reply, SIGNAL(Finished(bool)), + this, SLOT(SongSaveComplete(TagReaderReply*,QPersistentModelIndex)), + reply, QPersistentModelIndex(source_index)); } } } diff --git a/src/ui/mainwindow.h b/src/ui/mainwindow.h index 89fcab4d8..c8a0391bb 100644 --- a/src/ui/mainwindow.h +++ b/src/ui/mainwindow.h @@ -26,6 +26,7 @@ #include "config.h" #include "core/mac_startup.h" +#include "core/tagreaderclient.h" #include "engines/engine_fwd.h" #include "library/librarymodel.h" #include "playlist/playlistitem.h" @@ -68,7 +69,6 @@ class SongInfoBase; class SongInfoView; class SystemTrayIcon; class TagFetcher; -class TagReaderClient; class TaskManager; class TrackSelectionDialog; class TranscodeDialog; @@ -93,7 +93,6 @@ class MainWindow : public QMainWindow, public PlatformInterface { ArtLoader* art_loader, CoverProviders* cover_providers, GlobalSearch* global_search, - TagReaderClient* tag_reader_client, QWidget *parent = 0); ~MainWindow(); @@ -222,7 +221,8 @@ class MainWindow : public QMainWindow, public PlatformInterface { void NowPlayingWidgetPositionChanged(bool above_status_bar); - void SongSaveComplete(); + void SongSaveComplete(TagReaderReply* reply, + const QPersistentModelIndex& index); void ShowCoverManager(); #ifdef HAVE_LIBLASTFM @@ -281,7 +281,6 @@ class MainWindow : public QMainWindow, public PlatformInterface { Library* library_; GlobalShortcuts* global_shortcuts_; GlobalSearch* global_search_; - TagReaderClient* tag_reader_client_; Remote* remote_; DeviceManager* devices_; diff --git a/src/ui/organisedialog.cpp b/src/ui/organisedialog.cpp index 28f516ffc..cafb43278 100644 --- a/src/ui/organisedialog.cpp +++ b/src/ui/organisedialog.cpp @@ -21,6 +21,7 @@ #include "ui_organisedialog.h" #include "core/musicstorage.h" #include "core/organise.h" +#include "core/tagreaderclient.h" #include #include @@ -172,7 +173,8 @@ void OrganiseDialog::LoadPreviewSongs(const QString& filename) { } Song song; - song.InitFromFile(filename, -1); + TagReaderClient::Instance()->ReadFileBlocking(filename, &song); + if (song.is_valid()) preview_songs_ << song; } diff --git a/src/ui/trackselectiondialog.cpp b/src/ui/trackselectiondialog.cpp index af08f807e..14cb5dc9e 100644 --- a/src/ui/trackselectiondialog.cpp +++ b/src/ui/trackselectiondialog.cpp @@ -18,6 +18,7 @@ #include "iconloader.h" #include "trackselectiondialog.h" #include "ui_trackselectiondialog.h" +#include "core/tagreaderclient.h" #include #include @@ -241,7 +242,7 @@ void TrackSelectionDialog::SaveData(const QList& data) { copy.set_album(new_metadata.album()); copy.set_track(new_metadata.track()); - copy.Save(); + TagReaderClient::Instance()->SaveFileBlocking(copy.url().toLocalFile(), copy); } }