Update issue 1175:

Save rating and statistics in two distinct ways, and let users activate them separately in preferences.

ext/clementine-tagreader/tagreaderworker.cpp

@@ -53,6 +53,11 @@ void TagReaderWorker::MessageArrived(const pb::tagreader::Message& message) {
         tag_reader_.SaveSongStatisticsToFile(
             QStringFromStdString(message.save_song_statistics_to_file_request().filename()),
             message.save_song_statistics_to_file_request().metadata()));
+  } else if (message.has_save_song_rating_to_file_request()) {
+    reply.mutable_save_song_rating_to_file_response()->set_success(
+        tag_reader_.SaveSongRatingToFile(
+            QStringFromStdString(message.save_song_rating_to_file_request().filename()),
+            message.save_song_rating_to_file_request().metadata()));
   } else if (message.has_is_media_file_request()) {
     reply.mutable_is_media_file_response()->set_success(
         tag_reader_.IsMediaFile(QStringFromStdString(message.is_media_file_request().filename())));

ext/libclementine-tagreader/tagreader.cpp

@@ -69,6 +69,7 @@ using boost::scoped_ptr;
 # include "cloudstream.h"
 #endif
 
+#define NumberToASFAttribute(x) TagLib::ASF::Attribute(QStringToTaglibString(QString::number(x)))
 
 class FileRefFactory {
  public:
@@ -472,12 +473,16 @@ void TagReader::SetVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
   vorbis_comments->addField("COMPILATION", StdStringToTaglibString(song.compilation() ? "1" : "0"), true);
 }
 
-void TagReader::SetFMPSVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
+void TagReader::SetFMPSStatisticsVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
+                                  const pb::tagreader::SongMetadata& song) const {
+  vorbis_comments->addField("FMPS_PLAYCOUNT", QStringToTaglibString(QString::number(song.playcount())));
+  vorbis_comments->addField("FMPS_RATING_AMAROK_SCORE", QStringToTaglibString(QString::number(song.score() / 100.0)));
+}
+
+void TagReader::SetFMPSRatingVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
                                   const pb::tagreader::SongMetadata& song) const {
 
   vorbis_comments->addField("FMPS_RATING", QStringToTaglibString(QString::number(song.rating())));
-  vorbis_comments->addField("FMPS_PLAYCOUNT", QStringToTaglibString(QString::number(song.playcount())));
-  vorbis_comments->addField("FMPS_RATING_AMAROK_SCORE", QStringToTaglibString(QString::number(song.score() / 100.0)));
 }
 
 pb::tagreader::SongMetadata_Type TagReader::GuessFileType(
@@ -593,47 +598,83 @@ bool TagReader::SaveSongStatisticsToFile(const QString& filename,
     TagLib::ID3v2::Tag* tag = file->ID3v2Tag(true);
 
     // Save as FMPS
-    SetUserTextFrame("FMPS_Rating", QString::number(song.rating()), tag);
     SetUserTextFrame("FMPS_PlayCount", QString::number(song.playcount()), tag);
     SetUserTextFrame("FMPS_Rating_Amarok_Score", QString::number(song.score() / 100.0), tag);
 
     // Also save as POPM
-    TagLib::ID3v2::PopularimeterFrame* frame = NULL;
+    TagLib::ID3v2::PopularimeterFrame* frame = GetPOPMFrameFromTag(tag);
-
-    const TagLib::ID3v2::FrameListMap& map = file->ID3v2Tag()->frameListMap();
-    if (!map["POPM"].isEmpty()) {
-      frame = dynamic_cast<TagLib::ID3v2::PopularimeterFrame*>(map["POPM"].front());
-    }
-
-    if (!frame) {
-      frame = new TagLib::ID3v2::PopularimeterFrame();
-      tag->addFrame(frame);
-    }
-
-    frame->setRating(ConvertToPOPMRating(song.rating()));
     frame->setCounter(song.playcount());
 
   } else if (TagLib::FLAC::File* file = dynamic_cast<TagLib::FLAC::File*>(fileref->file())) {
     TagLib::Ogg::XiphComment* vorbis_comments = file->xiphComment(true);
-    SetFMPSVorbisComments(vorbis_comments, song);
+    SetFMPSStatisticsVorbisComments(vorbis_comments, song);
   } else if (TagLib::Ogg::XiphComment* tag = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref->file()->tag())) {
-    SetFMPSVorbisComments(tag, song);
+    SetFMPSStatisticsVorbisComments(tag, song);
   }
 #ifdef TAGLIB_WITH_ASF
   else if (TagLib::ASF::File* file = dynamic_cast<TagLib::ASF::File*>(fileref->file())) {
     TagLib::ASF::Tag* tag = file->tag();
-    #define ConvertASF(x) TagLib::ASF::Attribute(QStringToTaglibString(QString::number(x)))
+    tag->addAttribute("FMPS/Playcount", NumberToASFAttribute(song.playcount()));
-    tag->addAttribute("FMPS/Rating",    ConvertASF(song.rating()));
+    tag->addAttribute("FMPS/Rating_Amarok_Score", NumberToASFAttribute(song.score() / 100.0));
-    tag->addAttribute("FMPS/Playcount", ConvertASF(song.playcount()));
+  }
-    tag->addAttribute("FMPS/Rating_Amarok_Score", ConvertASF(song.score() / 100.0));
+#endif
-    #undef ConvertASF
+  else if (TagLib::MP4::File* file = dynamic_cast<TagLib::MP4::File*>(fileref->file())) {
+    TagLib::MP4::Tag* tag = file->tag();
+    tag->itemListMap()[kMP4_FMPS_Score_ID] =      TagLib::StringList(QStringToTaglibString(QString::number(song.score() / 100.0)));
+    tag->itemListMap()[kMP4_FMPS_Playcount_ID] =  TagLib::StringList(TagLib::String::number(song.playcount()));
+  } else {
+    // Nothing to save: stop now
+    return true;
+  }
+
+  bool ret = fileref->save();
+  #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(), NULL, 0);
+  }
+  #endif  // Q_OS_LINUX
+  return ret;
+}
+
+bool TagReader::SaveSongRatingToFile(const QString& filename,
+                                     const pb::tagreader::SongMetadata& song) const {
+  if (filename.isNull())
+    return false;
+
+  qLog(Debug) << "Saving song rating tags to" << filename;
+
+  scoped_ptr<TagLib::FileRef> fileref(factory_->GetFileRef(filename));
+
+  if (!fileref || fileref->isNull()) // The file probably doesn't exist
+    return false;
+
+  if (TagLib::MPEG::File* file = dynamic_cast<TagLib::MPEG::File*>(fileref->file())) {
+    TagLib::ID3v2::Tag* tag = file->ID3v2Tag(true);
+
+    // Save as FMPS
+    SetUserTextFrame("FMPS_Rating", QString::number(song.rating()), tag);
+
+    // Also save as POPM
+    TagLib::ID3v2::PopularimeterFrame* frame = GetPOPMFrameFromTag(tag);
+    frame->setRating(ConvertToPOPMRating(song.rating()));
+
+  } else if (TagLib::FLAC::File* file = dynamic_cast<TagLib::FLAC::File*>(fileref->file())) {
+    TagLib::Ogg::XiphComment* vorbis_comments = file->xiphComment(true);
+    SetFMPSRatingVorbisComments(vorbis_comments, song);
+  } else if (TagLib::Ogg::XiphComment* tag = dynamic_cast<TagLib::Ogg::XiphComment*>(fileref->file()->tag())) {
+    SetFMPSRatingVorbisComments(tag, song);
+  }
+#ifdef TAGLIB_WITH_ASF
+  else if (TagLib::ASF::File* file = dynamic_cast<TagLib::ASF::File*>(fileref->file())) {
+    TagLib::ASF::Tag* tag = file->tag();
+    tag->addAttribute("FMPS/Rating",    NumberToASFAttribute(song.rating()));
   }
 #endif
   else if (TagLib::MP4::File* file = dynamic_cast<TagLib::MP4::File*>(fileref->file())) {
     TagLib::MP4::Tag* tag = file->tag();
     tag->itemListMap()[kMP4_FMPS_Rating_ID] =     TagLib::StringList(QStringToTaglibString(QString::number(song.rating())));
-    tag->itemListMap()[kMP4_FMPS_Score_ID] =      TagLib::StringList(QStringToTaglibString(QString::number(song.score() / 100.0)));
-    tag->itemListMap()[kMP4_FMPS_Playcount_ID] =  TagLib::StringList(TagLib::String::number(song.playcount()));
   } else {
     // Nothing to save: stop now
     return true;
@@ -881,6 +922,21 @@ bool TagReader::ReadCloudFile(const QUrl& download_url,
 }
 #endif // HAVE_GOOGLE_DRIVE
 
+TagLib::ID3v2::PopularimeterFrame* TagReader::GetPOPMFrameFromTag(TagLib::ID3v2::Tag* tag) {
+  TagLib::ID3v2::PopularimeterFrame* frame = NULL;
+
+  const TagLib::ID3v2::FrameListMap& map = tag->frameListMap();
+  if (!map["POPM"].isEmpty()) {
+    frame = dynamic_cast<TagLib::ID3v2::PopularimeterFrame*>(map["POPM"].front());
+  }
+
+  if (!frame) {
+    frame = new TagLib::ID3v2::PopularimeterFrame();
+    tag->addFrame(frame);
+  }
+  return frame;
+}
+
 float TagReader::ConvertPOPMRating(const int POPM_rating) {
   if (POPM_rating < 0x01) {
     return 0.0;

ext/libclementine-tagreader/tagreader.h

@@ -37,6 +37,7 @@ namespace TagLib {
 
   namespace ID3v2 {
     class Tag;
+    class PopularimeterFrame;
   }
 }
 
@@ -57,6 +58,8 @@ class TagReader {
   // returns true if the file exists but nothing has been written inside because
   // statistics tag format is not supported for this kind of file)
   bool SaveSongStatisticsToFile(const QString& filename, const pb::tagreader::SongMetadata& song) const;
+  bool SaveSongRatingToFile(const QString& filename, const pb::tagreader::SongMetadata& song) const;
+
   bool IsMediaFile(const QString& filename) const;
   QByteArray LoadEmbeddedArt(const QString& filename) const;
 
@@ -82,8 +85,10 @@ class TagReader {
                    pb::tagreader::SongMetadata* song) const;
   void SetVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
                          const pb::tagreader::SongMetadata& song) const;
-  void SetFMPSVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
+  void SetFMPSStatisticsVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
-                             const pb::tagreader::SongMetadata& song) const;
+                                       const pb::tagreader::SongMetadata& song) const;
+  void SetFMPSRatingVorbisComments(TagLib::Ogg::XiphComment* vorbis_comments,
+                                   const pb::tagreader::SongMetadata& song) const;
 
   pb::tagreader::SongMetadata_Type GuessFileType(TagLib::FileRef* fileref) const;
 
@@ -105,6 +110,7 @@ private:
   static float ConvertPOPMRating(const int POPM_rating);
   // Reciprocal
   static int ConvertToPOPMRating(const float rating);
+  static TagLib::ID3v2::PopularimeterFrame* GetPOPMFrameFromTag(TagLib::ID3v2::Tag* tag);
 
   FileRefFactory* factory_;
   QNetworkAccessManager* network_;

ext/libclementine-tagreader/tagreadermessages.proto

@@ -98,7 +98,6 @@ message ReadCloudFileResponse {
   optional SongMetadata metadata = 1;
 }
 
-
 message SaveSongStatisticsToFileRequest {
   optional string filename = 1;
   optional SongMetadata metadata = 2;
@@ -108,6 +107,15 @@ message SaveSongStatisticsToFileResponse {
   optional bool success = 1;
 }
 
+message SaveSongRatingToFileRequest {
+  optional string filename = 1;
+  optional SongMetadata metadata = 2;
+}
+
+message SaveSongRatingToFileResponse {
+  optional bool success = 1;
+}
+
 message Message {
   optional int32 id = 1;
 
@@ -129,4 +137,6 @@ message Message {
   optional SaveSongStatisticsToFileRequest save_song_statistics_to_file_request = 12;
   optional SaveSongStatisticsToFileResponse save_song_statistics_to_file_response = 13;
   
+  optional SaveSongRatingToFileRequest save_song_rating_to_file_request = 14;
+  optional SaveSongRatingToFileResponse save_song_rating_to_file_response = 15;
 }

src/core/tagreaderclient.cpp

@@ -86,6 +86,24 @@ void TagReaderClient::UpdateSongsStatistics(const SongList& songs) {
   }
 }
 
+TagReaderReply* TagReaderClient::UpdateSongRating(const Song& metadata) {
+  pb::tagreader::Message message;
+  pb::tagreader::SaveSongRatingToFileRequest* req =
+      message.mutable_save_song_rating_to_file_request();
+
+  req->set_filename(DataCommaSizeFromQString(metadata.url().toLocalFile()));
+  metadata.ToProtobuf(req->mutable_metadata());
+
+  return worker_pool_->SendMessageWithReply(&message);
+}
+
+void TagReaderClient::UpdateSongsRating(const SongList& songs) {
+  foreach (const Song& song, songs) {
+    TagReaderReply* reply = UpdateSongRating(song);
+    connect(reply, SIGNAL(Finished(bool)), reply, SLOT(deleteLater()));
+  }
+}
+
 TagReaderReply* TagReaderClient::IsMediaFile(const QString& filename) {
   pb::tagreader::Message message;
   pb::tagreader::IsMediaFileRequest* req = message.mutable_is_media_file_request();
@@ -161,6 +179,20 @@ bool TagReaderClient::UpdateSongStatisticsBlocking(const Song& metadata) {
   return ret;
 }
 
+bool TagReaderClient::UpdateSongRatingBlocking(const Song& metadata) {
+  Q_ASSERT(QThread::currentThread() != thread());
+
+  bool ret = false;
+
+  TagReaderReply* reply = UpdateSongRating(metadata);
+  if (reply->WaitForFinished()) {
+    ret = reply->message().save_song_rating_to_file_response().success();
+  }
+  reply->deleteLater();
+
+  return ret;
+}
+
 bool TagReaderClient::IsMediaFileBlocking(const QString& filename) {
   Q_ASSERT(QThread::currentThread() != thread());
 

src/core/tagreaderclient.h

@@ -44,6 +44,7 @@ public:
   ReplyType* ReadFile(const QString& filename);
   ReplyType* SaveFile(const QString& filename, const Song& metadata);
   ReplyType* UpdateSongStatistics(const Song& metadata);
+  ReplyType* UpdateSongRating(const Song& metadata);
   ReplyType* IsMediaFile(const QString& filename);
   ReplyType* LoadEmbeddedArt(const QString& filename);
   ReplyType* ReadCloudFile(const QUrl& download_url,
@@ -58,6 +59,7 @@ public:
   void ReadFileBlocking(const QString& filename, Song* song);
   bool SaveFileBlocking(const QString& filename, const Song& metadata);
   bool UpdateSongStatisticsBlocking(const Song& metadata);
+  bool UpdateSongRatingBlocking(const Song& metadata);
   bool IsMediaFileBlocking(const QString& filename);
   QImage LoadEmbeddedArtBlocking(const QString& filename);
 
@@ -66,6 +68,7 @@ public:
 
 public slots:
   void UpdateSongsStatistics(const SongList& songs);
+  void UpdateSongsRating(const SongList& songs);
 
 private slots:
   void WorkerFailedToStart();

src/library/library.cpp

@@ -168,6 +168,7 @@ void Library::WriteAllSongsStatisticsToFiles() {
   int i = 0;
   foreach (const Song& song, all_songs) {
     TagReaderClient::Instance()->UpdateSongStatisticsBlocking(song);
+    TagReaderClient::Instance()->UpdateSongRatingBlocking(song);
     app_->task_manager()->SetTaskProgress(task_id, ++i, nb_songs);
   }
   app_->task_manager()->SetTaskFinished(task_id);

src/library/librarybackend.cpp

@@ -40,7 +40,8 @@ const char* LibraryBackend::kNewScoreSql =
 
 LibraryBackend::LibraryBackend(QObject *parent)
   : LibraryBackendInterface(parent),
-    save_statistics_in_file_(false)
+    save_statistics_in_file_(false),
+    save_ratings_in_file_(false)
 {
 }
 
@@ -1048,7 +1049,7 @@ void LibraryBackend::UpdateSongRating(int id, float rating) {
     return;
 
   Song new_song = GetSongById(id, db);
-  emit SongsStatisticsChanged(SongList() << new_song);
+  emit SongsRatingChanged(SongList() << new_song);
 }
 
 void LibraryBackend::DeleteAll() {
@@ -1080,15 +1081,34 @@ void LibraryBackend::ReloadSettingsAsync() {
 void LibraryBackend::ReloadSettings() {
   QSettings s;
   s.beginGroup(kSettingsGroup);
-  bool save_statistics_in_file = s.value("save_statistics_in_file", false).toBool();
+
-  // Compare with previous value to know if we should connect, disconnect or nothing
+  // Statistics
-  if (save_statistics_in_file_ && !save_statistics_in_file) {
+  {
-    disconnect(this, SIGNAL(SongsStatisticsChanged(SongList)),
+    bool save_statistics_in_file = s.value("save_statistics_in_file", false).toBool();
-        TagReaderClient::Instance(), SLOT(UpdateSongsStatistics(SongList)));
+    // Compare with previous value to know if we should connect, disconnect or nothing
-  } else if (!save_statistics_in_file_ && save_statistics_in_file) {
+    if (save_statistics_in_file_ && !save_statistics_in_file) {
-    connect(this, SIGNAL(SongsStatisticsChanged(SongList)),
+      disconnect(this, SIGNAL(SongsStatisticsChanged(SongList)),
-        TagReaderClient::Instance(), SLOT(UpdateSongsStatistics(SongList)));
+          TagReaderClient::Instance(), SLOT(UpdateSongsStatistics(SongList)));
+    } else if (!save_statistics_in_file_ && save_statistics_in_file) {
+      connect(this, SIGNAL(SongsStatisticsChanged(SongList)),
+          TagReaderClient::Instance(), SLOT(UpdateSongsStatistics(SongList)));
+    }
+    // Save old value
+    save_statistics_in_file_ = save_statistics_in_file;
+  }
+
+  // Rating
+  {
+    bool save_ratings_in_file = s.value("save_ratings_in_file", false).toBool();
+    // Compare with previous value to know if we should connect, disconnect or nothing
+    if (save_ratings_in_file_ && !save_ratings_in_file) {
+      disconnect(this, SIGNAL(SongsRatingChanged(SongList)),
+          TagReaderClient::Instance(), SLOT(UpdateSongsRating(SongList)));
+    } else if (!save_ratings_in_file_ && save_ratings_in_file) {
+      connect(this, SIGNAL(SongsRatingChanged(SongList)),
+          TagReaderClient::Instance(), SLOT(UpdateSongsRating(SongList)));
+    }
+    // Save old value
+    save_ratings_in_file_ = save_ratings_in_file;
   }
-  // Save old value
-  save_statistics_in_file_ = save_statistics_in_file;
 }

src/library/librarybackend.h

@@ -192,6 +192,7 @@ class LibraryBackend : public LibraryBackendInterface {
   void SongsDiscovered(const SongList& songs);
   void SongsDeleted(const SongList& songs);
   void SongsStatisticsChanged(const SongList& songs);
+  void SongsRatingChanged(const SongList& songs);
   void DatabaseReset();
 
   void TotalSongCountUpdated(int total);
@@ -227,6 +228,7 @@ class LibraryBackend : public LibraryBackendInterface {
   QString subdirs_table_;
   QString fts_table_;
   bool save_statistics_in_file_;
+  bool save_ratings_in_file_;
 };
 
 #endif // LIBRARYBACKEND_H

src/library/librarymodel.cpp

@@ -101,7 +101,8 @@ LibraryModel::LibraryModel(LibraryBackend* backend, Application* app,
 
   connect(backend_, SIGNAL(SongsDiscovered(SongList)), SLOT(SongsDiscovered(SongList)));
   connect(backend_, SIGNAL(SongsDeleted(SongList)), SLOT(SongsDeleted(SongList)));
-  connect(backend_, SIGNAL(SongsStatisticsChanged(SongList)), SLOT(SongsStatisticsChanged(SongList)));
+  connect(backend_, SIGNAL(SongsStatisticsChanged(SongList)), SLOT(SongsSlightlyChanged(SongList)));
+  connect(backend_, SIGNAL(SongsRatingChanged(SongList)), SLOT(SongsSlightlyChanged(SongList)));
   connect(backend_, SIGNAL(DatabaseReset()), SLOT(Reset()));
   connect(backend_, SIGNAL(TotalSongCountUpdated(int)), SLOT(TotalSongCountUpdatedSlot(int)));
 
@@ -219,7 +220,7 @@ void LibraryModel::SongsDiscovered(const SongList& songs) {
   }
 }
 
-void LibraryModel::SongsStatisticsChanged(const SongList& songs) {
+void LibraryModel::SongsSlightlyChanged(const SongList& songs) {
   // This is called if there was a minor change to the songs that will not
   // normally require the library to be restructured.  We can just update our
   // internal cache of Song objects without worrying about resetting the model.

src/library/librarymodel.h

@@ -183,7 +183,7 @@ class LibraryModel : public SimpleTreeModel<LibraryItem> {
   // From LibraryBackend
   void SongsDiscovered(const SongList& songs);
   void SongsDeleted(const SongList& songs);
-  void SongsStatisticsChanged(const SongList& songs);
+  void SongsSlightlyChanged(const SongList& songs);
   void TotalSongCountUpdatedSlot(int count);
 
   // Called after ResetAsync

src/library/librarysettingspage.cpp

@@ -101,6 +101,7 @@ void LibrarySettingsPage::Save() {
   s.endGroup();
 
   s.beginGroup(LibraryBackend::kSettingsGroup);
+  s.setValue("save_ratings_in_file", ui_->save_ratings_in_file->isChecked());
   s.setValue("save_statistics_in_file", ui_->save_statistics_in_file->isChecked());
   s.endGroup();
 }
@@ -139,6 +140,7 @@ void LibrarySettingsPage::Load() {
   s.endGroup();
 
   s.beginGroup(LibraryBackend::kSettingsGroup);
+  ui_->save_ratings_in_file->setChecked(s.value("save_ratings_in_file", false).toBool());
   ui_->save_statistics_in_file->setChecked(s.value("save_statistics_in_file", false).toBool());
   s.endGroup();
 }

src/library/librarysettingspage.ui

@@ -92,13 +92,20 @@
         </property>
        </widget>
       </item>
+      <item>
+       <widget class="QCheckBox" name="save_ratings_in_file">
+        <property name="text">
+         <string>Save ratings in file tags when possible</string>
+        </property>
+       </widget>
+      </item>
       <item>
        <widget class="QCheckBox" name="save_statistics_in_file">
         <property name="toolTip">
          <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;If not checked, Clementine will  try to save your ratings and other statistics only in a separate database and don't modify your files.&lt;/p&gt;&lt;p&gt;If checked, it will save statistics both in database and directly into the file each time they changed.&lt;/p&gt;&lt;p&gt;Please note it might not work for every format and, as there is no standard for doing so, other music players might not be able to read them.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
         </property>
         <property name="text">
-         <string>Save ratings and statistics in file tags when possible</string>
+         <string>Save statistics in file tags when possible</string>
         </property>
        </widget>
       </item>

src/playlist/playlistmanager.cpp

@@ -68,6 +68,7 @@ void PlaylistManager::Init(LibraryBackend* library_backend,
 
   connect(library_backend_, SIGNAL(SongsDiscovered(SongList)), SLOT(SongsDiscovered(SongList)));
   connect(library_backend_, SIGNAL(SongsStatisticsChanged(SongList)), SLOT(SongsDiscovered(SongList)));
+  connect(library_backend_, SIGNAL(SongsRatingChanged(SongList)), SLOT(SongsDiscovered(SongList)));
 
   foreach (const PlaylistBackend::Playlist& p, playlist_backend->GetAllOpenPlaylists()) {
     AddPlaylist(p.id, p.name, p.special_type, p.ui_path);

tests/song_test.cpp

@@ -69,6 +69,13 @@ class SongTest : public ::testing::Test {
     song.ToProtobuf(&pb_song);
     tag_reader.SaveSongStatisticsToFile(filename, pb_song);
   }
+
+  static void WriteSongRatingToFile(const Song& song, const QString& filename) {
+    TagReader tag_reader;
+    ::pb::tagreader::SongMetadata pb_song;
+    song.ToProtobuf(&pb_song);
+    tag_reader.SaveSongRatingToFile(filename, pb_song);
+  }
 };
 
 
@@ -112,7 +119,7 @@ TEST_F(SongTest, FMPSRatingUser) {
   EXPECT_FLOAT_EQ(0.10, song.rating());
 
   song.set_rating(0.20);
-  WriteSongStatisticsToFile(song, r.fileName());
+  WriteSongRatingToFile(song, r.fileName());
   Song new_song = ReadSongFromFile(r.fileName());
   EXPECT_FLOAT_EQ(0.20, new_song.rating());
 }
@@ -173,11 +180,22 @@ TEST_F(SongTest, BothFMPSPOPMRating) {
   EXPECT_FLOAT_EQ(0.42, song.rating());
 }
 
-TEST_F(SongTest, RatingAndStatisticsOgg) {
+TEST_F(SongTest, RatingOgg) {
   TemporaryResource r(":/testdata/beep.ogg");
   {
     Song song = ReadSongFromFile(r.fileName());
     song.set_rating(0.20);
+    WriteSongRatingToFile(song, r.fileName());
+  }
+
+  Song new_song = ReadSongFromFile(r.fileName());
+  EXPECT_FLOAT_EQ(0.20, new_song.rating());
+}
+
+TEST_F(SongTest, StatisticsOgg) {
+  TemporaryResource r(":/testdata/beep.ogg");
+  {
+    Song song = ReadSongFromFile(r.fileName());
     song.set_playcount(1337);
     song.set_score(87);
 
@@ -185,16 +203,26 @@ TEST_F(SongTest, RatingAndStatisticsOgg) {
   }
 
   Song new_song = ReadSongFromFile(r.fileName());
-  EXPECT_FLOAT_EQ(0.20, new_song.rating());
   EXPECT_EQ(1337, new_song.playcount());
   EXPECT_EQ(87, new_song.score());
 }
 
-TEST_F(SongTest, RatingAndStatisticsFLAC) {
+TEST_F(SongTest, RatingFLAC) {
   TemporaryResource r(":/testdata/beep.flac");
   {
     Song song = ReadSongFromFile(r.fileName());
     song.set_rating(0.20);
+    WriteSongRatingToFile(song, r.fileName());
+  }
+
+  Song new_song = ReadSongFromFile(r.fileName());
+  EXPECT_FLOAT_EQ(0.20, new_song.rating());
+}
+
+TEST_F(SongTest, StatisticsFLAC) {
+  TemporaryResource r(":/testdata/beep.flac");
+  {
+    Song song = ReadSongFromFile(r.fileName());
     song.set_playcount(1337);
     song.set_score(87);
 
@@ -202,17 +230,28 @@ TEST_F(SongTest, RatingAndStatisticsFLAC) {
   }
 
   Song new_song = ReadSongFromFile(r.fileName());
-  EXPECT_FLOAT_EQ(0.20, new_song.rating());
   EXPECT_EQ(1337, new_song.playcount());
   EXPECT_EQ(87, new_song.score());
 }
 
 #ifdef TAGLIB_WITH_ASF
-TEST_F(SongTest, RatingAndStatisticsASF) {
+TEST_F(SongTest, RatingASF) {
   TemporaryResource r(":/testdata/beep.wma");
   {
     Song song = ReadSongFromFile(r.fileName());
     song.set_rating(0.20);
+
+    WriteSongRatingToFile(song, r.fileName());
+  }
+
+  Song new_song = ReadSongFromFile(r.fileName());
+  EXPECT_FLOAT_EQ(0.20, new_song.rating());
+}
+
+TEST_F(SongTest, StatisticsASF) {
+  TemporaryResource r(":/testdata/beep.wma");
+  {
+    Song song = ReadSongFromFile(r.fileName());
     song.set_playcount(1337);
     song.set_score(87);
 
@@ -220,17 +259,28 @@ TEST_F(SongTest, RatingAndStatisticsASF) {
   }
 
   Song new_song = ReadSongFromFile(r.fileName());
-  EXPECT_FLOAT_EQ(0.20, new_song.rating());
   EXPECT_EQ(1337, new_song.playcount());
   EXPECT_EQ(87, new_song.score());
 }
 #endif // TAGLIB_WITH_ASF
 
-TEST_F(SongTest, RatingAndStatisticsMP4) {
+TEST_F(SongTest, RatingMP4) {
   TemporaryResource r(":/testdata/beep.m4a");
   {
     Song song = ReadSongFromFile(r.fileName());
     song.set_rating(0.20);
+
+    WriteSongRatingToFile(song, r.fileName());
+  }
+
+  Song new_song = ReadSongFromFile(r.fileName());
+  EXPECT_FLOAT_EQ(0.20, new_song.rating());
+}
+
+TEST_F(SongTest, StatisticsMP4) {
+  TemporaryResource r(":/testdata/beep.m4a");
+  {
+    Song song = ReadSongFromFile(r.fileName());
     song.set_playcount(1337);
     song.set_score(87);
 
@@ -238,7 +288,6 @@ TEST_F(SongTest, RatingAndStatisticsMP4) {
   }
 
   Song new_song = ReadSongFromFile(r.fileName());
-  EXPECT_FLOAT_EQ(0.20, new_song.rating());
   EXPECT_EQ(1337, new_song.playcount());
   EXPECT_EQ(87, new_song.score());
 }