Restore invalid cover image hashes

src/library/basesqltablemodel.cpp

@@ -943,16 +943,20 @@ TrackPointer BaseSqlTableModel::getTrack(const QModelIndex& index) const {
     return m_pTrackCollectionManager->internalCollection()->getTrackById(getTrackId(index));
 }
 
+TrackPointer BaseSqlTableModel::getTrackByRef(
+        const TrackRef& trackRef) const {
+    return m_pTrackCollectionManager->internalCollection()->getTrackByRef(trackRef);
+}
+
 QString BaseSqlTableModel::getTrackLocation(const QModelIndex& index) const {
     if (!index.isValid()) {
-        return "";
+        return QString();
     }
     QString nativeLocation =
             index.sibling(index.row(),
                     fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_NATIVELOCATION))
                             .data().toString();
-    QString location = QDir::fromNativeSeparators(nativeLocation);
-    return location;
+    return QDir::fromNativeSeparators(nativeLocation);
 }
 
 void BaseSqlTableModel::trackLoaded(QString group, TrackPointer pTrack) {

src/library/basesqltablemodel.h

@@ -66,6 +66,7 @@ class BaseSqlTableModel : public QAbstractTableModel, public TrackModel {
     ///////////////////////////////////////////////////////////////////////////
     bool isColumnHiddenByDefault(int column) override;
     TrackPointer getTrack(const QModelIndex& index) const override;
+    TrackPointer getTrackByRef(const TrackRef& trackRef) const override;
     TrackId getTrackId(const QModelIndex& index) const override;
     const QLinkedList<int> getTrackRows(TrackId trackId) const override {
         return m_trackIdToRows.value(trackId);

src/library/browse/browsetablemodel.cpp

@@ -154,13 +154,16 @@ void BrowseTableModel::setPath(const MDir& path) {
 }
 
 TrackPointer BrowseTableModel::getTrack(const QModelIndex& index) const {
-    QString trackLocation = getTrackLocation(index);
-    if (m_pRecordingManager->getRecordingLocation() == trackLocation) {
+    return getTrackByRef(TrackRef::fromFileInfo(getTrackLocation(index)));
+}
+
+TrackPointer BrowseTableModel::getTrackByRef(const TrackRef& trackRef) const {
+    if (m_pRecordingManager->getRecordingLocation() == trackRef.getLocation()) {
         QMessageBox::critical(
             0, tr("Mixxx Library"),
             tr("Could not load the following file because"
                " it is in use by Mixxx or another application.")
-            + "\n" + trackLocation);
+            + "\n" + trackRef.getLocation());
         return TrackPointer();
     }
     // NOTE(uklotzde, 2015-12-08): Accessing tracks from the browse view
@@ -171,8 +174,7 @@ TrackPointer BrowseTableModel::getTrack(const QModelIndex& index) const {
     // them edit the tracks in a way that persists across sessions
     // and we didn't want to edit the files on disk by default
     // unless the user opts in to that.
-    return m_pTrackCollectionManager->getOrAddTrack(
-            TrackRef::fromFileInfo(trackLocation));
+    return m_pTrackCollectionManager->getOrAddTrack(trackRef);
 }
 
 QString BrowseTableModel::getTrackLocation(const QModelIndex& index) const {

src/library/browse/browsetablemodel.h

@@ -51,6 +51,7 @@ class BrowseTableModel final : public QStandardItemModel, public virtual TrackMo
     void setPath(const MDir& path);
 
     TrackPointer getTrack(const QModelIndex& index) const override;
+    TrackPointer getTrackByRef(const TrackRef& trackRef) const override;
     TrackModel::CapabilitiesFlags getCapabilities() const override;
 
     QString getTrackLocation(const QModelIndex& index) const override;

src/library/coverart.cpp

@@ -1,6 +1,5 @@
-#include <QtDebug>
-
 #include "library/coverart.h"
+
 #include "library/coverartutils.h"
 #include "util/debug.h"
 #include "util/logger.h"
@@ -98,8 +97,12 @@ QImage CoverInfo::loadImage(
                 TrackFile(trackLocation),
                 pTrackLocationToken);
     } else if (type == CoverInfo::FILE) {
-        VERIFY_OR_DEBUG_ASSERT(!trackLocation.isEmpty()) {
-            kLogger.warning()
+        // NOTE(uklotzde): I have not idea when the trackLocation might ever
+        // become empty here?? But there is a test especially for this case!
+        // We cannot use VERIFY_OR_DEBUG_ASSERT unless someone is able to
+        // rule out this strange use case.
+        if (trackLocation.isEmpty()) {
+            kLogger.debug()
                     << "loadImage"
                     << type
                     << "cover with empty trackLocation."
@@ -134,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) &&

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;
 };