From 437df914fa88ad4cf2f723bfd5b13a01a5f9b436 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Tue, 19 Jan 2021 20:21:41 +0100 Subject: [PATCH 01/17] Regarding Issue #6468 (tagreader problems) When using Apple style Mp4 tags the tmpo property which normally is translated to BPM is not fetched from audio file metadata. This adds this feature without fixing other problems, e.g. when changing tags within clementine existing but unchanged tags become deranged. I strongly suggest to change taglib interface in a way kid3 is using (explicit adding/removing tags). The minor change within utilities is just to avoid searching for a file within huge direcories as simply showing directory without positioning or at least highlighting is unuseable. --- ext/libclementine-tagreader/tagreader.cpp | 10 +++++++--- src/core/utilities.cpp | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index 45704408c7..09bc172585 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -306,8 +306,7 @@ void TagReader::ReadFile(const QString& filename, } if (items.contains("BPM")) { - Decode(items["BPM"].toStringList().toString(", "), nullptr, - song->mutable_performer()); + song->set_bpm(TStringToQString(items["BPM"].toString()).trimmed().toFloat()); } if (items.contains("PERFORMER")) { @@ -502,7 +501,12 @@ void TagReader::ReadFile(const QString& filename, song->set_score(score); } } - + if (items.contains("tmpo")) { + float bpm = (float)items["tmpo"].toInt(); + if (song->bpm() <= 0 && bpm > 0) { + song->set_bpm(bpm); + } + } if (items.contains("\251wrt")) { Decode(items["\251wrt"].toStringList().toString(", "), nullptr, song->mutable_composer()); diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index ddbdb53d2c..340810e087 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -443,7 +443,7 @@ void OpenInFileBrowser(const QList& urls) { #elif defined(Q_OS_WIN32) ShowFileInExplorer(path); #else - QDesktopServices::openUrl(QUrl::fromLocalFile(directory)); + QDesktopServices::openUrl(QUrl::fromLocalFile(path)); #endif } } From 3628cbe6da72ecefec935ab81bc2b60e09d71219 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Tue, 26 Jan 2021 21:55:48 +0100 Subject: [PATCH 02/17] In order to enhance taglib writings into song files a new method is added to allow single tag editting while the original method SaveFile was overtwriting existing tags with invalid data. Therefor the original method now only effects basic attributes as required e.g. after ripping. --- ext/clementine-tagreader/tagreaderworker.cpp | 7 + ext/libclementine-tagreader/tagreader.cpp | 194 ++++++------------ ext/libclementine-tagreader/tagreader.h | 4 + .../tagreadermessages.proto | 14 ++ src/core/tagreaderclient.cpp | 29 +++ src/core/tagreaderclient.h | 3 + src/playlist/playlist.cpp | 57 ++++- src/playlist/playlist.h | 2 + src/ui/mainwindow.cpp | 9 +- 9 files changed, 181 insertions(+), 138 deletions(-) diff --git a/ext/clementine-tagreader/tagreaderworker.cpp b/ext/clementine-tagreader/tagreaderworker.cpp index 235aa3f11b..8f69efb065 100644 --- a/ext/clementine-tagreader/tagreaderworker.cpp +++ b/ext/clementine-tagreader/tagreaderworker.cpp @@ -46,6 +46,13 @@ void TagReaderWorker::MessageArrived(const pb::tagreader::Message& message) { reply.mutable_save_file_response()->set_success(tag_reader_.SaveFile( QStringFromStdString(message.save_file_request().filename()), message.save_file_request().metadata())); + } else if (message.has_save_song_tag_to_file_request()) { + reply.mutable_save_song_tag_to_file_response()->set_success( + tag_reader_.UpdateSongTag( + QStringFromStdString(message.save_song_tag_to_file_request().filename()), + QStringFromStdString(message.save_song_tag_to_file_request().tagname()), + QStringFromStdString(message.save_song_tag_to_file_request().tagvalue()) + )); } else if (message.has_save_song_statistics_to_file_request()) { reply.mutable_save_song_statistics_to_file_response()->set_success( tag_reader_.SaveSongStatisticsToFile( diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index 09bc172585..dacdeffb01 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -51,6 +51,10 @@ #include #include #include + +#include +#include + #include #include #include @@ -776,42 +780,6 @@ void TagReader::ParseOggTag(const TagLib::Ogg::FieldListMap& map, Decode(map["UNSYNCEDLYRICS"].front(), codec, song->mutable_lyrics()); } -void TagReader::SetVorbisComments( - TagLib::Ogg::XiphComment* vorbis_comments, - const pb::tagreader::SongMetadata& song) const { - vorbis_comments->addField("COMPOSER", - StdStringToTaglibString(song.composer()), true); - vorbis_comments->addField("PERFORMER", - StdStringToTaglibString(song.performer()), true); - vorbis_comments->addField("CONTENT GROUP", - StdStringToTaglibString(song.grouping()), true); - vorbis_comments->addField( - "BPM", - QStringToTaglibString(song.bpm() <= 0 - 1 ? QString() - : QString::number(song.bpm())), - true); - vorbis_comments->addField( - "DISCNUMBER", - QStringToTaglibString(song.disc() <= 0 ? QString() - : QString::number(song.disc())), - true); - vorbis_comments->addField( - "COMPILATION", - QStringToTaglibString(song.compilation() ? QString::number(1) - : QString()), - true); - - // Try to be coherent, the two forms are used but the first one is preferred - - vorbis_comments->addField("ALBUMARTIST", - StdStringToTaglibString(song.albumartist()), true); - vorbis_comments->removeField("ALBUM ARTIST"); - - vorbis_comments->addField("LYRICS", StdStringToTaglibString(song.lyrics()), - true); - vorbis_comments->removeField("UNSYNCEDLYRICS"); -} - void TagReader::SetFMPSStatisticsVorbisComments( TagLib::Ogg::XiphComment* vorbis_comments, const pb::tagreader::SongMetadata& song) const { @@ -873,111 +841,74 @@ pb::tagreader::SongMetadata_Type TagReader::GuessFileType( } bool TagReader::SaveFile(const QString& filename, - const pb::tagreader::SongMetadata& song) const { + const pb::tagreader::SongMetadata& song) const { if (filename.isNull()) return false; - qLog(Debug) << "Saving tags to" << filename; - std::unique_ptr fileref(factory_->GetFileRef(filename)); - if (!fileref || fileref->isNull()) // The file probably doesn't exist return false; - fileref->tag()->setTitle(StdStringToTaglibString(song.title())); - fileref->tag()->setArtist(StdStringToTaglibString(song.artist())); // TPE1 - fileref->tag()->setAlbum(StdStringToTaglibString(song.album())); - fileref->tag()->setGenre(StdStringToTaglibString(song.genre())); - fileref->tag()->setComment(StdStringToTaglibString(song.comment())); - fileref->tag()->setYear(song.year() <= 0 - 1 ? 0 : song.year()); - fileref->tag()->setTrack(song.track() <= 0 - 1 ? 0 : song.track()); - - auto saveApeTag = [&](TagLib::APE::Tag* tag) { - tag->addValue( - "disc", - QStringToTaglibString(song.disc() <= 0 ? QString() - : QString::number(song.disc())), - true); - tag->addValue("bpm", - QStringToTaglibString(song.bpm() <= 0 - 1 - ? QString() - : QString::number(song.bpm())), - true); - tag->setItem("composer", - TagLib::APE::Item( - "composer", TagLib::StringList(song.composer().c_str()))); - tag->setItem("grouping", - TagLib::APE::Item( - "grouping", TagLib::StringList(song.grouping().c_str()))); - tag->setItem("performer", - TagLib::APE::Item("performer", TagLib::StringList( - song.performer().c_str()))); - tag->setItem( - "album artist", - TagLib::APE::Item("album artist", - TagLib::StringList(song.albumartist().c_str()))); - tag->setItem("lyrics", - TagLib::APE::Item("lyrics", TagLib::String(song.lyrics()))); - tag->addValue("compilation", - QStringToTaglibString(song.compilation() ? QString::number(1) - : QString()), - true); - }; + // only basic tags as provided by ripper + TagLib::Tag *t = fileref->tag(); + t->setTitle(StdStringToTaglibString(song.title())); + t->setArtist(StdStringToTaglibString(song.artist())); + t->setAlbum(StdStringToTaglibString(song.album())); + t->setGenre(StdStringToTaglibString(song.genre())); + t->setComment(StdStringToTaglibString(song.comment())); + t->setYear(song.year() <= 0 - 1 ? 0 : song.year()); + t->setTrack(song.track() <= 0 - 1 ? 0 : song.track()); + // if more tags are required propertymap() shall be used - if (TagLib::MPEG::File* file = - dynamic_cast(fileref->file())) { - TagLib::ID3v2::Tag* tag = file->ID3v2Tag(true); - SetTextFrame("TPOS", - song.disc() <= 0 ? QString() : QString::number(song.disc()), - tag); - SetTextFrame("TBPM", - song.bpm() <= 0 - 1 ? QString() : QString::number(song.bpm()), - tag); - SetTextFrame("TCOM", song.composer(), tag); - SetTextFrame("TIT1", song.grouping(), tag); - SetTextFrame("TOPE", song.performer(), tag); - SetUnsyncLyricsFrame(song.lyrics(), tag); - // Skip TPE1 (which is the artist) here because we already set it - SetTextFrame("TPE2", song.albumartist(), tag); - SetTextFrame("TCMP", song.compilation() ? QString::number(1) : QString(), - tag); - } else if (TagLib::FLAC::File* file = - dynamic_cast(fileref->file())) { - TagLib::Ogg::XiphComment* tag = file->xiphComment(); - SetVorbisComments(tag, song); - } else if (TagLib::MP4::File* file = - dynamic_cast(fileref->file())) { - TagLib::MP4::Tag* tag = file->tag(); - tag->itemListMap()["disk"] = - TagLib::MP4::Item(song.disc() <= 0 - 1 ? 0 : song.disc(), 0); - tag->itemListMap()["tmpo"] = TagLib::StringList( - song.bpm() <= 0 - 1 ? "0" : TagLib::String::number(song.bpm())); - tag->itemListMap()["\251wrt"] = TagLib::StringList(song.composer().c_str()); - tag->itemListMap()["\251grp"] = TagLib::StringList(song.grouping().c_str()); - tag->itemListMap()["\251lyr"] = TagLib::StringList(song.lyrics().c_str()); - tag->itemListMap()["aART"] = TagLib::StringList(song.albumartist().c_str()); - tag->itemListMap()["cpil"] = - TagLib::StringList(song.compilation() ? "1" : "0"); - } else if (TagLib::APE::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } else if (TagLib::MPC::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } else if (TagLib::WavPack::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } + bool ret = fileref->save(); - // Handle all the files which have VorbisComments (Ogg, OPUS, ...) in the same - // way; - // apart, so we keep specific behavior for some formats by adding another - // "else if" block above. - if (TagLib::Ogg::XiphComment* tag = - dynamic_cast(fileref->file()->tag())) { - SetVorbisComments(tag, song); +#ifdef Q_OS_LINUX + if (ret) { + // Linux: inotify doesn't seem to notice the change to the file unless we + // change the timestamps as well. (this is what touch does) + utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } +#endif // Q_OS_LINUX + + return ret; +} +// Replacing SaveFile for single tag editting (e.g. in playlist) +// as full store operation (especially when file was mp4) failed +// This code performs single tag insert/update/delete and +// relies on taglib's PropertyMap to handle file variants. +bool TagReader::UpdateSongTag(const QString& filename, + const QString& tagname, + const QString& tagvalue) const { + + qLog(Debug) << "UpdateSongTag of " << filename << " tag " << tagname << " value " << tagvalue; + + if (filename.isNull()) return false; + + std::unique_ptr fileref(factory_->GetFileRef(filename)); + if (!fileref || fileref->isNull()) // The file probably doesn't exist + return false; + + qLog(Debug) << "UpdateSongTag begin"; + + // according taglib tagwriter example + // basic attributes see SaveFile + + // no check for available ones + TagLib::PropertyMap map = fileref->file()->properties(); + + TagLib::String key = QStringToTaglibString(tagname); + + if (tagvalue.isEmpty()) + map.erase(key); + else + // inserts too if not yet + map.replace(key, QStringToTaglibString(tagvalue)); + + qLog(Debug) << TStringToQString(map.toString()); + + fileref->file()->setProperties(map); bool ret = fileref->save(); + #ifdef Q_OS_LINUX if (ret) { // Linux: inotify doesn't seem to notice the change to the file unless we @@ -985,7 +916,8 @@ bool TagReader::SaveFile(const QString& filename, utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } #endif // Q_OS_LINUX - + + qLog(Debug) << "UpdateSongTag end"; return ret; } diff --git a/ext/libclementine-tagreader/tagreader.h b/ext/libclementine-tagreader/tagreader.h index 84a4302d91..8b9edc0257 100644 --- a/ext/libclementine-tagreader/tagreader.h +++ b/ext/libclementine-tagreader/tagreader.h @@ -62,6 +62,10 @@ class TagReader { const pb::tagreader::SongMetadata& song) const; // Returns false if something went wrong; returns true otherwise (might // returns true if the file exists but nothing has been written inside because + bool UpdateSongTag(const QString& filename, + const QString& tagname, + const QString& tagvalue) const; + // statistics tag format is not supported for this kind of file) bool SaveSongStatisticsToFile(const QString& filename, const pb::tagreader::SongMetadata& song) const; diff --git a/ext/libclementine-tagreader/tagreadermessages.proto b/ext/libclementine-tagreader/tagreadermessages.proto index 531efb7172..7bf708179b 100644 --- a/ext/libclementine-tagreader/tagreadermessages.proto +++ b/ext/libclementine-tagreader/tagreadermessages.proto @@ -106,6 +106,16 @@ message ReadCloudFileResponse { optional SongMetadata metadata = 1; } +message SaveSongTagToFileRequest { + optional string filename = 1; + optional string tagname = 2; + optional string tagvalue = 3; +} + +message SaveSongTagToFileResponse { + optional bool success = 1; +} + message SaveSongStatisticsToFileRequest { optional string filename = 1; optional SongMetadata metadata = 2; @@ -147,4 +157,8 @@ message Message { optional SaveSongRatingToFileRequest save_song_rating_to_file_request = 14; optional SaveSongRatingToFileResponse save_song_rating_to_file_response = 15; + optional SaveSongTagToFileRequest save_song_tag_to_file_request = 16; + + optional SaveSongTagToFileResponse save_song_tag_to_file_response = 17; + } diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index 521ce18249..f00902b4bc 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -66,17 +66,44 @@ TagReaderReply* TagReaderClient::ReadFile(const QString& filename) { return worker_pool_->SendMessageWithReply(&message); } +//TODO: remove form based metadataeditting (./src/ui/edittagdialog.cpp) +// shall be used only for new created (e.g. ripper, podcast, ...) files +// any other editting shall be replaced by field level changes, see below TagReaderReply* TagReaderClient::SaveFile(const QString& filename, const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveFileRequest* req = message.mutable_save_file_request(); + + qLog(Debug) << "TagReaderClient::SaveFile: " << filename; + req->set_filename(DataCommaSizeFromQString(filename)); metadata.ToProtobuf(req->mutable_metadata()); return worker_pool_->SendMessageWithReply(&message); } + +// Single field update, called by ./src/playlist/playlist.cpp +// Mayor change necessary, aka any "save..file" members be replaced by +// save (file, tagname, tagvalue) without sending whole song/metadata +TagReaderReply* TagReaderClient::UpdateSongTag(const QString& filename, + const QString& tagname, + const QVariant& tagvalue) { + pb::tagreader::Message message; + pb::tagreader::SaveSongTagToFileRequest* req = + message.mutable_save_song_tag_to_file_request(); + + qLog(Debug) << "TagReaderClient::SaveSongTag: " << filename + << " tag: " << tagname << " value: " << tagvalue; + + req->set_filename(DataCommaSizeFromQString(filename)); + req->set_tagname(DataCommaSizeFromQString(tagname)); + req->set_tagvalue(DataCommaSizeFromQString(tagvalue.toString())); + + return worker_pool_->SendMessageWithReply(&message); +} + TagReaderReply* TagReaderClient::UpdateSongStatistics(const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveSongStatisticsToFileRequest* req = @@ -166,6 +193,8 @@ bool TagReaderClient::SaveFileBlocking(const QString& filename, bool ret = false; + qLog(Debug) << "TagReaderClient::SaveFileBlocking: " << filename; + TagReaderReply* reply = SaveFile(filename, metadata); if (reply->WaitForFinished()) { ret = reply->message().save_file_response().success(); diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 5571f5d569..44d87208e4 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -46,6 +46,9 @@ class TagReaderClient : public QObject { ReplyType* ReadFile(const QString& filename); ReplyType* SaveFile(const QString& filename, const Song& metadata); + ReplyType* UpdateSongTag(const QString& filename, + const QString& tagname, + const QVariant& tagvalue); ReplyType* UpdateSongStatistics(const Song& metadata); ReplyType* UpdateSongRating(const Song& metadata); ReplyType* IsMediaFile(const QString& filename); diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index c4c20ec2a2..adb4c353b8 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -98,6 +98,40 @@ const int Playlist::kUndoItemLimit = 500; const qint64 Playlist::kMinScrobblePointNsecs = 31ll * kNsecPerSec; const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; +// see enum Columns, when playlist column is editable the names are +// used as names for taglib writing +const QStringList Playlist::kColumns = QStringList() + << "title" + << "artist" + << "album" + << "albumartist" + << "composer" + << "length" + << "track" + << "disc" + << "year" + << "genre" + << "bpm" + << "bitrate" + << "samplerate" + << "filename" + << "basefilename" + << "filesize" + << "filetype" + << "ctime" + << "mtime" + << "rating" + << "playcount" + << "skipcount" + << "lastplayed" + << "score" + << "comment" + << "source" + << "mood" + << "performer" + << "grouping" + << "originalyear"; + namespace { QString removePrefix(const QString& a, const QStringList& prefixes) { for (const QString& prefix : prefixes) { @@ -212,6 +246,7 @@ bool Playlist::column_is_editable(Playlist::Column column) { case Column_Disc: case Column_Year: case Column_Genre: + case Column_BPM: case Column_Score: case Column_Comment: return true; @@ -223,8 +258,10 @@ bool Playlist::column_is_editable(Playlist::Column column) { bool Playlist::set_column_value(Song& song, Playlist::Column column, const QVariant& value) { - if (!song.IsEditable()) return false; - + if (!song.IsEditable()) { + qLog(Debug) << "PlayList::set_column_value not editable: " << column; + return false; + } switch (column) { case Column_Title: song.set_title(value.toString()); @@ -259,6 +296,9 @@ bool Playlist::set_column_value(Song& song, Playlist::Column column, case Column_Genre: song.set_genre(value.toString()); break; + case Column_BPM: + song.set_bpm(value.toFloat()); + break; case Column_Score: song.set_score(value.toInt()); break; @@ -431,8 +471,15 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, library_->AddOrUpdateSongs(SongList() << song); emit EditingFinished(index); } else { + qLog(Debug) << "Playlist::setData file: " << song.url().toLocalFile(); + const QString name = Playlist::kColumns.at(index.column()); + qLog(Debug) << "Playlist::setData name: " << name; + qLog(Debug) << "Playlist::setData value: " << value; + + // Update just that tag TagReaderReply* reply = - TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); + TagReaderClient::Instance()->UpdateSongTag(song.url().toLocalFile(), + name, value); NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)), @@ -444,7 +491,7 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, void Playlist::SongSaveComplete(TagReaderReply* reply, const QPersistentModelIndex& index) { if (reply->is_successful() && index.isValid()) { - if (reply->message().save_file_response().success()) { + if (reply->message().save_song_tag_to_file_response().success()) { QFuture future = item_at(index.row())->BackgroundReload(); NewClosure(future, this, SLOT(ItemReloadComplete(QPersistentModelIndex)), index); @@ -452,7 +499,7 @@ void Playlist::SongSaveComplete(TagReaderReply* reply, emit Error( tr("An error occurred writing metadata to '%1'") .arg(QString::fromStdString( - reply->request_message().save_file_request().filename()))); + reply->request_message().save_song_tag_to_file_request().filename()))); } } reply->deleteLater(); diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index 833f311cd5..61c7d189fc 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -167,6 +167,8 @@ class Playlist : public QAbstractListModel { static const qint64 kMinScrobblePointNsecs; static const qint64 kMaxScrobblePointNsecs; + static const QStringList kColumns; + static bool CompareItems(int column, Qt::SortOrder order, PlaylistItemPtr a, PlaylistItemPtr b, const QStringList& prefixes = {}); diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index e9a927ef9b..4cc7522c83 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -2106,7 +2106,8 @@ void MainWindow::SelectionSetValue() { app_->playlist_manager()->current()->proxy()->mapToSource(index); int row = source_index.row(); Song song = app_->playlist_manager()->current()->item_at(row)->Metadata(); - + qLog(Debug) << "MainWindow: SelectionSetValue: song: " << row << " value: " << column_value; + if (Playlist::set_column_value(song, column, column_value)) { TagReaderReply* reply = TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); @@ -2125,6 +2126,8 @@ void MainWindow::EditValue() { // Edit the last column that was right-clicked on. If nothing's ever been // right clicked then look for the first editable column. int column = playlist_menu_index_.column(); + + qLog(Debug) << "MainWindow: EditValue: column: " << column; if (column == -1) { for (int i = 0; i < ui_->playlist->view()->model()->columnCount(); ++i) { if (ui_->playlist->view()->isColumnHidden(i)) continue; @@ -2133,7 +2136,7 @@ void MainWindow::EditValue() { break; } } - + qLog(Debug) << "MainWindow: EditValue: column: " << column; ui_->playlist->view()->edit(current.sibling(current.row(), column)); } @@ -2487,9 +2490,11 @@ void MainWindow::CopyFilesToDevice(const QList& urls) { void MainWindow::EditFileTags(const QList& urls) { SongList songs; + for (const QUrl& url : urls) { Song song; song.set_url(url); + qLog(Debug) << "MainWindow: EditFileTags: url: " << url; song.set_valid(true); song.set_filetype(Song::Type_Mpeg); songs << song; From 61799b5a312e6f1910d72f6462442ec031930cc2 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Thu, 28 Jan 2021 10:19:19 +0100 Subject: [PATCH 03/17] Playlist::kColumns provides names to enum Columns in order to use taglib in single tag mode. This avoids to overwrite uneffected properties within audiofiles as it occured when using SaveFile interface. Quite sure not all names are used for tagging, only those marked edittable can be updated/inserted/removed. The names are taken from taglib translations when available (by convention written in uppercase allthough taglib name mapping is caseinsensitive). Main source: 3rdparty/taglib/.../id3v2frame translations. Names written lowercase miss a taglib equivalent and are used for internal informations. Note: When adding lines to column enumeration: Try to add mappable taglib properties. --- src/playlist/playlist.cpp | 41 +++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index adb4c353b8..4750883750 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -98,20 +98,27 @@ const int Playlist::kUndoItemLimit = 500; const qint64 Playlist::kMinScrobblePointNsecs = 31ll * kNsecPerSec; const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; -// see enum Columns, when playlist column is editable the names are -// used as names for taglib writing +// kColumns provides names to enum Columns. +// When the playlist column is editable those names are used as names +// for taglib writing to file. Names are caseinsensitive and often +// 3rdparty/taglib/.../id3v2frame translations is base for naming used. +// When playlist enum columns is changed the list below has to be adapted +// accordingly. Lowercase written names miss a taglib equivalent and are +// used for internal informations. +// When adding lines to column enumeration: Try to add mappable taglib +// properties. const QStringList Playlist::kColumns = QStringList() - << "title" - << "artist" - << "album" - << "albumartist" - << "composer" - << "length" - << "track" - << "disc" - << "year" - << "genre" - << "bpm" + << "TITLE" + << "ARTIST" + << "ALBUM" + << "ALBUMARTIST" + << "COMPOSER" + << "LENGTH" + << "TRACKNUMBER" + << "DISCNUMBER" + << "DATE" + << "GENRE" + << "BPM" << "bitrate" << "samplerate" << "filename" @@ -125,12 +132,12 @@ const QStringList Playlist::kColumns = QStringList() << "skipcount" << "lastplayed" << "score" - << "comment" + << "COMMENT" << "source" - << "mood" + << "MOOD" << "performer" - << "grouping" - << "originalyear"; + << "GROUPING" + << "ORIGINALDATE"; namespace { QString removePrefix(const QString& a, const QStringList& prefixes) { From 42eef4f53764c1f21b84240aeb2cefb4e4a90e12 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Tue, 19 Jan 2021 20:21:41 +0100 Subject: [PATCH 04/17] Regarding Issue #6468 (tagreader problems) When using Apple style Mp4 tags the tmpo property which normally is translated to BPM is not fetched from audio file metadata. This adds this feature without fixing other problems, e.g. when changing tags within clementine existing but unchanged tags become deranged. I strongly suggest to change taglib interface in a way kid3 is using (explicit adding/removing tags). The minor change within utilities is just to avoid searching for a file within huge direcories as simply showing directory without positioning or at least highlighting is unuseable. --- ext/libclementine-tagreader/tagreader.cpp | 10 +++++++--- src/core/utilities.cpp | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index 45704408c7..09bc172585 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -306,8 +306,7 @@ void TagReader::ReadFile(const QString& filename, } if (items.contains("BPM")) { - Decode(items["BPM"].toStringList().toString(", "), nullptr, - song->mutable_performer()); + song->set_bpm(TStringToQString(items["BPM"].toString()).trimmed().toFloat()); } if (items.contains("PERFORMER")) { @@ -502,7 +501,12 @@ void TagReader::ReadFile(const QString& filename, song->set_score(score); } } - + if (items.contains("tmpo")) { + float bpm = (float)items["tmpo"].toInt(); + if (song->bpm() <= 0 && bpm > 0) { + song->set_bpm(bpm); + } + } if (items.contains("\251wrt")) { Decode(items["\251wrt"].toStringList().toString(", "), nullptr, song->mutable_composer()); diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index ddbdb53d2c..340810e087 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -443,7 +443,7 @@ void OpenInFileBrowser(const QList& urls) { #elif defined(Q_OS_WIN32) ShowFileInExplorer(path); #else - QDesktopServices::openUrl(QUrl::fromLocalFile(directory)); + QDesktopServices::openUrl(QUrl::fromLocalFile(path)); #endif } } From 98883a0df8a972d59c24ac1ddd77d3dc9348fdc1 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Tue, 26 Jan 2021 21:55:48 +0100 Subject: [PATCH 05/17] In order to enhance taglib writings into song files a new method is added to allow single tag editting while the original method SaveFile was overtwriting existing tags with invalid data. Therefor the original method now only effects basic attributes as required e.g. after ripping. --- ext/clementine-tagreader/tagreaderworker.cpp | 7 + ext/libclementine-tagreader/tagreader.cpp | 194 ++++++------------ ext/libclementine-tagreader/tagreader.h | 4 + .../tagreadermessages.proto | 14 ++ src/core/tagreaderclient.cpp | 29 +++ src/core/tagreaderclient.h | 3 + src/playlist/playlist.cpp | 57 ++++- src/playlist/playlist.h | 2 + src/ui/mainwindow.cpp | 9 +- 9 files changed, 181 insertions(+), 138 deletions(-) diff --git a/ext/clementine-tagreader/tagreaderworker.cpp b/ext/clementine-tagreader/tagreaderworker.cpp index 235aa3f11b..8f69efb065 100644 --- a/ext/clementine-tagreader/tagreaderworker.cpp +++ b/ext/clementine-tagreader/tagreaderworker.cpp @@ -46,6 +46,13 @@ void TagReaderWorker::MessageArrived(const pb::tagreader::Message& message) { reply.mutable_save_file_response()->set_success(tag_reader_.SaveFile( QStringFromStdString(message.save_file_request().filename()), message.save_file_request().metadata())); + } else if (message.has_save_song_tag_to_file_request()) { + reply.mutable_save_song_tag_to_file_response()->set_success( + tag_reader_.UpdateSongTag( + QStringFromStdString(message.save_song_tag_to_file_request().filename()), + QStringFromStdString(message.save_song_tag_to_file_request().tagname()), + QStringFromStdString(message.save_song_tag_to_file_request().tagvalue()) + )); } else if (message.has_save_song_statistics_to_file_request()) { reply.mutable_save_song_statistics_to_file_response()->set_success( tag_reader_.SaveSongStatisticsToFile( diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index 09bc172585..dacdeffb01 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -51,6 +51,10 @@ #include #include #include + +#include +#include + #include #include #include @@ -776,42 +780,6 @@ void TagReader::ParseOggTag(const TagLib::Ogg::FieldListMap& map, Decode(map["UNSYNCEDLYRICS"].front(), codec, song->mutable_lyrics()); } -void TagReader::SetVorbisComments( - TagLib::Ogg::XiphComment* vorbis_comments, - const pb::tagreader::SongMetadata& song) const { - vorbis_comments->addField("COMPOSER", - StdStringToTaglibString(song.composer()), true); - vorbis_comments->addField("PERFORMER", - StdStringToTaglibString(song.performer()), true); - vorbis_comments->addField("CONTENT GROUP", - StdStringToTaglibString(song.grouping()), true); - vorbis_comments->addField( - "BPM", - QStringToTaglibString(song.bpm() <= 0 - 1 ? QString() - : QString::number(song.bpm())), - true); - vorbis_comments->addField( - "DISCNUMBER", - QStringToTaglibString(song.disc() <= 0 ? QString() - : QString::number(song.disc())), - true); - vorbis_comments->addField( - "COMPILATION", - QStringToTaglibString(song.compilation() ? QString::number(1) - : QString()), - true); - - // Try to be coherent, the two forms are used but the first one is preferred - - vorbis_comments->addField("ALBUMARTIST", - StdStringToTaglibString(song.albumartist()), true); - vorbis_comments->removeField("ALBUM ARTIST"); - - vorbis_comments->addField("LYRICS", StdStringToTaglibString(song.lyrics()), - true); - vorbis_comments->removeField("UNSYNCEDLYRICS"); -} - void TagReader::SetFMPSStatisticsVorbisComments( TagLib::Ogg::XiphComment* vorbis_comments, const pb::tagreader::SongMetadata& song) const { @@ -873,111 +841,74 @@ pb::tagreader::SongMetadata_Type TagReader::GuessFileType( } bool TagReader::SaveFile(const QString& filename, - const pb::tagreader::SongMetadata& song) const { + const pb::tagreader::SongMetadata& song) const { if (filename.isNull()) return false; - qLog(Debug) << "Saving tags to" << filename; - std::unique_ptr fileref(factory_->GetFileRef(filename)); - if (!fileref || fileref->isNull()) // The file probably doesn't exist return false; - fileref->tag()->setTitle(StdStringToTaglibString(song.title())); - fileref->tag()->setArtist(StdStringToTaglibString(song.artist())); // TPE1 - fileref->tag()->setAlbum(StdStringToTaglibString(song.album())); - fileref->tag()->setGenre(StdStringToTaglibString(song.genre())); - fileref->tag()->setComment(StdStringToTaglibString(song.comment())); - fileref->tag()->setYear(song.year() <= 0 - 1 ? 0 : song.year()); - fileref->tag()->setTrack(song.track() <= 0 - 1 ? 0 : song.track()); - - auto saveApeTag = [&](TagLib::APE::Tag* tag) { - tag->addValue( - "disc", - QStringToTaglibString(song.disc() <= 0 ? QString() - : QString::number(song.disc())), - true); - tag->addValue("bpm", - QStringToTaglibString(song.bpm() <= 0 - 1 - ? QString() - : QString::number(song.bpm())), - true); - tag->setItem("composer", - TagLib::APE::Item( - "composer", TagLib::StringList(song.composer().c_str()))); - tag->setItem("grouping", - TagLib::APE::Item( - "grouping", TagLib::StringList(song.grouping().c_str()))); - tag->setItem("performer", - TagLib::APE::Item("performer", TagLib::StringList( - song.performer().c_str()))); - tag->setItem( - "album artist", - TagLib::APE::Item("album artist", - TagLib::StringList(song.albumartist().c_str()))); - tag->setItem("lyrics", - TagLib::APE::Item("lyrics", TagLib::String(song.lyrics()))); - tag->addValue("compilation", - QStringToTaglibString(song.compilation() ? QString::number(1) - : QString()), - true); - }; + // only basic tags as provided by ripper + TagLib::Tag *t = fileref->tag(); + t->setTitle(StdStringToTaglibString(song.title())); + t->setArtist(StdStringToTaglibString(song.artist())); + t->setAlbum(StdStringToTaglibString(song.album())); + t->setGenre(StdStringToTaglibString(song.genre())); + t->setComment(StdStringToTaglibString(song.comment())); + t->setYear(song.year() <= 0 - 1 ? 0 : song.year()); + t->setTrack(song.track() <= 0 - 1 ? 0 : song.track()); + // if more tags are required propertymap() shall be used - if (TagLib::MPEG::File* file = - dynamic_cast(fileref->file())) { - TagLib::ID3v2::Tag* tag = file->ID3v2Tag(true); - SetTextFrame("TPOS", - song.disc() <= 0 ? QString() : QString::number(song.disc()), - tag); - SetTextFrame("TBPM", - song.bpm() <= 0 - 1 ? QString() : QString::number(song.bpm()), - tag); - SetTextFrame("TCOM", song.composer(), tag); - SetTextFrame("TIT1", song.grouping(), tag); - SetTextFrame("TOPE", song.performer(), tag); - SetUnsyncLyricsFrame(song.lyrics(), tag); - // Skip TPE1 (which is the artist) here because we already set it - SetTextFrame("TPE2", song.albumartist(), tag); - SetTextFrame("TCMP", song.compilation() ? QString::number(1) : QString(), - tag); - } else if (TagLib::FLAC::File* file = - dynamic_cast(fileref->file())) { - TagLib::Ogg::XiphComment* tag = file->xiphComment(); - SetVorbisComments(tag, song); - } else if (TagLib::MP4::File* file = - dynamic_cast(fileref->file())) { - TagLib::MP4::Tag* tag = file->tag(); - tag->itemListMap()["disk"] = - TagLib::MP4::Item(song.disc() <= 0 - 1 ? 0 : song.disc(), 0); - tag->itemListMap()["tmpo"] = TagLib::StringList( - song.bpm() <= 0 - 1 ? "0" : TagLib::String::number(song.bpm())); - tag->itemListMap()["\251wrt"] = TagLib::StringList(song.composer().c_str()); - tag->itemListMap()["\251grp"] = TagLib::StringList(song.grouping().c_str()); - tag->itemListMap()["\251lyr"] = TagLib::StringList(song.lyrics().c_str()); - tag->itemListMap()["aART"] = TagLib::StringList(song.albumartist().c_str()); - tag->itemListMap()["cpil"] = - TagLib::StringList(song.compilation() ? "1" : "0"); - } else if (TagLib::APE::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } else if (TagLib::MPC::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } else if (TagLib::WavPack::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } + bool ret = fileref->save(); - // Handle all the files which have VorbisComments (Ogg, OPUS, ...) in the same - // way; - // apart, so we keep specific behavior for some formats by adding another - // "else if" block above. - if (TagLib::Ogg::XiphComment* tag = - dynamic_cast(fileref->file()->tag())) { - SetVorbisComments(tag, song); +#ifdef Q_OS_LINUX + if (ret) { + // Linux: inotify doesn't seem to notice the change to the file unless we + // change the timestamps as well. (this is what touch does) + utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } +#endif // Q_OS_LINUX + + return ret; +} +// Replacing SaveFile for single tag editting (e.g. in playlist) +// as full store operation (especially when file was mp4) failed +// This code performs single tag insert/update/delete and +// relies on taglib's PropertyMap to handle file variants. +bool TagReader::UpdateSongTag(const QString& filename, + const QString& tagname, + const QString& tagvalue) const { + + qLog(Debug) << "UpdateSongTag of " << filename << " tag " << tagname << " value " << tagvalue; + + if (filename.isNull()) return false; + + std::unique_ptr fileref(factory_->GetFileRef(filename)); + if (!fileref || fileref->isNull()) // The file probably doesn't exist + return false; + + qLog(Debug) << "UpdateSongTag begin"; + + // according taglib tagwriter example + // basic attributes see SaveFile + + // no check for available ones + TagLib::PropertyMap map = fileref->file()->properties(); + + TagLib::String key = QStringToTaglibString(tagname); + + if (tagvalue.isEmpty()) + map.erase(key); + else + // inserts too if not yet + map.replace(key, QStringToTaglibString(tagvalue)); + + qLog(Debug) << TStringToQString(map.toString()); + + fileref->file()->setProperties(map); bool ret = fileref->save(); + #ifdef Q_OS_LINUX if (ret) { // Linux: inotify doesn't seem to notice the change to the file unless we @@ -985,7 +916,8 @@ bool TagReader::SaveFile(const QString& filename, utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } #endif // Q_OS_LINUX - + + qLog(Debug) << "UpdateSongTag end"; return ret; } diff --git a/ext/libclementine-tagreader/tagreader.h b/ext/libclementine-tagreader/tagreader.h index 84a4302d91..8b9edc0257 100644 --- a/ext/libclementine-tagreader/tagreader.h +++ b/ext/libclementine-tagreader/tagreader.h @@ -62,6 +62,10 @@ class TagReader { const pb::tagreader::SongMetadata& song) const; // Returns false if something went wrong; returns true otherwise (might // returns true if the file exists but nothing has been written inside because + bool UpdateSongTag(const QString& filename, + const QString& tagname, + const QString& tagvalue) const; + // statistics tag format is not supported for this kind of file) bool SaveSongStatisticsToFile(const QString& filename, const pb::tagreader::SongMetadata& song) const; diff --git a/ext/libclementine-tagreader/tagreadermessages.proto b/ext/libclementine-tagreader/tagreadermessages.proto index 531efb7172..7bf708179b 100644 --- a/ext/libclementine-tagreader/tagreadermessages.proto +++ b/ext/libclementine-tagreader/tagreadermessages.proto @@ -106,6 +106,16 @@ message ReadCloudFileResponse { optional SongMetadata metadata = 1; } +message SaveSongTagToFileRequest { + optional string filename = 1; + optional string tagname = 2; + optional string tagvalue = 3; +} + +message SaveSongTagToFileResponse { + optional bool success = 1; +} + message SaveSongStatisticsToFileRequest { optional string filename = 1; optional SongMetadata metadata = 2; @@ -147,4 +157,8 @@ message Message { optional SaveSongRatingToFileRequest save_song_rating_to_file_request = 14; optional SaveSongRatingToFileResponse save_song_rating_to_file_response = 15; + optional SaveSongTagToFileRequest save_song_tag_to_file_request = 16; + + optional SaveSongTagToFileResponse save_song_tag_to_file_response = 17; + } diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index 521ce18249..f00902b4bc 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -66,17 +66,44 @@ TagReaderReply* TagReaderClient::ReadFile(const QString& filename) { return worker_pool_->SendMessageWithReply(&message); } +//TODO: remove form based metadataeditting (./src/ui/edittagdialog.cpp) +// shall be used only for new created (e.g. ripper, podcast, ...) files +// any other editting shall be replaced by field level changes, see below TagReaderReply* TagReaderClient::SaveFile(const QString& filename, const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveFileRequest* req = message.mutable_save_file_request(); + + qLog(Debug) << "TagReaderClient::SaveFile: " << filename; + req->set_filename(DataCommaSizeFromQString(filename)); metadata.ToProtobuf(req->mutable_metadata()); return worker_pool_->SendMessageWithReply(&message); } + +// Single field update, called by ./src/playlist/playlist.cpp +// Mayor change necessary, aka any "save..file" members be replaced by +// save (file, tagname, tagvalue) without sending whole song/metadata +TagReaderReply* TagReaderClient::UpdateSongTag(const QString& filename, + const QString& tagname, + const QVariant& tagvalue) { + pb::tagreader::Message message; + pb::tagreader::SaveSongTagToFileRequest* req = + message.mutable_save_song_tag_to_file_request(); + + qLog(Debug) << "TagReaderClient::SaveSongTag: " << filename + << " tag: " << tagname << " value: " << tagvalue; + + req->set_filename(DataCommaSizeFromQString(filename)); + req->set_tagname(DataCommaSizeFromQString(tagname)); + req->set_tagvalue(DataCommaSizeFromQString(tagvalue.toString())); + + return worker_pool_->SendMessageWithReply(&message); +} + TagReaderReply* TagReaderClient::UpdateSongStatistics(const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveSongStatisticsToFileRequest* req = @@ -166,6 +193,8 @@ bool TagReaderClient::SaveFileBlocking(const QString& filename, bool ret = false; + qLog(Debug) << "TagReaderClient::SaveFileBlocking: " << filename; + TagReaderReply* reply = SaveFile(filename, metadata); if (reply->WaitForFinished()) { ret = reply->message().save_file_response().success(); diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 5571f5d569..44d87208e4 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -46,6 +46,9 @@ class TagReaderClient : public QObject { ReplyType* ReadFile(const QString& filename); ReplyType* SaveFile(const QString& filename, const Song& metadata); + ReplyType* UpdateSongTag(const QString& filename, + const QString& tagname, + const QVariant& tagvalue); ReplyType* UpdateSongStatistics(const Song& metadata); ReplyType* UpdateSongRating(const Song& metadata); ReplyType* IsMediaFile(const QString& filename); diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index c4c20ec2a2..adb4c353b8 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -98,6 +98,40 @@ const int Playlist::kUndoItemLimit = 500; const qint64 Playlist::kMinScrobblePointNsecs = 31ll * kNsecPerSec; const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; +// see enum Columns, when playlist column is editable the names are +// used as names for taglib writing +const QStringList Playlist::kColumns = QStringList() + << "title" + << "artist" + << "album" + << "albumartist" + << "composer" + << "length" + << "track" + << "disc" + << "year" + << "genre" + << "bpm" + << "bitrate" + << "samplerate" + << "filename" + << "basefilename" + << "filesize" + << "filetype" + << "ctime" + << "mtime" + << "rating" + << "playcount" + << "skipcount" + << "lastplayed" + << "score" + << "comment" + << "source" + << "mood" + << "performer" + << "grouping" + << "originalyear"; + namespace { QString removePrefix(const QString& a, const QStringList& prefixes) { for (const QString& prefix : prefixes) { @@ -212,6 +246,7 @@ bool Playlist::column_is_editable(Playlist::Column column) { case Column_Disc: case Column_Year: case Column_Genre: + case Column_BPM: case Column_Score: case Column_Comment: return true; @@ -223,8 +258,10 @@ bool Playlist::column_is_editable(Playlist::Column column) { bool Playlist::set_column_value(Song& song, Playlist::Column column, const QVariant& value) { - if (!song.IsEditable()) return false; - + if (!song.IsEditable()) { + qLog(Debug) << "PlayList::set_column_value not editable: " << column; + return false; + } switch (column) { case Column_Title: song.set_title(value.toString()); @@ -259,6 +296,9 @@ bool Playlist::set_column_value(Song& song, Playlist::Column column, case Column_Genre: song.set_genre(value.toString()); break; + case Column_BPM: + song.set_bpm(value.toFloat()); + break; case Column_Score: song.set_score(value.toInt()); break; @@ -431,8 +471,15 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, library_->AddOrUpdateSongs(SongList() << song); emit EditingFinished(index); } else { + qLog(Debug) << "Playlist::setData file: " << song.url().toLocalFile(); + const QString name = Playlist::kColumns.at(index.column()); + qLog(Debug) << "Playlist::setData name: " << name; + qLog(Debug) << "Playlist::setData value: " << value; + + // Update just that tag TagReaderReply* reply = - TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); + TagReaderClient::Instance()->UpdateSongTag(song.url().toLocalFile(), + name, value); NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)), @@ -444,7 +491,7 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, void Playlist::SongSaveComplete(TagReaderReply* reply, const QPersistentModelIndex& index) { if (reply->is_successful() && index.isValid()) { - if (reply->message().save_file_response().success()) { + if (reply->message().save_song_tag_to_file_response().success()) { QFuture future = item_at(index.row())->BackgroundReload(); NewClosure(future, this, SLOT(ItemReloadComplete(QPersistentModelIndex)), index); @@ -452,7 +499,7 @@ void Playlist::SongSaveComplete(TagReaderReply* reply, emit Error( tr("An error occurred writing metadata to '%1'") .arg(QString::fromStdString( - reply->request_message().save_file_request().filename()))); + reply->request_message().save_song_tag_to_file_request().filename()))); } } reply->deleteLater(); diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index 833f311cd5..61c7d189fc 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -167,6 +167,8 @@ class Playlist : public QAbstractListModel { static const qint64 kMinScrobblePointNsecs; static const qint64 kMaxScrobblePointNsecs; + static const QStringList kColumns; + static bool CompareItems(int column, Qt::SortOrder order, PlaylistItemPtr a, PlaylistItemPtr b, const QStringList& prefixes = {}); diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index d4da9300d0..9407dcf7b8 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -2126,7 +2126,8 @@ void MainWindow::SelectionSetValue() { app_->playlist_manager()->current()->proxy()->mapToSource(index); int row = source_index.row(); Song song = app_->playlist_manager()->current()->item_at(row)->Metadata(); - + qLog(Debug) << "MainWindow: SelectionSetValue: song: " << row << " value: " << column_value; + if (Playlist::set_column_value(song, column, column_value)) { TagReaderReply* reply = TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); @@ -2145,6 +2146,8 @@ void MainWindow::EditValue() { // Edit the last column that was right-clicked on. If nothing's ever been // right clicked then look for the first editable column. int column = playlist_menu_index_.column(); + + qLog(Debug) << "MainWindow: EditValue: column: " << column; if (column == -1) { for (int i = 0; i < ui_->playlist->view()->model()->columnCount(); ++i) { if (ui_->playlist->view()->isColumnHidden(i)) continue; @@ -2153,7 +2156,7 @@ void MainWindow::EditValue() { break; } } - + qLog(Debug) << "MainWindow: EditValue: column: " << column; ui_->playlist->view()->edit(current.sibling(current.row(), column)); } @@ -2507,9 +2510,11 @@ void MainWindow::CopyFilesToDevice(const QList& urls) { void MainWindow::EditFileTags(const QList& urls) { SongList songs; + for (const QUrl& url : urls) { Song song; song.set_url(url); + qLog(Debug) << "MainWindow: EditFileTags: url: " << url; song.set_valid(true); song.set_filetype(Song::Type_Mpeg); songs << song; From d48bc5f85f3c5f76110bfa16e1988f5794a1c0ba Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Thu, 28 Jan 2021 10:19:19 +0100 Subject: [PATCH 06/17] Playlist::kColumns provides names to enum Columns in order to use taglib in single tag mode. This avoids to overwrite uneffected properties within audiofiles as it occured when using SaveFile interface. Quite sure not all names are used for tagging, only those marked edittable can be updated/inserted/removed. The names are taken from taglib translations when available (by convention written in uppercase allthough taglib name mapping is caseinsensitive). Main source: 3rdparty/taglib/.../id3v2frame translations. Names written lowercase miss a taglib equivalent and are used for internal informations. Note: When adding lines to column enumeration: Try to add mappable taglib properties. --- src/playlist/playlist.cpp | 41 +++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index adb4c353b8..4750883750 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -98,20 +98,27 @@ const int Playlist::kUndoItemLimit = 500; const qint64 Playlist::kMinScrobblePointNsecs = 31ll * kNsecPerSec; const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; -// see enum Columns, when playlist column is editable the names are -// used as names for taglib writing +// kColumns provides names to enum Columns. +// When the playlist column is editable those names are used as names +// for taglib writing to file. Names are caseinsensitive and often +// 3rdparty/taglib/.../id3v2frame translations is base for naming used. +// When playlist enum columns is changed the list below has to be adapted +// accordingly. Lowercase written names miss a taglib equivalent and are +// used for internal informations. +// When adding lines to column enumeration: Try to add mappable taglib +// properties. const QStringList Playlist::kColumns = QStringList() - << "title" - << "artist" - << "album" - << "albumartist" - << "composer" - << "length" - << "track" - << "disc" - << "year" - << "genre" - << "bpm" + << "TITLE" + << "ARTIST" + << "ALBUM" + << "ALBUMARTIST" + << "COMPOSER" + << "LENGTH" + << "TRACKNUMBER" + << "DISCNUMBER" + << "DATE" + << "GENRE" + << "BPM" << "bitrate" << "samplerate" << "filename" @@ -125,12 +132,12 @@ const QStringList Playlist::kColumns = QStringList() << "skipcount" << "lastplayed" << "score" - << "comment" + << "COMMENT" << "source" - << "mood" + << "MOOD" << "performer" - << "grouping" - << "originalyear"; + << "GROUPING" + << "ORIGINALDATE"; namespace { QString removePrefix(const QString& a, const QStringList& prefixes) { From 56f0ff1c2c3eeb3865d87583b8d4a73a0e05fa55 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 11:58:56 +0100 Subject: [PATCH 07/17] Obsolete qLog(Debug) removed --- src/ui/mainwindow.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index 9407dcf7b8..d4da9300d0 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -2126,8 +2126,7 @@ void MainWindow::SelectionSetValue() { app_->playlist_manager()->current()->proxy()->mapToSource(index); int row = source_index.row(); Song song = app_->playlist_manager()->current()->item_at(row)->Metadata(); - qLog(Debug) << "MainWindow: SelectionSetValue: song: " << row << " value: " << column_value; - + if (Playlist::set_column_value(song, column, column_value)) { TagReaderReply* reply = TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); @@ -2146,8 +2145,6 @@ void MainWindow::EditValue() { // Edit the last column that was right-clicked on. If nothing's ever been // right clicked then look for the first editable column. int column = playlist_menu_index_.column(); - - qLog(Debug) << "MainWindow: EditValue: column: " << column; if (column == -1) { for (int i = 0; i < ui_->playlist->view()->model()->columnCount(); ++i) { if (ui_->playlist->view()->isColumnHidden(i)) continue; @@ -2156,7 +2153,7 @@ void MainWindow::EditValue() { break; } } - qLog(Debug) << "MainWindow: EditValue: column: " << column; + ui_->playlist->view()->edit(current.sibling(current.row(), column)); } @@ -2510,11 +2507,9 @@ void MainWindow::CopyFilesToDevice(const QList& urls) { void MainWindow::EditFileTags(const QList& urls) { SongList songs; - for (const QUrl& url : urls) { Song song; song.set_url(url); - qLog(Debug) << "MainWindow: EditFileTags: url: " << url; song.set_valid(true); song.set_filetype(Song::Type_Mpeg); songs << song; From 50bce31afc34c503877a3ca4726c995704130a13 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 12:01:14 +0100 Subject: [PATCH 08/17] Source formatting, trying to satisfy Lint clang --- src/playlist/playlist.cpp | 76 +++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 4750883750..12e0eeefaf 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -107,37 +107,36 @@ const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; // used for internal informations. // When adding lines to column enumeration: Try to add mappable taglib // properties. -const QStringList Playlist::kColumns = QStringList() - << "TITLE" - << "ARTIST" - << "ALBUM" - << "ALBUMARTIST" - << "COMPOSER" - << "LENGTH" - << "TRACKNUMBER" - << "DISCNUMBER" - << "DATE" - << "GENRE" - << "BPM" - << "bitrate" - << "samplerate" - << "filename" - << "basefilename" - << "filesize" - << "filetype" - << "ctime" - << "mtime" - << "rating" - << "playcount" - << "skipcount" - << "lastplayed" - << "score" - << "COMMENT" - << "source" - << "MOOD" - << "performer" - << "GROUPING" - << "ORIGINALDATE"; +const QStringList Playlist::kColumns = QStringList() << "TITLE" + << "ARTIST" + << "ALBUM" + << "ALBUMARTIST" + << "COMPOSER" + << "LENGTH" + << "TRACKNUMBER" + << "DISCNUMBER" + << "DATE" + << "GENRE" + << "BPM" + << "bitrate" + << "samplerate" + << "filename" + << "basefilename" + << "filesize" + << "filetype" + << "ctime" + << "mtime" + << "rating" + << "playcount" + << "skipcount" + << "lastplayed" + << "score" + << "COMMENT" + << "source" + << "MOOD" + << "performer" + << "GROUPING" + << "ORIGINALDATE"; namespace { QString removePrefix(const QString& a, const QStringList& prefixes) { @@ -478,15 +477,11 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, library_->AddOrUpdateSongs(SongList() << song); emit EditingFinished(index); } else { - qLog(Debug) << "Playlist::setData file: " << song.url().toLocalFile(); - const QString name = Playlist::kColumns.at(index.column()); - qLog(Debug) << "Playlist::setData name: " << name; - qLog(Debug) << "Playlist::setData value: " << value; + const QString name = Playlist::kColumns.at(index.column()); // Update just that tag - TagReaderReply* reply = - TagReaderClient::Instance()->UpdateSongTag(song.url().toLocalFile(), - name, value); + TagReaderReply* reply = TagReaderClient::Instance()->UpdateSongTag( + song.url().toLocalFile(), name, value); NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)), @@ -505,8 +500,9 @@ void Playlist::SongSaveComplete(TagReaderReply* reply, } else { emit Error( tr("An error occurred writing metadata to '%1'") - .arg(QString::fromStdString( - reply->request_message().save_song_tag_to_file_request().filename()))); + .arg(QString::fromStdString(reply->request_message() + .save_song_tag_to_file_request() + .filename()))); } } reply->deleteLater(); From adc1fab9420dff37583b7b72bb30c983760cfa6c Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 12:05:50 +0100 Subject: [PATCH 09/17] Source formatting changed to satisfy Ling clang and obsolete gLog(Debug) removed --- src/core/tagreaderclient.cpp | 9 --------- src/core/tagreaderclient.h | 3 +-- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index f00902b4bc..d6c124ba5c 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -66,17 +66,11 @@ TagReaderReply* TagReaderClient::ReadFile(const QString& filename) { return worker_pool_->SendMessageWithReply(&message); } -//TODO: remove form based metadataeditting (./src/ui/edittagdialog.cpp) -// shall be used only for new created (e.g. ripper, podcast, ...) files -// any other editting shall be replaced by field level changes, see below TagReaderReply* TagReaderClient::SaveFile(const QString& filename, const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveFileRequest* req = message.mutable_save_file_request(); - - qLog(Debug) << "TagReaderClient::SaveFile: " << filename; - req->set_filename(DataCommaSizeFromQString(filename)); metadata.ToProtobuf(req->mutable_metadata()); @@ -94,9 +88,6 @@ TagReaderReply* TagReaderClient::UpdateSongTag(const QString& filename, pb::tagreader::SaveSongTagToFileRequest* req = message.mutable_save_song_tag_to_file_request(); - qLog(Debug) << "TagReaderClient::SaveSongTag: " << filename - << " tag: " << tagname << " value: " << tagvalue; - req->set_filename(DataCommaSizeFromQString(filename)); req->set_tagname(DataCommaSizeFromQString(tagname)); req->set_tagvalue(DataCommaSizeFromQString(tagvalue.toString())); diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 44d87208e4..8ec2f41809 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -46,8 +46,7 @@ class TagReaderClient : public QObject { ReplyType* ReadFile(const QString& filename); ReplyType* SaveFile(const QString& filename, const Song& metadata); - ReplyType* UpdateSongTag(const QString& filename, - const QString& tagname, + ReplyType* UpdateSongTag(const QString& filename, const QString& tagname, const QVariant& tagvalue); ReplyType* UpdateSongStatistics(const Song& metadata); ReplyType* UpdateSongRating(const Song& metadata); From 1c590bb53401b8274b1e029482797e0dd4bbc1b4 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 13:44:27 +0100 Subject: [PATCH 10/17] Revert "Source formatting changed to satisfy Ling clang and obsolete gLog(Debug) removed" This reverts commit adc1fab9420dff37583b7b72bb30c983760cfa6c. --- src/core/tagreaderclient.h | 2 +- src/playlist/playlist.cpp | 17 ----------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 191365cade..c833ddfc4c 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -45,7 +45,7 @@ class TagReaderClient : public QObject { void Start(); ReplyType* ReadFile(const QString& filename); - ReplyType* SaveFile(const QString& filename, const Song& + ReplyType* SaveFile(const QString& filename, const Song&); ReplyType* UpdateSongTag(const QString& filename, const QString& tagname, const QVariant& tagvalue); ReplyType* UpdateSongStatistics(const Song& metadata); diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index cd258b8311..12e0eeefaf 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -477,23 +477,11 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, library_->AddOrUpdateSongs(SongList() << song); emit EditingFinished(index); } else { -<<<<<<< HEAD const QString name = Playlist::kColumns.at(index.column()); // Update just that tag TagReaderReply* reply = TagReaderClient::Instance()->UpdateSongTag( song.url().toLocalFile(), name, value); -======= - qLog(Debug) << "Playlist::setData file: " << song.url().toLocalFile(); - const QString name = Playlist::kColumns.at(index.column()); - qLog(Debug) << "Playlist::setData name: " << name; - qLog(Debug) << "Playlist::setData value: " << value; - - // Update just that tag - TagReaderReply* reply = - TagReaderClient::Instance()->UpdateSongTag(song.url().toLocalFile(), - name, value); ->>>>>>> 61799b5a312e6f1910d72f6462442ec031930cc2 NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)), @@ -512,14 +500,9 @@ void Playlist::SongSaveComplete(TagReaderReply* reply, } else { emit Error( tr("An error occurred writing metadata to '%1'") -<<<<<<< HEAD .arg(QString::fromStdString(reply->request_message() .save_song_tag_to_file_request() .filename()))); -======= - .arg(QString::fromStdString( - reply->request_message().save_song_tag_to_file_request().filename()))); ->>>>>>> 61799b5a312e6f1910d72f6462442ec031930cc2 } } reply->deleteLater(); From 9f94b7e67804d2b8e322a3034c66700bd17a27e2 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 13:51:10 +0100 Subject: [PATCH 11/17] Furher qLog(Debug) and comment lines removed --- src/core/tagreaderclient.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index f00902b4bc..f5e936dbbd 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -83,10 +83,6 @@ TagReaderReply* TagReaderClient::SaveFile(const QString& filename, return worker_pool_->SendMessageWithReply(&message); } - -// Single field update, called by ./src/playlist/playlist.cpp -// Mayor change necessary, aka any "save..file" members be replaced by -// save (file, tagname, tagvalue) without sending whole song/metadata TagReaderReply* TagReaderClient::UpdateSongTag(const QString& filename, const QString& tagname, const QVariant& tagvalue) { @@ -94,9 +90,6 @@ TagReaderReply* TagReaderClient::UpdateSongTag(const QString& filename, pb::tagreader::SaveSongTagToFileRequest* req = message.mutable_save_song_tag_to_file_request(); - qLog(Debug) << "TagReaderClient::SaveSongTag: " << filename - << " tag: " << tagname << " value: " << tagvalue; - req->set_filename(DataCommaSizeFromQString(filename)); req->set_tagname(DataCommaSizeFromQString(tagname)); req->set_tagvalue(DataCommaSizeFromQString(tagvalue.toString())); From 0d5c3516d54009cd96842d7bea7176fc0590c6d4 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 16:16:39 +0100 Subject: [PATCH 12/17] Another fault introduced by lint cleanup attempts --- src/core/tagreaderclient.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index c833ddfc4c..8ec2f41809 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -45,7 +45,7 @@ class TagReaderClient : public QObject { void Start(); ReplyType* ReadFile(const QString& filename); - ReplyType* SaveFile(const QString& filename, const Song&); + ReplyType* SaveFile(const QString& filename, const Song& metadata); ReplyType* UpdateSongTag(const QString& filename, const QString& tagname, const QVariant& tagvalue); ReplyType* UpdateSongStatistics(const Song& metadata); From f0c6f8ebab819861bc610fa539a00e8905213405 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 17:46:58 +0100 Subject: [PATCH 13/17] Enhanced tageditting using playlist view including fix to get BPM from mp4 tag 'tmpo'. Main change: Instead of posting whole song properties to external tagreader only single tag (aka the one just editted) is posted. That required a new tagreader interface. As a side effect the tag translation which differs depending on audio file type (mp3, aac, flac, ...) is delegated to propertymap in 3rdparty tagreader. On the other hand tagname are made available to playlist class (matching the columns enumeration) which should provide an easier way to add additional filepersistent tags. --- ext/clementine-tagreader/tagreaderworker.cpp | 7 + ext/libclementine-tagreader/tagreader.cpp | 201 ++++++------------ ext/libclementine-tagreader/tagreader.h | 4 + .../tagreadermessages.proto | 14 ++ src/core/tagreaderclient.cpp | 22 ++ src/core/tagreaderclient.h | 2 + src/playlist/playlist.cpp | 64 +++++- src/playlist/playlist.h | 2 + 8 files changed, 175 insertions(+), 141 deletions(-) diff --git a/ext/clementine-tagreader/tagreaderworker.cpp b/ext/clementine-tagreader/tagreaderworker.cpp index 235aa3f11b..8f69efb065 100644 --- a/ext/clementine-tagreader/tagreaderworker.cpp +++ b/ext/clementine-tagreader/tagreaderworker.cpp @@ -46,6 +46,13 @@ void TagReaderWorker::MessageArrived(const pb::tagreader::Message& message) { reply.mutable_save_file_response()->set_success(tag_reader_.SaveFile( QStringFromStdString(message.save_file_request().filename()), message.save_file_request().metadata())); + } else if (message.has_save_song_tag_to_file_request()) { + reply.mutable_save_song_tag_to_file_response()->set_success( + tag_reader_.UpdateSongTag( + QStringFromStdString(message.save_song_tag_to_file_request().filename()), + QStringFromStdString(message.save_song_tag_to_file_request().tagname()), + QStringFromStdString(message.save_song_tag_to_file_request().tagvalue()) + )); } else if (message.has_save_song_statistics_to_file_request()) { reply.mutable_save_song_statistics_to_file_response()->set_success( tag_reader_.SaveSongStatisticsToFile( diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index 45704408c7..ab12077002 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -51,6 +51,10 @@ #include #include #include + +#include +#include + #include #include #include @@ -306,8 +310,7 @@ void TagReader::ReadFile(const QString& filename, } if (items.contains("BPM")) { - Decode(items["BPM"].toStringList().toString(", "), nullptr, - song->mutable_performer()); + song->set_bpm(TStringToQString(items["BPM"].toString()).trimmed().toFloat()); } if (items.contains("PERFORMER")) { @@ -502,7 +505,12 @@ void TagReader::ReadFile(const QString& filename, song->set_score(score); } } - + if (items.contains("tmpo")) { + float bpm = (float)items["tmpo"].toInt(); + if (song->bpm() <= 0 && bpm > 0) { + song->set_bpm(bpm); + } + } if (items.contains("\251wrt")) { Decode(items["\251wrt"].toStringList().toString(", "), nullptr, song->mutable_composer()); @@ -772,42 +780,6 @@ void TagReader::ParseOggTag(const TagLib::Ogg::FieldListMap& map, Decode(map["UNSYNCEDLYRICS"].front(), codec, song->mutable_lyrics()); } -void TagReader::SetVorbisComments( - TagLib::Ogg::XiphComment* vorbis_comments, - const pb::tagreader::SongMetadata& song) const { - vorbis_comments->addField("COMPOSER", - StdStringToTaglibString(song.composer()), true); - vorbis_comments->addField("PERFORMER", - StdStringToTaglibString(song.performer()), true); - vorbis_comments->addField("CONTENT GROUP", - StdStringToTaglibString(song.grouping()), true); - vorbis_comments->addField( - "BPM", - QStringToTaglibString(song.bpm() <= 0 - 1 ? QString() - : QString::number(song.bpm())), - true); - vorbis_comments->addField( - "DISCNUMBER", - QStringToTaglibString(song.disc() <= 0 ? QString() - : QString::number(song.disc())), - true); - vorbis_comments->addField( - "COMPILATION", - QStringToTaglibString(song.compilation() ? QString::number(1) - : QString()), - true); - - // Try to be coherent, the two forms are used but the first one is preferred - - vorbis_comments->addField("ALBUMARTIST", - StdStringToTaglibString(song.albumartist()), true); - vorbis_comments->removeField("ALBUM ARTIST"); - - vorbis_comments->addField("LYRICS", StdStringToTaglibString(song.lyrics()), - true); - vorbis_comments->removeField("UNSYNCEDLYRICS"); -} - void TagReader::SetFMPSStatisticsVorbisComments( TagLib::Ogg::XiphComment* vorbis_comments, const pb::tagreader::SongMetadata& song) const { @@ -869,111 +841,72 @@ pb::tagreader::SongMetadata_Type TagReader::GuessFileType( } bool TagReader::SaveFile(const QString& filename, - const pb::tagreader::SongMetadata& song) const { + const pb::tagreader::SongMetadata& song) const { if (filename.isNull()) return false; - qLog(Debug) << "Saving tags to" << filename; - std::unique_ptr fileref(factory_->GetFileRef(filename)); - if (!fileref || fileref->isNull()) // The file probably doesn't exist return false; - fileref->tag()->setTitle(StdStringToTaglibString(song.title())); - fileref->tag()->setArtist(StdStringToTaglibString(song.artist())); // TPE1 - fileref->tag()->setAlbum(StdStringToTaglibString(song.album())); - fileref->tag()->setGenre(StdStringToTaglibString(song.genre())); - fileref->tag()->setComment(StdStringToTaglibString(song.comment())); - fileref->tag()->setYear(song.year() <= 0 - 1 ? 0 : song.year()); - fileref->tag()->setTrack(song.track() <= 0 - 1 ? 0 : song.track()); - - auto saveApeTag = [&](TagLib::APE::Tag* tag) { - tag->addValue( - "disc", - QStringToTaglibString(song.disc() <= 0 ? QString() - : QString::number(song.disc())), - true); - tag->addValue("bpm", - QStringToTaglibString(song.bpm() <= 0 - 1 - ? QString() - : QString::number(song.bpm())), - true); - tag->setItem("composer", - TagLib::APE::Item( - "composer", TagLib::StringList(song.composer().c_str()))); - tag->setItem("grouping", - TagLib::APE::Item( - "grouping", TagLib::StringList(song.grouping().c_str()))); - tag->setItem("performer", - TagLib::APE::Item("performer", TagLib::StringList( - song.performer().c_str()))); - tag->setItem( - "album artist", - TagLib::APE::Item("album artist", - TagLib::StringList(song.albumartist().c_str()))); - tag->setItem("lyrics", - TagLib::APE::Item("lyrics", TagLib::String(song.lyrics()))); - tag->addValue("compilation", - QStringToTaglibString(song.compilation() ? QString::number(1) - : QString()), - true); - }; + // only basic tags as provided by ripper + TagLib::Tag *t = fileref->tag(); + t->setTitle(StdStringToTaglibString(song.title())); + t->setArtist(StdStringToTaglibString(song.artist())); + t->setAlbum(StdStringToTaglibString(song.album())); + t->setGenre(StdStringToTaglibString(song.genre())); + t->setComment(StdStringToTaglibString(song.comment())); + t->setYear(song.year() <= 0 - 1 ? 0 : song.year()); + t->setTrack(song.track() <= 0 - 1 ? 0 : song.track()); + // if more tags are required propertymap() shall be used - if (TagLib::MPEG::File* file = - dynamic_cast(fileref->file())) { - TagLib::ID3v2::Tag* tag = file->ID3v2Tag(true); - SetTextFrame("TPOS", - song.disc() <= 0 ? QString() : QString::number(song.disc()), - tag); - SetTextFrame("TBPM", - song.bpm() <= 0 - 1 ? QString() : QString::number(song.bpm()), - tag); - SetTextFrame("TCOM", song.composer(), tag); - SetTextFrame("TIT1", song.grouping(), tag); - SetTextFrame("TOPE", song.performer(), tag); - SetUnsyncLyricsFrame(song.lyrics(), tag); - // Skip TPE1 (which is the artist) here because we already set it - SetTextFrame("TPE2", song.albumartist(), tag); - SetTextFrame("TCMP", song.compilation() ? QString::number(1) : QString(), - tag); - } else if (TagLib::FLAC::File* file = - dynamic_cast(fileref->file())) { - TagLib::Ogg::XiphComment* tag = file->xiphComment(); - SetVorbisComments(tag, song); - } else if (TagLib::MP4::File* file = - dynamic_cast(fileref->file())) { - TagLib::MP4::Tag* tag = file->tag(); - tag->itemListMap()["disk"] = - TagLib::MP4::Item(song.disc() <= 0 - 1 ? 0 : song.disc(), 0); - tag->itemListMap()["tmpo"] = TagLib::StringList( - song.bpm() <= 0 - 1 ? "0" : TagLib::String::number(song.bpm())); - tag->itemListMap()["\251wrt"] = TagLib::StringList(song.composer().c_str()); - tag->itemListMap()["\251grp"] = TagLib::StringList(song.grouping().c_str()); - tag->itemListMap()["\251lyr"] = TagLib::StringList(song.lyrics().c_str()); - tag->itemListMap()["aART"] = TagLib::StringList(song.albumartist().c_str()); - tag->itemListMap()["cpil"] = - TagLib::StringList(song.compilation() ? "1" : "0"); - } else if (TagLib::APE::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } else if (TagLib::MPC::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } else if (TagLib::WavPack::File* file = - dynamic_cast(fileref->file())) { - saveApeTag(file->APETag(true)); - } + bool ret = fileref->save(); - // Handle all the files which have VorbisComments (Ogg, OPUS, ...) in the same - // way; - // apart, so we keep specific behavior for some formats by adding another - // "else if" block above. - if (TagLib::Ogg::XiphComment* tag = - dynamic_cast(fileref->file()->tag())) { - SetVorbisComments(tag, song); +#ifdef Q_OS_LINUX + if (ret) { + // Linux: inotify doesn't seem to notice the change to the file unless we + // change the timestamps as well. (this is what touch does) + utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } +#endif // Q_OS_LINUX + + return ret; +} + +// Replacing SaveFile for single tag editting (e.g. in playlist) +// as full store operation (especially when file was mp4) failed +// This code performs single tag insert/update/delete and +// relies on taglib's PropertyMap to handle file variants. +bool TagReader::UpdateSongTag(const QString& filename, + const QString& tagname, + const QString& tagvalue) const { + + //qLog(Debug) << "UpdateSongTag of " << filename << " tag " << tagname << " value " << tagvalue; + + if (filename.isNull()) return false; + + std::unique_ptr fileref(factory_->GetFileRef(filename)); + if (!fileref || fileref->isNull()) // The file probably doesn't exist + return false; + + + // according taglib tagwriter example + // to save basic attributes see SaveFile + + TagLib::PropertyMap map = fileref->file()->properties(); + + TagLib::String key = QStringToTaglibString(tagname); + if (tagvalue.isEmpty()) + map.erase(key); + else + // inserts too if not yet + map.replace(key, QStringToTaglibString(tagvalue)); + + //qLog(Debug) << TStringToQString(map.toString()); + + fileref->file()->setProperties(map); bool ret = fileref->save(); + #ifdef Q_OS_LINUX if (ret) { // Linux: inotify doesn't seem to notice the change to the file unless we @@ -981,7 +914,7 @@ bool TagReader::SaveFile(const QString& filename, utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } #endif // Q_OS_LINUX - + return ret; } diff --git a/ext/libclementine-tagreader/tagreader.h b/ext/libclementine-tagreader/tagreader.h index 84a4302d91..8b9edc0257 100644 --- a/ext/libclementine-tagreader/tagreader.h +++ b/ext/libclementine-tagreader/tagreader.h @@ -62,6 +62,10 @@ class TagReader { const pb::tagreader::SongMetadata& song) const; // Returns false if something went wrong; returns true otherwise (might // returns true if the file exists but nothing has been written inside because + bool UpdateSongTag(const QString& filename, + const QString& tagname, + const QString& tagvalue) const; + // statistics tag format is not supported for this kind of file) bool SaveSongStatisticsToFile(const QString& filename, const pb::tagreader::SongMetadata& song) const; diff --git a/ext/libclementine-tagreader/tagreadermessages.proto b/ext/libclementine-tagreader/tagreadermessages.proto index 531efb7172..7bf708179b 100644 --- a/ext/libclementine-tagreader/tagreadermessages.proto +++ b/ext/libclementine-tagreader/tagreadermessages.proto @@ -106,6 +106,16 @@ message ReadCloudFileResponse { optional SongMetadata metadata = 1; } +message SaveSongTagToFileRequest { + optional string filename = 1; + optional string tagname = 2; + optional string tagvalue = 3; +} + +message SaveSongTagToFileResponse { + optional bool success = 1; +} + message SaveSongStatisticsToFileRequest { optional string filename = 1; optional SongMetadata metadata = 2; @@ -147,4 +157,8 @@ message Message { optional SaveSongRatingToFileRequest save_song_rating_to_file_request = 14; optional SaveSongRatingToFileResponse save_song_rating_to_file_response = 15; + optional SaveSongTagToFileRequest save_song_tag_to_file_request = 16; + + optional SaveSongTagToFileResponse save_song_tag_to_file_response = 17; + } diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index 521ce18249..f5e936dbbd 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -66,17 +66,37 @@ TagReaderReply* TagReaderClient::ReadFile(const QString& filename) { return worker_pool_->SendMessageWithReply(&message); } +//TODO: remove form based metadataeditting (./src/ui/edittagdialog.cpp) +// shall be used only for new created (e.g. ripper, podcast, ...) files +// any other editting shall be replaced by field level changes, see below TagReaderReply* TagReaderClient::SaveFile(const QString& filename, const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveFileRequest* req = message.mutable_save_file_request(); + + qLog(Debug) << "TagReaderClient::SaveFile: " << filename; + req->set_filename(DataCommaSizeFromQString(filename)); metadata.ToProtobuf(req->mutable_metadata()); return worker_pool_->SendMessageWithReply(&message); } +TagReaderReply* TagReaderClient::UpdateSongTag(const QString& filename, + const QString& tagname, + const QVariant& tagvalue) { + pb::tagreader::Message message; + pb::tagreader::SaveSongTagToFileRequest* req = + message.mutable_save_song_tag_to_file_request(); + + req->set_filename(DataCommaSizeFromQString(filename)); + req->set_tagname(DataCommaSizeFromQString(tagname)); + req->set_tagvalue(DataCommaSizeFromQString(tagvalue.toString())); + + return worker_pool_->SendMessageWithReply(&message); +} + TagReaderReply* TagReaderClient::UpdateSongStatistics(const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveSongStatisticsToFileRequest* req = @@ -166,6 +186,8 @@ bool TagReaderClient::SaveFileBlocking(const QString& filename, bool ret = false; + qLog(Debug) << "TagReaderClient::SaveFileBlocking: " << filename; + TagReaderReply* reply = SaveFile(filename, metadata); if (reply->WaitForFinished()) { ret = reply->message().save_file_response().success(); diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 5571f5d569..8ec2f41809 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -46,6 +46,8 @@ class TagReaderClient : public QObject { ReplyType* ReadFile(const QString& filename); ReplyType* SaveFile(const QString& filename, const Song& metadata); + ReplyType* UpdateSongTag(const QString& filename, const QString& tagname, + const QVariant& tagvalue); ReplyType* UpdateSongStatistics(const Song& metadata); ReplyType* UpdateSongRating(const Song& metadata); ReplyType* IsMediaFile(const QString& filename); diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index c4c20ec2a2..12e0eeefaf 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -98,6 +98,46 @@ const int Playlist::kUndoItemLimit = 500; const qint64 Playlist::kMinScrobblePointNsecs = 31ll * kNsecPerSec; const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; +// kColumns provides names to enum Columns. +// When the playlist column is editable those names are used as names +// for taglib writing to file. Names are caseinsensitive and often +// 3rdparty/taglib/.../id3v2frame translations is base for naming used. +// When playlist enum columns is changed the list below has to be adapted +// accordingly. Lowercase written names miss a taglib equivalent and are +// used for internal informations. +// When adding lines to column enumeration: Try to add mappable taglib +// properties. +const QStringList Playlist::kColumns = QStringList() << "TITLE" + << "ARTIST" + << "ALBUM" + << "ALBUMARTIST" + << "COMPOSER" + << "LENGTH" + << "TRACKNUMBER" + << "DISCNUMBER" + << "DATE" + << "GENRE" + << "BPM" + << "bitrate" + << "samplerate" + << "filename" + << "basefilename" + << "filesize" + << "filetype" + << "ctime" + << "mtime" + << "rating" + << "playcount" + << "skipcount" + << "lastplayed" + << "score" + << "COMMENT" + << "source" + << "MOOD" + << "performer" + << "GROUPING" + << "ORIGINALDATE"; + namespace { QString removePrefix(const QString& a, const QStringList& prefixes) { for (const QString& prefix : prefixes) { @@ -212,6 +252,7 @@ bool Playlist::column_is_editable(Playlist::Column column) { case Column_Disc: case Column_Year: case Column_Genre: + case Column_BPM: case Column_Score: case Column_Comment: return true; @@ -223,8 +264,10 @@ bool Playlist::column_is_editable(Playlist::Column column) { bool Playlist::set_column_value(Song& song, Playlist::Column column, const QVariant& value) { - if (!song.IsEditable()) return false; - + if (!song.IsEditable()) { + qLog(Debug) << "PlayList::set_column_value not editable: " << column; + return false; + } switch (column) { case Column_Title: song.set_title(value.toString()); @@ -259,6 +302,9 @@ bool Playlist::set_column_value(Song& song, Playlist::Column column, case Column_Genre: song.set_genre(value.toString()); break; + case Column_BPM: + song.set_bpm(value.toFloat()); + break; case Column_Score: song.set_score(value.toInt()); break; @@ -431,8 +477,11 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, library_->AddOrUpdateSongs(SongList() << song); emit EditingFinished(index); } else { - TagReaderReply* reply = - TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); + const QString name = Playlist::kColumns.at(index.column()); + + // Update just that tag + TagReaderReply* reply = TagReaderClient::Instance()->UpdateSongTag( + song.url().toLocalFile(), name, value); NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)), @@ -444,15 +493,16 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, void Playlist::SongSaveComplete(TagReaderReply* reply, const QPersistentModelIndex& index) { if (reply->is_successful() && index.isValid()) { - if (reply->message().save_file_response().success()) { + if (reply->message().save_song_tag_to_file_response().success()) { QFuture future = item_at(index.row())->BackgroundReload(); NewClosure(future, this, SLOT(ItemReloadComplete(QPersistentModelIndex)), index); } else { emit Error( tr("An error occurred writing metadata to '%1'") - .arg(QString::fromStdString( - reply->request_message().save_file_request().filename()))); + .arg(QString::fromStdString(reply->request_message() + .save_song_tag_to_file_request() + .filename()))); } } reply->deleteLater(); diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index 833f311cd5..61c7d189fc 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -167,6 +167,8 @@ class Playlist : public QAbstractListModel { static const qint64 kMinScrobblePointNsecs; static const qint64 kMaxScrobblePointNsecs; + static const QStringList kColumns; + static bool CompareItems(int column, Qt::SortOrder order, PlaylistItemPtr a, PlaylistItemPtr b, const QStringList& prefixes = {}); From ae34e41510ec8b1b7016a00d1614c393088b304f Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 18:42:37 +0100 Subject: [PATCH 14/17] Merge branch 'master' of github.com:HorstFiedler/Clementine --- ext/libclementine-tagreader/tagreader.cpp | 2 ++ src/core/utilities.cpp | 2 +- src/translations/sv.po | 4 ++-- src/ui/mainwindow.cpp | 9 +++++++-- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index ab12077002..9f158c3cbf 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -892,6 +892,7 @@ bool TagReader::UpdateSongTag(const QString& filename, // according taglib tagwriter example // to save basic attributes see SaveFile + // no check for available ones TagLib::PropertyMap map = fileref->file()->properties(); TagLib::String key = QStringToTaglibString(tagname); @@ -902,6 +903,7 @@ bool TagReader::UpdateSongTag(const QString& filename, // inserts too if not yet map.replace(key, QStringToTaglibString(tagvalue)); + //qLog(Debug) << TStringToQString(map.toString()); fileref->file()->setProperties(map); diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index ddbdb53d2c..340810e087 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -443,7 +443,7 @@ void OpenInFileBrowser(const QList& urls) { #elif defined(Q_OS_WIN32) ShowFileInExplorer(path); #else - QDesktopServices::openUrl(QUrl::fromLocalFile(directory)); + QDesktopServices::openUrl(QUrl::fromLocalFile(path)); #endif } } diff --git a/src/translations/sv.po b/src/translations/sv.po index 15498d3dc4..26cb9bb177 100644 --- a/src/translations/sv.po +++ b/src/translations/sv.po @@ -32,7 +32,7 @@ msgid "" msgstr "" "Project-Id-Version: Clementine Music Player\n" -"PO-Revision-Date: 2021-01-27 10:52+0000\n" +"PO-Revision-Date: 2021-01-28 11:19+0000\n" "Last-Translator: Jonatan Nyberg \n" "Language-Team: Swedish (http://www.transifex.com/davidsansome/clementine/language/sv/)\n" "Content-Type: text/plain; charset=UTF-8\n" @@ -1280,7 +1280,7 @@ msgstr "Klicka här för att lägga till musik" msgid "" "Click here to favorite this playlist so it will be saved and remain " "accessible through the \"Playlists\" panel on the left side bar" -msgstr "Klicka här för att favorisera denna spellista så att den sparas och förblir tillgänglig från \"Spellistor\" på det vänstra sidofältet" +msgstr "Klicka här för att favorisera denna spellista så att den sparas och förblir tillgänglig från panelen \"Spellistor\" på det vänstra sidofältet" #: ../bin/src/ui_trackslider.h:71 msgid "Click to toggle between remaining time and total time" diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index d4da9300d0..9407dcf7b8 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -2126,7 +2126,8 @@ void MainWindow::SelectionSetValue() { app_->playlist_manager()->current()->proxy()->mapToSource(index); int row = source_index.row(); Song song = app_->playlist_manager()->current()->item_at(row)->Metadata(); - + qLog(Debug) << "MainWindow: SelectionSetValue: song: " << row << " value: " << column_value; + if (Playlist::set_column_value(song, column, column_value)) { TagReaderReply* reply = TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); @@ -2145,6 +2146,8 @@ void MainWindow::EditValue() { // Edit the last column that was right-clicked on. If nothing's ever been // right clicked then look for the first editable column. int column = playlist_menu_index_.column(); + + qLog(Debug) << "MainWindow: EditValue: column: " << column; if (column == -1) { for (int i = 0; i < ui_->playlist->view()->model()->columnCount(); ++i) { if (ui_->playlist->view()->isColumnHidden(i)) continue; @@ -2153,7 +2156,7 @@ void MainWindow::EditValue() { break; } } - + qLog(Debug) << "MainWindow: EditValue: column: " << column; ui_->playlist->view()->edit(current.sibling(current.row(), column)); } @@ -2507,9 +2510,11 @@ void MainWindow::CopyFilesToDevice(const QList& urls) { void MainWindow::EditFileTags(const QList& urls) { SongList songs; + for (const QUrl& url : urls) { Song song; song.set_url(url); + qLog(Debug) << "MainWindow: EditFileTags: url: " << url; song.set_valid(true); song.set_filetype(Song::Type_Mpeg); songs << song; From 2799eb6a4727122849d095944b6254139b3f6da5 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 19:01:21 +0100 Subject: [PATCH 15/17] Reverted to original state, change has nothing to do with taglib --- src/core/utilities.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index 340810e087..ddbdb53d2c 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -443,7 +443,7 @@ void OpenInFileBrowser(const QList& urls) { #elif defined(Q_OS_WIN32) ShowFileInExplorer(path); #else - QDesktopServices::openUrl(QUrl::fromLocalFile(path)); + QDesktopServices::openUrl(QUrl::fromLocalFile(directory)); #endif } } From c1cecdab0ef5841c823e7ffcb310c8e91aa96770 Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Fri, 29 Jan 2021 19:11:24 +0100 Subject: [PATCH 16/17] Reverted, debug messages removed --- src/ui/mainwindow.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index 9407dcf7b8..d4da9300d0 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -2126,8 +2126,7 @@ void MainWindow::SelectionSetValue() { app_->playlist_manager()->current()->proxy()->mapToSource(index); int row = source_index.row(); Song song = app_->playlist_manager()->current()->item_at(row)->Metadata(); - qLog(Debug) << "MainWindow: SelectionSetValue: song: " << row << " value: " << column_value; - + if (Playlist::set_column_value(song, column, column_value)) { TagReaderReply* reply = TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); @@ -2146,8 +2145,6 @@ void MainWindow::EditValue() { // Edit the last column that was right-clicked on. If nothing's ever been // right clicked then look for the first editable column. int column = playlist_menu_index_.column(); - - qLog(Debug) << "MainWindow: EditValue: column: " << column; if (column == -1) { for (int i = 0; i < ui_->playlist->view()->model()->columnCount(); ++i) { if (ui_->playlist->view()->isColumnHidden(i)) continue; @@ -2156,7 +2153,7 @@ void MainWindow::EditValue() { break; } } - qLog(Debug) << "MainWindow: EditValue: column: " << column; + ui_->playlist->view()->edit(current.sibling(current.row(), column)); } @@ -2510,11 +2507,9 @@ void MainWindow::CopyFilesToDevice(const QList& urls) { void MainWindow::EditFileTags(const QList& urls) { SongList songs; - for (const QUrl& url : urls) { Song song; song.set_url(url); - qLog(Debug) << "MainWindow: EditFileTags: url: " << url; song.set_valid(true); song.set_filetype(Song::Type_Mpeg); songs << song; From 9a16f2e0162a4201ca180ed0c7efcd0a74e01eec Mon Sep 17 00:00:00 2001 From: Horst Fiedler Date: Sat, 30 Jan 2021 10:38:28 +0100 Subject: [PATCH 17/17] Lint styling --- ext/clementine-tagreader/tagreaderworker.cpp | 12 ++-- ext/libclementine-tagreader/tagreader.cpp | 40 ++++++------ ext/libclementine-tagreader/tagreader.h | 12 ++-- src/core/tagreaderclient.cpp | 10 +-- src/core/tagreaderclient.h | 2 +- src/playlist/playlist.cpp | 66 ++++++++++---------- 6 files changed, 67 insertions(+), 75 deletions(-) diff --git a/ext/clementine-tagreader/tagreaderworker.cpp b/ext/clementine-tagreader/tagreaderworker.cpp index 8f69efb065..78a1fb0afc 100644 --- a/ext/clementine-tagreader/tagreaderworker.cpp +++ b/ext/clementine-tagreader/tagreaderworker.cpp @@ -48,11 +48,13 @@ void TagReaderWorker::MessageArrived(const pb::tagreader::Message& message) { message.save_file_request().metadata())); } else if (message.has_save_song_tag_to_file_request()) { reply.mutable_save_song_tag_to_file_response()->set_success( - tag_reader_.UpdateSongTag( - QStringFromStdString(message.save_song_tag_to_file_request().filename()), - QStringFromStdString(message.save_song_tag_to_file_request().tagname()), - QStringFromStdString(message.save_song_tag_to_file_request().tagvalue()) - )); + tag_reader_.UpdateSongTag( + QStringFromStdString( + message.save_song_tag_to_file_request().filename()), + QStringFromStdString( + message.save_song_tag_to_file_request().tagname()), + QStringFromStdString( + message.save_song_tag_to_file_request().tagvalue()))); } else if (message.has_save_song_statistics_to_file_request()) { reply.mutable_save_song_statistics_to_file_response()->set_success( tag_reader_.SaveSongStatisticsToFile( diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index 9f158c3cbf..5a1240911e 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -49,12 +49,10 @@ #include #include #include +#include +#include #include #include - -#include -#include - #include #include #include @@ -310,7 +308,8 @@ void TagReader::ReadFile(const QString& filename, } if (items.contains("BPM")) { - song->set_bpm(TStringToQString(items["BPM"].toString()).trimmed().toFloat()); + song->set_bpm( + TStringToQString(items["BPM"].toString()).trimmed().toFloat()); } if (items.contains("PERFORMER")) { @@ -506,10 +505,10 @@ void TagReader::ReadFile(const QString& filename, } } if (items.contains("tmpo")) { - float bpm = (float)items["tmpo"].toInt(); - if (song->bpm() <= 0 && bpm > 0) { - song->set_bpm(bpm); - } + float bpm = (float)items["tmpo"].toInt(); + if (song->bpm() <= 0 && bpm > 0) { + song->set_bpm(bpm); + } } if (items.contains("\251wrt")) { Decode(items["\251wrt"].toStringList().toString(", "), nullptr, @@ -841,7 +840,7 @@ pb::tagreader::SongMetadata_Type TagReader::GuessFileType( } bool TagReader::SaveFile(const QString& filename, - const pb::tagreader::SongMetadata& song) const { + const pb::tagreader::SongMetadata& song) const { if (filename.isNull()) return false; std::unique_ptr fileref(factory_->GetFileRef(filename)); @@ -849,7 +848,7 @@ bool TagReader::SaveFile(const QString& filename, return false; // only basic tags as provided by ripper - TagLib::Tag *t = fileref->tag(); + TagLib::Tag* t = fileref->tag(); t->setTitle(StdStringToTaglibString(song.title())); t->setArtist(StdStringToTaglibString(song.artist())); t->setAlbum(StdStringToTaglibString(song.album())); @@ -868,7 +867,7 @@ bool TagReader::SaveFile(const QString& filename, utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } #endif // Q_OS_LINUX - + return ret; } @@ -876,11 +875,10 @@ bool TagReader::SaveFile(const QString& filename, // as full store operation (especially when file was mp4) failed // This code performs single tag insert/update/delete and // relies on taglib's PropertyMap to handle file variants. -bool TagReader::UpdateSongTag(const QString& filename, - const QString& tagname, - const QString& tagvalue) const { - - //qLog(Debug) << "UpdateSongTag of " << filename << " tag " << tagname << " value " << tagvalue; +bool TagReader::UpdateSongTag(const QString& filename, const QString& tagname, + const QString& tagvalue) const { + // qLog(Debug) << "UpdateSongTag of " << filename << " tag " << tagname << " + // value " << tagvalue; if (filename.isNull()) return false; @@ -888,10 +886,9 @@ bool TagReader::UpdateSongTag(const QString& filename, if (!fileref || fileref->isNull()) // The file probably doesn't exist return false; - // according taglib tagwriter example // to save basic attributes see SaveFile - + // no check for available ones TagLib::PropertyMap map = fileref->file()->properties(); @@ -903,8 +900,7 @@ bool TagReader::UpdateSongTag(const QString& filename, // inserts too if not yet map.replace(key, QStringToTaglibString(tagvalue)); - - //qLog(Debug) << TStringToQString(map.toString()); + // qLog(Debug) << TStringToQString(map.toString()); fileref->file()->setProperties(map); bool ret = fileref->save(); @@ -916,7 +912,7 @@ bool TagReader::UpdateSongTag(const QString& filename, utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); } #endif // Q_OS_LINUX - + return ret; } diff --git a/ext/libclementine-tagreader/tagreader.h b/ext/libclementine-tagreader/tagreader.h index 8b9edc0257..307c766645 100644 --- a/ext/libclementine-tagreader/tagreader.h +++ b/ext/libclementine-tagreader/tagreader.h @@ -18,10 +18,10 @@ #ifndef TAGREADER_H #define TAGREADER_H +#include + #include #include - -#include #include #include "config.h" @@ -62,9 +62,8 @@ class TagReader { const pb::tagreader::SongMetadata& song) const; // Returns false if something went wrong; returns true otherwise (might // returns true if the file exists but nothing has been written inside because - bool UpdateSongTag(const QString& filename, - const QString& tagname, - const QString& tagvalue) const; + bool UpdateSongTag(const QString& filename, const QString& tagname, + const QString& tagvalue) const; // statistics tag format is not supported for this kind of file) bool SaveSongStatisticsToFile(const QString& filename, @@ -101,7 +100,8 @@ class TagReader { const pb::tagreader::SongMetadata& song) const; void GuessArtistAndTitle(pb::tagreader::SongMetadata* song) const; - void GuessAlbum(const QFileInfo &info, pb::tagreader::SongMetadata* song) const; + void GuessAlbum(const QFileInfo& info, + pb::tagreader::SongMetadata* song) const; pb::tagreader::SongMetadata_Type GuessFileType( TagLib::FileRef* fileref) const; diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index f5e936dbbd..0b83b28a1c 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -66,17 +66,11 @@ TagReaderReply* TagReaderClient::ReadFile(const QString& filename) { return worker_pool_->SendMessageWithReply(&message); } -//TODO: remove form based metadataeditting (./src/ui/edittagdialog.cpp) -// shall be used only for new created (e.g. ripper, podcast, ...) files -// any other editting shall be replaced by field level changes, see below TagReaderReply* TagReaderClient::SaveFile(const QString& filename, const Song& metadata) { pb::tagreader::Message message; pb::tagreader::SaveFileRequest* req = message.mutable_save_file_request(); - - qLog(Debug) << "TagReaderClient::SaveFile: " << filename; - req->set_filename(DataCommaSizeFromQString(filename)); metadata.ToProtobuf(req->mutable_metadata()); @@ -84,8 +78,8 @@ TagReaderReply* TagReaderClient::SaveFile(const QString& filename, } TagReaderReply* TagReaderClient::UpdateSongTag(const QString& filename, - const QString& tagname, - const QVariant& tagvalue) { + const QString& tagname, + const QVariant& tagvalue) { pb::tagreader::Message message; pb::tagreader::SaveSongTagToFileRequest* req = message.mutable_save_song_tag_to_file_request(); diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 8ec2f41809..d88095e1c5 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -47,7 +47,7 @@ class TagReaderClient : public QObject { ReplyType* ReadFile(const QString& filename); ReplyType* SaveFile(const QString& filename, const Song& metadata); ReplyType* UpdateSongTag(const QString& filename, const QString& tagname, - const QVariant& tagvalue); + const QVariant& tagvalue); ReplyType* UpdateSongStatistics(const Song& metadata); ReplyType* UpdateSongRating(const Song& metadata); ReplyType* IsMediaFile(const QString& filename); diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 12e0eeefaf..537a8fc2a4 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -108,35 +108,35 @@ const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; // When adding lines to column enumeration: Try to add mappable taglib // properties. const QStringList Playlist::kColumns = QStringList() << "TITLE" - << "ARTIST" - << "ALBUM" - << "ALBUMARTIST" - << "COMPOSER" - << "LENGTH" - << "TRACKNUMBER" - << "DISCNUMBER" - << "DATE" - << "GENRE" - << "BPM" - << "bitrate" - << "samplerate" - << "filename" - << "basefilename" - << "filesize" - << "filetype" - << "ctime" - << "mtime" - << "rating" - << "playcount" - << "skipcount" - << "lastplayed" - << "score" - << "COMMENT" - << "source" - << "MOOD" - << "performer" - << "GROUPING" - << "ORIGINALDATE"; + << "ARTIST" + << "ALBUM" + << "ALBUMARTIST" + << "COMPOSER" + << "LENGTH" + << "TRACKNUMBER" + << "DISCNUMBER" + << "DATE" + << "GENRE" + << "BPM" + << "bitrate" + << "samplerate" + << "filename" + << "basefilename" + << "filesize" + << "filetype" + << "ctime" + << "mtime" + << "rating" + << "playcount" + << "skipcount" + << "lastplayed" + << "score" + << "COMMENT" + << "source" + << "MOOD" + << "performer" + << "GROUPING" + << "ORIGINALDATE"; namespace { QString removePrefix(const QString& a, const QStringList& prefixes) { @@ -478,10 +478,10 @@ bool Playlist::setData(const QModelIndex& index, const QVariant& value, emit EditingFinished(index); } else { const QString name = Playlist::kColumns.at(index.column()); - + // Update just that tag TagReaderReply* reply = TagReaderClient::Instance()->UpdateSongTag( - song.url().toLocalFile(), name, value); + song.url().toLocalFile(), name, value); NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)), @@ -501,8 +501,8 @@ void Playlist::SongSaveComplete(TagReaderReply* reply, emit Error( tr("An error occurred writing metadata to '%1'") .arg(QString::fromStdString(reply->request_message() - .save_song_tag_to_file_request() - .filename()))); + .save_song_tag_to_file_request() + .filename()))); } } reply->deleteLater();