Fix freeze cause by incorrect duplicate entry in thumbnail cache.

Maybe related to #1364

src/bin/projectclip.cpp

@@ -1154,13 +1154,14 @@ QPoint ProjectClip::zone() const
     return ClipController::zone();
 }
 
-const QString ProjectClip::hash(bool createIfEmpty)
+const QString ProjectClip::hash()
 {
     if (m_clipStatus == FileStatus::StatusWaiting) {
+        // Clip is not ready
         return QString();
     }
     QString clipHash = getProducerProperty(QStringLiteral("kdenlive:file_hash"));
-    if (!clipHash.isEmpty() || createIfEmpty) {
+    if (!clipHash.isEmpty()) {
         return clipHash;
     }
     return getFileHash();
@@ -1627,7 +1628,7 @@ const QString ProjectClip::getAudioThumbPath(int stream)
     if (!ok) {
         return QString();
     }
-    const QString clipHash = hash(false);
+    const QString clipHash = hash();
     if (clipHash.isEmpty()) {
         return QString();
     }
@@ -1910,7 +1911,7 @@ void ProjectClip::getThumbFromPercent(int percent, bool storeFrame)
     if (percent < 0) {
         if (hasProducerProperty(QStringLiteral("kdenlive:thumbnailFrame"))) {
             int framePos = qMax(0, getProducerIntProperty(QStringLiteral("kdenlive:thumbnailFrame")));
-            QImage thumb = ThumbnailCache::get()->getThumbnail(hash(false), m_binId, framePos);
+            QImage thumb = ThumbnailCache::get()->getThumbnail(hash(), m_binId, framePos);
             if (!thumb.isNull()) {
                 setThumbnail(thumb, -1, -1);
             }
@@ -1921,7 +1922,7 @@ void ProjectClip::getThumbFromPercent(int percent, bool storeFrame)
     int steps = qCeil(qMax(pCore->getCurrentFps(), double(duration) / 30));
     int framePos = duration * percent / 100;
     framePos -= framePos%steps;
-    QImage thumb = ThumbnailCache::get()->getThumbnail(hash(false), m_binId, framePos);
+    QImage thumb = ThumbnailCache::get()->getThumbnail(hash(), m_binId, framePos);
     if (!thumb.isNull()) {
         setThumbnail(thumb, -1, -1);
     } else {

src/bin/projectclip.h

@@ -144,7 +144,7 @@ public:
     QString getToolTip() const override;
 
     /** @brief The clip hash created from the clip's resource. */
-    const QString hash(bool createIfEmpty = true);
+    const QString hash();
     /** @brief Callculate a file hash from a path. */
     static const QPair<QByteArray, qint64> calculateHash(const QString &path);
 

src/jobs/cachetask.cpp

@@ -97,7 +97,7 @@ void CacheTask::generateThumbnail(std::shared_ptr<ProjectClip>binClip)
                 frame->set("consumer.rescale", "nearest");
 #endif
                 QImage result = KThumb::getFrame(frame.data(), 0, 0, m_fullWidth);
-                if (!result.isNull()) {
+                if (!result.isNull() && !m_isCanceled) {
                     qDebug()<<"==== CACHING FRAME: "<<i;
                     ThumbnailCache::get()->storeThumbnail(clipId, i, result, true);
                 }

src/jobs/cliploadtask.cpp

@@ -232,7 +232,7 @@ void ClipLoadTask::generateThumbnail(std::shared_ptr<ProjectClip>binClip, std::s
     qDebug()<<"===== \nREADY FOR THUMB"<<binClip->clipType()<<"\n\n=========";
     int frameNumber = m_in > -1 ? m_in : qMax(0, binClip->getProducerIntProperty(QStringLiteral("kdenlive:thumbnailFrame")));
     if (producer->get_int("video_index") > -1) {
-        QImage thumb = ThumbnailCache::get()->getThumbnail(binClip->hash(false), QString::number(m_owner.second), frameNumber);
+        QImage thumb = ThumbnailCache::get()->getThumbnail(binClip->hash(), QString::number(m_owner.second), frameNumber);
         if (!thumb.isNull()) {
             // Thumbnail found in cache
             qDebug()<<"=== FOUND THUMB IN CACHe";

src/timeline2/view/qmltypes/thumbnailprovider.cpp

@@ -35,7 +35,7 @@ QImage ThumbnailProvider::requestImage(const QString &id, QSize *size, const QSi
     if (ok) {
         std::shared_ptr<ProjectClip> binClip = pCore->projectItemModel()->getClipByBinID(binId);
         if (binClip) {
-            result = ThumbnailCache::get()->getThumbnail(binClip->hash(false), binId, frameNumber);
+            result = ThumbnailCache::get()->getThumbnail(binClip->hash(), binId, frameNumber);
             if (!result.isNull()) {
                 *size = result.size();
                 return result;

src/utils/thumbnailcache.cpp

@@ -93,7 +93,7 @@ std::unique_ptr<ThumbnailCache> &ThumbnailCache::get()
 
 bool ThumbnailCache::hasThumbnail(const QString &binId, int pos, bool volatileOnly) const
 {
-    QMutexLocker locker(&m_mutex);
+    QReadLocker locker(&m_mutex);
     bool ok = false;
     auto key = pos < 0 ? getAudioKey(binId, &ok).constFirst() : getKey(binId, pos, &ok);
     if (ok && m_volatileCache->contains(key)) {
@@ -108,7 +108,7 @@ bool ThumbnailCache::hasThumbnail(const QString &binId, int pos, bool volatileOn
 
 QImage ThumbnailCache::getAudioThumbnail(const QString &binId, bool volatileOnly) const
 {
-    QMutexLocker locker(&m_mutex);
+    QReadLocker locker(&m_mutex);
     bool ok = false;
     auto key = getAudioKey(binId, &ok).constFirst();
     if (ok && m_volatileCache->contains(key)) {
@@ -127,7 +127,7 @@ QImage ThumbnailCache::getAudioThumbnail(const QString &binId, bool volatileOnly
 
 const QList <QUrl> ThumbnailCache::getAudioThumbPath(const QString &binId) const
 {
-    QMutexLocker locker(&m_mutex);
+    QReadLocker locker(&m_mutex);
     bool ok = false;
     auto key = getAudioKey(binId, &ok);
     QDir thumbFolder = getDir(true, &ok);
@@ -148,7 +148,7 @@ QImage ThumbnailCache::getThumbnail(QString hash, const QString &binId, int pos,
         return QImage();
     }
     hash.append(QString("#%1.jpg").arg(pos));
-    QMutexLocker locker(&m_mutex);
+    QReadLocker locker(&m_mutex);
     if (m_volatileCache->contains(hash)) {
         return m_volatileCache->get(hash);
     }
@@ -168,7 +168,7 @@ QImage ThumbnailCache::getThumbnail(QString hash, const QString &binId, int pos,
 
 QImage ThumbnailCache::getThumbnail(const QString &binId, int pos, bool volatileOnly) const
 {
-    QMutexLocker locker(&m_mutex);
+    QReadLocker locker(&m_mutex);
     bool ok = false;
     auto key = getKey(binId, pos, &ok);
     if (ok && m_volatileCache->contains(key)) {
@@ -189,7 +189,7 @@ QImage ThumbnailCache::getThumbnail(const QString &binId, int pos, bool volatile
 
 void ThumbnailCache::storeThumbnail(const QString &binId, int pos, const QImage &img, bool persistent)
 {
-    QMutexLocker locker(&m_mutex);
+    QWriteLocker locker(&m_mutex);
     bool ok = false;
     const QString key = getKey(binId, pos, &ok);
     if (!ok) {
@@ -204,18 +204,15 @@ void ThumbnailCache::storeThumbnail(const QString &binId, int pos, const QImage
             if (m_storedOnDisk.find(binId) == m_storedOnDisk.end() || std::find(m_storedOnDisk[binId].begin(), m_storedOnDisk[binId].end(), pos) == m_storedOnDisk[binId].end()) {
                 m_storedOnDisk[binId].push_back(pos);
             }
-            // if volatile cache also contains this entry, update it
-            if (m_volatileCache->contains(key)) {
-                m_volatileCache->remove(key);
-            } else {
-                m_storedVolatile[binId].push_back(pos);
-            }
-            m_volatileCache->insert(key, img, (int)img.sizeInBytes());
         }
+    }
+    // if volatile cache also contains this entry, update it
+    if (m_volatileCache->contains(key)) {
+        m_volatileCache->remove(key);
     } else {
-        m_volatileCache->insert(key, img, (int)img.sizeInBytes());
         m_storedVolatile[binId].push_back(pos);
     }
+    m_volatileCache->insert(key, img, (int)img.sizeInBytes());
 }
 
 void ThumbnailCache::saveCachedThumbs(const std::unordered_map<QString, std::vector<int>> &keys)
@@ -225,6 +222,7 @@ void ThumbnailCache::saveCachedThumbs(const std::unordered_map<QString, std::vec
     if (!ok) {
         return;
     }
+    QReadLocker locker(&m_mutex);
     for (auto &key : keys) {
         bool ok;
         for(const auto& pos: key.second) {
@@ -249,7 +247,7 @@ void ThumbnailCache::saveCachedThumbs(const std::unordered_map<QString, std::vec
 
 void ThumbnailCache::invalidateThumbsForClip(const QString &binId)
 {
-    QMutexLocker locker(&m_mutex);
+    QWriteLocker locker(&m_mutex);
     if (m_storedVolatile.find(binId) != m_storedVolatile.end()) {
         bool ok = false;
         for (int pos : m_storedVolatile.at(binId)) {
@@ -279,7 +277,7 @@ void ThumbnailCache::invalidateThumbsForClip(const QString &binId)
 
 void ThumbnailCache::clearCache()
 {
-    QMutexLocker locker(&m_mutex);
+    QWriteLocker locker(&m_mutex);
     m_volatileCache->clear();
     m_storedVolatile.clear();
     m_storedOnDisk.clear();
@@ -293,7 +291,7 @@ QString ThumbnailCache::getKey(const QString &binId, int pos, bool *ok)
         return QString();
     }
     auto binClip = pCore->projectItemModel()->getClipByBinID(binId);
-    *ok = binClip != nullptr;
+    *ok = binClip != nullptr && binClip->statusReady();
     return *ok ? binClip->hash() + QLatin1Char('#') + QString::number(pos) + QStringLiteral(".jpg") : QString();
 }
 

src/utils/thumbnailcache.hpp

@@ -9,7 +9,7 @@
 #include <QDir>
 #include <QUrl>
 #include <QImage>
-#include <QMutex>
+#include <QReadWriteLock>
 #include <memory>
 #include <mutex>
 #include <unordered_map>
@@ -80,7 +80,7 @@ protected:
 
     class Cache_t;
     std::unique_ptr<Cache_t> m_volatileCache;
-    mutable QMutex m_mutex;
+    mutable QReadWriteLock m_mutex;
 
     // the following maps keeps track of the positions that we store for each clip in volatile caches.
     // Note that we don't track deletions due to items dropped from the cache. So the maps can contain more items that are currently stored.