Save embedded cover in the same process as tags

Possible fix for #1158

ext/libstrawberry-tagreader/CMakeLists.txt

@@ -46,6 +46,7 @@ target_link_libraries(libstrawberry-tagreader PRIVATE
   ${Protobuf_LIBRARIES}
   ${QtCore_LIBRARIES}
   ${QtNetwork_LIBRARIES}
+  ${QtGui_LIBRARIES}
   libstrawberry-common
 )
 

ext/libstrawberry-tagreader/tagreaderbase.cpp

@@ -19,6 +19,15 @@
 
 #include <string>
 
+#include <QByteArray>
+#include <QString>
+#include <QIODevice>
+#include <QFile>
+#include <QBuffer>
+#include <QImage>
+#include <QMimeDatabase>
+
+#include "core/logging.h"
 #include "tagreaderbase.h"
 
 const std::string TagReaderBase::kEmbeddedCover = "(embedded)";
@@ -28,7 +37,8 @@ TagReaderBase::~TagReaderBase() = default;
 
 void TagReaderBase::Decode(const QString &tag, std::string *output) {
 
-  output->assign(DataCommaSizeFromQString(tag));
+  const QByteArray data = tag.toUtf8();
+  output->assign(data.constData(), data.size());
 
 }
 
@@ -55,3 +65,86 @@ int TagReaderBase::ConvertToPOPMRating(const float rating) {
   return 0xFF;
 
 }
+
+QByteArray TagReaderBase::LoadCoverDataFromRequest(const spb::tagreader::SaveFileRequest &request) {
+
+  if (!request.has_save_cover() || !request.save_cover()) {
+    return QByteArray();
+  }
+
+  const QString song_filename = QString::fromUtf8(request.filename().data(), request.filename().size());
+  QString cover_filename;
+  if (request.has_cover_filename()) {
+    cover_filename = QString::fromUtf8(request.cover_filename().data(), request.cover_filename().size());
+  }
+  QByteArray cover_data;
+  if (request.has_cover_data()) {
+    cover_data = QByteArray(request.cover_data().data(), request.cover_data().size());
+  }
+  bool cover_is_jpeg = false;
+  if (request.has_cover_is_jpeg()) {
+    cover_is_jpeg = request.cover_is_jpeg();
+  }
+
+  return LoadCoverDataFromRequest(song_filename, cover_filename, cover_data, cover_is_jpeg);
+
+}
+
+QByteArray TagReaderBase::LoadCoverDataFromRequest(const spb::tagreader::SaveEmbeddedArtRequest &request) {
+
+  const QString song_filename = QString::fromUtf8(request.filename().data(), request.filename().size());
+  QString cover_filename;
+  if (request.has_cover_filename()) {
+    cover_filename = QString::fromUtf8(request.cover_filename().data(), request.cover_filename().size());
+  }
+  QByteArray cover_data;
+  if (request.has_cover_data()) {
+    cover_data = QByteArray(request.cover_data().data(), request.cover_data().size());
+  }
+  bool cover_is_jpeg = false;
+  if (request.has_cover_is_jpeg()) {
+    cover_is_jpeg = request.cover_is_jpeg();
+  }
+
+  return LoadCoverDataFromRequest(song_filename, cover_filename, cover_data, cover_is_jpeg);
+
+}
+
+QByteArray TagReaderBase::LoadCoverDataFromRequest(const QString &song_filename, const QString &cover_filename, QByteArray cover_data, const bool cover_is_jpeg) {
+
+  if (!cover_data.isEmpty() && cover_is_jpeg) {
+    qLog(Debug) << "Using cover from JPEG data for" << song_filename;
+    return cover_data;
+  }
+
+  if (cover_data.isEmpty() && !cover_filename.isEmpty()) {
+    qLog(Debug) << "Loading cover from" << cover_filename << "for" << song_filename;
+    QFile file(cover_filename);
+    if (!file.open(QIODevice::ReadOnly)) {
+      qLog(Error) << "Failed to open file" << cover_filename << "for reading:" << file.errorString();
+      return QByteArray();
+    }
+    cover_data = file.readAll();
+    file.close();
+  }
+
+  if (!cover_data.isEmpty()) {
+    if (QMimeDatabase().mimeTypeForData(cover_data).name() == "image/jpeg") {
+      qLog(Debug) << "Using cover from JPEG data for" << song_filename;
+      return cover_data;
+    }
+    // Convert image to JPEG.
+    qLog(Debug) << "Converting cover to JPEG data for" << song_filename;
+    QImage cover_image(cover_data);
+    cover_data.clear();
+    QBuffer buffer(&cover_data);
+    if (buffer.open(QIODevice::WriteOnly)) {
+      cover_image.save(&buffer, "JPEG");
+      buffer.close();
+    }
+    return cover_data;
+  }
+
+  return QByteArray();
+
+}

ext/libstrawberry-tagreader/tagreaderbase.h

@@ -27,9 +27,6 @@
 
 #include "tagreadermessages.pb.h"
 
-#define QStringFromStdString(x) QString::fromUtf8((x).data(), (x).size())
-#define DataCommaSizeFromQString(x) (x).toUtf8().constData(), (x).toUtf8().length()
-
 /*
  * This class holds all useful methods to read and write tags from/to files.
  * You should not use it directly in the main process but rather use a TagReaderWorker process (using TagReaderClient)
@@ -42,10 +39,10 @@ class TagReaderBase {
   virtual bool IsMediaFile(const QString &filename) const = 0;
 
   virtual bool ReadFile(const QString &filename, spb::tagreader::SongMetadata *song) const = 0;
-  virtual bool SaveFile(const QString &filename, const spb::tagreader::SongMetadata &song) const = 0;
+  virtual bool SaveFile(const spb::tagreader::SaveFileRequest &request) const = 0;
 
   virtual QByteArray LoadEmbeddedArt(const QString &filename) const = 0;
-  virtual bool SaveEmbeddedArt(const QString &filename, const QByteArray &data) = 0;
+  virtual bool SaveEmbeddedArt(const spb::tagreader::SaveEmbeddedArtRequest &request) const = 0;
 
   virtual bool SaveSongPlaycountToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const = 0;
   virtual bool SaveSongRatingToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const = 0;
@@ -55,6 +52,12 @@ class TagReaderBase {
   static float ConvertPOPMRating(const int POPM_rating);
   static int ConvertToPOPMRating(const float rating);
 
+  static QByteArray LoadCoverDataFromRequest(const spb::tagreader::SaveFileRequest &request);
+  static QByteArray LoadCoverDataFromRequest(const spb::tagreader::SaveEmbeddedArtRequest &request);
+
+ private:
+  static QByteArray LoadCoverDataFromRequest(const QString &song_filename, const QString &cover_filename, QByteArray cover_data, const bool cover_is_jpeg);
+
  protected:
   static const std::string kEmbeddedCover;
 

ext/libstrawberry-tagreader/tagreadergme.cpp

@@ -278,7 +278,7 @@ bool TagReaderGME::ReadFile(const QString &filename, spb::tagreader::SongMetadat
   return GME::ReadFile(fileinfo, song);
 }
 
-bool TagReaderGME::SaveFile(const QString&, const spb::tagreader::SongMetadata&) const {
+bool TagReaderGME::SaveFile(const spb::tagreader::SaveFileRequest&) const {
   return false;
 }
 
@@ -286,7 +286,7 @@ QByteArray TagReaderGME::LoadEmbeddedArt(const QString&) const {
   return QByteArray();
 }
 
-bool TagReaderGME::SaveEmbeddedArt(const QString&, const QByteArray&) {
+bool TagReaderGME::SaveEmbeddedArt(const spb::tagreader::SaveEmbeddedArtRequest&) const {
   return false;
 }
 

ext/libstrawberry-tagreader/tagreadergme.h

@@ -107,10 +107,10 @@ class TagReaderGME : public TagReaderBase {
   bool IsMediaFile(const QString &filename) const override;
 
   bool ReadFile(const QString &filename, spb::tagreader::SongMetadata *song) const override;
-  bool SaveFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
+  bool SaveFile(const spb::tagreader::SaveFileRequest &request) const override;
 
   QByteArray LoadEmbeddedArt(const QString &filename) const override;
-  bool SaveEmbeddedArt(const QString &filename, const QByteArray &data) override;
+  bool SaveEmbeddedArt(const spb::tagreader::SaveEmbeddedArtRequest &request) const override;
 
   bool SaveSongPlaycountToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
   bool SaveSongRatingToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;

ext/libstrawberry-tagreader/tagreadermessages.proto

@@ -77,6 +77,14 @@ message SongMetadata {
 
 }
 
+message IsMediaFileRequest {
+  optional string filename = 1;
+}
+
+message IsMediaFileResponse {
+  optional bool success = 1;
+}
+
 message ReadFileRequest {
   optional string filename = 1;
 }
@@ -87,21 +95,20 @@ message ReadFileResponse {
 
 message SaveFileRequest {
   optional string filename = 1;
-  optional SongMetadata metadata = 2;
+  optional bool save_tags = 2;
+  optional bool save_playcount = 3;
+  optional bool save_rating = 4;
+  optional bool save_cover = 5;
+  optional SongMetadata metadata = 6;
+  optional string cover_filename = 7;
+  optional bytes cover_data = 8;
+  optional bool cover_is_jpeg = 9;
 }
 
 message SaveFileResponse {
   optional bool success = 1;
 }
 
-message IsMediaFileRequest {
-  optional string filename = 1;
-}
-
-message IsMediaFileResponse {
-  optional bool success = 1;
-}
-
 message LoadEmbeddedArtRequest {
   optional string filename = 1;
 }
@@ -112,7 +119,9 @@ message LoadEmbeddedArtResponse {
 
 message SaveEmbeddedArtRequest {
   optional string filename = 1;
-  optional bytes data = 2;
+  optional string cover_filename = 2;
+  optional bytes cover_data = 3;
+  optional bool cover_is_jpeg = 4;
 }
 
 message SaveEmbeddedArtResponse {

ext/libstrawberry-tagreader/tagreadertaglib.cpp

@@ -81,11 +81,12 @@
 #endif
 
 #include <QtGlobal>
-#include <QFile>
-#include <QFileInfo>
 #include <QVector>
 #include <QByteArray>
 #include <QString>
+#include <QStringList>
+#include <QFile>
+#include <QFileInfo>
 #include <QUrl>
 #include <QDateTime>
 #include <QtDebug>
@@ -193,7 +194,8 @@ bool TagReaderTagLib::ReadFile(const QString &filename, spb::tagreader::SongMeta
 
   qLog(Debug) << "Reading tags from" << filename;
 
-  song->set_basefilename(DataCommaSizeFromQString(fileinfo.fileName()));
+  const QByteArray basefilename = fileinfo.fileName().toUtf8();
+  song->set_basefilename(basefilename.constData(), basefilename.length());
   song->set_url(url.constData(), url.size());
   song->set_filesize(fileinfo.size());
   song->set_mtime(fileinfo.lastModified().isValid() ? std::max(fileinfo.lastModified().toSecsSinceEpoch(), 0LL) : 0LL);
@@ -494,7 +496,9 @@ bool TagReaderTagLib::ReadFile(const QString &filename, spb::tagreader::SongMeta
 
   if (compilation.isEmpty()) {
     // well, it wasn't set, but if the artist is VA assume it's a compilation
-    if (QStringFromStdString(song->artist()).compare("various artists") == 0 || QStringFromStdString(song->albumartist()).compare("various artists") == 0) {
+    const QString albumartist = QString::fromUtf8(song->albumartist().data(), song->albumartist().size());
+    const QString artist = QString::fromUtf8(song->artist().data(), song->artist().size());
+    if (artist.compare("various artists") == 0 || albumartist.compare("various artists") == 0) {
       song->set_compilation(true);
     }
   }
@@ -521,8 +525,9 @@ bool TagReaderTagLib::ReadFile(const QString &filename, spb::tagreader::SongMeta
 
 void TagReaderTagLib::Decode(const TagLib::String &tag, std::string *output) {
 
-  QString tmp = TStringToQString(tag).trimmed();
-  output->assign(DataCommaSizeFromQString(tmp));
+  const QString tmp = TStringToQString(tag).trimmed();
+  const QByteArray data = tmp.toUtf8();
+  output->assign(data.constData(), data.size());
 
 }
 
@@ -624,79 +629,175 @@ void TagReaderTagLib::SetVorbisComments(TagLib::Ogg::XiphComment *vorbis_comment
 
 }
 
-bool TagReaderTagLib::SaveFile(const QString &filename, const spb::tagreader::SongMetadata &song) const {
+bool TagReaderTagLib::SaveFile(const spb::tagreader::SaveFileRequest &request) const {
 
-  if (filename.isEmpty()) return false;
+  if (request.filename().empty()) return false;
+
+  const QString filename = QString::fromUtf8(request.filename().data(), request.filename().size());
+  const spb::tagreader::SongMetadata song = request.metadata();
+  const bool save_tags = request.has_save_tags() && request.save_tags();
+  const bool save_playcount = request.has_save_playcount() && request.save_playcount();
+  const bool save_rating = request.has_save_rating() && request.save_rating();
+  const bool save_cover = request.has_save_cover() && request.save_cover();
+
+  QStringList save_tags_options;
+  if (save_tags) {
+    save_tags_options << "tags";
+  }
+  if (save_playcount) {
+    save_tags_options << "playcount";
+  }
+  if (save_rating) {
+    save_tags_options << "rating";
+  }
+  if (save_cover) {
+    save_tags_options << "embedded cover";
+  }
+
+  qLog(Debug) << "Saving" << save_tags_options.join(", ") << "to" << filename;
+
+  const QByteArray cover_data = LoadCoverDataFromRequest(request);
 
-  qLog(Debug) << "Saving tags to" << filename;
   std::unique_ptr<TagLib::FileRef> fileref(factory_->GetFileRef(filename));;
   if (!fileref || fileref->isNull()) return false;
 
-  fileref->tag()->setTitle(song.title().empty() ? TagLib::String() : StdStringToTaglibString(song.title()));
-  fileref->tag()->setArtist(song.artist().empty() ? TagLib::String() : StdStringToTaglibString(song.artist()));
-  fileref->tag()->setAlbum(song.album().empty() ? TagLib::String() : StdStringToTaglibString(song.album()));
-  fileref->tag()->setGenre(song.genre().empty() ? TagLib::String() : StdStringToTaglibString(song.genre()));
-  fileref->tag()->setComment(song.comment().empty() ? TagLib::String() : StdStringToTaglibString(song.comment()));
-  fileref->tag()->setYear(song.year() <= 0 ? 0 : song.year());
-  fileref->tag()->setTrack(song.track() <= 0 ? 0 : song.track());
-
-  bool result = false;
+  if (save_tags) {
+    fileref->tag()->setTitle(song.title().empty() ? TagLib::String() : StdStringToTaglibString(song.title()));
+    fileref->tag()->setArtist(song.artist().empty() ? TagLib::String() : StdStringToTaglibString(song.artist()));
+    fileref->tag()->setAlbum(song.album().empty() ? TagLib::String() : StdStringToTaglibString(song.album()));
+    fileref->tag()->setGenre(song.genre().empty() ? TagLib::String() : StdStringToTaglibString(song.genre()));
+    fileref->tag()->setComment(song.comment().empty() ? TagLib::String() : StdStringToTaglibString(song.comment()));
+    fileref->tag()->setYear(song.year() <= 0 ? 0 : song.year());
+    fileref->tag()->setTrack(song.track() <= 0 ? 0 : song.track());
+  }
 
+  bool is_flac = false;
   if (TagLib::FLAC::File *file_flac = dynamic_cast<TagLib::FLAC::File*>(fileref->file())) {
-    TagLib::Ogg::XiphComment *tag = file_flac->xiphComment(true);
-    if (!tag) return false;
-    SetVorbisComments(tag, song);
+    is_flac = true;
+    TagLib::Ogg::XiphComment *xiph_comment = file_flac->xiphComment(true);
+    if (!xiph_comment) return false;
+    if (save_tags) {
+      SetVorbisComments(xiph_comment, song);
+    }
+    if (save_playcount) {
+      SetPlaycount(xiph_comment, song);
+    }
+    if (save_rating) {
+      SetRating(xiph_comment, song);
+    }
+    if (save_cover) {
+      SetEmbeddedArt(file_flac, xiph_comment, cover_data);
+    }
   }
 
   else if (TagLib::WavPack::File *file_wavpack = dynamic_cast<TagLib::WavPack::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = file_wavpack->APETag(true);
     if (!tag) return false;
-    SaveAPETag(tag, song);
+    if (save_tags) {
+      SaveAPETag(tag, song);
+    }
+    if (save_playcount) {
+      SetPlaycount(tag, song);
+    }
+    if (save_rating) {
+      SetRating(tag, song);
+    }
   }
 
   else if (TagLib::APE::File *file_ape = dynamic_cast<TagLib::APE::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = file_ape->APETag(true);
     if (!tag) return false;
-    SaveAPETag(tag, song);
+    if (save_tags) {
+      SaveAPETag(tag, song);
+    }
+    if (save_playcount) {
+      SetPlaycount(tag, song);
+    }
+    if (save_rating) {
+      SetRating(tag, song);
+    }
   }
 
   else if (TagLib::MPC::File *file_mpc = dynamic_cast<TagLib::MPC::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = file_mpc->APETag(true);
     if (!tag) return false;
-    SaveAPETag(tag, song);
+    if (save_tags) {
+      SaveAPETag(tag, song);
+    }
+    if (save_playcount) {
+      SetPlaycount(tag, song);
+    }
+    if (save_rating) {
+      SetRating(tag, song);
+    }
   }
 
   else if (TagLib::MPEG::File *file_mpeg = dynamic_cast<TagLib::MPEG::File*>(fileref->file())) {
     TagLib::ID3v2::Tag *tag = file_mpeg->ID3v2Tag(true);
     if (!tag) return false;
-    SetTextFrame("TPOS", song.disc() <= 0 ? QString() : QString::number(song.disc()), tag);
-    SetTextFrame("TCOM", song.composer().empty() ? std::string() : song.composer(), tag);
-    SetTextFrame("TIT1", song.grouping().empty() ? std::string() : song.grouping(), tag);
-    SetTextFrame("TOPE", song.performer().empty() ? std::string() : song.performer(), tag);
-    // Skip TPE1 (which is the artist) here because we already set it
-    SetTextFrame("TPE2", song.albumartist().empty() ? std::string() : song.albumartist(), tag);
-    SetTextFrame("TCMP", song.compilation() ? QString::number(1) : QString(), tag);
-    SetUnsyncLyricsFrame(song.lyrics().empty() ? std::string() : song.lyrics(), tag);
+    if (save_tags) {
+      SetTextFrame("TPOS", song.disc() <= 0 ? QString() : QString::number(song.disc()), tag);
+      SetTextFrame("TCOM", song.composer().empty() ? std::string() : song.composer(), tag);
+      SetTextFrame("TIT1", song.grouping().empty() ? std::string() : song.grouping(), tag);
+      SetTextFrame("TOPE", song.performer().empty() ? std::string() : song.performer(), tag);
+      // Skip TPE1 (which is the artist) here because we already set it
+      SetTextFrame("TPE2", song.albumartist().empty() ? std::string() : song.albumartist(), tag);
+      SetTextFrame("TCMP", song.compilation() ? QString::number(1) : QString(), tag);
+      SetUnsyncLyricsFrame(song.lyrics().empty() ? std::string() : song.lyrics(), tag);
+    }
+    if (save_playcount) {
+      SetPlaycount(tag, song);
+    }
+    if (save_rating) {
+      SetRating(tag, song);
+    }
+    if (save_cover) {
+      SetEmbeddedArt(file_mpeg, tag, cover_data);
+    }
   }
 
   else if (TagLib::MP4::File *file_mp4 = dynamic_cast<TagLib::MP4::File*>(fileref->file())) {
     TagLib::MP4::Tag *tag = file_mp4->tag();
     if (!tag) return false;
-    tag->setItem("disk", TagLib::MP4::Item(song.disc() <= 0 - 1 ? 0 : song.disc(), 0));
-    tag->setItem("\251wrt", TagLib::StringList(TagLib::String(song.composer(), TagLib::String::UTF8)));
-    tag->setItem("\251grp", TagLib::StringList(TagLib::String(song.grouping(), TagLib::String::UTF8)));
-    tag->setItem("\251lyr", TagLib::StringList(TagLib::String(song.lyrics(), TagLib::String::UTF8)));
-    tag->setItem("aART", TagLib::StringList(TagLib::String(song.albumartist(), TagLib::String::UTF8)));
-    tag->setItem("cpil", TagLib::MP4::Item(song.compilation()));
+    if (save_tags) {
+      tag->setItem("disk", TagLib::MP4::Item(song.disc() <= 0 - 1 ? 0 : song.disc(), 0));
+      tag->setItem("\251wrt", TagLib::StringList(TagLib::String(song.composer(), TagLib::String::UTF8)));
+      tag->setItem("\251grp", TagLib::StringList(TagLib::String(song.grouping(), TagLib::String::UTF8)));
+      tag->setItem("\251lyr", TagLib::StringList(TagLib::String(song.lyrics(), TagLib::String::UTF8)));
+      tag->setItem("aART", TagLib::StringList(TagLib::String(song.albumartist(), TagLib::String::UTF8)));
+      tag->setItem("cpil", TagLib::MP4::Item(song.compilation()));
+    }
+    if (save_playcount) {
+      SetPlaycount(tag, song);
+    }
+    if (save_rating) {
+      SetRating(tag, song);
+    }
+    if (save_cover) {
+      SetEmbeddedArt(file_mp4, tag, cover_data);
+    }
   }
 
   // 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 *xiph_comment = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref->file()->tag())) {
-    SetVorbisComments(xiph_comment, song);
+  if (!is_flac) {
+    if (TagLib::Ogg::XiphComment *xiph_comment = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref->file()->tag())) {
+      if (save_tags) {
+        SetVorbisComments(xiph_comment, song);
+      }
+      if (save_playcount) {
+        SetPlaycount(xiph_comment, song);
+      }
+      if (save_rating) {
+        SetRating(xiph_comment, song);
+      }
+      if (save_cover) {
+        SetEmbeddedArt(xiph_comment, cover_data);
+      }
+    }
   }
 
-  result = fileref->save();
+  const bool result = fileref->save();
 #ifdef Q_OS_LINUX
   if (result) {
     // Linux: inotify doesn't seem to notice the change to the file unless we change the timestamps as well. (this is what touch does)
@@ -705,6 +806,7 @@ bool TagReaderTagLib::SaveFile(const QString &filename, const spb::tagreader::So
 #endif  // Q_OS_LINUX
 
   return result;
+
 }
 
 void TagReaderTagLib::SaveAPETag(TagLib::APE::Tag *tag, const spb::tagreader::SongMetadata &song) const {
@@ -939,12 +1041,84 @@ QByteArray TagReaderTagLib::LoadEmbeddedAPEArt(const TagLib::APE::ItemListMap &m
 
 }
 
-bool TagReaderTagLib::SaveEmbeddedArt(const QString &filename, const QByteArray &data) {
+void TagReaderTagLib::SetEmbeddedArt(TagLib::FLAC::File *flac_file, TagLib::Ogg::XiphComment *xiph_comment, const QByteArray &data) const {
 
-  if (filename.isEmpty()) return false;
+  (void)xiph_comment;
+
+  flac_file->removePictures();
+
+  if (!data.isEmpty()) {
+    TagLib::FLAC::Picture *picture = new TagLib::FLAC::Picture();
+    picture->setType(TagLib::FLAC::Picture::FrontCover);
+    picture->setMimeType("image/jpeg");
+    picture->setData(TagLib::ByteVector(data.constData(), data.size()));
+    flac_file->addPicture(picture);
+  }
+
+}
+
+void TagReaderTagLib::SetEmbeddedArt(TagLib::Ogg::XiphComment *xiph_comment, const QByteArray &data) const {
+
+  xiph_comment->removeAllPictures();
+
+  if (!data.isEmpty()) {
+    TagLib::FLAC::Picture *picture = new TagLib::FLAC::Picture();
+    picture->setType(TagLib::FLAC::Picture::FrontCover);
+    picture->setMimeType("image/jpeg");
+    picture->setData(TagLib::ByteVector(data.constData(), data.size()));
+    xiph_comment->addPicture(picture);
+  }
+
+}
+
+void TagReaderTagLib::SetEmbeddedArt(TagLib::MPEG::File *file_mp3, TagLib::ID3v2::Tag *tag, const QByteArray &data) const {
+
+  (void)file_mp3;
+
+  // Remove existing covers
+  TagLib::ID3v2::FrameList apiclist = tag->frameListMap()["APIC"];
+  for (TagLib::ID3v2::FrameList::ConstIterator it = apiclist.begin(); it != apiclist.end(); ++it) {
+    TagLib::ID3v2::AttachedPictureFrame *frame = dynamic_cast<TagLib::ID3v2::AttachedPictureFrame*>(*it);
+    tag->removeFrame(frame, false);
+  }
+
+  if (!data.isEmpty()) {
+    // Add new cover
+    TagLib::ID3v2::AttachedPictureFrame *frontcover = nullptr;
+    frontcover = new TagLib::ID3v2::AttachedPictureFrame("APIC");
+    frontcover->setType(TagLib::ID3v2::AttachedPictureFrame::FrontCover);
+    frontcover->setMimeType("image/jpeg");
+    frontcover->setPicture(TagLib::ByteVector(data.constData(), data.size()));
+    tag->addFrame(frontcover);
+  }
+
+}
+
+void TagReaderTagLib::SetEmbeddedArt(TagLib::MP4::File *aac_file, TagLib::MP4::Tag *tag, const QByteArray &data) const {
+
+  (void)aac_file;
+
+  TagLib::MP4::CoverArtList covers;
+  if (data.isEmpty()) {
+    if (tag->contains("covr")) tag->removeItem("covr");
+  }
+  else {
+    covers.append(TagLib::MP4::CoverArt(TagLib::MP4::CoverArt::JPEG, TagLib::ByteVector(data.constData(), data.size())));
+    tag->setItem("covr", covers);
+  }
+
+}
+
+bool TagReaderTagLib::SaveEmbeddedArt(const spb::tagreader::SaveEmbeddedArtRequest &request) const {
+
+  if (request.filename().empty()) return false;
+
+  const QString filename = QString::fromUtf8(request.filename().data(), request.filename().size());
 
   qLog(Debug) << "Saving art to" << filename;
 
+  const QByteArray cover_data = LoadCoverDataFromRequest(request);
+
 #ifdef Q_OS_WIN32
   TagLib::FileRef fileref(filename.toStdWString().c_str());
 #else
@@ -955,65 +1129,28 @@ bool TagReaderTagLib::SaveEmbeddedArt(const QString &filename, const QByteArray
 
   // FLAC
   if (TagLib::FLAC::File *flac_file = dynamic_cast<TagLib::FLAC::File*>(fileref.file())) {
-    TagLib::Ogg::XiphComment *vorbis_comments = flac_file->xiphComment(true);
-    if (!vorbis_comments) return false;
-    flac_file->removePictures();
-    if (!data.isEmpty()) {
-      TagLib::FLAC::Picture *picture = new TagLib::FLAC::Picture();
-      picture->setType(TagLib::FLAC::Picture::FrontCover);
-      picture->setMimeType("image/jpeg");
-      picture->setData(TagLib::ByteVector(data.constData(), data.size()));
-      flac_file->addPicture(picture);
-    }
+    TagLib::Ogg::XiphComment *xiph_comment = flac_file->xiphComment(true);
+    if (!xiph_comment) return false;
+    SetEmbeddedArt(flac_file, xiph_comment, cover_data);
   }
 
   // Ogg Vorbis / Opus / Speex
   else if (TagLib::Ogg::XiphComment *xiph_comment = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref.file()->tag())) {
-    xiph_comment->removeAllPictures();
-    if (!data.isEmpty()) {
-      TagLib::FLAC::Picture *picture = new TagLib::FLAC::Picture();
-      picture->setType(TagLib::FLAC::Picture::FrontCover);
-      picture->setMimeType("image/jpeg");
-      picture->setData(TagLib::ByteVector(data.constData(), data.size()));
-      xiph_comment->addPicture(picture);
-    }
+    SetEmbeddedArt(xiph_comment, cover_data);
   }
 
   // MP3
   else if (TagLib::MPEG::File *file_mp3 = dynamic_cast<TagLib::MPEG::File*>(fileref.file())) {
     TagLib::ID3v2::Tag *tag = file_mp3->ID3v2Tag();
     if (!tag) return false;
-
-    // Remove existing covers
-    TagLib::ID3v2::FrameList apiclist = tag->frameListMap()["APIC"];
-    for (TagLib::ID3v2::FrameList::ConstIterator it = apiclist.begin(); it != apiclist.end(); ++it) {
-      TagLib::ID3v2::AttachedPictureFrame *frame = dynamic_cast<TagLib::ID3v2::AttachedPictureFrame*>(*it);
-      tag->removeFrame(frame, false);
-    }
-
-    if (!data.isEmpty()) {
-      // Add new cover
-      TagLib::ID3v2::AttachedPictureFrame *frontcover = nullptr;
-      frontcover = new TagLib::ID3v2::AttachedPictureFrame("APIC");
-      frontcover->setType(TagLib::ID3v2::AttachedPictureFrame::FrontCover);
-      frontcover->setMimeType("image/jpeg");
-      frontcover->setPicture(TagLib::ByteVector(data.constData(), data.size()));
-      tag->addFrame(frontcover);
-    }
+    SetEmbeddedArt(file_mp3, tag, cover_data);
   }
 
   // MP4/AAC
   else if (TagLib::MP4::File *aac_file = dynamic_cast<TagLib::MP4::File*>(fileref.file())) {
     TagLib::MP4::Tag *tag = aac_file->tag();
     if (!tag) return false;
-    TagLib::MP4::CoverArtList covers;
-    if (data.isEmpty()) {
-      if (tag->contains("covr")) tag->removeItem("covr");
-    }
-    else {
-      covers.append(TagLib::MP4::CoverArt(TagLib::MP4::CoverArt::JPEG, TagLib::ByteVector(data.constData(), data.size())));
-      tag->setItem("covr", covers);
-    }
+    SetEmbeddedArt(aac_file, tag, cover_data);
   }
 
   // Not supported.
@@ -1041,6 +1178,60 @@ TagLib::ID3v2::PopularimeterFrame *TagReaderTagLib::GetPOPMFrameFromTag(TagLib::
 
 }
 
+void TagReaderTagLib::SetPlaycount(TagLib::Ogg::XiphComment *xiph_comment, const spb::tagreader::SongMetadata &song) const {
+
+  if (song.playcount() > 0) {
+    xiph_comment->addField("FMPS_PLAYCOUNT", TagLib::String::number(static_cast<int>(song.playcount())), true);
+  }
+  else {
+    xiph_comment->removeFields("FMPS_PLAYCOUNT");
+  }
+
+}
+
+void TagReaderTagLib::SetPlaycount(TagLib::APE::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  if (song.playcount() > 0) {
+    tag->setItem("FMPS_Playcount", TagLib::APE::Item("FMPS_Playcount", TagLib::String::number(static_cast<int>(song.playcount()))));
+  }
+  else {
+    tag->removeItem("FMPS_Playcount");
+  }
+
+}
+
+void TagReaderTagLib::SetPlaycount(TagLib::ID3v2::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  SetUserTextFrame("FMPS_Playcount", QString::number(song.playcount()), tag);
+  TagLib::ID3v2::PopularimeterFrame *frame = GetPOPMFrameFromTag(tag);
+  if (frame) {
+    frame->setCounter(song.playcount());
+  }
+
+}
+
+void TagReaderTagLib::SetPlaycount(TagLib::MP4::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  if (song.playcount() > 0) {
+    tag->setItem(kMP4_FMPS_Playcount_ID, TagLib::MP4::Item(TagLib::String::number(static_cast<int>(song.playcount()))));
+  }
+  else {
+    tag->removeItem(kMP4_FMPS_Playcount_ID);
+  }
+
+}
+
+void TagReaderTagLib::SetPlaycount(TagLib::ASF::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  if (song.playcount() > 0) {
+    tag->setAttribute("FMPS/Playcount", TagLib::ASF::Attribute(QStringToTaglibString(QString::number(song.playcount()))));
+  }
+  else {
+    tag->removeItem("FMPS/Playcount");
+  }
+
+}
+
 bool TagReaderTagLib::SaveSongPlaycountToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const {
 
   if (filename.isEmpty()) return false;
@@ -1051,62 +1242,37 @@ bool TagReaderTagLib::SaveSongPlaycountToFile(const QString &filename, const spb
   if (!fileref || fileref->isNull()) return false;
 
   if (TagLib::FLAC::File *flac_file = dynamic_cast<TagLib::FLAC::File*>(fileref->file())) {
-    TagLib::Ogg::XiphComment *vorbis_comments = flac_file->xiphComment(true);
-    if (!vorbis_comments) return false;
-    if (song.playcount() > 0) {
-      vorbis_comments->addField("FMPS_PLAYCOUNT", TagLib::String::number(static_cast<int>(song.playcount())), true);
-    }
-    else {
-      vorbis_comments->removeFields("FMPS_PLAYCOUNT");
-    }
+    TagLib::Ogg::XiphComment *xiph_comment = flac_file->xiphComment(true);
+    if (!xiph_comment) return false;
+    SetPlaycount(xiph_comment, song);
   }
   else if (TagLib::WavPack::File *wavpack_file = dynamic_cast<TagLib::WavPack::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = wavpack_file->APETag(true);
     if (!tag) return false;
-    if (song.playcount() > 0) {
-      tag->setItem("FMPS_Playcount", TagLib::APE::Item("FMPS_Playcount", TagLib::String::number(static_cast<int>(song.playcount()))));
-    }
+    SetPlaycount(tag, song);
   }
   else if (TagLib::APE::File *ape_file = dynamic_cast<TagLib::APE::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = ape_file->APETag(true);
     if (!tag) return false;
-    if (song.playcount() > 0) {
-      tag->setItem("FMPS_Playcount", TagLib::APE::Item("FMPS_Playcount", TagLib::String::number(static_cast<int>(song.playcount()))));
-    }
+    SetPlaycount(tag, song);
   }
   else if (TagLib::Ogg::XiphComment *xiph_comment = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref->file()->tag())) {
-    if (song.playcount() > 0) {
-      xiph_comment->addField("FMPS_PLAYCOUNT", TagLib::String::number(static_cast<int>(song.playcount())), true);
-    }
-    else {
-      xiph_comment->removeFields("FMPS_PLAYCOUNT");
-    }
+    SetPlaycount(xiph_comment, song);
   }
   else if (TagLib::MPEG::File *mpeg_file = dynamic_cast<TagLib::MPEG::File*>(fileref->file())) {
     TagLib::ID3v2::Tag *tag = mpeg_file->ID3v2Tag(true);
     if (!tag) return false;
-    if (song.playcount() > 0) {
-      SetUserTextFrame("FMPS_Playcount", QString::number(song.playcount()), tag);
-      TagLib::ID3v2::PopularimeterFrame *frame = GetPOPMFrameFromTag(tag);
-      if (frame) {
-        frame->setCounter(song.playcount());
-      }
-    }
-
+    SetPlaycount(tag, song);
   }
   else if (TagLib::MP4::File *mp4_file = dynamic_cast<TagLib::MP4::File*>(fileref->file())) {
     TagLib::MP4::Tag *tag = mp4_file->tag();
     if (!tag) return false;
-    if (song.playcount() > 0) {
-      tag->setItem(kMP4_FMPS_Playcount_ID, TagLib::MP4::Item(TagLib::String::number(static_cast<int>(song.playcount()))));
-    }
+    SetPlaycount(tag, song);
   }
   else if (TagLib::MPC::File *mpc_file = dynamic_cast<TagLib::MPC::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = mpc_file->APETag(true);
     if (!tag) return false;
-    if (song.playcount() > 0) {
-      tag->setItem("FMPS_Playcount", TagLib::APE::Item("FMPS_Playcount", TagLib::String::number(static_cast<int>(song.playcount()))));
-    }
+    SetPlaycount(tag, song);
   }
   else if (TagLib::ASF::File *asf_file = dynamic_cast<TagLib::ASF::File*>(fileref->file())) {
     TagLib::ASF::Tag *tag = asf_file->tag();
@@ -1130,6 +1296,50 @@ bool TagReaderTagLib::SaveSongPlaycountToFile(const QString &filename, const spb
   return ret;
 }
 
+void TagReaderTagLib::SetRating(TagLib::Ogg::XiphComment *xiph_comment, const spb::tagreader::SongMetadata &song) const {
+
+  if (song.rating() > 0.0F) {
+    xiph_comment->addField("FMPS_RATING", QStringToTaglibString(QString::number(song.rating())), true);
+  }
+  else {
+    xiph_comment->removeFields("FMPS_RATING");
+  }
+
+}
+
+void TagReaderTagLib::SetRating(TagLib::APE::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  if (song.rating() > 0.0F) {
+    tag->setItem("FMPS_Rating", TagLib::APE::Item("FMPS_Rating", TagLib::StringList(QStringToTaglibString(QString::number(song.rating())))));
+  }
+  else {
+    tag->removeItem("FMPS_Rating");
+  }
+
+}
+
+void TagReaderTagLib::SetRating(TagLib::ID3v2::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  SetUserTextFrame("FMPS_Rating", QString::number(song.rating()), tag);
+  TagLib::ID3v2::PopularimeterFrame *frame = GetPOPMFrameFromTag(tag);
+  if (frame) {
+    frame->setRating(ConvertToPOPMRating(song.rating()));
+  }
+
+}
+
+void TagReaderTagLib::SetRating(TagLib::MP4::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  tag->setItem(kMP4_FMPS_Rating_ID, TagLib::StringList(QStringToTaglibString(QString::number(song.rating()))));
+
+}
+
+void TagReaderTagLib::SetRating(TagLib::ASF::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  tag->addAttribute("FMPS/Rating", TagLib::ASF::Attribute(QStringToTaglibString(QString::number(song.rating()))));
+
+}
+
 bool TagReaderTagLib::SaveSongRatingToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const {
 
   if (filename.isNull()) return false;
@@ -1145,56 +1355,42 @@ bool TagReaderTagLib::SaveSongRatingToFile(const QString &filename, const spb::t
   if (!fileref || fileref->isNull()) return false;
 
   if (TagLib::FLAC::File *flac_file = dynamic_cast<TagLib::FLAC::File*>(fileref->file())) {
-    TagLib::Ogg::XiphComment *vorbis_comments = flac_file->xiphComment(true);
-    if (!vorbis_comments) return false;
-    if (song.rating() > 0) {
-      vorbis_comments->addField("FMPS_RATING", QStringToTaglibString(QString::number(song.rating())), true);
-    }
-    else {
-      vorbis_comments->removeFields("FMPS_RATING");
-    }
+    TagLib::Ogg::XiphComment *xiph_comment = flac_file->xiphComment(true);
+    if (!xiph_comment) return false;
+    SetRating(xiph_comment, song);
   }
   else if (TagLib::WavPack::File *wavpack_file = dynamic_cast<TagLib::WavPack::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = wavpack_file->APETag(true);
     if (!tag) return false;
-    tag->setItem("FMPS_Rating", TagLib::APE::Item("FMPS_Rating", TagLib::StringList(QStringToTaglibString(QString::number(song.rating())))));
+    SetRating(tag, song);
   }
   else if (TagLib::APE::File *ape_file = dynamic_cast<TagLib::APE::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = ape_file->APETag(true);
     if (!tag) return false;
-    tag->setItem("FMPS_Rating", TagLib::APE::Item("FMPS_Rating", TagLib::StringList(QStringToTaglibString(QString::number(song.rating())))));
+    SetRating(tag, song);
   }
   else if (TagLib::Ogg::XiphComment *xiph_comment = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref->file()->tag())) {
-    if (song.rating() > 0) {
-      xiph_comment->addField("FMPS_RATING", QStringToTaglibString(QString::number(song.rating())), true);
-    }
-    else {
-      xiph_comment->removeFields("FMPS_RATING");
-    }
+    SetRating(xiph_comment, song);
   }
   else if (TagLib::MPEG::File *mpeg_file = dynamic_cast<TagLib::MPEG::File*>(fileref->file())) {
     TagLib::ID3v2::Tag *tag = mpeg_file->ID3v2Tag(true);
     if (!tag) return false;
-    SetUserTextFrame("FMPS_Rating", QString::number(song.rating()), tag);
-    TagLib::ID3v2::PopularimeterFrame *frame = GetPOPMFrameFromTag(tag);
-    if (frame) {
-      frame->setRating(ConvertToPOPMRating(song.rating()));
-    }
+    SetRating(tag, song);
   }
   else if (TagLib::MP4::File *mp4_file = dynamic_cast<TagLib::MP4::File*>(fileref->file())) {
     TagLib::MP4::Tag *tag = mp4_file->tag();
     if (!tag) return false;
-    tag->setItem(kMP4_FMPS_Rating_ID, TagLib::StringList(QStringToTaglibString(QString::number(song.rating()))));
+    SetRating(tag, song);
   }
   else if (TagLib::ASF::File *asf_file = dynamic_cast<TagLib::ASF::File*>(fileref->file())) {
     TagLib::ASF::Tag *tag = asf_file->tag();
     if (!tag) return false;
-    tag->addAttribute("FMPS/Rating", TagLib::ASF::Attribute(QStringToTaglibString(QString::number(song.rating()))));
+    SetRating(tag, song);
   }
   else if (TagLib::MPC::File *mpc_file = dynamic_cast<TagLib::MPC::File*>(fileref->file())) {
     TagLib::APE::Tag *tag = mpc_file->APETag(true);
     if (!tag) return false;
-    tag->setItem("FMPS_Rating", TagLib::APE::Item("FMPS_Rating", TagLib::StringList(QStringToTaglibString(QString::number(song.rating())))));
+    SetRating(tag, song);
   }
   else {
     return true;

ext/libstrawberry-tagreader/tagreadertaglib.h

@@ -29,8 +29,12 @@
 #include <taglib/tstring.h>
 #include <taglib/fileref.h>
 #include <taglib/xiphcomment.h>
+#include <taglib/flacfile.h>
+#include <taglib/mpegfile.h>
+#include <taglib/mp4file.h>
 #include <taglib/apetag.h>
 #include <taglib/apefile.h>
+#include <taglib/asffile.h>
 #include <taglib/id3v2tag.h>
 #include <taglib/popularimeterframe.h>
 
@@ -51,10 +55,10 @@ class TagReaderTagLib : public TagReaderBase {
   bool IsMediaFile(const QString &filename) const override;
 
   bool ReadFile(const QString &filename, spb::tagreader::SongMetadata *song) const override;
-  bool SaveFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
+  bool SaveFile(const spb::tagreader::SaveFileRequest &request) const override;
 
   QByteArray LoadEmbeddedArt(const QString &filename) const override;
-  bool SaveEmbeddedArt(const QString &filename, const QByteArray &data) override;
+  bool SaveEmbeddedArt(const spb::tagreader::SaveEmbeddedArtRequest &request) const override;
 
   bool SaveSongPlaycountToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
   bool SaveSongRatingToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
@@ -80,6 +84,23 @@ class TagReaderTagLib : public TagReaderBase {
 
   static TagLib::ID3v2::PopularimeterFrame *GetPOPMFrameFromTag(TagLib::ID3v2::Tag *tag);
 
+  void SetPlaycount(TagLib::Ogg::XiphComment *xiph_comment, const spb::tagreader::SongMetadata &song) const;
+  void SetPlaycount(TagLib::APE::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SetPlaycount(TagLib::ID3v2::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SetPlaycount(TagLib::MP4::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SetPlaycount(TagLib::ASF::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+
+  void SetRating(TagLib::Ogg::XiphComment *xiph_comment, const spb::tagreader::SongMetadata &song) const;
+  void SetRating(TagLib::APE::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SetRating(TagLib::ID3v2::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SetRating(TagLib::MP4::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SetRating(TagLib::ASF::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+
+  void SetEmbeddedArt(TagLib::FLAC::File *flac_file, TagLib::Ogg::XiphComment *xiph_comment, const QByteArray &data) const;
+  void SetEmbeddedArt(TagLib::Ogg::XiphComment *xiph_comment, const QByteArray &data) const;
+  void SetEmbeddedArt(TagLib::MPEG::File *file_mp3, TagLib::ID3v2::Tag *tag, const QByteArray &data) const;
+  void SetEmbeddedArt(TagLib::MP4::File *aac_file, TagLib::MP4::Tag *tag, const QByteArray &data) const;
+
  private:
   FileRefFactory *factory_;
 

ext/libstrawberry-tagreader/tagreadertagparser.cpp

@@ -22,6 +22,7 @@
 #include <string>
 #include <memory>
 #include <algorithm>
+#include <vector>
 #include <sys/stat.h>
 
 #include <tagparser/mediafileinfo.h>
@@ -79,7 +80,7 @@ bool TagReaderTagParser::IsMediaFile(const QString &filename) const {
     }
 
     const auto tracks = taginfo.tracks();
-    for (const auto track : tracks) {
+    for (TagParser::AbstractTrack *track : tracks) {
       if (track->mediaType() == TagParser::MediaType::Audio) {
         taginfo.close();
         return true;
@@ -102,8 +103,9 @@ bool TagReaderTagParser::ReadFile(const QString &filename, spb::tagreader::SongM
   if (!fileinfo.exists() || fileinfo.suffix().compare("bak", Qt::CaseInsensitive) == 0) return false;
 
   const QByteArray url(QUrl::fromLocalFile(filename).toEncoded());
+  const QByteArray basefilename = fileinfo.fileName().toUtf8();
 
-  song->set_basefilename(DataCommaSizeFromQString(fileinfo.fileName()));
+  song->set_basefilename(basefilename.constData(), basefilename.size());
   song->set_url(url.constData(), url.size());
   song->set_filesize(fileinfo.size());
   song->set_mtime(fileinfo.lastModified().isValid() ? std::max(fileinfo.lastModified().toSecsSinceEpoch(), 0LL) : 0LL);
@@ -154,8 +156,8 @@ bool TagReaderTagParser::ReadFile(const QString &filename, spb::tagreader::SongM
       qLog(Debug) << QString::fromStdString(msg.message());
     }
 
-    const auto tracks = taginfo.tracks();
-    for (const auto track : tracks) {
+    std::vector<TagParser::AbstractTrack*> tracks = taginfo.tracks();
+    for (TagParser::AbstractTrack *track : tracks) {
       switch (track->format().general) {
         case TagParser::GeneralMediaFormat::Flac:
           song->set_filetype(spb::tagreader::SongMetadata_FileType::SongMetadata_FileType_FLAC);
@@ -209,7 +211,7 @@ bool TagReaderTagParser::ReadFile(const QString &filename, spb::tagreader::SongM
       return false;
     }
 
-    for (const auto tag : taginfo.tags()) {
+    for (TagParser::Tag *tag : taginfo.tags()) {
       song->set_albumartist(tag->value(TagParser::KnownField::AlbumArtist).toString(TagParser::TagTextEncoding::Utf8));
       song->set_artist(tag->value(TagParser::KnownField::Artist).toString(TagParser::TagTextEncoding::Utf8));
       song->set_album(tag->value(TagParser::KnownField::Album).toString(TagParser::TagTextEncoding::Utf8));
@@ -256,11 +258,34 @@ bool TagReaderTagParser::ReadFile(const QString &filename, spb::tagreader::SongM
 
 }
 
-bool TagReaderTagParser::SaveFile(const QString &filename, const spb::tagreader::SongMetadata &song) const {
+bool TagReaderTagParser::SaveFile(const spb::tagreader::SaveFileRequest &request) const {
 
-  if (filename.isEmpty()) return false;
+  if (request.filename().empty()) return false;
 
-  qLog(Debug) << "Saving tags to" << filename;
+  const QString filename = QString::fromUtf8(request.filename().data(), request.filename().size());
+  const spb::tagreader::SongMetadata song = request.metadata();
+  const bool save_tags = request.has_save_tags() && request.save_tags();
+  const bool save_playcount = request.has_save_playcount() && request.save_playcount();
+  const bool save_rating = request.has_save_rating() && request.save_rating();
+  const bool save_cover = request.has_save_cover() && request.save_cover();
+
+  QStringList save_tags_options;
+  if (save_tags) {
+    save_tags_options << "tags";
+  }
+  if (save_playcount) {
+    save_tags_options << "playcount";
+  }
+  if (save_rating) {
+    save_tags_options << "rating";
+  }
+  if (save_cover) {
+    save_tags_options << "embedded cover";
+  }
+
+  qLog(Debug) << "Saving" << save_tags_options.join(", ") << "to" << filename;
+
+  const QByteArray cover_data = LoadCoverDataFromRequest(request);
 
   try {
     TagParser::MediaFileInfo taginfo;
@@ -295,22 +320,34 @@ bool TagReaderTagParser::SaveFile(const QString &filename, const spb::tagreader:
       taginfo.createAppropriateTags();
     }
 
-    for (const auto tag : taginfo.tags()) {
-      tag->setValue(TagParser::KnownField::AlbumArtist, TagParser::TagValue(song.albumartist(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Artist, TagParser::TagValue(song.artist(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Album, TagParser::TagValue(song.album(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Title, TagParser::TagValue(song.title(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Genre, TagParser::TagValue(song.genre(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Composer, TagParser::TagValue(song.composer(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Performers, TagParser::TagValue(song.performer(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Grouping, TagParser::TagValue(song.grouping(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Comment, TagParser::TagValue(song.comment(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::Lyrics, TagParser::TagValue(song.lyrics(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
-      tag->setValue(TagParser::KnownField::TrackPosition, TagParser::TagValue(song.track()));
-      tag->setValue(TagParser::KnownField::DiskPosition, TagParser::TagValue(song.disc()));
-      tag->setValue(TagParser::KnownField::RecordDate, TagParser::TagValue(song.year()));
-      tag->setValue(TagParser::KnownField::ReleaseDate, TagParser::TagValue(song.originalyear()));
+    for (TagParser::Tag *tag : taginfo.tags()) {
+      if (save_tags) {
+        tag->setValue(TagParser::KnownField::AlbumArtist, TagParser::TagValue(song.albumartist(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Artist, TagParser::TagValue(song.artist(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Album, TagParser::TagValue(song.album(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Title, TagParser::TagValue(song.title(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Genre, TagParser::TagValue(song.genre(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Composer, TagParser::TagValue(song.composer(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Performers, TagParser::TagValue(song.performer(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Grouping, TagParser::TagValue(song.grouping(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Comment, TagParser::TagValue(song.comment(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::Lyrics, TagParser::TagValue(song.lyrics(), TagParser::TagTextEncoding::Utf8, tag->proposedTextEncoding()));
+        tag->setValue(TagParser::KnownField::TrackPosition, TagParser::TagValue(song.track()));
+        tag->setValue(TagParser::KnownField::DiskPosition, TagParser::TagValue(song.disc()));
+        tag->setValue(TagParser::KnownField::RecordDate, TagParser::TagValue(song.year()));
+        tag->setValue(TagParser::KnownField::ReleaseDate, TagParser::TagValue(song.originalyear()));
+      }
+      if (save_playcount) {
+        SaveSongPlaycountToFile(tag, song);
+      }
+      if (save_rating) {
+        SaveSongRatingToFile(tag, song);
+      }
+      if (save_cover) {
+        SaveEmbeddedArt(tag, cover_data);
+      }
     }
+
     taginfo.applyChanges(diag, progress);
     taginfo.close();
 
@@ -358,7 +395,7 @@ QByteArray TagReaderTagParser::LoadEmbeddedArt(const QString &filename) const {
       return QByteArray();
     }
 
-    for (const auto tag : taginfo.tags()) {
+    for (TagParser::Tag *tag : taginfo.tags()) {
       if (!tag->value(TagParser::KnownField::Cover).empty() && tag->value(TagParser::KnownField::Cover).dataSize() > 0) {
         QByteArray data(tag->value(TagParser::KnownField::Cover).dataPointer(), tag->value(TagParser::KnownField::Cover).dataSize());
         taginfo.close();
@@ -379,12 +416,22 @@ QByteArray TagReaderTagParser::LoadEmbeddedArt(const QString &filename) const {
 
 }
 
-bool TagReaderTagParser::SaveEmbeddedArt(const QString &filename, const QByteArray &data) {
+void TagReaderTagParser::SaveEmbeddedArt(TagParser::Tag *tag, const QByteArray &data) const {
 
-  if (filename.isEmpty()) return false;
+  tag->setValue(TagParser::KnownField::Cover, TagParser::TagValue(data.toStdString()));
+
+}
+
+bool TagReaderTagParser::SaveEmbeddedArt(const spb::tagreader::SaveEmbeddedArtRequest &request) const {
+
+  if (request.filename().empty()) return false;
+
+  const QString filename = QString::fromUtf8(request.filename().data(), request.filename().size());
 
   qLog(Debug) << "Saving art to" << filename;
 
+  const QByteArray cover_data = LoadCoverDataFromRequest(request);
+
   try {
 
     TagParser::MediaFileInfo taginfo;
@@ -415,8 +462,8 @@ bool TagReaderTagParser::SaveEmbeddedArt(const QString &filename, const QByteArr
       taginfo.createAppropriateTags();
     }
 
-    for (const auto tag : taginfo.tags()) {
-      tag->setValue(TagParser::KnownField::Cover, TagParser::TagValue(data.toStdString()));
+    for (TagParser::Tag *tag : taginfo.tags()) {
+      SaveEmbeddedArt(tag, cover_data);
     }
 
     taginfo.applyChanges(diag, progress);
@@ -435,8 +482,16 @@ bool TagReaderTagParser::SaveEmbeddedArt(const QString &filename, const QByteArr
 
 }
 
+void TagReaderTagParser::SaveSongPlaycountToFile(TagParser::Tag*, const spb::tagreader::SongMetadata&) const {}
+
 bool TagReaderTagParser::SaveSongPlaycountToFile(const QString&, const spb::tagreader::SongMetadata&) const { return false; }
 
+void TagReaderTagParser::SaveSongRatingToFile(TagParser::Tag *tag, const spb::tagreader::SongMetadata &song) const {
+
+  tag->setValue(TagParser::KnownField::Rating, TagParser::TagValue(ConvertToPOPMRating(song.rating())));
+
+}
+
 bool TagReaderTagParser::SaveSongRatingToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const {
 
     if (filename.isEmpty()) return false;
@@ -476,9 +531,10 @@ bool TagReaderTagParser::SaveSongRatingToFile(const QString &filename, const spb
         taginfo.createAppropriateTags();
       }
 
-      for (const auto tag : taginfo.tags()) {
-        tag->setValue(TagParser::KnownField::Rating, TagParser::TagValue(ConvertToPOPMRating(song.rating())));
+      for (TagParser::Tag *tag : taginfo.tags()) {
+        SaveSongRatingToFile(tag, song);
       }
+
       taginfo.applyChanges(diag, progress);
       taginfo.close();
 

ext/libstrawberry-tagreader/tagreadertagparser.h

@@ -22,6 +22,8 @@
 
 #include <string>
 
+#include <tagparser/tag.h>
+
 #include <QByteArray>
 #include <QString>
 
@@ -40,14 +42,20 @@ class TagReaderTagParser : public TagReaderBase {
   bool IsMediaFile(const QString &filename) const override;
 
   bool ReadFile(const QString &filename, spb::tagreader::SongMetadata *song) const override;
-  bool SaveFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
+  bool SaveFile(const spb::tagreader::SaveFileRequest &request) const override;
 
   QByteArray LoadEmbeddedArt(const QString &filename) const override;
-  bool SaveEmbeddedArt(const QString &filename, const QByteArray &data) override;
+  bool SaveEmbeddedArt(const spb::tagreader::SaveEmbeddedArtRequest &request) const override;
 
   bool SaveSongPlaycountToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
   bool SaveSongRatingToFile(const QString &filename, const spb::tagreader::SongMetadata &song) const override;
 
+ private:
+  void SaveSongPlaycountToFile(TagParser::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SaveSongRatingToFile(TagParser::Tag *tag, const spb::tagreader::SongMetadata &song) const;
+  void SaveEmbeddedArt(TagParser::Tag *tag, const QByteArray &data) const;
+
+ public:
   Q_DISABLE_COPY(TagReaderTagParser)
 };
 

ext/strawberry-tagreader/tagreaderworker.cpp

@@ -56,36 +56,41 @@ void TagReaderWorker::DeviceClosed() {
 bool TagReaderWorker::HandleMessage(const spb::tagreader::Message &message, spb::tagreader::Message &reply, TagReaderBase *reader) {
 
   if (message.has_is_media_file_request()) {
-    bool success = reader->IsMediaFile(QStringFromStdString(message.is_media_file_request().filename()));
+    const QString filename = QString::fromUtf8(message.is_media_file_request().filename().data(), message.is_media_file_request().filename().size());
+    bool success = reader->IsMediaFile(filename);
     reply.mutable_is_media_file_response()->set_success(success);
     return success;
   }
   else if (message.has_read_file_request()) {
-    bool success = reader->ReadFile(QStringFromStdString(message.read_file_request().filename()), reply.mutable_read_file_response()->mutable_metadata());
+    const QString filename = QString::fromUtf8(message.read_file_request().filename().data(), message.read_file_request().filename().size());
+    bool success = reader->ReadFile(filename, reply.mutable_read_file_response()->mutable_metadata());
     return success;
   }
   else if (message.has_save_file_request()) {
-    bool success = reader->SaveFile(QStringFromStdString(message.save_file_request().filename()), message.save_file_request().metadata());
+    bool success = reader->SaveFile(message.save_file_request());
     reply.mutable_save_file_response()->set_success(success);
     return success;
   }
   else if (message.has_load_embedded_art_request()) {
-    QByteArray data = reader->LoadEmbeddedArt(QStringFromStdString(message.load_embedded_art_request().filename()));
+    const QString filename = QString::fromUtf8(message.load_embedded_art_request().filename().data(), message.load_embedded_art_request().filename().size());
+    QByteArray data = reader->LoadEmbeddedArt(filename);
     reply.mutable_load_embedded_art_response()->set_data(data.constData(), data.size());
     return true;
   }
   else if (message.has_save_embedded_art_request()) {
-    bool success = reader->SaveEmbeddedArt(QStringFromStdString(message.save_embedded_art_request().filename()), QByteArray(message.save_embedded_art_request().data().data(), static_cast<qint64>(message.save_embedded_art_request().data().size())));
+    bool success = reader->SaveEmbeddedArt(message.save_embedded_art_request());
     reply.mutable_save_embedded_art_response()->set_success(success);
     return success;
   }
   else if (message.has_save_song_playcount_to_file_request()) {
-    bool success = reader->SaveSongPlaycountToFile(QStringFromStdString(message.save_song_playcount_to_file_request().filename()), message.save_song_playcount_to_file_request().metadata());
+    const QString filename = QString::fromUtf8(message.save_song_playcount_to_file_request().filename().data(), message.save_song_playcount_to_file_request().filename().size());
+    bool success = reader->SaveSongPlaycountToFile(filename, message.save_song_playcount_to_file_request().metadata());
     reply.mutable_save_song_playcount_to_file_response()->set_success(success);
     return success;
   }
   else if (message.has_save_song_rating_to_file_request()) {
-    bool success = reader->SaveSongRatingToFile(QStringFromStdString(message.save_song_rating_to_file_request().filename()), message.save_song_rating_to_file_request().metadata());
+    const QString filename = QString::fromUtf8(message.save_song_rating_to_file_request().filename().data(), message.save_song_rating_to_file_request().filename().size());
+    bool success = reader->SaveSongRatingToFile(filename, message.save_song_rating_to_file_request().metadata());
     reply.mutable_save_song_rating_to_file_response()->set_success(success);
     return success;
   }