improved renderer reliability

The renderer backend can now distinguish between jobs. Previously if two jobs
of the same frame were started (which is legal if the user made a change while
frames were still being rendered), an earlier job in some situations could
finish AFTER a later job, and the backend would have no way of distinguishing
between them. This meant a frame could be erroneously set to an old value
rather than the newest. This commit introduces job identification so that old
jobs are automatically discarded.

app/render/backend/audio/audiobackend.cpp

@@ -49,42 +49,46 @@ void AudioBackend::DecompileInternal()
 
 void AudioBackend::ConnectWorkerToThis(RenderWorker *worker)
 {
-  connect(worker, SIGNAL(CompletedCache(NodeDependency, NodeValueTable)), this, SLOT(ThreadCompletedCache(NodeDependency, NodeValueTable)));
+  connect(worker, &RenderWorker::CompletedCache, this, &AudioBackend::ThreadCompletedCache);
 }
 
-void AudioBackend::ThreadCompletedCache(NodeDependency dep, NodeValueTable data)
+void AudioBackend::ThreadCompletedCache(NodeDependency dep, NodeValueTable data, qint64 job_time)
 {
   SetWorkerBusyState(static_cast<RenderWorker*>(sender()), false);
 
-  QByteArray cached_samples = data.Get(NodeParam::kSamples).toByteArray();
+  if (job_time == render_job_info_.value(dep.range())) {
+    render_job_info_.remove(dep.range());
 
-  int offset = params().time_to_bytes(dep.in());
-  int length = params().time_to_bytes(dep.range().length());
-  int out_point = offset + length;
+    QByteArray cached_samples = data.Get(NodeParam::kSamples).toByteArray();
 
-  QFile f(CachePathName());
-  if (f.open(QFile::WriteOnly | QFile::Append)) {
+    int offset = params().time_to_bytes(dep.in());
+    int length = params().time_to_bytes(dep.range().length());
+    int out_point = offset + length;
 
-    if (f.size() < out_point) {
-      f.resize(out_point);
+    QFile f(CachePathName());
+    if (f.open(QFile::WriteOnly | QFile::Append)) {
+
+      if (f.size() < out_point) {
+        f.resize(out_point);
+      }
+
+      f.seek(offset);
+
+      // Replace data with this data
+      int copy_length = qMin(length, cached_samples.size());
+
+      f.write(cached_samples.data(), copy_length);
+
+      if (copy_length < length) {
+        // Fill in remainder with silence
+        QByteArray empty_space(length - copy_length, 0);
+        f.write(empty_space);
+      }
+
+      f.close();
+    } else {
+      qWarning() << "Failed to write to cached PCM file";
     }
-
-    f.seek(offset);
-
-    // Replace data with this data
-    int copy_length = qMin(length, cached_samples.size());
-
-    f.write(cached_samples.data(), copy_length);
-
-    if (copy_length < length) {
-      // Fill in remainder with silence
-      QByteArray empty_space(length - copy_length, 0);
-      f.write(empty_space);
-    }
-
-    f.close();
-  } else {
-    qWarning() << "Failed to write to cached PCM file";
   }
 
   CacheNext();

app/render/backend/audio/audiobackend.h

@@ -27,7 +27,7 @@ protected:
   virtual void ConnectWorkerToThis(RenderWorker* worker) override;
 
 private slots:
-  void ThreadCompletedCache(NodeDependency dep, NodeValueTable data);
+  void ThreadCompletedCache(NodeDependency dep, NodeValueTable data, qint64 job_time);
 
 private:
   QFile pull_device_;

app/render/backend/renderbackend.cpp

@@ -258,14 +258,25 @@ void RenderBackend::CacheNext()
     if (!WorkerIsBusy(worker)) {
       TimeRange cache_frame = cache_queue_.takeFirst();
 
-      NodeDependency dep = NodeDependency(node_connected_to_viewer, cache_frame.in(), cache_frame.out());
+      NodeDependency dep = NodeDependency(node_connected_to_viewer,
+                                          cache_frame);
+
+      // Timestamp this render job
+      qint64 job_time = QDateTime::currentMSecsSinceEpoch();
+      if (render_job_info_.contains(cache_frame)
+          && render_job_info_.value(cache_frame) == job_time) {
+        // Ensure the job's time is unique
+        job_time = render_job_info_.value(cache_frame) + 1;
+      }
+      render_job_info_.insert(cache_frame, job_time);
 
       SetWorkerBusyState(worker, true);
 
       QMetaObject::invokeMethod(worker,
                                 "Render",
                                 Qt::QueuedConnection,
-                                Q_ARG(NodeDependency, dep));
+                                Q_ARG(NodeDependency, dep),
+                                Q_ARG(qint64, job_time));
     }
   }
 }

app/render/backend/renderbackend.h

@@ -96,6 +96,8 @@ protected:
 
   bool compiled_;
 
+  QHash<TimeRange, qint64> render_job_info_;
+
 private:
   bool AllProcessorsAreAvailable() const;
 

app/render/backend/renderworker.cpp

@@ -32,13 +32,15 @@ void RenderWorker::Close()
   started_ = false;
 }
 
-void RenderWorker::Render(NodeDependency path)
+void RenderWorker::Render(NodeDependency path, qint64 job_time)
 {
-  emit CompletedCache(path, RenderInternal(path));
+  emit CompletedCache(path, RenderInternal(path, job_time), job_time);
 }
 
-NodeValueTable RenderWorker::RenderInternal(const NodeDependency &path)
+NodeValueTable RenderWorker::RenderInternal(const NodeDependency &path, const qint64 &job_time)
 {
+  Q_UNUSED(job_time)
+
   return ProcessNode(path);
 }
 
@@ -114,7 +116,7 @@ NodeValueTable RenderWorker::ProcessInput(const NodeInput *input, const TimeRang
   }
 }
 
-NodeValueDatabase RenderWorker::GenerateDatabase(const Node* node, const TimeRange& range)
+NodeValueDatabase RenderWorker::GenerateDatabase(const Node* node, const TimeRange &range)
 {
   NodeValueDatabase database;
 

app/render/backend/renderworker.h

@@ -23,17 +23,17 @@ public:
 public slots:
   void Close();
 
-  void Render(NodeDependency path);
+  void Render(NodeDependency path, qint64 job_time);
 
 signals:
-  void CompletedCache(NodeDependency dep, NodeValueTable data);
+  void CompletedCache(NodeDependency dep, NodeValueTable data, qint64 job_time);
 
 protected:
   virtual bool InitInternal() = 0;
 
   virtual void CloseInternal() = 0;
 
-  virtual NodeValueTable RenderInternal(const NodeDependency& path);
+  virtual NodeValueTable RenderInternal(const NodeDependency& path, const qint64& job_time);
 
   virtual void RunNodeAccelerated(const Node *node, const TimeRange& range, const NodeValueDatabase &input_params, NodeValueTable* output_params);
 

app/render/backend/videorenderbackend.cpp

@@ -33,8 +33,7 @@
 
 VideoRenderBackend::VideoRenderBackend(QObject *parent) :
   RenderBackend(parent),
-  export_mode_(false),
-  last_download_thread_(0)
+  export_mode_(false)
 {
 }
 
@@ -182,10 +181,12 @@ void VideoRenderBackend::CacheIDChangedEvent(const QString &id)
 
 void VideoRenderBackend::ConnectWorkerToThis(RenderWorker *processor)
 {
-  connect(processor, SIGNAL(CompletedFrame(NodeDependency, QByteArray, NodeValueTable)), this, SLOT(ThreadCompletedFrame(NodeDependency, QByteArray, NodeValueTable)));
-  connect(processor, SIGNAL(HashAlreadyBeingCached(NodeDependency, QByteArray)), this, SLOT(ThreadSkippedFrame(NodeDependency, QByteArray)));
-  connect(processor, SIGNAL(CompletedDownload(NodeDependency, QByteArray)), this, SLOT(ThreadCompletedDownload(NodeDependency, QByteArray)));
-  connect(processor, SIGNAL(HashAlreadyExists(NodeDependency, QByteArray)), this, SLOT(ThreadHashAlreadyExists(NodeDependency, QByteArray)));
+  VideoRenderWorker* video_processor = static_cast<VideoRenderWorker*>(processor);
+
+  connect(video_processor, &VideoRenderWorker::CompletedFrame, this, &VideoRenderBackend::ThreadCompletedFrame);
+  connect(video_processor, &VideoRenderWorker::HashAlreadyBeingCached, this, &VideoRenderBackend::ThreadSkippedFrame);
+  connect(video_processor, &VideoRenderWorker::CompletedDownload, this, &VideoRenderBackend::ThreadCompletedDownload);
+  connect(video_processor, &VideoRenderWorker::HashAlreadyExists, this, &VideoRenderBackend::ThreadHashAlreadyExists);
 }
 
 VideoRenderFrameCache *VideoRenderBackend::frame_cache()
@@ -246,95 +247,82 @@ bool VideoRenderBackend::CanRender()
   return params_.is_valid();
 }
 
-void VideoRenderBackend::ThreadCompletedFrame(NodeDependency path, QByteArray hash, NodeValueTable table)
+void VideoRenderBackend::ThreadCompletedFrame(NodeDependency path, qint64 job_time, QByteArray hash, QVariant value)
 {
-  SetWorkerBusyState(static_cast<RenderWorker*>(sender()), false);
+  // Here, we received a frame resident in memory that can be forwarded along to a viewer or exporter if necessary.
 
-  QVariant texture = table.Get(NodeParam::kTexture);
-
-  // Check if this frame has changed once again, in which case we may not want to draw it (it'll look jittery to the user)
   QList<rational> times_with_this_hash;
 
-  if (last_time_requested_ == path.in() || export_mode_) {
+  // If the viewer last requested this time, presumably it hasn't moved from there and should know this frame has now
+  // changed
+  if (last_time_requested_ == path.in()
+      && JobIsCurrent(path, job_time)) {
     times_with_this_hash.append(path.in());
   }
 
-  if (export_mode_) {
+  // Send all deferred frames to the exporter
+  /*if (export_mode_) {
     times_with_this_hash.append(frame_cache()->DeferredMapsWithHash(hash));
-  }
+  }*/
 
+  // If we have frames to forward along to a viewer/exporter, forward them here
   if (!times_with_this_hash.isEmpty()) {
-    EmitCachedFrameReady(times_with_this_hash, texture);
+    EmitCachedFrameReady(times_with_this_hash, value);
   }
+}
 
-  if (!export_mode_) {
-    if (texture.isNull()) {
-      // No frame received, we set hash to an empty
-      frame_cache()->RemoveHash(path.in(), hash);
-    } else {
-      // Received a texture, let's download it
-      QString cache_fn = frame_cache()->CachePathName(hash);
+void VideoRenderBackend::ThreadCompletedDownload(NodeDependency dep, qint64 job_time, QByteArray hash)
+{
+  SetWorkerBusyState(static_cast<RenderWorker*>(sender()), false);
 
-      // Find an available worker to download this texture
-      QMetaObject::invokeMethod(processors_.at(last_download_thread_%processors_.size()),
-                                "Download",
-                                Q_ARG(NodeDependency, path),
-                                Q_ARG(QByteArray, hash),
-                                Q_ARG(QVariant, texture),
-                                Q_ARG(QString, cache_fn));
+  SetFrameHash(dep, hash, job_time);
 
-      last_download_thread_++;
-    }
+  // Queue up a new frame for this worker
+  CacheNext();
+}
+
+void VideoRenderBackend::ThreadSkippedFrame(NodeDependency dep, qint64 job_time, QByteArray hash)
+{
+  SetWorkerBusyState(static_cast<RenderWorker*>(sender()), false);
+
+  SetFrameHash(dep, hash, job_time);
+
+  // Queue up a new frame for this worker
+  CacheNext();
+}
+
+void VideoRenderBackend::ThreadHashAlreadyExists(NodeDependency dep, qint64 job_time, QByteArray hash)
+{
+  SetWorkerBusyState(static_cast<RenderWorker*>(sender()), false);
+
+  if (SetFrameHash(dep, hash, job_time) && dep.in() == last_time_requested_) {
+    emit CachedTimeReady(dep.in());
   }
 
   // Queue up a new frame for this worker
   CacheNext();
 }
 
-void VideoRenderBackend::ThreadCompletedDownload(NodeDependency dep, QByteArray hash)
-{
-  // Set hash, but DON'T signal time because it's most likely this frame has been signalled in ThreadCompletedFrame()
-  frame_cache()->SetHash(dep.in(), hash);
-
-  // Emit for each frame that has this hash (some may have been added in ThreadSkippedFrame)
-  DumpDeferredMappings(frame_cache()->DeferredMapsWithHash(hash), hash);
-}
-
-void VideoRenderBackend::ThreadSkippedFrame(NodeDependency dep, QByteArray hash)
-{
-  SetWorkerBusyState(static_cast<RenderWorker*>(sender()), false);
-
-  // Queue up a new frame for this worker
-  CacheNext();
-}
-
-void VideoRenderBackend::ThreadHashAlreadyExists(NodeDependency dep, QByteArray hash)
-{
-  // Emit for each frame that has this hash (some may have been added in ThreadSkippedFrame)
-  QList<rational> times_with_this_hash = frame_cache()->DeferredMapsWithHash(hash);
-  times_with_this_hash.append(dep.in());
-  DumpDeferredMappings(times_with_this_hash, hash);
-
-  //ThreadCompletedDownload(dep, hash);
-  SetWorkerBusyState(static_cast<RenderWorker*>(sender()), false);
-
-  // Queue up a new frame for this worker
-  CacheNext();
-}
-
-bool VideoRenderBackend::TimeIsQueued(const TimeRange &time)
+bool VideoRenderBackend::TimeIsQueued(const TimeRange &time) const
 {
   return cache_queue_.contains(time);
 }
 
-void VideoRenderBackend::DumpDeferredMappings(const QList<rational>& times_with_this_hash, const QByteArray& hash)
+bool VideoRenderBackend::JobIsCurrent(const NodeDependency &dep, const qint64& job_time) const
 {
-  foreach (const rational& t, times_with_this_hash) {
-    if (frame_cache()->TimeToHash(t) != hash) {
-      frame_cache()->SetHash(t, hash);
-      if (last_time_requested_ == t && !TimeIsQueued(TimeRange(t, t))) {
-        emit CachedTimeReady(t);
-      }
-    }
+  return (render_job_info_.value(dep.range()) == job_time && !TimeIsQueued(dep.range()));
+}
+
+bool VideoRenderBackend::SetFrameHash(const NodeDependency &dep, const QByteArray &hash, const qint64& job_time)
+{
+  if (JobIsCurrent(dep, job_time)) {
+    frame_cache_.SetHash(dep.in(), hash);
+    render_job_info_.remove(dep.range());
+
+    return true;
   }
+
+  qDebug() << "Discarded frame" << dep.in().toDouble();
+
+  return false;
 }

app/render/backend/videorenderbackend.h

@@ -106,9 +106,11 @@ signals:
   void CachedTimeReady(const rational& time);
 
 private:
-  bool TimeIsQueued(const TimeRange &time);
+  bool TimeIsQueued(const TimeRange &time) const;
 
-  void DumpDeferredMappings(const QList<rational> &times_with_this_hash, const QByteArray &hash);
+  bool JobIsCurrent(const NodeDependency &dep, const qint64& job_time) const;
+
+  bool SetFrameHash(const NodeDependency& dep, const QByteArray& hash, const qint64& job_time);
 
   VideoRenderingParams params_;
 
@@ -118,13 +120,11 @@ private:
 
   rational last_time_requested_;
 
-  int last_download_thread_;
-
 private slots:
-  void ThreadCompletedFrame(NodeDependency path, QByteArray hash, NodeValueTable table);
-  void ThreadCompletedDownload(NodeDependency dep, QByteArray hash);
-  void ThreadSkippedFrame(NodeDependency dep, QByteArray hash);
-  void ThreadHashAlreadyExists(NodeDependency dep, QByteArray hash);
+  void ThreadCompletedFrame(NodeDependency path, qint64 job_time, QByteArray hash, QVariant value);
+  void ThreadCompletedDownload(NodeDependency dep, qint64 job_time, QByteArray hash);
+  void ThreadSkippedFrame(NodeDependency dep, qint64 job_time, QByteArray hash);
+  void ThreadHashAlreadyExists(NodeDependency dep, qint64 job_time, QByteArray hash);
 
 };
 

app/render/backend/videorenderframecache.cpp

@@ -32,9 +32,7 @@ bool VideoRenderFrameCache::TryCache(const rational& time, const QByteArray &has
 
   bool is_caching = currently_caching_list_.contains(hash);
 
-  if (is_caching) {
-    deferred_maps_.insert(time, hash);
-  } else {
+  if (!is_caching) {
     currently_caching_list_.append(hash);
   }
 
@@ -82,30 +80,6 @@ void VideoRenderFrameCache::Truncate(const rational &time)
   }
 }
 
-QList<rational> VideoRenderFrameCache::DeferredMapsWithHash(const QByteArray &hash)
-{
-  QList<rational> list;
-
-  currently_caching_lock_.lock();
-
-  QMap<rational, QByteArray>::iterator iterator = deferred_maps_.begin();
-
-  while (iterator != deferred_maps_.end()) {
-    if (iterator.value() == hash) {
-      list.append(iterator.key());
-      iterator = deferred_maps_.erase(iterator);
-    } else {
-      iterator++;
-    }
-  }
-
-  currently_caching_list_.removeOne(hash);
-
-  currently_caching_lock_.unlock();
-
-  return list;
-}
-
 void VideoRenderFrameCache::RemoveHashFromCurrentlyCaching(const QByteArray &hash)
 {
   currently_caching_lock_.lock();

app/render/backend/videorenderframecache.h

@@ -39,13 +39,10 @@ public:
 
   void Truncate(const rational& time);
 
-  QList<rational> DeferredMapsWithHash(const QByteArray& hash);
-
 private:
   void RemoveHashFromCurrentlyCaching(const QByteArray& hash);
 
   QMap<rational, QByteArray> time_hash_map_;
-  QMap<rational, QByteArray> deferred_maps_;
 
   QMutex currently_caching_lock_;
   QVector<QByteArray> currently_caching_list_;

app/render/backend/videorenderworker.cpp

@@ -17,7 +17,7 @@ const VideoRenderingParams &VideoRenderWorker::video_params()
   return video_params_;
 }
 
-NodeValueTable VideoRenderWorker::RenderInternal(const NodeDependency& path)
+NodeValueTable VideoRenderWorker::RenderInternal(const NodeDependency& path, const qint64 &job_time)
 {
   // Get hash of node graph
   // We use SHA-1 for speed (benchmarks show it's the fastest hash available to us)
@@ -29,15 +29,27 @@ NodeValueTable VideoRenderWorker::RenderInternal(const NodeDependency& path)
 
   if (frame_cache_->HasHash(hash)) {
     // We've already cached this hash, no need to continue
-    emit HashAlreadyExists(path, hash);
+    emit HashAlreadyExists(path, job_time, hash);
   } else if (frame_cache_->TryCache(path.in(), hash)) {
     // This hash is available for us to cache, start traversing graph
     value = ProcessNode(path);
 
-    emit CompletedFrame(path, hash, value);
+    // Find texture in hash
+    QVariant texture = value.Get(NodeParam::kTexture);
+
+    // Signal that we have a frame in memory that could be shown right now
+    emit CompletedFrame(path, job_time, hash, texture);
+
+    // If we actually have a texture, download it into the disk cache
+    if (!texture.isNull()) {
+      Download(path, hash, texture, frame_cache_->CachePathName(hash));
+    }
+
+    // Signal that this job is complete
+    emit CompletedDownload(path, job_time, hash);
   } else {
     // Another thread must be caching this already, nothing to be done
-    emit HashAlreadyBeingCached(path, hash);
+    emit HashAlreadyBeingCached(path, job_time, hash);
   }
 
   return value;
@@ -152,8 +164,6 @@ void VideoRenderWorker::Download(NodeDependency dep, QByteArray hash, QVariant t
     out->open(working_fn_std, spec);
     out->write_image(format_info.oiio_desc, download_buffer_.data());
     out->close();
-
-    emit CompletedDownload(dep, hash);
   } else {
     qWarning() << "Failed to open output file:" << filename;
   }

app/render/backend/videorenderworker.h

@@ -16,17 +16,14 @@ public:
 
   void SetParameters(const VideoRenderingParams& video_params);
 
-public slots:
-  void Download(NodeDependency dep, QByteArray hash, QVariant texture, QString filename);
-
 signals:
-  void CompletedFrame(NodeDependency path, QByteArray hash, NodeValueTable value);
+  void CompletedFrame(NodeDependency path, qint64 job_time, QByteArray hash, QVariant value);
 
-  void CompletedDownload(NodeDependency path, QByteArray hash);
+  void CompletedDownload(NodeDependency path, qint64 job_time, QByteArray hash);
 
-  void HashAlreadyBeingCached(NodeDependency path, QByteArray hash);
+  void HashAlreadyBeingCached(NodeDependency path, qint64 job_time, QByteArray hash);
 
-  void HashAlreadyExists(NodeDependency path, QByteArray hash);
+  void HashAlreadyExists(NodeDependency path, qint64 job_time, QByteArray hash);
 
 protected:
   virtual bool InitInternal() override;
@@ -39,7 +36,7 @@ protected:
 
   virtual void TextureToBuffer(const QVariant& texture, QByteArray& buffer) = 0;
 
-  virtual NodeValueTable RenderInternal(const NodeDependency& path) override;
+  virtual NodeValueTable RenderInternal(const NodeDependency& path, const qint64& job_time) override;
 
   virtual FramePtr RetrieveFromDecoder(DecoderPtr decoder, const TimeRange& range) override;
 
@@ -50,6 +47,8 @@ protected:
 private:
   void HashNodeRecursively(QCryptographicHash* hash, const Node *n, const rational &time);
 
+  void Download(NodeDependency dep, QByteArray hash, QVariant texture, QString filename);
+
   VideoRenderingParams video_params_;
 
   VideoRenderFrameCache* frame_cache_;