renderer: fixed bug that caused threads to hang indefinitely

app/render/renderprocessor.cpp

@@ -265,39 +265,32 @@ QVariant RenderProcessor::ProcessVideoFootage(StreamPtr stream, const rational &
 
   ColorManager* color_manager = Node::ValueToPtr<ColorManager>(ticket_->property("colormanager"));
 
-  StillImageCache::Entry want_entry = {nullptr,
-                                       stream,
-                                       ColorProcessor::GenerateID(color_manager, video_stream->colorspace(), color_manager->GetReferenceColorSpace()),
-                                       video_stream->premultiplied_alpha(),
-                                       video_params.divider(),
-                                       (video_stream->video_type() == VideoStream::kVideoTypeStill) ? 0 : input_time};
+  StillImageCache::EntryPtr want_entry = std::make_shared<StillImageCache::Entry>(
+        nullptr,
+        stream,
+        ColorProcessor::GenerateID(color_manager, video_stream->colorspace(), color_manager->GetReferenceColorSpace()),
+        video_stream->premultiplied_alpha(),
+        video_params.divider(),
+        (video_stream->video_type() == VideoStream::kVideoTypeStill) ? 0 : input_time,
+        true);
+
+  bool found_existing = false;
 
   still_image_cache_->mutex()->lock();
 
-  foreach (const StillImageCache::Entry& e, still_image_cache_->entries()) {
+  foreach (StillImageCache::EntryPtr e, still_image_cache_->entries()) {
     if (StillImageCache::CompareEntryMetadata(want_entry, e)) {
-      // Found an exact match of the texture we want in the cache, use it instead of reading it
-      // ourselves
-      value = e.texture;
-      break;
-    }
-  }
+      // Found an exact match of the texture we want in the cache. See if it's working or if it's
+      // ready.
+      want_entry = e;
+      found_existing = true;
 
-  if (!value) {
-    // Failed to find the texture, let's see if it's being generated by another processor
-    foreach (const StillImageCache::Entry& e, still_image_cache_->pending()) {
-      if (StillImageCache::CompareEntryMetadata(want_entry, e)) {
-        // An exact match of this texture is pending, let's wait for it
-        while (!value) {
-          // FIXME: Hacky way of waiting for other threads
-          still_image_cache_->mutex()->unlock();
-          QThread::msleep(1);
-          still_image_cache_->mutex()->lock();
-
-          value = e.texture;
-        }
-        break;
+      while (want_entry->working) {
+        still_image_cache_->wait_cond()->wait(still_image_cache_->mutex());
       }
+
+      value = want_entry->texture;
+      break;
     }
   }
 
@@ -307,8 +300,11 @@ QVariant RenderProcessor::ProcessVideoFootage(StreamPtr stream, const rational &
   } else {
     // Wasn't in still image cache, so we'll have to retrieve it from the decoder
 
-    // Let other processors know we're getting this texture
-    still_image_cache_->PushPending(want_entry);
+    // Let other processors know we're getting this texture (want_entry's `working` field is
+    // already set to true in the initializer above)
+    if (!found_existing) {
+      still_image_cache_->PushEntry(want_entry);
+    }
 
     still_image_cache_->mutex()->unlock();
 
@@ -330,7 +326,7 @@ QVariant RenderProcessor::ProcessVideoFootage(StreamPtr stream, const rational &
         managed_params.set_format(video_params.format());
         value = render_ctx_->CreateTexture(managed_params);
 
-        qDebug() << "FIXME: Accessing video_stream->colorspace() and video_stream->premultiplied_alpha() may cause race conditions";
+        //qDebug() << "FIXME: Accessing video_stream->colorspace() and video_stream->premultiplied_alpha() may cause race conditions";
 
         ColorProcessorPtr processor = ColorProcessor::Create(color_manager,
                                                              video_stream->colorspace(),
@@ -342,11 +338,11 @@ QVariant RenderProcessor::ProcessVideoFootage(StreamPtr stream, const rational &
 
         still_image_cache_->mutex()->lock();
 
-        still_image_cache_->RemovePending(want_entry);
-
         // Put this into the image cache instead
-        want_entry.texture = value;
-        still_image_cache_->PushEntry(want_entry);
+        want_entry->texture = value;
+        want_entry->working = false;
+
+        still_image_cache_->wait_cond()->wakeAll();
 
         still_image_cache_->mutex()->unlock();
       }
@@ -481,8 +477,6 @@ QVariant RenderProcessor::GetCachedFrame(const Node *node, const rational &time)
 
     FramePtr f = FrameHashCache::LoadCacheFrame(ticket_->property("cache").toString(), hash);
 
-    qDebug() << ticket_->property("cache").toString() << hash.toHex();
-
     if (f) {
       // The cached frame won't load with the correct divider by default, so we enforce it here
       VideoParams p = f->video_params();
@@ -497,8 +491,6 @@ QVariant RenderProcessor::GetCachedFrame(const Node *node, const rational &time)
 
       TexturePtr texture = render_ctx_->CreateTexture(f->video_params(), f->data(), f->linesize_pixels());
       return QVariant::fromValue(texture);
-    } else {
-      qDebug() << "Not using cached frame because frame is null";
     }
   }
 

app/render/stillimagecache.h

@@ -2,6 +2,7 @@
 #define STILLIMAGECACHE_H
 
 #include <QHash>
+#include <QWaitCondition>
 
 #include "common/rational.h"
 #include "project/item/footage/stream.h"
@@ -13,44 +14,53 @@ class StillImageCache
 {
 public:
   struct Entry {
+    Entry(TexturePtr t, StreamPtr s, const QString& cs, bool a, int d, const rational& i, bool w)
+    {
+      texture = t;
+      stream = s;
+      colorspace = cs;
+      alpha_is_associated = a;
+      divider = d;
+      time = i;
+      working = w;
+    }
+
     TexturePtr texture;
     StreamPtr stream;
     QString colorspace;
     bool alpha_is_associated;
     int divider;
     rational time;
+    bool working;
   };
 
+  using EntryPtr = std::shared_ptr<Entry>;
+
   QMutex* mutex()
   {
     return &mutex_;
   }
 
-  const QVector<Entry>& entries() const
+  QWaitCondition* wait_cond()
+  {
+    return &wait_cond_;
+  }
+
+  const QVector<EntryPtr>& entries() const
   {
     return entries_;
   }
 
-  const QVector<Entry>& pending() const
+  static bool CompareEntryMetadata(EntryPtr a, EntryPtr b)
   {
-    return pending_;
+    return (a->stream == b->stream
+            && a->colorspace == b->colorspace
+            && a->alpha_is_associated == b->alpha_is_associated
+            && a->divider == b->divider
+            && a->time == b->time);
   }
 
-  static bool CompareEntryMetadata(const Entry& a, const Entry& b)
-  {
-    return (a.stream == b.stream
-            && a.colorspace == b.colorspace
-            && a.alpha_is_associated == b.alpha_is_associated
-            && a.divider == b.divider
-            && a.time == b.time);
-  }
-
-  void PushPending(const Entry& e)
-  {
-    pending_.prepend(e);
-  }
-
-  void PushEntry(const Entry& e)
+  void PushEntry(EntryPtr e)
   {
     entries_.prepend(e);
 
@@ -59,22 +69,12 @@ public:
     }
   }
 
-  void RemovePending(const Entry& e)
-  {
-    for (int i=0; i<pending_.size(); i++) {
-      if (CompareEntryMetadata(pending_.at(i), e)) {
-        pending_.removeAt(i);
-        break;
-      }
-    }
-  }
-
 private:
   QMutex mutex_;
 
-  QVector<Entry> entries_;
+  QWaitCondition wait_cond_;
 
-  QVector<Entry> pending_;
+  QVector<EntryPtr> entries_;
 
 };