Hiding some of Song's cover art logic complexity in Song's API (namely: embedded and manually unset covers).

src/core/albumcoverloader.cpp

@@ -25,9 +25,6 @@
 #include <QUrl>
 #include <QNetworkReply>
 
-const char* AlbumCoverLoader::kManuallyUnsetCover = "(unset)";
-const char* AlbumCoverLoader::kEmbeddedCover = "(embedded)";
-
 AlbumCoverLoader::AlbumCoverLoader(QObject* parent)
   : QObject(parent),
     stop_requested_(false),
@@ -127,10 +124,10 @@ AlbumCoverLoader::TryLoadResult AlbumCoverLoader::TryLoadImage(
     case State_TryingManual: filename = task.art_manual;    break;
   }
 
-  if (filename == kManuallyUnsetCover)
+  if (filename == Song::kManuallyUnsetCover)
     return TryLoadResult(false, true, default_);
 
-  if (filename == kEmbeddedCover && !task.song_filename.isEmpty()) {
+  if (filename == Song::kEmbeddedCover && !task.song_filename.isEmpty()) {
     QImage taglib_image = Song::LoadEmbeddedArt(task.song_filename);
     if (!taglib_image.isNull())
       return TryLoadResult(false, true, ScaleAndPad(taglib_image));
@@ -217,12 +214,12 @@ QPixmap AlbumCoverLoader::TryLoadPixmap(const QString& automatic,
                                         const QString& manual,
                                         const QString& filename) {
   QPixmap ret;
-  if (manual == kManuallyUnsetCover)
+  if (manual == Song::kManuallyUnsetCover)
     return ret;
   if (!manual.isEmpty())
     ret.load(manual);
   if (ret.isNull()) {
-    if (automatic == kEmbeddedCover && !filename.isNull())
+    if (automatic == Song::kEmbeddedCover && !filename.isNull())
       ret = QPixmap::fromImage(Song::LoadEmbeddedArt(filename));
     else if (!automatic.isEmpty())
       ret.load(automatic);

src/core/albumcoverloader.h

@@ -57,9 +57,6 @@ class AlbumCoverLoader : public QObject {
   static QPixmap TryLoadPixmap(const QString& automatic, const QString& manual,
                                const QString& filename = QString());
 
-  static const char* kManuallyUnsetCover;
-  static const char* kEmbeddedCover;
-
  signals:
   void ImageLoaded(quint64 id, const QImage& image);
   void ImageLoaded(quint64 id, const QImage& scaled, const QImage& original);

src/core/song.cpp

@@ -110,6 +110,9 @@ const QString Song::kFtsColumnSpec = Song::kFtsColumns.join(", ");
 const QString Song::kFtsBindSpec = Prepend(":", Song::kFtsColumns).join(", ");
 const QString Song::kFtsUpdateSpec = Updateify(Song::kFtsColumns).join(", ");
 
+const QString Song::kManuallyUnsetCover = "(unset)";
+const QString Song::kEmbeddedCover = "(embedded)";
+
 QString Song::JoinSpec(const QString& table) {
   return Prepend(table + ".", kColumns).join(", ");
 }
@@ -304,7 +307,7 @@ void Song::InitFromFile(const QString& filename, int directory_id) {
         compilation = TStringToQString(map["TCMP"].front()->toString()).trimmed();
 
       if (!map["APIC"].isEmpty())
-        d->art_automatic_ = AlbumCoverLoader::kEmbeddedCover;
+        set_embedded_cover();
 
       // Find a suitable comment tag.  For now we ignore iTunNORM comments.
       for (int i=0 ; i<map["COMM"].size() ; ++i) {
@@ -423,7 +426,7 @@ void Song::ParseOggTag(const TagLib::Ogg::FieldListMap& map, const QTextCodec* c
     *compilation = TStringToQString( map["COMPILATION"].front() ).trimmed();
   
   if (!map["COVERART"].isEmpty())
-    d->art_automatic_ = AlbumCoverLoader::kEmbeddedCover;
+    set_embedded_cover();
 }
 
 void Song::GuessFileType(TagLib::FileRef* fileref) {

src/core/song.h

@@ -88,6 +88,9 @@ class Song {
   static const QString kFtsBindSpec;
   static const QString kFtsUpdateSpec;
 
+  static const QString kManuallyUnsetCover;
+  static const QString kEmbeddedCover;
+
   static QString JoinSpec(const QString& table);
 
   // Don't change these values - they're stored in the database
@@ -198,6 +201,18 @@ class Song {
   const QString& art_automatic() const { return d->art_automatic_; }
   const QString& art_manual() const { return d->art_manual_; }
 
+  // Returns true if this Song had it's cover manually unset by user.
+  bool has_manually_unset_cover() const { return d->art_manual_ == kManuallyUnsetCover; }
+  // This method represents an explicit request to unset this song's
+  // cover.
+  void manually_unset_cover() { d->art_manual_ = kManuallyUnsetCover; }
+
+  // Returns true if this song (it's media file) has an embedded cover.
+  bool has_embedded_cover() const { return d->art_automatic_ == kEmbeddedCover; }
+  // Sets a flag saying that this song (it's media file) has an embedded
+  // cover.
+  void set_embedded_cover() { d->art_automatic_ = kEmbeddedCover; }
+
   const QImage& image() const { return d->image_; }
 
   // Pretty accessors

src/library/librarywatcher.cpp

@@ -17,7 +17,6 @@
 
 #include "librarywatcher.h"
 #include "librarybackend.h"
-#include "core/albumcoverloader.h"
 #include "core/taskmanager.h"
 #include "playlistparsers/cueparser.h"
 
@@ -321,7 +320,7 @@ void LibraryWatcher::ScanSubdirectory(
       QString image = ImageForSong(file, album_art);
       if ((matching_song.art_automatic().isEmpty() && !image.isEmpty()) ||
           (!matching_song.art_automatic().isEmpty()
-           && matching_song.art_automatic() != AlbumCoverLoader::kEmbeddedCover
+           && !matching_song.has_embedded_cover()
            && !QFile::exists(matching_song.art_automatic()))) {
         changed = true;
       }
@@ -499,7 +498,7 @@ void LibraryWatcher::PreserveUserSetData(const QString& file, const QString& ima
   // Previous versions of Clementine incorrectly overwrote this and
   // stored it in the DB, so we can't rely on matching_song to
   // know if it has embedded artwork or not, but we can check here.
-  if (out->art_automatic() != AlbumCoverLoader::kEmbeddedCover)
+  if (!out->has_embedded_cover())
     out->set_art_automatic(image);
 
   out->set_playcount(matching_song.playcount());

src/ui/albumcoverchoicecontroller.cpp

@@ -87,7 +87,7 @@ void AlbumCoverChoiceController::SetLibrary(LibraryBackend* library) {
 QString AlbumCoverChoiceController::LoadCoverFromFile(Song* song) {
   QString dir;
 
-  if (!song->art_automatic().isEmpty() && song->art_automatic() != AlbumCoverLoader::kEmbeddedCover) {
+  if (!song->art_automatic().isEmpty() && !song->has_embedded_cover()) {
     dir = song->art_automatic();
   } else if (!song->filename().isEmpty() && song->filename().contains('/')) {
     // we get rid of the filename because it's extension is screwing with the dialog's
@@ -186,7 +186,7 @@ QString AlbumCoverChoiceController::SearchForCover(Song* song) {
 }
 
 QString AlbumCoverChoiceController::UnsetCover(Song* song) {
-  QString cover = AlbumCoverLoader::kManuallyUnsetCover;
+  QString cover = Song::kManuallyUnsetCover;
   SaveCover(song, cover);
 
   return cover;

src/ui/albumcovermanager.cpp

@@ -537,7 +537,7 @@ void AlbumCoverManager::SaveCoverToFile() {
   QImage image;
 
   // load the image from disk
-  if(song.art_manual() == AlbumCoverLoader::kManuallyUnsetCover) {
+  if(song.has_manually_unset_cover()) {
     image = QImage(":/nocover.png");
   } else {
     if(!song.art_manual().isEmpty() && QFile::exists(song.art_manual())) {

src/ui/edittagdialog.cpp

@@ -405,12 +405,12 @@ void EditTagDialog::UpdateSummaryTab(const Song& song) {
   QString summary = "<b>" + Qt::escape(song.PrettyTitleWithArtist()) + "</b><br/>";
 
   bool art_is_set = true;
-  if (song.art_manual() == AlbumCoverLoader::kManuallyUnsetCover) {
+  if (song.has_manually_unset_cover()) {
     summary += Qt::escape(tr("Cover art manually unset"));
     art_is_set = false;
   } else if (!song.art_manual().isEmpty()) {
     summary += Qt::escape(tr("Cover art set from %1").arg(song.art_manual()));
-  } else if (song.art_automatic() == AlbumCoverLoader::kEmbeddedCover) {
+  } else if (song.has_embedded_cover()) {
     summary += Qt::escape(tr("Cover art from embedded image"));
   } else if (!song.art_automatic().isEmpty()) {
     summary += Qt::escape(tr("Cover art loaded automatically from %1").arg(song.art_manual()));

src/widgets/nowplayingwidget.cpp

@@ -377,8 +377,7 @@ void NowPlayingWidget::contextMenuEvent(QContextMenuEvent* e) {
     album_cover_choice_controller_->search_for_cover_action()->setEnabled(false);
   #endif
 
-    const bool art_is_not_set =
+    const bool art_is_not_set = metadata_.has_manually_unset_cover()
-        metadata_.art_manual() == AlbumCoverLoader::kManuallyUnsetCover
         || (metadata_.art_automatic().isEmpty() && metadata_.art_manual().isEmpty());
 
     album_cover_choice_controller_->unset_cover_action()->setEnabled(!art_is_not_set);