diff --git a/app/render/previewautocacher.cpp b/app/render/previewautocacher.cpp index 375c700b5..c1ba01ce5 100644 --- a/app/render/previewautocacher.cpp +++ b/app/render/previewautocacher.cpp @@ -33,20 +33,17 @@ PreviewAutoCacher::~PreviewAutoCacher() RenderTicketPtr PreviewAutoCacher::GetSingleFrame(const rational &t) { - if (single_frame_render_) { - single_frame_render_->Cancel(); - } + CancelQueuedSingleFrameRender(); - single_frame_render_ = std::make_shared(); - - single_frame_render_->setProperty("time", QVariant::fromValue(t)); - - // Copy because TryRender() might set this to null and we still want to return a handle to this - RenderTicketPtr copy = single_frame_render_; + auto sfr = std::make_shared(); + sfr->Start(); + sfr->setProperty("time", QVariant::fromValue(t)); + // Attempt to queue + single_frame_render_ = sfr; TryRender(); - return copy; + return sfr; } void PreviewAutoCacher::SetPaused(bool paused) @@ -139,7 +136,7 @@ void PreviewAutoCacher::AudioRendered() RenderTicketWatcher* watcher = static_cast(sender()); if (audio_tasks_.contains(watcher)) { - if (!watcher->WasCancelled()) { + if (watcher->HasResult()) { viewer_node_->audio_playback_cache()->WritePCM(audio_tasks_.value(watcher), watcher->Get().value(), watcher->GetTicket()->GetJobTime()); @@ -190,10 +187,7 @@ void PreviewAutoCacher::VideoRendered() RenderTicketWatcher* watcher = static_cast(sender()); if (video_tasks_.contains(watcher)) { - if (watcher->WasCancelled()) { - // We didn't get this hash - currently_caching_hashes_.removeOne(watcher->property("hash").toByteArray()); - } else { + if (watcher->HasResult()) { const QByteArray& hash = video_tasks_.value(watcher); // Download frame in another thread @@ -204,6 +198,9 @@ void PreviewAutoCacher::VideoRendered() watcher->Get().value(), hash, true)); + } else { + // We didn't get this hash + currently_caching_hashes_.removeOne(watcher->property("hash").toByteArray()); } video_tasks_.remove(watcher); @@ -222,7 +219,7 @@ void PreviewAutoCacher::VideoDownloaded() RenderTicketWatcher* watcher = static_cast(sender()); if (video_download_tasks_.contains(watcher)) { - if (!watcher->WasCancelled()) { + if (watcher->HasResult()) { if (watcher->Get().toBool()) { const QByteArray& hash = video_download_tasks_.value(watcher); @@ -237,6 +234,11 @@ void PreviewAutoCacher::VideoDownloaded() video_download_tasks_.remove(watcher); } + // The cacher might be waiting for this job to finish + if (!graph_update_queue_.isEmpty()) { + TryRender(); + } + delete watcher; } @@ -244,7 +246,13 @@ void PreviewAutoCacher::SingleFrameFinished() { RenderTicketWatcher* watcher = static_cast(sender()); RenderTicketPtr passthrough = watcher->property("passthrough").value(); - passthrough->Finish(watcher->GetTicket()->Get(), watcher->GetTicket()->WasCancelled()); + + if (watcher->HasResult()) { + passthrough->Finish(watcher->Get()); + } else { + passthrough->Finish(); + } + single_frame_tasks_.removeOne(watcher); delete watcher; } @@ -346,6 +354,15 @@ void PreviewAutoCacher::UpdateLastSyncedValue() last_update_time_ = QDateTime::currentMSecsSinceEpoch(); } +void PreviewAutoCacher::CancelQueuedSingleFrameRender() +{ + if (single_frame_render_) { + // Signal that this ticket was cancelled with no value + single_frame_render_->Finish(); + single_frame_render_ = nullptr; + } +} + void PreviewAutoCacher::SetPlayhead(const rational &playhead) { cache_range_ = TimeRange(playhead - Config::Current()[QStringLiteral("DiskCacheBehind")].value(), @@ -374,74 +391,26 @@ void PreviewAutoCacher::ClearHashQueue(bool wait) } } -void PreviewAutoCacher::ClearVideoQueue(bool wait) +void PreviewAutoCacher::ClearVideoQueue(bool hard) { - // Copy because tasks that cancel immediately will be automatically removed from the list - auto vt_copy = video_tasks_; - auto sft_copy = single_frame_tasks_; + ClearQueueInternal(video_tasks_, hard, &PreviewAutoCacher::VideoRendered); - for (auto it=vt_copy.cbegin(); it!=vt_copy.cend(); it++) { - it.key()->Cancel(); - } - foreach (RenderTicketWatcher* watcher, sft_copy) { - watcher->Cancel(); - } - if (wait) { - // Re-copy because the above cancels may have deleted these watchers - vt_copy = video_tasks_; - sft_copy = single_frame_tasks_; - - for (auto it=vt_copy.cbegin(); it!=vt_copy.cend(); it++) { - it.key()->WaitForFinished(); - } - foreach (RenderTicketWatcher* watcher, sft_copy) { - watcher->WaitForFinished(); - } - - // If we're waiting, we prioritize clearing the cache. Otherwise, we assume that tasks can still - // finish after this function returns. - video_tasks_.clear(); - single_frame_tasks_.clear(); + if (hard) { + ClearQueueInternal(single_frame_tasks_, hard, &PreviewAutoCacher::SingleFrameFinished); } has_changed_ = true; use_custom_range_ = false; } -void PreviewAutoCacher::ClearAudioQueue(bool wait) +void PreviewAutoCacher::ClearAudioQueue(bool hard) { - // Create a copy because otherwise - auto copy = audio_tasks_; - - for (auto it=copy.cbegin(); it!=copy.cend(); it++) { - it.key()->Cancel(); - } - if (wait) { - copy = audio_tasks_; - for (auto it=copy.cbegin(); it!=copy.cend(); it++) { - it.key()->WaitForFinished(); - } - - audio_tasks_.clear(); - } + ClearQueueInternal(audio_tasks_, hard, &PreviewAutoCacher::AudioRendered); } -void PreviewAutoCacher::ClearVideoDownloadQueue(bool wait) +void PreviewAutoCacher::ClearVideoDownloadQueue(bool hard) { - // Create a copy because otherwise - auto copy = video_download_tasks_; - - for (auto it=copy.cbegin(); it!=copy.cend(); it++) { - it.key()->Cancel(); - } - if (wait) { - copy = video_download_tasks_; - for (auto it=copy.cbegin(); it!=copy.cend(); it++) { - it.key()->WaitForFinished(); - } - - video_download_tasks_.clear(); - } + ClearQueueInternal(video_download_tasks_, hard, &PreviewAutoCacher::VideoDownloaded); } void PreviewAutoCacher::NodeAdded(Node *node) @@ -474,6 +443,7 @@ void PreviewAutoCacher::TryRender() if (!graph_update_queue_.isEmpty()) { if (HasActiveJobs()) { // Still waiting for jobs to finish + qDebug() << "Returning because active jobs still running:" << video_tasks_.size() << video_download_tasks_.size() << audio_tasks_.size() << single_frame_tasks_.size(); return; } @@ -520,8 +490,6 @@ void PreviewAutoCacher::TryRender() connect(watcher, &RenderTicketWatcher::Finished, this, &PreviewAutoCacher::SingleFrameFinished); single_frame_tasks_.append(watcher); - single_frame_render_->Start(); - watcher->SetTicket(RenderManager::instance()->RenderFrame(copied_viewer_node_, copied_color_manager_, single_frame_render_->property("time").value(), @@ -578,8 +546,14 @@ void PreviewAutoCacher::RequeueFrames() } else if (currently_caching_hash) { // Cancel this frame unless it's already started RenderTicketWatcher* watcher = video_tasks_.key(hash); - if (watcher && watcher->HasStarted()) { - watcher->Cancel(); + if (watcher) { + QMutexLocker locker(watcher->GetTicket()->lock()); + + if (!watcher->GetTicket()->IsRunning(false)) { + video_tasks_.remove(watcher); + currently_caching_hashes_.removeOne(hash); + delete watcher; + } } } } @@ -626,7 +600,7 @@ void PreviewAutoCacher::SetViewerNode(ViewerOutput *viewer_node) ClearVideoDownloadQueue(true); // Clear any single frame render that might be queued - single_frame_render_ = nullptr; + CancelQueuedSingleFrameRender(); // No longer caching any hashes currently_caching_hashes_.clear(); @@ -711,4 +685,71 @@ void PreviewAutoCacher::SetViewerNode(ViewerOutput *viewer_node) } } +RenderTicketWatcher* RetrieveFromQueueIterator(QMap::iterator it) +{ + return it.key(); +} + +RenderTicketWatcher* RetrieveFromQueueIterator(QMap::iterator it) +{ + return it.key(); +} + +RenderTicketWatcher* RetrieveFromQueueIterator(QVector::iterator it) +{ + return *it; +} + +void PreviewAutoCacher::ClearQueueRemoveEventInternal(QMap::iterator it) +{ + currently_caching_hashes_.removeOne(it.value()); +} + +void PreviewAutoCacher::ClearQueueRemoveEventInternal(QMap::iterator it) +{ + Q_UNUSED(it) +} + +void PreviewAutoCacher::ClearQueueRemoveEventInternal(QVector::iterator it) +{ + Q_UNUSED(it) +} + +template +void PreviewAutoCacher::ClearQueueInternal(T& list, bool hard, Func member) +{ + for (auto it=list.begin(); it!=list.end(); ) { + RenderTicketWatcher* ticket = RetrieveFromQueueIterator(it); + + QMutexLocker locker(ticket->GetTicket()->lock()); + + bool ticket_is_running = ticket->GetTicket()->IsRunning(false); + + if (hard || !ticket_is_running) { + // Override default signalling + disconnect(ticket, &RenderTicketWatcher::Finished, this, member); + + if (ticket_is_running) { + // If ticket is running, assume we're hard clearing and wait for the ticket to finish + ticket->GetTicket()->WaitForFinished(ticket->GetTicket()->lock()); + } else { + // Just remove the ticket from the queue + RenderManager::instance()->RemoveTicket(ticket->GetTicket()); + } + + // Special functionality for certain queues + ClearQueueRemoveEventInternal(it); + + // Destroy ticket + locker.unlock(); + delete ticket; + + it = list.erase(it); + } else { + // We can't clear this, probably because we're soft clearing and the ticket is currently running + it++; + } + } +} + } diff --git a/app/render/previewautocacher.h b/app/render/previewautocacher.h index f319540d2..576f05fec 100644 --- a/app/render/previewautocacher.h +++ b/app/render/previewautocacher.h @@ -104,6 +104,15 @@ private: void UpdateLastSyncedValue(); + void CancelQueuedSingleFrameRender(); + + template + void ClearQueueInternal(T& list, bool hard, Func member); + + void ClearQueueRemoveEventInternal(QMap::iterator it); + void ClearQueueRemoveEventInternal(QMap::iterator it); + void ClearQueueRemoveEventInternal(QVector::iterator it); + class QueuedJob { public: enum Type { diff --git a/app/render/renderprocessor.cpp b/app/render/renderprocessor.cpp index c6785fdd9..f026cb13a 100644 --- a/app/render/renderprocessor.cpp +++ b/app/render/renderprocessor.cpp @@ -45,12 +45,6 @@ void RenderProcessor::Run() // Depending on the render ticket type, start a job RenderManager::TicketType type = ticket_->property("type").value(); - ticket_->Start(); - - if (ticket_->WasCancelled()) { - return; - } - switch (type) { case RenderManager::kTypeVideo: { @@ -82,13 +76,7 @@ void RenderProcessor::Run() frame_params.set_format(frame_format); } - if (RenderManager::instance()->backend() == RenderManager::kOpenGL - && QOpenGLContext::openGLModuleType() == QOpenGLContext::LibGLES) { - // HACK: From what I can tell, ANGLE only supports texture reading to RGBA - frame_params.set_channel_count(VideoParams::kRGBAChannelCount); - } else if (texture) { - frame_params.set_channel_count(texture->channel_count()); - } + frame_params.set_channel_count(texture ? texture->channel_count() : VideoParams::kRGBChannelCount); FramePtr frame = Frame::Create(); frame->set_timestamp(time); @@ -130,7 +118,7 @@ void RenderProcessor::Run() render_ctx_->DownloadFromTexture(texture.get(), frame->data(), frame->linesize_pixels()); } - ticket_->Finish(QVariant::fromValue(frame), IsCancelled()); + ticket_->Finish(QVariant::fromValue(frame)); break; } case RenderManager::kTypeAudio: @@ -144,7 +132,7 @@ void RenderProcessor::Run() table = GenerateTable(texture_output.node(), texture_output.output(), time); } - ticket_->Finish(table.Get(NodeValue::kSamples), IsCancelled()); + ticket_->Finish(table.Get(NodeValue::kSamples)); break; } case RenderManager::kTypeVideoDownload: @@ -153,12 +141,12 @@ void RenderProcessor::Run() FramePtr frame = ticket_->property("frame").value(); QByteArray hash = ticket_->property("hash").toByteArray(); - ticket_->Finish(FrameHashCache::SaveCacheFrame(cache, hash, frame), false); + ticket_->Finish(FrameHashCache::SaveCacheFrame(cache, hash, frame)); break; } default: // Fail - ticket_->Cancel(); + ticket_->Finish(); } } diff --git a/app/task/render/render.cpp b/app/task/render/render.cpp index 1f6b8e88a..9f327b217 100644 --- a/app/task/render/render.cpp +++ b/app/task/render/render.cpp @@ -202,7 +202,7 @@ bool RenderTask::Render(ColorManager* manager, // Cancel every watcher we created foreach (RenderTicketWatcher* watcher, running_watchers_) { disconnect(watcher, &RenderTicketWatcher::Finished, this, &RenderTask::TicketDone); - watcher->Cancel(); + RenderManager::instance()->RemoveTicket(watcher->GetTicket()); } } diff --git a/app/threading/threadpool.cpp b/app/threading/threadpool.cpp index 9490e2433..38047a5ec 100644 --- a/app/threading/threadpool.cpp +++ b/app/threading/threadpool.cpp @@ -54,6 +54,17 @@ ThreadPool::~ThreadPool() } } +bool ThreadPool::RemoveTicket(RenderTicketPtr ticket) +{ + auto it = std::find(ticket_queue_.begin(), ticket_queue_.end(), ticket); + if (it == ticket_queue_.end()) { + return false; + } + + ticket_queue_.erase(it); + return true; +} + void ThreadPool::AddTicket(RenderTicketPtr ticket, bool prioritize) { if (prioritize) { @@ -72,16 +83,15 @@ void ThreadPool::RunNext() RenderTicketPtr ticket = ticket_queue_.front(); ticket_queue_.pop_front(); - if (!ticket->WasCancelled()) { - ThreadPoolThread* thread = available_threads_.front(); - available_threads_.pop_front(); + ThreadPoolThread* thread = available_threads_.front(); + available_threads_.pop_front(); - // Move ticket to other thread so event processing can occur there - ticket->moveToThread(thread); + // Move ticket to other thread so event processing can occur there + ticket->Start(); + ticket->moveToThread(thread); - // Run the ticket in the thread, which actually just calls our virtual function RunTicket - thread->RunTicket(ticket); - } + // Run the ticket in the thread, which actually just calls our virtual function RunTicket + thread->RunTicket(ticket); } } diff --git a/app/threading/threadpool.h b/app/threading/threadpool.h index 235e8924e..ee1e29b61 100644 --- a/app/threading/threadpool.h +++ b/app/threading/threadpool.h @@ -42,6 +42,8 @@ public: virtual void RunTicket(RenderTicketPtr ticket) const = 0; + bool RemoveTicket(RenderTicketPtr ticket); + public slots: void AddTicket(olive::RenderTicketPtr ticket, bool prioritize = false); diff --git a/app/threading/threadticket.cpp b/app/threading/threadticket.cpp index 250c08345..5dfd29d4c 100644 --- a/app/threading/threadticket.cpp +++ b/app/threading/threadticket.cpp @@ -23,20 +23,37 @@ namespace olive { RenderTicket::RenderTicket() : - started_(false), - finished_(false), - cancelled_(false) + is_running_(false), + has_result_(false), + finish_count_(0) { SetJobTime(); } -void RenderTicket::WaitForFinished() +void RenderTicket::WaitForFinished(QMutex *mutex) +{ + if (is_running_) { + wait_.wait(mutex); + } +} + +void RenderTicket::Start() { QMutexLocker locker(&lock_); - if (!finished_) { - wait_.wait(&lock_); - } + is_running_ = true; + has_result_ = false; + result_.clear(); +} + +void RenderTicket::Finish() +{ + FinishInternal(false, QVariant()); +} + +void RenderTicket::Finish(QVariant result) +{ + FinishInternal(true, result); } QVariant RenderTicket::Get() @@ -48,57 +65,61 @@ QVariant RenderTicket::Get() return result_; } -bool RenderTicket::HasStarted() +void RenderTicket::WaitForFinished() { QMutexLocker locker(&lock_); - return started_; + + WaitForFinished(&lock_); } -bool RenderTicket::IsFinished(bool lock) +bool RenderTicket::IsRunning(bool lock) { if (lock) { lock_.lock(); } - bool finished = finished_; + bool running = is_running_; if (lock) { lock_.unlock(); } - return finished; + return running; } -bool RenderTicket::WasCancelled() +int RenderTicket::GetFinishCount(bool lock) { - QMutexLocker locker(&lock_); - - return cancelled_; -} - -void RenderTicket::Start() -{ - QMutexLocker locker(&lock_); - - if (!started_ && !finished_) { - started_ = true; + if (lock) { + lock_.lock(); } + + int count = finish_count_; + + if (lock) { + lock_.unlock(); + } + + return count; } -void RenderTicket::Finish(QVariant result, bool cancelled) +bool RenderTicket::HasResult() { QMutexLocker locker(&lock_); - if (!started_) { - qWarning() << "Tried to finish a ticket that hadn't started"; - } else if (finished_) { - // Do nothing - return; - } else { - finished_ = true; - cancelled_ = cancelled; + return has_result_; +} +void RenderTicket::FinishInternal(bool has_result, QVariant result) +{ + QMutexLocker locker(&lock_); + + if (!is_running_) { + qWarning() << "Tried to finish ticket that wasn't running"; + } else { + is_running_ = false; + has_result_ = has_result; result_ = result; + finish_count_++; wait_.wakeAll(); @@ -108,23 +129,4 @@ void RenderTicket::Finish(QVariant result, bool cancelled) } } -void RenderTicket::Cancel() -{ - QMutexLocker locker(&lock_); - - if (!finished_) { - cancelled_ = true; - - if (!started_) { - finished_ = true; - - wait_.wakeAll(); - - locker.unlock(); - - emit Finished(); - } - } -} - } diff --git a/app/threading/threadticket.h b/app/threading/threadticket.h index 61364c9db..47f2bb586 100644 --- a/app/threading/threadticket.h +++ b/app/threading/threadticket.h @@ -48,39 +48,91 @@ public: job_time_ = QDateTime::currentMSecsSinceEpoch(); } - void WaitForFinished(); + /** + * @brief Get the ticket's current state + * + * This function is thread safe, unless `lock` is set to false. Then the caller has responsibility + * of locking the mutex before and unlocking after this function is called. + */ + bool IsRunning(bool lock = true); + /** + * @brief Determine how many times ticket has been finished + * + * This function is thread safe, unless `lock` is set to false. Then the caller has responsibility + * of locking the mutex before and unlocking after this function is called. + */ + int GetFinishCount(bool lock = true); + + /** + * @brief Check if this ticket has a result + * + * If this ticket is running, this will always return false. + */ + bool HasResult(); + + /** + * @brief Get value, if any + */ QVariant Get(); - bool HasStarted(); - - bool IsFinished(bool lock = true); - - bool WasCancelled(); + /** + * @brief Wait for ticket to be finished + * + * If this ticket is not running, this function returns immediately. + */ + void WaitForFinished(); + void WaitForFinished(QMutex* mutex); + /** + * @brief Access this ticket's mutex + * + * Use if you're doing several operations on a ticket and need to ensure thread safety while + * doing so. Most of the time this isn't necessary since all functions are thread safe by default. + */ QMutex* lock() { return &lock_; } + /** + * @brief Signal to the ticket that it is running + * + * If any value is set, it is cleared. + */ void Start(); - void Finish(QVariant result, bool cancelled); + /** + * @brief Finish ticket with no value + * + * Sets ticket to no longer running and assume it has received no result. + */ + void Finish(); - void Cancel(); + /** + * @brief Finish ticket with value + * + * Sets ticket to no longer running and provide a value generated by the operation requested. + */ + void Finish(QVariant result); signals: + /** + * @brief Emitted when finish has been called by any means (either cancelled or with a result) + */ void Finished(); private: - bool started_; + void FinishInternal(bool has_result, QVariant result); - bool finished_; - - bool cancelled_; + bool is_running_; QVariant result_; + bool has_result_; + + int finish_count_; + QMutex lock_; QWaitCondition wait_; diff --git a/app/threading/threadticketwatcher.cpp b/app/threading/threadticketwatcher.cpp index 18b3ac8eb..495f45721 100644 --- a/app/threading/threadticketwatcher.cpp +++ b/app/threading/threadticketwatcher.cpp @@ -42,38 +42,22 @@ void RenderTicketWatcher::SetTicket(RenderTicketPtr ticket) ticket_ = ticket; + // Lock ticket so we can query if it's already finished by the time this code runs QMutexLocker locker(ticket->lock()); - if (ticket_->IsFinished(false)) { + connect(ticket_.get(), &RenderTicket::Finished, this, &RenderTicketWatcher::TicketFinished); + + if (!ticket_->IsRunning(false) && ticket_->GetFinishCount(false) > 0) { + // Ticket has already finished before, so we emit a signal locker.unlock(); - emit Finished(this); - } else { - connect(ticket_.get(), &RenderTicket::Finished, this, &RenderTicketWatcher::TicketFinished); + TicketFinished(); } } -bool RenderTicketWatcher::WasCancelled() +bool RenderTicketWatcher::IsRunning() { if (ticket_) { - return ticket_->WasCancelled(); - } else { - return false; - } -} - -bool RenderTicketWatcher::IsFinished() -{ - if (ticket_) { - return ticket_->IsFinished(); - } else { - return false; - } -} - -bool RenderTicketWatcher::HasStarted() -{ - if (ticket_) { - return ticket_->HasStarted(); + return ticket_->IsRunning(); } else { return false; } @@ -95,10 +79,12 @@ QVariant RenderTicketWatcher::Get() } } -void RenderTicketWatcher::Cancel() +bool RenderTicketWatcher::HasResult() { if (ticket_) { - ticket_->Cancel(); + return ticket_->HasResult(); + } else { + return false; } } diff --git a/app/threading/threadticketwatcher.h b/app/threading/threadticketwatcher.h index b04ce83b8..993b56ab3 100644 --- a/app/threading/threadticketwatcher.h +++ b/app/threading/threadticketwatcher.h @@ -38,26 +38,23 @@ public: void SetTicket(RenderTicketPtr ticket); - void Cancel(); - - bool WasCancelled(); - - bool IsFinished(); - - bool HasStarted(); + bool IsRunning(); void WaitForFinished(); QVariant Get(); + bool HasResult(); + signals: void Finished(RenderTicketWatcher* watcher); private: - void TicketFinished(); - RenderTicketPtr ticket_; +private slots: + void TicketFinished(); + }; } diff --git a/app/widget/viewer/viewer.cpp b/app/widget/viewer/viewer.cpp index fbb2b971a..fbbd89b80 100644 --- a/app/widget/viewer/viewer.cpp +++ b/app/widget/viewer/viewer.cpp @@ -397,7 +397,7 @@ FramePtr ViewerWidget::DecodeCachedImage(const QString &fn, const rational& time void ViewerWidget::DecodeCachedImage(RenderTicketPtr ticket, const QString &fn, const rational& time) const { ticket->Start(); - ticket->Finish(QVariant::fromValue(DecodeCachedImage(fn, time)), false); + ticket->Finish(QVariant::fromValue(DecodeCachedImage(fn, time))); } bool ViewerWidget::ShouldForceWaveform() const @@ -816,7 +816,7 @@ void ViewerWidget::RendererGeneratedFrame() { RenderTicketWatcher* ticket = static_cast(sender()); - if (!ticket->WasCancelled()) { + if (ticket->HasResult()) { FramePtr frame = ticket->Get().value(); if (nonqueue_watchers_.contains(ticket)) { @@ -837,7 +837,7 @@ void ViewerWidget::RendererGeneratedFrameForQueue() { RenderTicketWatcher* watcher = static_cast(sender()); - if (!watcher->WasCancelled()) { + if (watcher->HasResult()) { FramePtr frame = watcher->Get().value(); // Ignore this signal if we've paused now