Prepare CoverArtCache for migration to new hash function

10 files changed, 317 insertions, 154 deletions

diff --git a/src/library/coverart.cpp b/src/library/coverart.cpp
index 27f317d6ba..3b43890ceb 100644
--- a/src/library/coverart.cpp
+++ b/src/library/coverart.cpp
@@ -137,6 +137,32 @@ QImage CoverInfo::loadImage(
     }
 }
 
+bool CoverInfo::refreshImageHash(
+        const QImage& loadedImage,
+        const SecurityTokenPointer& pTrackLocationToken) {
+    if (CoverImageUtils::isValidHash(hash)) {
+        // Trust that a valid hash has been calculated from the
+        // corresponding image. Otherwise we would refresh all
+        // hashes over and over again.
+        return false;
+    }
+    QImage image = loadedImage;
+    if (loadedImage.isNull()) {
+        image = loadImage(pTrackLocationToken);
+    }
+    if (image.isNull() && type != CoverInfo::NONE) {
+        kLogger.warning()
+                << "Resetting cover info"
+                << *this;
+        reset();
+        return true;
+    }
+    hash = CoverImageUtils::calculateHash(image);
+    DEBUG_ASSERT(image.isNull() || CoverImageUtils::isValidHash(hash));
+    DEBUG_ASSERT(!image.isNull() || hash == CoverImageUtils::defaultHash());
+    return true;
+}
+
 bool operator==(const CoverInfo& a, const CoverInfo& b) {
     return static_cast<const CoverInfoRelative&>(a) ==
                     static_cast<const CoverInfoRelative&>(b) &&
diff --git a/src/library/coverart.h b/src/library/coverart.h
index 9a79ecc344..d9179c3d08 100644
--- a/src/library/coverart.h
+++ b/src/library/coverart.h
@@ -52,6 +52,13 @@ class CoverInfoRelative {
     CoverInfoRelative(CoverInfoRelative&&) = default;
     CoverInfoRelative& operator=(CoverInfoRelative&&) = default;
 
+    /*non-virtual*/ void reset() {
+        // Slicing when invoked from a subclass is intended!
+        // Only the contents of this base class are supposed
+        // to be reset, i.e. trackLocation should be preserved.
+        *this = CoverInfoRelative();
+    }
+
     Source source;
     Type type;
     QString coverLocation; // relative path, from track location
@@ -81,6 +88,14 @@ class CoverInfo : public CoverInfoRelative {
     QImage loadImage(
             const SecurityTokenPointer& pTrackLocationToken = SecurityTokenPointer()) const;
 
+    // Verify the image hash and update it if necessary.
+    // If the corresponding image has already been loaded it
+    // could be provided as a parameter to avoid reloading
+    // if actually needed.
+    bool refreshImageHash(
+            const QImage& loadedImage = QImage(),
+            const SecurityTokenPointer& pTrackLocationToken = SecurityTokenPointer());
+
     QString trackLocation;
 };
 
diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp
index d518d111f8..019afdb0dd 100644
--- a/src/library/coverartcache.cpp
+++ b/src/library/coverartcache.cpp
@@ -13,6 +13,15 @@ namespace {
 
 mixxx::Logger kLogger("CoverArtCache");
 
+// The initial QPixmapCache limit is 10MB.
+// But it is not used just by the coverArt stuff,
+// it is also used by Qt to handle other things behind the scenes.
+// Consequently coverArt cache will always have less than those
+// 10MB available to store the pixmaps.
+// So, we must increase this size a bit more,
+// in order to allow CoverCache handle more covers (performance gain).
+constexpr int kPixmapCacheLimit = 20480;
+
 QString pixmapCacheKey(quint16 hash, int width) {
     return QString("CoverArtCache_%1_%2")
             .arg(QString::number(hash), QString::number(width));
@@ -26,47 +35,69 @@ inline QImage resizeImageWidth(const QImage& image, int width) {
     return image.scaledToWidth(width, kTransformationMode);
 }
 
-const bool sDebug = false;
-
 } // anonymous namespace
 
 CoverArtCache::CoverArtCache() {
-    // The initial QPixmapCache limit is 10MB.
-    // But it is not used just by the coverArt stuff,
-    // it is also used by Qt to handle other things behind the scenes.
-    // Consequently coverArt cache will always have less than those
-    // 10MB available to store the pixmaps.
-    // So, we must increase this size a bit more,
-    // in order to allow CoverCache handle more covers (performance gain).
-    QPixmapCache::setCacheLimit(20480);
+    QPixmapCache::setCacheLimit(kPixmapCacheLimit);
 }
 
-CoverArtCache::~CoverArtCache() {
-    qDebug() << "~CoverArtCache()";
+//static
+void CoverArtCache::requestCover(
+        const QObject* pRequestor,
+        const CoverInfo& coverInfo,
+        const TrackPointer& pTrack) {
+    CoverArtCache* pCache = CoverArtCache::instance();
+    VERIFY_OR_DEBUG_ASSERT(pCache) {
+        return;
+    }
+    pCache->tryLoadCover(
+            pRequestor,
+            pTrack,
+            coverInfo,
+            0, // original size
+            Loading::Default);
 }
 
-QPixmap CoverArtCache::requestCover(const CoverInfo& requestInfo,
-                                    const QObject* pRequestor,
-                                    const int desiredWidth,
-                                    const bool onlyCached,
-                                    const bool signalWhenDone) {
-    if (sDebug) {
-        kLogger.debug() << "requestCover"
-                 << requestInfo << pRequestor <<
-                desiredWidth << onlyCached << signalWhenDone;
+//static
+void CoverArtCache::requestTrackCover(
+        const QObject* pRequestor,
+        const TrackPointer& pTrack) {
+    VERIFY_OR_DEBUG_ASSERT(pTrack) {
+        return;
+    }
+    requestCover(
+            pRequestor,
+            pTrack->getCoverInfoWithLocation(),
+            pTrack);
+}
+
+QPixmap CoverArtCache::tryLoadCover(
+        const QObject* pRequestor,
+        const TrackPointer& pTrack,
+        const CoverInfo& coverInfo,
+        int desiredWidth,
+        Loading loading) {
+    if (kLogger.traceEnabled()) {
+        kLogger.trace()
+                << "requestCover"
+                << pRequestor
+                << coverInfo
+                << desiredWidth
+                << loading;
     }
+    DEBUG_ASSERT(!pTrack ||
+                pTrack->getLocation() == coverInfo.trackLocation);
 
-    if (requestInfo.type == CoverInfo::NONE) {
-        if (signalWhenDone) {
-            emit coverFound(pRequestor, requestInfo,
-                            QPixmap(), true);
+    if (coverInfo.type == CoverInfo::NONE) {
+        if (loading == Loading::Default) {
+            emit coverFound(pRequestor, coverInfo, QPixmap(), coverInfo.hash, false);
         }
         return QPixmap();
     }
 
     // keep a list of trackIds for which a future is currently running
     // to avoid loading the same picture again while we are loading it
-    QPair<const QObject*, quint16> requestId = qMakePair(pRequestor, requestInfo.hash);
+    QPair<const QObject*, quint16> requestId = qMakePair(pRequestor, coverInfo.hash);
     if (m_runningRequests.contains(requestId)) {
         return QPixmap();
     }
@@ -76,35 +107,44 @@ QPixmap CoverArtCache::requestCover(const CoverInfo& requestInfo,
     // column). It's very important to keep the cropped covers in cache because
     // it avoids having to rescale+crop it ALWAYS (which brings a lot of
     // performance issues).
-    QString cacheKey = pixmapCacheKey(requestInfo.hash, desiredWidth);
+    QString cacheKey = pixmapCacheKey(coverInfo.hash, desiredWidth);
 
     QPixmap pixmap;
     if (QPixmapCache::find(cacheKey, &pixmap)) {
-        if (sDebug) {
-            kLogger.debug() << "CoverArtCache::requestCover cover found in cache" << requestInfo << signalWhenDone;
+        if (kLogger.traceEnabled()) {
+            kLogger.trace()
+                    << "requestCover cache hit"
+                    << coverInfo
+                    << loading;
         }
-        if (signalWhenDone) {
-            emit coverFound(pRequestor, requestInfo, pixmap, true);
+        if (loading == Loading::Default) {
+            emit coverFound(pRequestor, coverInfo, pixmap, coverInfo.hash, false);
         }
         return pixmap;
     }
 
-    if (onlyCached) {
-        if (sDebug) {
-            kLogger.debug() << "requestCover cache miss";
+    if (loading == Loading::CachedOnly) {
+        if (kLogger.traceEnabled()) {
+            kLogger.trace() << "requestCover cache miss";
         }
         return QPixmap();
     }
 
-    if (sDebug) {
-        kLogger.debug() << "CoverArtCache::requestCover starting future for" << requestInfo;
+    if (kLogger.traceEnabled()) {
+        kLogger.trace()
+                << "requestCover starting future for"
+                << coverInfo;
     }
     m_runningRequests.insert(requestId);
     // The watcher will be deleted in coverLoaded()
     QFutureWatcher<FutureResult>* watcher = new QFutureWatcher<FutureResult>(this);
     QFuture<FutureResult> future = QtConcurrent::run(
-            this, &CoverArtCache::loadCover, requestInfo, pRequestor,
-            desiredWidth, signalWhenDone);
+            &CoverArtCache::loadCover,
+            pRequestor,
+            pTrack,
+            coverInfo,
+            desiredWidth,
+            loading == Loading::Default);
     connect(watcher,
             &QFutureWatcher<FutureResult>::finished,
             this,
@@ -114,43 +154,48 @@ QPixmap CoverArtCache::requestCover(const CoverInfo& requestInfo,
 }
 
 //static
-void CoverArtCache::requestCover(const Track& track,
-                         const QObject* pRequestor) {
-    CoverArtCache* pCache = CoverArtCache::instance();
-    if (pCache == nullptr) return;
-
-    CoverInfo info = track.getCoverInfoWithLocation();
-    pCache->requestCover(info, pRequestor, 0, false, true);
-}
-
 CoverArtCache::FutureResult CoverArtCache::loadCover(
-        const CoverInfo& info,
         const QObject* pRequestor,
-        const int desiredWidth,
-        const bool signalWhenDone) {
-    if (sDebug) {
-        kLogger.debug() << "loadCover"
-                 << info << desiredWidth << signalWhenDone;
+        TrackPointer pTrack,
+        CoverInfo coverInfo,
+        int desiredWidth,
+        bool signalWhenDone) {
+    if (kLogger.traceEnabled()) {
+        kLogger.trace()
+                << "loadCover"
+                << coverInfo
+                << desiredWidth
+                << signalWhenDone;
     }
+    DEBUG_ASSERT(!pTrack ||
+            pTrack->getLocation() == coverInfo.trackLocation);
 
-    QImage image = info.loadImage();
-
-    // TODO(XXX) Should we re-hash here? If the cover file (or track metadata)
-    // has changed then info.hash may be incorrect. The fix
-    // will also require noticing a hash mis-match at higher levels and
-    // recording the hash change in the database.
+    FutureResult res;
+    res.pRequestor = pRequestor;
+    res.requestedHash = coverInfo.hash;
+    res.signalWhenDone = signalWhenDone;
+    DEBUG_ASSERT(!res.coverInfoUpdated);
+
+    QImage image = coverInfo.loadImage(
+            pTrack ? pTrack->getSecurityToken() : SecurityTokenPointer());
+
+    // Refresh hash before resizing the original image!
+    res.coverInfoUpdated = coverInfo.refreshImageHash(image);
+    if (pTrack && res.coverInfoUpdated) {
+        kLogger.info()
+                << "Updating cover info of track"
+                << coverInfo.trackLocation;
+        pTrack->setCoverInfo(coverInfo);
+    }
 
-    // Adjust the cover size according to the request or downsize the image for
-    // efficiency.
+    // Resize image to requested size
     if (!image.isNull() && desiredWidth > 0) {
+        // Adjust the cover size according to the request
+        // or downsize the image for efficiency.
         image = resizeImageWidth(image, desiredWidth);
     }
 
-    FutureResult res;
-    res.pRequestor = pRequestor;
-    res.cover = CoverArt(info, image, desiredWidth);
-    res.signalWhenDone = signalWhenDone;
-
+    res.cover = CoverArt(coverInfo, image, desiredWidth);
     return res;
 }
 
@@ -158,14 +203,17 @@ CoverArtCache::FutureResult CoverArtCache::loadCover(
 void CoverArtCache::coverLoaded() {
     FutureResult res;
     {
-        QFutureWatcher<FutureResult>* watcher =
+        QFutureWatcher<FutureResult>* pFutureWatcher =
                 static_cast<QFutureWatcher<FutureResult>*>(sender());
-        res = watcher->result();
-        watcher->deleteLater();
+        VERIFY_OR_DEBUG_ASSERT(pFutureWatcher) {
+            return;
+        }
+        res = pFutureWatcher->result();
+        pFutureWatcher->deleteLater();
     }
 
-    if (sDebug) {
-        kLogger.debug() << "coverLoaded" << res.cover;
+    if (kLogger.traceEnabled()) {
+        kLogger.trace() << "coverLoaded" << res.cover;
     }
 
     // Don't cache full size covers (resizedToWidth = 0)
@@ -186,9 +234,9 @@ void CoverArtCache::coverLoaded() {
         QPixmapCache::insert(cacheKey, pixmap);
     }
 
-    m_runningRequests.remove(qMakePair(res.pRequestor, res.cover.hash));
+    m_runningRequests.remove(qMakePair(res.pRequestor, res.requestedHash));
 
     if (res.signalWhenDone) {
-        emit coverFound(res.pRequestor, res.cover, pixmap, false);
+        emit coverFound(res.pRequestor, res.cover, pixmap, res.requestedHash, res.coverInfoUpdated);
     }
 }
diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h
index b1541b27c4..c89b5c8a63 100644
--- a/src/library/coverartcache.h
+++ b/src/library/coverartcache.h
@@ -1,8 +1,10 @@
-#ifndef COVERARTCACHE_H
-#define COVERARTCACHE_H
+#pragma once
 
 #include <QObject>
+#include <QPair>
 #include <QPixmap>
+#include <QSet>
+#include <QtDebug>
 
 #include "library/coverart.h"
 #include "util/singleton.h"
@@ -11,6 +13,14 @@
 class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
     Q_OBJECT
   public:
+    static void requestCover(
+            const QObject* pRequestor,
+            const CoverInfo& coverInfo,
+            const TrackPointer& pTrack = TrackPointer());
+    static void requestTrackCover(
+            const QObject* pRequestor,
+            const TrackPointer& pTrack);
+
     /* This method is used to request a cover art pixmap.
      *
      * @param pRequestor : an arbitrary pointer (can be any number you'd like,
@@ -23,48 +33,78 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
      *      In this way, the method will just look into CoverCache and return
      *      a Pixmap if it is already loaded in the QPixmapCache.
      */
-    QPixmap requestCover(const CoverInfo& info,
-                         const QObject* pRequestor,
-                         const int desiredWidth,
-                         const bool onlyCached,
-                         const bool signalWhenDone);
-
-    static void requestCover(const Track& track,
-                             const QObject* pRequestor);
+    enum class Loading {
+        CachedOnly,
+        NoSignal,
+        Default, // signal when done
+    };
+    QPixmap tryLoadCover(
+            const QObject* pRequestor,
+            const CoverInfo& info,
+            int desiredWidth = 0, // <= 0: original size
+            Loading loading = Loading::Default) {
+        return tryLoadCover(
+                pRequestor,
+                TrackPointer(),
+                info,
+                desiredWidth,
+                loading);
+    }
 
+    // Only public for testing
     struct FutureResult {
         FutureResult()
-                : pRequestor(NULL),
-                  signalWhenDone(false) {
+                : pRequestor(nullptr),
+                  requestedHash(CoverImageUtils::defaultHash()),
+                  signalWhenDone(false),
+                  coverInfoUpdated(false) {
         }
 
-        CoverArt cover;
         const QObject* pRequestor;
+        quint16 requestedHash;
         bool signalWhenDone;
+
+        CoverArt cover;
+        bool coverInfoUpdated;
     };
+    // Load cover from path indicated in coverInfo. WARNING: This is run in a
+    // worker thread.
+    static FutureResult loadCover(
+            const QObject* pRequestor,
+            TrackPointer pTrack,
+            CoverInfo coverInfo,
+            int desiredWidth,
+            bool emitSignals);
 
-  public slots:
+  private slots:
     // Called when loadCover is complete in the main thread.
     void coverLoaded();
 
   signals:
-    void coverFound(const QObject* requestor,
-                    const CoverInfoRelative& info, QPixmap pixmap, bool fromCache);
+    void coverFound(
+            const QObject* requestor,
+            const CoverInfo& coverInfo,
+            const QPixmap& pixmap,
+            quint16 requestedHash,
+            bool coverInfoUpdated);
 
   protected:
     CoverArtCache();
-    virtual ~CoverArtCache();
+    ~CoverArtCache() override = default;
     friend class Singleton<CoverArtCache>;
 
-    // Load cover from path indicated in coverInfo. WARNING: This is run in a
-    // worker thread.
-    FutureResult loadCover(const CoverInfo& coverInfo,
-                           const QObject* pRequestor,
-                           const int desiredWidth,
-                           const bool emitSignals);
-
   private:
+    QPixmap tryLoadCover(
+            const QObject* pRequestor,
+            const TrackPointer& pTrack,
+            const CoverInfo& info,
+            int desiredWidth,
+            Loading loading);
+
     QSet<QPair<const QObject*, quint16> > m_runningRequests;
 };
 
-#endif // COVERARTCACHE_H
+inline
+QDebug operator<<(QDebug dbg, CoverArtCache::Loading loading) {
+    return dbg << static_cast<int>(loading);
+}
diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp
index 3e31c6cf8b..f9d7c5175e 100644
--- a/src/library/coverartdelegate.cpp
+++ b/src/library/coverartdelegate.cpp
@@ -6,8 +6,14 @@
 #include "library/trackmodel.h"
 #include "widget/wlibrarytableview.h"
 #include "util/compatibility.h"
+#include "util/logger.h"
 #include "util/math.h"
 
+namespace {
+
+const mixxx::Logger kLogger("CoverArtDelegate");
+
+} // anonymous namespace
 
 CoverArtDelegate::CoverArtDelegate(WLibraryTableView* parent)
         : TableItemDelegate(parent),
@@ -71,13 +77,18 @@ void CoverArtDelegate::slotOnlyCachedCoverArt(bool b) {
     }
 }
 
-void CoverArtDelegate::slotCoverFound(const QObject* pRequestor,
-                                      const CoverInfoRelative& info,
-                                      QPixmap pixmap, bool fromCache) {
-    if (pRequestor == this && !pixmap.isNull() && !fromCache) {
-        // qDebug() << "CoverArtDelegate::slotCoverFound" << pRequestor << info
-        //          << pixmap.size();
-        QLinkedList<int> rows = m_hashToRow.take(info.hash);
+void CoverArtDelegate::slotCoverFound(
+        const QObject* pRequestor,
+        const CoverInfo& coverInfo,
+        const QPixmap& pixmap,
+        quint16 requestedHash,
+        bool coverInfoUpdated) {
+    Q_UNUSED(coverInfo);
+    Q_UNUSED(pixmap);
+    Q_UNUSED(coverInfoUpdated);
+    if (pRequestor == this) {
+        const QLinkedList<int> rows =
+                m_hashToRow.take(requestedHash);
         foreach(int row, rows) {
             emit coverReadyForCell(row, m_iCoverColumn);
         }
@@ -87,9 +98,10 @@ void CoverArtDelegate::slotCoverFound(const QObject* pRequestor,
 void CoverArtDelegate::paintItem(QPainter *painter,
                              const QStyleOptionViewItem &option,
                              const QModelIndex &index) const {
-    CoverArtCache* pCache = CoverArtCache::instance();
-    if (pCache == NULL || m_iIdColumn == -1 || m_iCoverSourceColumn == -1 ||
-            m_iCoverTypeColumn == -1 || m_iCoverLocationColumn == -1 ||
+    if (m_iIdColumn < 0 ||
+        m_iCoverSourceColumn == -1 ||
+            m_iCoverTypeColumn == -1 ||
+            m_iCoverLocationColumn == -1 ||
             m_iCoverHashColumn == -1) {
         return;
     }
@@ -112,19 +124,31 @@ void CoverArtDelegate::paintItem(QPainter *painter,
     double scaleFactor = getDevicePixelRatioF(static_cast<QWidget*>(parent()));
     // We listen for updates via slotCoverFound above and signal to
     // BaseSqlTableModel when a row's cover is ready.
-    QPixmap pixmap = pCache->requestCover(info, this, option.rect.width() * scaleFactor,
-                                          m_bOnlyCachedCover, true);
+    CoverArtCache* const pCache = CoverArtCache::instance();
+    VERIFY_OR_DEBUG_ASSERT(pCache) {
+        return;
+    }
+    QPixmap pixmap = pCache->tryLoadCover(
+            this,
+            info,
+            option.rect.width() * scaleFactor,
+            m_bOnlyCachedCover ? CoverArtCache::Loading::CachedOnly : CoverArtCache::Loading::Default);
     if (!pixmap.isNull()) {
+        // Cache hit
         pixmap.setDevicePixelRatio(scaleFactor);
         painter->drawPixmap(option.rect.topLeft(), pixmap);
-    } else if (!m_bOnlyCachedCover) {
+        return;
+    }
+
+    if (m_bOnlyCachedCover) {
+        // We are requesting cache-only covers and got a cache
+        // miss. Record this row so that when we switch to requesting
+        // non-cache we can request an update.
+        m_cacheMissRows.append(index.row());
+    } else {
         // If we asked for a non-cache image and got a null pixmap, then our
-        // request was queued.
+        // request was queued. We cannot use the cover image hash, because this
+        // might be refreshed while loading the image!
         m_hashToRow[info.hash].append(index.row());
-    } else {
-        // Otherwise, we are requesting cache-only covers and got a cache
-        // miss. Record this row so that when we switch to requesting non-cache
-        // we can request an update.
-        m_cacheMissRows.append(index.row());
     }
 }
diff --git a/src/library/coverartdelegate.h b/src/library/coverartdelegate.h
index 026d67bca5..e21da8143f 100644
--- a/src/library/coverartdelegate.h
+++ b/src/library/coverartdelegate.h
@@ -7,7 +7,7 @@
 
 #include "library/tableitemdelegate.h"
 
-class CoverInfoRelative;
+class CoverInfo;
 class TrackModel;
 class WLibraryTableView;
 
@@ -37,9 +37,12 @@ class CoverArtDelegate : public TableItemDelegate {
     // which could bring performance issues.
     void slotOnlyCachedCoverArt(bool b);
 
-    void slotCoverFound(const QObject* pRequestor,
-                        const CoverInfoRelative& info,
-                        QPixmap pixmap, bool fromCache);
+    void slotCoverFound(
+            const QObject* pRequestor,
+            const CoverInfo& coverInfo,
+            const QPixmap& pixmap,
+            quint16 requestedHash,
+            bool coverInfoUpdated);
 
   private:
     QTableView* m_pTableView;
diff --git a/src/library/dlgcoverartfullsize.cpp b/src/library/dlgcoverartfullsize.cpp
index 3a120ad432..1393cf9f44 100644
--- a/src/library/dlgcoverartfullsize.cpp
+++ b/src/library/dlgcoverartfullsize.cpp
@@ -112,21 +112,22 @@ void DlgCoverArtFullSize::slotLoadTrack(TrackPointer pTrack) {
 }
 
 void DlgCoverArtFullSize::slotTrackCoverArtUpdated() {
-    if (m_pLoadedTrack != nullptr) {
-        CoverArtCache::requestCover(*m_pLoadedTrack, this);
+    if (m_pLoadedTrack) {
+        CoverArtCache::requestTrackCover(this, m_pLoadedTrack);
     }
 }
 
-void DlgCoverArtFullSize::slotCoverFound(const QObject* pRequestor,
-                                         const CoverInfoRelative& info, QPixmap pixmap,
-                                         bool fromCache) {
-    Q_UNUSED(info);
-    Q_UNUSED(fromCache);
-
-    if (pRequestor == this && m_pLoadedTrack != nullptr &&
-            m_pLoadedTrack->getCoverHash() == info.hash) {
-        // qDebug() << "DlgCoverArtFullSize::slotCoverFound" << pRequestor << info
-        //          << pixmap.size();
+void DlgCoverArtFullSize::slotCoverFound(
+        const QObject* pRequestor,
+        const CoverInfo& coverInfo,
+        const QPixmap& pixmap,
+        quint16 requestedHash,
+        bool coverInfoUpdated) {
+    Q_UNUSED(requestedHash);
+    Q_UNUSED(coverInfoUpdated);
+    if (pRequestor == this &&
+            m_pLoadedTrack &&
+            m_pLoadedTrack->getLocation() == coverInfo.trackLocation) {
         m_pixmap = pixmap;
         // Scale down dialog if the pixmap is larger than the screen.
         // Use 90% of screen size instead of 100% to prevent an issue with
diff --git a/src/library/dlgcoverartfullsize.h b/src/library/dlgcoverartfullsize.h
index 7be9d57191..7ae2a7ba71 100644
--- a/src/library/dlgcoverartfullsize.h
+++ b/src/library/dlgcoverartfullsize.h
@@ -28,8 +28,12 @@ class DlgCoverArtFullSize
 
   public slots:
     void slotLoadTrack(TrackPointer);
-    void slotCoverFound(const QObject* pRequestor,
-            const CoverInfoRelative& info, QPixmap pixmap, bool fromCache);
+    void slotCoverFound(
+            const QObject* pRequestor,
+            const CoverInfo& coverInfo,
+            const QPixmap& pixmap,
+            quint16 requestedHash,
+            bool coverInfoUpdated);
     void slotTrackCoverArtUpdated();
 
     // slots that handle signals from WCoverArtMenu
diff --git a/src/library/dlgtrackinfo.cpp b/src/library/dlgtrackinfo.cpp
index 64ae4c8b29..45ee553d06 100644
--- a/src/library/dlgtrackinfo.cpp
+++ b/src/library/dlgtrackinfo.cpp
@@ -225,10 +225,7 @@ void DlgTrackInfo::populateFields(const Track& track) {
 
     m_loadedCoverInfo = track.getCoverInfoWithLocation();
     m_pWCoverArtLabel->setCoverArt(m_loadedCoverInfo, QPixmap());
-    CoverArtCache* pCache = CoverArtCache::instance();
-    if (pCache != NULL) {
-        pCache->requestCover(m_loadedCoverInfo, this, 0, false, true);
-    }
+    CoverArtCache::requestCover(this, m_loadedCoverInfo);
 }
 
 void DlgTrackInfo::reloadTrackBeats(const Track& track) {
@@ -274,15 +271,19 @@ void DlgTrackInfo::loadTrack(TrackPointer pTrack) {
             &DlgTrackInfo::slotTrackChanged);
 }
 
-void DlgTrackInfo::slotCoverFound(const QObject* pRequestor,
-                                  const CoverInfoRelative& info,
-                                  QPixmap pixmap, bool fromCache) {
-    Q_UNUSED(fromCache);
-    if (pRequestor == this && m_pLoadedTrack &&
-            m_loadedCoverInfo.hash == info.hash) {
-        qDebug() << "DlgTrackInfo::slotPixmapFound" << pRequestor << info
-                 << pixmap.size();
-        m_pWCoverArtLabel->setCoverArt(m_loadedCoverInfo, pixmap);
+void DlgTrackInfo::slotCoverFound(
+        const QObject* pRequestor,
+        const CoverInfo& coverInfo,
+        const QPixmap& pixmap,
+        quint16 requestedHash,
+        bool coverInfoUpdated) {
+    Q_UNUSED(requestedHash);
+    Q_UNUSED(coverInfoUpdated);
+    if (pRequestor == this &&
+            m_pLoadedTrack &&
+            m_loadedCoverInfo.trackLocation == coverInfo.trackLocation) {
+        m_loadedCoverInfo = coverInfo;
+        m_pWCoverArtLabel->setCoverArt(coverInfo, pixmap);
     }
 }
 
@@ -301,10 +302,7 @@ void DlgTrackInfo::slotCoverInfoSelected(const CoverInfoRelative& coverInfo) {
         return;
     }
     m_loadedCoverInfo = CoverInfo(coverInfo, m_pLoadedTrack->getLocation());
-    CoverArtCache* pCache = CoverArtCache::instance();
-    if (pCache) {
-        pCache->requestCover(m_loadedCoverInfo, this, 0, false, true);
-    }
+    CoverArtCache::requestCover(this, m_loadedCoverInfo);
 }
 
 void DlgTrackInfo::slotOpenInFileBrowser() {
diff --git a/src/library/dlgtrackinfo.h b/src/library/dlgtrackinfo.h
index 677483ead3..eb721c96c7 100644
--- a/src/library/dlgtrackinfo.h
+++ b/src/library/dlgtrackinfo.h
@@ -61,8 +61,12 @@ class DlgTrackInfo : public QDialog, public Ui::DlgTrackInfo {
     void slotTrackChanged(TrackId trackId);
     void slotOpenInFileBrowser();
 
-    void slotCoverFound(const QObject* pRequestor,
-                        const CoverInfoRelative& info, QPixmap pixmap, bool fromCache);
+    void slotCoverFound(
+            const QObject* pRequestor,
+            const CoverInfo& info,
+            const QPixmap& pixmap,
+            quint16 requestedHash,
+            bool coverInfoUpdated);
     void slotCoverInfoSelected(const CoverInfoRelative& coverInfo);
     void slotReloadCoverArt();