From f6064b40dfe9c855d5fc4f2c0871e0064d56afc1 Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Fri, 6 Dec 2019 19:38:57 +1100 Subject: [PATCH] hold worker busy state in renderbackend rather than in renderworker Workers run in different threads and the backends can poll whether the worker is currently busy or not. However the previous iteration has the worker (and an atomic int) provide the busy state which could easily desync with the main thread (since all workers run in different threads). By holding the busy states in the main thread, the main thread will always be able to poll the busy state accurately. --- app/render/backend/audio/audiobackend.cpp | 2 ++ app/render/backend/opengl/openglbackend.cpp | 28 ++++++++--------- app/render/backend/opengl/openglbackend.h | 2 ++ app/render/backend/renderbackend.cpp | 34 ++++++++++----------- app/render/backend/renderbackend.h | 7 +++-- app/render/backend/renderworker.cpp | 12 -------- app/render/backend/renderworker.h | 6 ---- app/render/backend/videorenderworker.cpp | 6 ---- app/widget/timelinewidget/tool/pointer.cpp | 2 +- 9 files changed, 41 insertions(+), 58 deletions(-) diff --git a/app/render/backend/audio/audiobackend.cpp b/app/render/backend/audio/audiobackend.cpp index f1c5ae55d..19c0cbce2 100644 --- a/app/render/backend/audio/audiobackend.cpp +++ b/app/render/backend/audio/audiobackend.cpp @@ -67,6 +67,8 @@ void AudioBackend::ConnectWorkerToThis(RenderWorker *worker) void AudioBackend::ThreadCompletedCache(NodeDependency dep, NodeValueTable data) { + SetWorkerBusyState(static_cast(sender()), false); + QByteArray cached_samples = data.Get(NodeParam::kSamples).toByteArray(); int offset = params().time_to_bytes(dep.in()); diff --git a/app/render/backend/opengl/openglbackend.cpp b/app/render/backend/opengl/openglbackend.cpp index dae35f97e..120c610f5 100644 --- a/app/render/backend/opengl/openglbackend.cpp +++ b/app/render/backend/opengl/openglbackend.cpp @@ -6,7 +6,8 @@ #include "functions.h" OpenGLBackend::OpenGLBackend(QObject *parent) : - VideoRenderBackend(parent) + VideoRenderBackend(parent), + last_download_thread_(0) { } @@ -135,6 +136,8 @@ bool OpenGLBackend::TimeIsCached(const TimeRange &time) void OpenGLBackend::ThreadCompletedFrame(NodeDependency path, QByteArray hash, NodeValueTable table) { + SetWorkerBusyState(static_cast(sender()), false); + QVariant value = table.Get(NodeParam::kTexture); OpenGLTexturePtr texture = value.value(); @@ -146,19 +149,14 @@ void OpenGLBackend::ThreadCompletedFrame(NodeDependency path, QByteArray hash, N QString cache_fn = frame_cache()->CachePathName(hash); // Find an available worker to download this texture - foreach (RenderWorker* worker, processors_) { - // Check if one is available, but worst case if none of them are available, just queue it on the last worker since - // it's the least likely to get work - if (worker->IsAvailable() || worker == processors_.last()) { - QMetaObject::invokeMethod(worker, - "Download", - Q_ARG(NodeDependency, path), - Q_ARG(QByteArray, hash), - Q_ARG(QVariant, QVariant::fromValue(texture)), - Q_ARG(QString, cache_fn)); - break; - } - } + QMetaObject::invokeMethod(processors_.at(last_download_thread_%processors_.size()), + "Download", + Q_ARG(NodeDependency, path), + Q_ARG(QByteArray, hash), + Q_ARG(QVariant, QVariant::fromValue(texture)), + Q_ARG(QString, cache_fn)); + + last_download_thread_++; } // Set as push texture @@ -184,6 +182,7 @@ void OpenGLBackend::ThreadCompletedDownload(NodeDependency dep, QByteArray hash) void OpenGLBackend::ThreadSkippedFrame(NodeDependency dep, QByteArray hash) { frame_cache()->SetHash(dep.in(), hash); + SetWorkerBusyState(static_cast(sender()), false); // Queue up a new frame for this worker CacheNext(); @@ -192,6 +191,7 @@ void OpenGLBackend::ThreadSkippedFrame(NodeDependency dep, QByteArray hash) void OpenGLBackend::ThreadHashAlreadyExists(NodeDependency dep, QByteArray hash) { ThreadCompletedDownload(dep, hash); + SetWorkerBusyState(static_cast(sender()), false); // Queue up a new frame for this worker CacheNext(); diff --git a/app/render/backend/opengl/openglbackend.h b/app/render/backend/opengl/openglbackend.h index 099f3415a..e05303d74 100644 --- a/app/render/backend/opengl/openglbackend.h +++ b/app/render/backend/opengl/openglbackend.h @@ -34,6 +34,8 @@ private: OpenGLShaderCache shader_cache_; + int last_download_thread_; + private slots: void ThreadCompletedFrame(NodeDependency path, QByteArray hash, NodeValueTable table); void ThreadCompletedDownload(NodeDependency dep, QByteArray hash); diff --git a/app/render/backend/renderbackend.cpp b/app/render/backend/renderbackend.cpp index ad4b56ac4..63e6e4687 100644 --- a/app/render/backend/renderbackend.cpp +++ b/app/render/backend/renderbackend.cpp @@ -240,7 +240,7 @@ void RenderBackend::CacheNext() break; } - if (worker->IsAvailable()) { + if (!WorkerIsBusy(worker)) { TimeRange cache_frame = cache_queue_.takeFirst(); NodeDependency dep = NodeDependency(GetDependentInput()->get_connected_node(), cache_frame.in(), cache_frame.out()); @@ -249,6 +249,8 @@ void RenderBackend::CacheNext() "Render", Qt::QueuedConnection, Q_ARG(NodeDependency, dep)); + + SetWorkerBusyState(worker, true); } } } @@ -288,10 +290,20 @@ void RenderBackend::UpdateNodeInputs() } } +bool RenderBackend::WorkerIsBusy(RenderWorker *worker) const +{ + return processor_busy_state_.at(processors_.indexOf(worker)); +} + +void RenderBackend::SetWorkerBusyState(RenderWorker *worker, bool busy) +{ + processor_busy_state_.replace(processors_.indexOf(worker), busy); +} + bool RenderBackend::AllProcessorsAreAvailable() const { - foreach (RenderWorker* worker, processors_) { - if (!worker->IsAvailable()) { + foreach (bool busy, processor_busy_state_) { + if (busy) { return false; } } @@ -316,7 +328,6 @@ void RenderBackend::InitWorkers() QThread* thread = threads().at(i); // Connect to it - connect(processor, SIGNAL(RequestSibling(NodeDependency)), this, SLOT(ThreadRequestedSibling(NodeDependency))); ConnectWorkerToThis(processor); // Finally, we can move it to its own thread @@ -325,20 +336,9 @@ void RenderBackend::InitWorkers() // This function blocks the main thread intentionally. See the documentation for this function to see why. processor->Init(); } -} -void RenderBackend::ThreadRequestedSibling(NodeDependency dep) -{ - // Try to queue another thread to run this dep in advance - foreach (RenderWorker* worker, processors_) { - if (worker->IsAvailable()) { - QMetaObject::invokeMethod(worker, - "RenderAsSibling", - Qt::QueuedConnection, - Q_ARG(NodeDependency, dep)); - return; - } - } + processor_busy_state_.resize(processors_.size()); + processor_busy_state_.fill(false); } void RenderBackend::QueueRecompile() diff --git a/app/render/backend/renderbackend.h b/app/render/backend/renderbackend.h index f159149db..d9d913381 100644 --- a/app/render/backend/renderbackend.h +++ b/app/render/backend/renderbackend.h @@ -86,6 +86,9 @@ protected: void UpdateNodeInputs(); + bool WorkerIsBusy(RenderWorker* worker) const; + void SetWorkerBusyState(RenderWorker* worker, bool busy); + QList cache_queue_; QVector processors_; @@ -133,9 +136,9 @@ private: bool recompile_queued_; bool input_update_queued_; -private slots: - void ThreadRequestedSibling(NodeDependency dep); + QVector processor_busy_state_; +private slots: void QueueRecompile(); }; diff --git a/app/render/backend/renderworker.cpp b/app/render/backend/renderworker.cpp index 913b2defb..49d02b8cd 100644 --- a/app/render/backend/renderworker.cpp +++ b/app/render/backend/renderworker.cpp @@ -6,16 +6,10 @@ RenderWorker::RenderWorker(QObject *parent) : QObject(parent), - working_(0), started_(false) { } -bool RenderWorker::IsAvailable() -{ - return !working_; -} - bool RenderWorker::Init() { if (started_) { @@ -51,9 +45,6 @@ NodeValueTable RenderWorker::RenderAsSibling(NodeDependency dep) //qDebug() << "Processing" << node->id(); - // Set working state - working_++; - // Firstly we check if this node is a "Block", if it is that means it's part of a linked list of mutually exclusive // nodes based on time and we might need to locate which Block to attach to if (node->IsTrack()) { @@ -65,9 +56,6 @@ NodeValueTable RenderWorker::RenderAsSibling(NodeDependency dep) // We're done! - // End this working state - working_--; - return value; } diff --git a/app/render/backend/renderworker.h b/app/render/backend/renderworker.h index 0664012d4..21530c069 100644 --- a/app/render/backend/renderworker.h +++ b/app/render/backend/renderworker.h @@ -20,8 +20,6 @@ public: bool IsStarted(); - bool IsAvailable(); - public slots: void Close(); @@ -30,8 +28,6 @@ public slots: NodeValueTable RenderAsSibling(NodeDependency dep); signals: - void RequestSibling(NodeDependency path); - void CompletedCache(NodeDependency dep, NodeValueTable data); protected: @@ -56,8 +52,6 @@ protected: virtual NodeValueTable RenderBlock(TrackOutput *track, const TimeRange& range) = 0; - QAtomicInt working_; - private: bool started_; diff --git a/app/render/backend/videorenderworker.cpp b/app/render/backend/videorenderworker.cpp index acfd9035c..b5a33c2d1 100644 --- a/app/render/backend/videorenderworker.cpp +++ b/app/render/backend/videorenderworker.cpp @@ -18,8 +18,6 @@ const VideoRenderingParams &VideoRenderWorker::video_params() NodeValueTable VideoRenderWorker::RenderInternal(const NodeDependency& path) { - qDebug() << "Rendering" << path.in().toDouble() << "on" << this; - // Get hash of node graph // We use SHA-1 for speed (benchmarks show it's the fastest hash available to us) QCryptographicHash hasher(QCryptographicHash::Sha1); @@ -141,8 +139,6 @@ void VideoRenderWorker::CloseInternal() void VideoRenderWorker::Download(NodeDependency dep, QByteArray hash, QVariant texture, QString filename) { - working_++; - PixelFormatInfo format_info = PixelService::GetPixelFormatInfo(video_params().format()); // Set up OIIO::ImageSpec for compressing cached images on disk @@ -164,8 +160,6 @@ void VideoRenderWorker::Download(NodeDependency dep, QByteArray hash, QVariant t } else { qWarning() << "Failed to open output file:" << filename; } - - working_--; } NodeValueTable VideoRenderWorker::RenderBlock(TrackOutput *track, const TimeRange &range) diff --git a/app/widget/timelinewidget/tool/pointer.cpp b/app/widget/timelinewidget/tool/pointer.cpp index 1493861b9..147c96b30 100644 --- a/app/widget/timelinewidget/tool/pointer.cpp +++ b/app/widget/timelinewidget/tool/pointer.cpp @@ -239,7 +239,7 @@ void TimelineWidget::PointerTool::InitiateDrag(const TimelineCoordinate &mouse_p olive::timeline::MovementMode trim_mode = olive::timeline::kNone; // FIXME: Hardcoded number - const int kTrimHandle = 20; + const int kTrimHandle = 10; qreal mouse_x = parent()->TimeToScene(mouse_pos.GetFrame());