Fix possible freeze on clip job deletion, ensure jobs are deleted when completed

src/bin/bin.cpp

@@ -212,10 +212,12 @@ public:
                 if (opt.decorationSize.height() > 0) {
                     r.setWidth(r.height() * pCore->getCurrentDar());
                     QPixmap pix = opt.icon.pixmap(opt.icon.actualSize(r.size()));
-                    // Draw icon
-                    decoWidth += r.width() + textMargin;
-                    r.setWidth(r.height() * pix.width() / pix.height());
-                    painter->drawPixmap(r, pix, QRect(0, 0, pix.width(), pix.height()));
+                    if (!pix.isNull()) {
+                        // Draw icon
+                        decoWidth += r.width() + textMargin;
+                        r.setWidth(r.height() * pix.width() / pix.height());
+                        painter->drawPixmap(r, pix, QRect(0, 0, pix.width(), pix.height()));
+                    }
                     m_thumbRect = r;
                 }
                 int mid = (int)((r1.height() / 2));
@@ -3756,6 +3758,20 @@ void Bin::reloadAllProducers()
     }
 }
 
+void Bin::checkAudioThumbs()
+{
+    if (!KdenliveSettings::audiothumbnails() || m_itemModel->getRootFolder() == nullptr || m_itemModel->getRootFolder()->childCount() == 0 || !isEnabled()) {
+        return;
+    }
+    QList<std::shared_ptr<ProjectClip>> clipList = m_itemModel->getRootFolder()->childClips();
+    for (auto clip : clipList) {
+        ClipType::ProducerType type = clip->clipType();
+        if (type == ClipType::AV || type == ClipType::Audio || type == ClipType::Playlist || type == ClipType::Unknown) {
+            pCore->jobManager()->startJob<AudioThumbJob>({clip->clipId()}, -1, QString());
+        }
+    }
+}
+
 void Bin::slotMessageActionTriggered()
 {
     m_infoMessage->animatedHide();

src/bin/bin.h

@@ -256,6 +256,8 @@ public:
     bool isEmpty() const;
     /** @brief Trigger reload of all clips. */
     void reloadAllProducers();
+    /** @brief Ensure all audio thumbs have been created */
+    void checkAudioThumbs();
     /** @brief Get usage stats for project bin. */
     void getBinStats(uint *used, uint *unused, qint64 *usedSize, qint64 *unusedSize);
     /** @brief Returns the clip properties dockwidget. */

src/bin/projectclip.cpp

@@ -197,6 +197,9 @@ QString ProjectClip::getXmlProperty(const QDomElement &producer, const QString &
 
 void ProjectClip::updateAudioThumbnail(const QVector<uint8_t> audioLevels)
 {
+    if (!KdenliveSettings::audiothumbnails()) {
+        return;
+    }
     audioFrameCache = audioLevels;
     m_audioThumbCreated = true;
     updateTimelineClips({TimelineModel::ReloadThumbRole});
@@ -357,6 +360,9 @@ QDomElement ProjectClip::toXml(QDomDocument &document, bool includeMeta, bool in
 
 void ProjectClip::setThumbnail(const QImage &img)
 {
+    if (img.isNull()) {
+        return;
+    }
     QPixmap thumb = roundedPixmap(QPixmap::fromImage(img));
     if (hasProxy() && !thumb.isNull()) {
         // Overlay proxy icon

src/bin/projectsubclip.cpp

@@ -145,6 +145,9 @@ std::shared_ptr<ProjectSubClip> ProjectSubClip::subClip(int in, int out)
 
 void ProjectSubClip::setThumbnail(const QImage &img)
 {
+    if (img.isNull()) {
+        return;
+    }
     QPixmap thumb = roundedPixmap(QPixmap::fromImage(img));
     int duration = m_parentDuration;
     double factor = ((double) thumb.width()) / duration;

src/doc/kthumb.cpp

@@ -117,7 +117,7 @@ QImage KThumb::getFrame(Mlt::Frame *frame, int width, int height, int scaledWidt
         if (scaledWidth == 0 || scaledWidth == width) {
             return temp.rgbSwapped();
         }
-        return temp.rgbSwapped().scaled(scaledWidth, height);
+        return temp.rgbSwapped().scaled(scaledWidth, height == 0 ? oh : height);
     }
     return QImage();
 }

src/jobs/abstractclipjob.cpp

@@ -63,3 +63,4 @@ AbstractClipJob::JOBTYPE AbstractClipJob::jobType() const
 {
     return m_jobType;
 }
+

src/jobs/audiothumbjob.cpp

@@ -119,6 +119,7 @@ bool AudioThumbJob::computeWithFFMPEG()
     }
     m_ffmpegProcess.reset(new QProcess);
     if (!m_thumbInCache) {
+        // Generate thumbnail used in monitor overlay
         QStringList args;
         args << QStringLiteral("-hide_banner") << QStringLiteral("-y")<< QStringLiteral("-i") << QUrl::fromLocalFile(filePath).toLocalFile() << QStringLiteral("-filter_complex:a");
         args << QString("showwavespic=s=%1x%2:split_channels=1:scale=cbrt:colors=0xffdddd|0xddffdd").arg(m_thumbSize.width()).arg(m_thumbSize.height());
@@ -137,7 +138,8 @@ bool AudioThumbJob::computeWithFFMPEG()
         m_ffmpegProcess->waitForFinished(-1);
         if (m_ffmpegProcess->exitStatus() != QProcess::CrashExit) {
             m_thumbInCache = true;
-            if (m_dataInCache) {
+            if (m_dataInCache || !KdenliveSettings::audiothumbnails()) {
+                m_binClip->audioThumbReady();
                 m_done = true;
                 return true;
             } else {
@@ -146,7 +148,8 @@ bool AudioThumbJob::computeWithFFMPEG()
             }
         }
     }
-    if (!m_dataInCache && !m_done) {
+    if (!m_dataInCache && !m_done && KdenliveSettings::audiothumbnails()) {
+        // Generate timeline audio thumbnail data
         m_audioLevels.clear();
         std::vector<std::unique_ptr<QTemporaryFile>> channelFiles;
         for (int i = 0; i < m_channels; i++) {
@@ -262,6 +265,10 @@ bool AudioThumbJob::computeWithFFMPEG()
             return true;
         }
     }
+    if (!KdenliveSettings::audiothumbnails()) {
+        // We only wanted the thumb generation
+        return true;
+    }
     QString err = m_ffmpegProcess->readAllStandardError();
     m_ffmpegProcess.reset();
     // m_errorMessage += err;
@@ -345,13 +352,13 @@ bool AudioThumbJob::startJob()
     if (ThumbnailCache::get()->hasThumbnail(m_clipId, -1, false)) {
         m_thumbInCache = true;
     }
-    if (m_thumbInCache && m_dataInCache) {
+    if (m_thumbInCache && (m_dataInCache || !KdenliveSettings::audiothumbnails())) {
         m_done = true;
         m_successful = true;
         return true;
     }
 
-    bool ok = m_binClip->clipType() == ClipType::Playlist ? false : computeWithFFMPEG();
+    bool ok = m_binClip->clipType() == ClipType::Playlist ? (KdenliveSettings::audiothumbnails() ? false : true) : computeWithFFMPEG();
     ok = ok ? ok : computeWithMlt();
     Q_ASSERT(ok == m_done);
 
@@ -378,7 +385,7 @@ bool AudioThumbJob::startJob()
         image.save(m_cachePath);
         m_successful = true;
         return true;
-    } else if (ok && m_thumbInCache && m_done) {
+    } else if (ok && m_thumbInCache && (m_done || !KdenliveSettings::audiothumbnails())) {
         m_successful = true;
         return true;
     }
@@ -400,12 +407,18 @@ bool AudioThumbJob::commitResult(Fun &undo, Fun &redo)
         return false;
     }
     QVector <uint8_t>old = m_binClip->audioFrameCache;
-    QImage oldImage = m_binClip->thumbnail(m_thumbSize.width(), m_thumbSize.height()).toImage();
-    QImage result = ThumbnailCache::get()->getAudioThumbnail(m_clipId);
+    QImage oldImage;
+    QImage result;
+    if (m_binClip->clipType() == ClipType::Audio) {
+        oldImage = m_binClip->thumbnail(m_thumbSize.width(), m_thumbSize.height()).toImage();
+        result = ThumbnailCache::get()->getAudioThumbnail(m_clipId);
+    }
 
     // note that the image is moved into lambda, it won't be available from this class anymore
     auto operation = [clip = m_binClip, audio = std::move(m_audioLevels), image = std::move(result)]() {
-        clip->updateAudioThumbnail(audio);
+        if (!audio.isEmpty()) {
+            clip->updateAudioThumbnail(audio);
+        }
         if (!image.isNull() && clip->clipType() == ClipType::Audio) {
             clip->setThumbnail(image);
         }

src/jobs/cachejob.cpp

@@ -30,15 +30,15 @@
 #include "utils/thumbnailcache.hpp"
 #include <QImage>
 #include <QScopedPointer>
+#include <QThread>
 #include <mlt++/MltProducer.h>
 
 #include <set>
 
 CacheJob::CacheJob(const QString &binId, int thumbsCount, int inPoint, int outPoint)
     : AbstractClipJob(CACHEJOB, binId)
-    , m_imageHeight(pCore->thumbProfile()->height())
-    , m_imageWidth(pCore->thumbProfile()->width())
-    , m_fullWidth(m_imageHeight * pCore->getCurrentDar() + 0.5)
+    , m_fullWidth(qFuzzyCompare(pCore->getCurrentSar(), 1.0) ? 0 : pCore->thumbProfile()->height() * pCore->getCurrentDar() + 0.5)
+    , m_semaphore(1)
     , m_done(false)
     , m_thumbsCount(thumbsCount)
     , m_inPoint(inPoint)
@@ -48,11 +48,13 @@ CacheJob::CacheJob(const QString &binId, int thumbsCount, int inPoint, int outPo
     if (m_fullWidth % 2 > 0) {
         m_fullWidth ++;
     }
-    m_imageHeight += m_imageHeight % 2;
     connect(this, &CacheJob::jobCanceled, [&] () {
-        QMutexLocker lk(&m_mutex);
+        if (m_done) {
+            return;
+        }
         m_done = true;
         m_clipId.clear();
+        m_semaphore.acquire();
     });
 }
 
@@ -102,9 +104,8 @@ bool CacheJob::startJob()
         if (m_clipId.isEmpty() || ThumbnailCache::get()->hasThumbnail(m_clipId, i)) {
             continue;
         }
-        m_mutex.lock();
-        if (m_done) {
-            m_mutex.unlock();
+        if (m_done || !m_semaphore.tryAcquire(1)) {
+            m_semaphore.release();
             break;
         }
         m_prod->seek(i);
@@ -113,10 +114,10 @@ bool CacheJob::startJob()
         frame->set("top_field_first", -1);
         frame->set("rescale.interp", "nearest");
         if (frame != nullptr && frame->is_valid()) {
-            QImage result = KThumb::getFrame(frame.data(), m_imageWidth, m_imageHeight, m_fullWidth);
+            QImage result = KThumb::getFrame(frame.data(), 0, 0, m_fullWidth);
             ThumbnailCache::get()->storeThumbnail(m_clipId, i, result, true);
         }
-        m_mutex.unlock();
+        m_semaphore.release(1);
     }
     m_done = true;
     return true;

src/jobs/cachejob.hpp

@@ -23,7 +23,7 @@
 
 #include "abstractclipjob.h"
 
-#include <QMutex>
+#include <QSemaphore>
 #include <memory>
 
 /* @brief This class represents the job that corresponds to computing the thumb of a clip
@@ -54,13 +54,11 @@ public:
     bool commitResult(Fun &undo, Fun &redo) override;
 
 private:
-    int m_imageHeight;
-    int m_imageWidth;
     int m_fullWidth;
 
     std::shared_ptr<ProjectClip> m_binClip;
     std::shared_ptr<Mlt::Producer> m_prod;
-    QMutex m_mutex;
+    QSemaphore m_semaphore;
 
     bool m_done{false};
     int m_thumbsCount;

src/jobs/jobmanager.cpp

@@ -212,7 +212,9 @@ void JobManager::slotCancelJobs()
 {
     QWriteLocker locker(&m_lock);
     for (const auto &j : m_jobs) {
-        j.second->m_processed = true;
+        if (j.second->m_processed) {
+            continue;
+        }
         for (const std::shared_ptr<AbstractClipJob> &job : j.second->m_job) {
             job->jobCanceled();
         }
@@ -222,27 +224,6 @@ void JobManager::slotCancelJobs()
 
 void JobManager::createJob(const std::shared_ptr<Job_t> &job)
 {
-    /*
-    // This thread wait mechanism was broken and caused a race condition locking the application
-    // so I switched to a simpler model
-    bool ok = false;
-    // wait for parents to finish
-    while (!ok) {
-        ok = true;
-        for (int p : parents) {
-            if (!m_jobs[p]->m_completionMutex.tryLock()) {
-                ok = false;
-                qDebug()<<"********\nWAITING FOR JOB COMPLETION MUTEX!!: "<<job->m_id<<" : "<<m_jobs[p]->m_id<<"="<<m_jobs[p]->m_type;
-                break;
-            } else {
-                qDebug()<<">>>>>>>>>>\nJOB COMPLETION MUTEX DONE: "<<job->m_id;
-                m_jobs[p]->m_completionMutex.unlock();
-            }
-        }
-        if (!ok) {
-            QThread::msleep(10);
-        }
-    }*/
     // connect progress signals
     QReadLocker locker(&m_lock);
     for (const auto &it : job->m_indices) {
@@ -256,24 +237,15 @@ void JobManager::createJob(const std::shared_ptr<Job_t> &job)
         });
     }
     connect(&job->m_future, &QFutureWatcher<bool>::started, this, &JobManager::updateJobCount);
-    connect(&job->m_future, &QFutureWatcher<bool>::finished, [this, id = job->m_id]() { slotManageFinishedJob(id); });
+    connect(&job->m_future, &QFutureWatcher<bool>::finished, [this, id = job->m_id]() { if (m_jobs.count(id)> 0) slotManageFinishedJob(id); });
     connect(&job->m_future, &QFutureWatcher<bool>::canceled, [this, id = job->m_id]() { slotManageCanceledJob(id); });
     job->m_actualFuture = QtConcurrent::mapped(job->m_job, AbstractClipJob::execute);
     job->m_future.setFuture(job->m_actualFuture);
-
-    // In the unlikely event that the job finished before the signal connection was made, we check manually for finish and cancel
-    /*if (job->m_future.isFinished()) {
-        //emit job->m_future.finished();
-        slotManageFinishedJob(job->m_id);
-    }
-    if (job->m_future.isCanceled()) {
-        //emit job->m_future.canceled();
-        slotManageCanceledJob(job->m_id);
-    }*/
 }
 
 void JobManager::slotManageCanceledJob(int id)
 {
+    qDebug() << "################### JOB canceled: " << id;
     QReadLocker locker(&m_lock);
     Q_ASSERT(m_jobs.count(id) > 0);
     if (m_jobs[id]->m_processed) return;
@@ -282,13 +254,17 @@ void JobManager::slotManageCanceledJob(int id)
     // send notification to refresh view
     for (const auto &it : m_jobs[id]->m_indices) {
         pCore->projectItemModel()->onItemUpdated(it.first, AbstractProjectItem::JobStatus);
+        m_jobsByClip.erase(it.first);
+    }
+    if (m_jobsByParents.count(id) > 0) {
+        m_jobsByParents.erase(id);
     }
-    // TODO: delete child jobs
+    m_jobs.erase(id);
     updateJobCount();
 }
 void JobManager::slotManageFinishedJob(int id)
 {
-    qDebug() << "################### JOB finished" << id;
+    qDebug() << "################### JOB finished: " << id;
     QReadLocker locker(&m_lock);
     Q_ASSERT(m_jobs.count(id) > 0);
     if (m_jobs[id]->m_processed) return;
@@ -296,6 +272,9 @@ void JobManager::slotManageFinishedJob(int id)
     // send notification to refresh view
     for (const auto &it : m_jobs[id]->m_indices) {
         pCore->projectItemModel()->onItemUpdated(it.first, AbstractProjectItem::JobStatus);
+        if (m_jobsByClip.count(it.first) > 0) {
+            m_jobsByClip.at(it.first).erase(std::remove(m_jobsByClip.at(it.first).begin(), m_jobsByClip.at(it.first).end(), id), m_jobsByClip.at(it.first).end());
+        }
     }
     bool ok = true;
     for (bool res : m_jobs[id]->m_future.future()) {
@@ -331,6 +310,7 @@ void JobManager::slotManageFinishedJob(int id)
                 }
             }
         }
+        m_jobs.erase(id);
         updateJobCount();
         return;
     }
@@ -344,7 +324,6 @@ void JobManager::slotManageFinishedJob(int id)
     if (!ok) {
         m_jobs[id]->m_failed = true;
         const QString bid = m_jobs.at(id)->m_indices.cbegin()->first;
-        qDebug() << "ERROR: Job " << id << " failed, BID: " << bid<<", TYPE: "<<m_jobs.at(id)->m_type;
         QPair<QString, QString> message = getJobMessageForClip(id, bid);
         qDebug()<<"MESSAGE LOG: "<<message;
         if (!message.first.isEmpty()) {
@@ -368,6 +347,7 @@ void JobManager::slotManageFinishedJob(int id)
         }
         m_jobsByParents.erase(id);
     }
+    m_jobs.erase(id);
     updateJobCount();
 }
 

src/mainwindow.cpp

@@ -2391,6 +2391,7 @@ void MainWindow::slotSwitchVideoThumbs()
 void MainWindow::slotSwitchAudioThumbs()
 {
     KdenliveSettings::setAudiothumbnails(!KdenliveSettings::audiothumbnails());
+    pCore->bin()->checkAudioThumbs();
     m_timelineTabs->showAudioThumbnailsChanged();
     m_buttonAudioThumbs->setChecked(KdenliveSettings::audiothumbnails());
 }