]> git.sesse.net Git - nageru/commitdiff
Move to RAII queue counting, to fix some underflows.
authorSteinar H. Gunderson <sgunderson@bigfoot.com>
Sat, 27 Oct 2018 19:46:55 +0000 (21:46 +0200)
committerSteinar H. Gunderson <sgunderson@bigfoot.com>
Sun, 28 Oct 2018 00:08:45 +0000 (02:08 +0200)
player.cpp
player.h
queue_spot_holder.h [new file with mode: 0644]
video_stream.cpp
video_stream.h

index 1432f04bad93d2b5df348755e0b7e5349d0f038a..dd37143fb497a5035ece9456d101912e045ca251 100644 (file)
@@ -73,7 +73,7 @@ wait_for_clip:
 
                if (!clip_ready) {
                        if (video_stream != nullptr) {
-                               video_stream->schedule_refresh_frame(steady_clock::now(), pts, /*display_func=*/nullptr);
+                               video_stream->schedule_refresh_frame(steady_clock::now(), pts, /*display_func=*/nullptr, QueueSpotHolder());
                        }
                        continue;
                }
@@ -193,8 +193,9 @@ got_clip:
                                });
                                if (new_clip_ready) {
                                        if (video_stream != nullptr) {
+                                               lock.unlock();  // Urg.
                                                video_stream->clear_queue();
-                                               num_queued_frames = 0;
+                                               lock.lock();
                                        }
                                        goto wait_for_clip;
                                }
@@ -208,22 +209,18 @@ got_clip:
                        if (in_pts_lower == in_pts_upper) {
                                auto display_func = [this, primary_stream_idx, in_pts_lower, secondary_stream_idx, secondary_pts, fade_alpha]{
                                        destination->setFrame(primary_stream_idx, in_pts_lower, /*interpolated=*/false, secondary_stream_idx, secondary_pts, fade_alpha);
-                                       unique_lock<mutex> lock(queue_state_mu);
-                                       assert(num_queued_frames > 0);
-                                       --num_queued_frames;
-                                       new_clip_changed.notify_all();
                                };
                                if (video_stream == nullptr) {
                                        display_func();
                                } else {
                                        if (secondary_stream_idx == -1) {
-                                               video_stream->schedule_original_frame(next_frame_start, pts, display_func, primary_stream_idx, in_pts_lower);
-                                               unique_lock<mutex> lock(queue_state_mu);
-                                               ++num_queued_frames;
+                                               video_stream->schedule_original_frame(
+                                                       next_frame_start, pts, display_func, QueueSpotHolder(this),
+                                                       primary_stream_idx, in_pts_lower);
                                        } else {
-                                               bool ok = video_stream->schedule_faded_frame(next_frame_start, pts, display_func, primary_stream_idx, in_pts_lower, secondary_stream_idx, secondary_pts, fade_alpha);
-                                               unique_lock<mutex> lock(queue_state_mu);
-                                               if (ok) ++num_queued_frames;
+                                               video_stream->schedule_faded_frame(next_frame_start, pts, display_func,
+                                                       QueueSpotHolder(this), primary_stream_idx, in_pts_lower,
+                                                       secondary_stream_idx, secondary_pts, fade_alpha);
                                        }
                                }
                                continue;
@@ -238,22 +235,18 @@ got_clip:
                                if (fabs(snap_pts_as_frameno - frameno) < 0.01) {
                                        auto display_func = [this, primary_stream_idx, snap_pts, secondary_stream_idx, secondary_pts, fade_alpha]{
                                                destination->setFrame(primary_stream_idx, snap_pts, /*interpolated=*/false, secondary_stream_idx, secondary_pts, fade_alpha);
-                                               unique_lock<mutex> lock(queue_state_mu);
-                                               assert(num_queued_frames > 0);
-                                               --num_queued_frames;
-                                               new_clip_changed.notify_all();
                                        };
                                        if (video_stream == nullptr) {
                                                display_func();
                                        } else {
                                                if (secondary_stream_idx == -1) {
-                                                       video_stream->schedule_original_frame(next_frame_start, pts, display_func, primary_stream_idx, snap_pts);
-                                                       unique_lock<mutex> lock(queue_state_mu);
-                                                       ++num_queued_frames;
+                                                       video_stream->schedule_original_frame(
+                                                               next_frame_start, pts, display_func,
+                                                               QueueSpotHolder(this), primary_stream_idx, snap_pts);
                                                } else {
-                                                       bool ok = video_stream->schedule_faded_frame(next_frame_start, pts, display_func, primary_stream_idx, snap_pts, secondary_stream_idx, secondary_pts, fade_alpha);
-                                                       unique_lock<mutex> lock(queue_state_mu);
-                                                       if (ok) ++num_queued_frames;
+                                                       video_stream->schedule_faded_frame(
+                                                               next_frame_start, pts, display_func, QueueSpotHolder(this),
+                                                               primary_stream_idx, snap_pts, secondary_stream_idx, secondary_pts, fade_alpha);
                                                }
                                        }
                                        in_pts_origin += snap_pts - in_pts;
@@ -280,13 +273,11 @@ got_clip:
                        } else {
                                auto display_func = [this, primary_stream_idx, pts, secondary_stream_idx, secondary_pts, fade_alpha]{
                                        destination->setFrame(primary_stream_idx, pts, /*interpolated=*/true, secondary_stream_idx, secondary_pts, fade_alpha);
-                                       assert(num_queued_frames > 0);
-                                       --num_queued_frames;
-                                       new_clip_changed.notify_all();
                                };
-                               bool ok = video_stream->schedule_interpolated_frame(next_frame_start, pts, display_func, primary_stream_idx, in_pts_lower, in_pts_upper, alpha, secondary_stream_idx, secondary_pts, fade_alpha);
-                               unique_lock<mutex> lock(queue_state_mu);
-                               if (ok) ++num_queued_frames;
+                               video_stream->schedule_interpolated_frame(
+                                       next_frame_start, pts, display_func, QueueSpotHolder(this),
+                                       primary_stream_idx, in_pts_lower, in_pts_upper, alpha,
+                                       secondary_stream_idx, secondary_pts, fade_alpha);
                        }
                }
 
@@ -414,3 +405,17 @@ void Player::override_angle(unsigned stream_idx)
        }
        destination->setFrame(stream_idx, *it, /*interpolated=*/false);
 }
+
+void Player::take_queue_spot()
+{
+       unique_lock<mutex> lock(queue_state_mu);
+       ++num_queued_frames;
+}
+
+void Player::release_queue_spot()
+{
+       unique_lock<mutex> lock(queue_state_mu);
+       assert(num_queued_frames > 0);
+       --num_queued_frames;
+       new_clip_changed.notify_all();
+}
index f5adcf7a505dfed1636466bb06cf689c788bba5a..fb9f4a0f01ea6a90544609954191d59148f33683 100644 (file)
--- a/player.h
+++ b/player.h
@@ -2,6 +2,7 @@
 #define _PLAYER_H 1
 
 #include "clip_list.h"
+#include "queue_spot_holder.h"
 
 extern "C" {
 #include <libavformat/avio.h>
@@ -16,7 +17,7 @@ class VideoStream;
 class QSurface;
 class QSurfaceFormat;
 
-class Player {
+class Player : public QueueInterface {
 public:
        Player(JPEGFrameView *destination, bool also_output_to_stream);
 
@@ -38,6 +39,10 @@ public:
        using progress_callback_func = std::function<void(double played_this_clip, double total_length)>;
        void set_progress_callback(progress_callback_func cb) { progress_callback = cb; }
 
+       // QueueInterface.
+       void take_queue_spot() override;
+       void release_queue_spot() override;
+
 private:
        void thread_func(bool also_output_to_stream);
        void open_output_stream();
diff --git a/queue_spot_holder.h b/queue_spot_holder.h
new file mode 100644 (file)
index 0000000..b9dee06
--- /dev/null
@@ -0,0 +1,46 @@
+#ifndef _QUEUE_SPOT_HOLDER
+#define _QUEUE_SPOT_HOLDER 1
+
+// A RAII class to hold a shared resource, in our case an (unordered!) spot in a queue,
+// for as long as a frame is under computation.
+
+class QueueInterface {
+public:
+       virtual ~QueueInterface() {}
+       virtual void take_queue_spot() = 0;
+       virtual void release_queue_spot() = 0;
+};
+
+class QueueSpotHolder {
+public:
+       QueueSpotHolder() : queue(nullptr) {}
+
+       explicit QueueSpotHolder(QueueInterface *queue) : queue(queue) {
+               queue->take_queue_spot();
+       }
+
+       QueueSpotHolder(QueueSpotHolder &&other) : queue(other.queue) {
+               other.queue = nullptr;
+       }
+
+       QueueSpotHolder &operator=(QueueSpotHolder &&other) {
+               queue = other.queue;
+               other.queue = nullptr;
+               return *this;
+       }
+
+       ~QueueSpotHolder() {
+               if (queue != nullptr) {
+                       queue->release_queue_spot();
+               }
+       }
+
+       // Movable only.
+       QueueSpotHolder(QueueSpotHolder &) = delete;
+       QueueSpotHolder &operator=(QueueSpotHolder &) = delete;
+
+private:
+       QueueInterface *queue;  
+};
+
+#endif // !defined(_QUEUE_SPOT_HOLDER)
index 49458f85811fb529129e4fa014b83f708395a8cf..bc8cada93e9f999729e77d26ab8d317c77207613 100644 (file)
@@ -290,6 +290,7 @@ void VideoStream::clear_queue()
 
 void VideoStream::schedule_original_frame(steady_clock::time_point local_pts,
                                           int64_t output_pts, function<void()> &&display_func,
+                                          QueueSpotHolder &&queue_spot_holder,
                                           unsigned stream_idx, int64_t input_pts)
 {
        fprintf(stderr, "output_pts=%ld  original      input_pts=%ld\n", output_pts, input_pts);
@@ -305,15 +306,17 @@ void VideoStream::schedule_original_frame(steady_clock::time_point local_pts,
        qf.stream_idx = stream_idx;
        qf.input_first_pts = input_pts;
        qf.display_func = move(display_func);
+       qf.queue_spot_holder = move(queue_spot_holder);
 
        unique_lock<mutex> lock(queue_lock);
        frame_queue.push_back(move(qf));
        queue_changed.notify_all();
 }
 
-bool VideoStream::schedule_faded_frame(steady_clock::time_point local_pts, int64_t output_pts,
-                                       function<void()> &&display_func, unsigned stream_idx,
-                                       int64_t input_pts, int secondary_stream_idx,
+void VideoStream::schedule_faded_frame(steady_clock::time_point local_pts, int64_t output_pts,
+                                       function<void()> &&display_func,
+                                       QueueSpotHolder &&queue_spot_holder,
+                                       unsigned stream_idx, int64_t input_pts, int secondary_stream_idx,
                                        int64_t secondary_input_pts, float fade_alpha)
 {
        fprintf(stderr, "output_pts=%ld  faded         input_pts=%ld,%ld  fade_alpha=%.2f\n", output_pts, input_pts, secondary_input_pts, fade_alpha);
@@ -327,7 +330,7 @@ bool VideoStream::schedule_faded_frame(steady_clock::time_point local_pts, int64
                unique_lock<mutex> lock(queue_lock);
                if (interpolate_resources.empty()) {
                        fprintf(stderr, "WARNING: Too many interpolated frames already in transit; dropping one.\n");
-                       return false;
+                       return;
                }
                resources = interpolate_resources.front();
                interpolate_resources.pop_front();
@@ -357,6 +360,7 @@ bool VideoStream::schedule_faded_frame(steady_clock::time_point local_pts, int64
        qf.resources = resources;
        qf.input_first_pts = input_pts;
        qf.display_func = move(display_func);
+       qf.queue_spot_holder = move(queue_spot_holder);
 
        qf.secondary_stream_idx = secondary_stream_idx;
        qf.secondary_input_pts = secondary_input_pts;
@@ -385,11 +389,11 @@ bool VideoStream::schedule_faded_frame(steady_clock::time_point local_pts, int64
        unique_lock<mutex> lock(queue_lock);
        frame_queue.push_back(move(qf));
        queue_changed.notify_all();
-       return true;
 }
 
-bool VideoStream::schedule_interpolated_frame(steady_clock::time_point local_pts,
+void VideoStream::schedule_interpolated_frame(steady_clock::time_point local_pts,
                                               int64_t output_pts, function<void()> &&display_func,
+                                              QueueSpotHolder &&queue_spot_holder,
                                               unsigned stream_idx, int64_t input_first_pts,
                                               int64_t input_second_pts, float alpha,
                                               int secondary_stream_idx, int64_t secondary_input_pts,
@@ -414,7 +418,7 @@ bool VideoStream::schedule_interpolated_frame(steady_clock::time_point local_pts
                unique_lock<mutex> lock(queue_lock);
                if (interpolate_resources.empty()) {
                        fprintf(stderr, "WARNING: Too many interpolated frames already in transit; dropping one.\n");
-                       return false;
+                       return;
                }
                resources = interpolate_resources.front();
                interpolate_resources.pop_front();
@@ -427,6 +431,7 @@ bool VideoStream::schedule_interpolated_frame(steady_clock::time_point local_pts
        qf.resources = resources;
        qf.id = id;
        qf.display_func = move(display_func);
+       qf.queue_spot_holder = move(queue_spot_holder);
 
        check_error();
 
@@ -507,16 +512,17 @@ bool VideoStream::schedule_interpolated_frame(steady_clock::time_point local_pts
        unique_lock<mutex> lock(queue_lock);
        frame_queue.push_back(move(qf));
        queue_changed.notify_all();
-       return true;
 }
 
 void VideoStream::schedule_refresh_frame(steady_clock::time_point local_pts,
-                                         int64_t output_pts, function<void()> &&display_func)
+                                         int64_t output_pts, function<void()> &&display_func,
+                                         QueueSpotHolder &&queue_spot_holder)
 {
        QueuedFrame qf;
        qf.type = QueuedFrame::REFRESH;
        qf.output_pts = output_pts;
        qf.display_func = move(display_func);
+       qf.queue_spot_holder = move(queue_spot_holder);
 
        unique_lock<mutex> lock(queue_lock);
        frame_queue.push_back(move(qf));
index 8038c7b8ef1ee34ad768a7b1ca7345a998b17df8..8e0b0abd597e2242f69ebd2524a8524ab72fc530 100644 (file)
@@ -10,6 +10,7 @@ extern "C" {
 
 #include "jpeg_frame_view.h"
 #include "ref_counted_gl_sync.h"
+#include "queue_spot_holder.h"
 
 #include <chrono>
 #include <condition_variable>
@@ -39,22 +40,25 @@ public:
        void clear_queue();
 
        // “display_func” is called after the frame has been calculated (if needed)
-       // and has gone out to the stream. Returns false on failure (ie., couldn't
-       // schedule the frame due to lack of resources).
+       // and has gone out to the stream.
        void schedule_original_frame(std::chrono::steady_clock::time_point,
                                     int64_t output_pts, std::function<void()> &&display_func,
+                                    QueueSpotHolder &&queue_spot_holder,
                                     unsigned stream_idx, int64_t input_pts);
-       bool schedule_faded_frame(std::chrono::steady_clock::time_point, int64_t output_pts,
-                                 std::function<void()> &&display_func, unsigned stream_idx,
-                                 int64_t input_pts, int secondary_stream_idx,
+       void schedule_faded_frame(std::chrono::steady_clock::time_point, int64_t output_pts,
+                                 std::function<void()> &&display_func,
+                                 QueueSpotHolder &&queue_spot_holder,
+                                 unsigned stream_idx, int64_t input_pts, int secondary_stream_idx,
                                  int64_t secondary_input_pts, float fade_alpha);
-       bool schedule_interpolated_frame(std::chrono::steady_clock::time_point, int64_t output_pts,
-                                 std::function<void()> &&display_func, unsigned stream_idx,
-                                 int64_t input_first_pts, int64_t input_second_pts, float alpha,
-                                 int secondary_stream_idx = -1, int64_t secondary_inputs_pts = -1,
+       void schedule_interpolated_frame(std::chrono::steady_clock::time_point, int64_t output_pts,
+                                 std::function<void()> &&display_func,
+                                 QueueSpotHolder &&queue_spot_holder,
+                                 unsigned stream_idx, int64_t input_first_pts, int64_t input_second_pts,
+                                 float alpha, int secondary_stream_idx = -1, int64_t secondary_inputs_pts = -1,
                                  float fade_alpha = 0.0f);  // -1 = no secondary frame.
        void schedule_refresh_frame(std::chrono::steady_clock::time_point, int64_t output_pts,
-                                   std::function<void()> &&display_func);
+                                   std::function<void()> &&display_func,
+                                   QueueSpotHolder &&queue_spot_holder);
 
 private:
        void encode_thread_func();
@@ -103,6 +107,8 @@ private:
                JPEGID id;
 
                std::function<void()> display_func;  // Called when the image is done decoding.
+
+               QueueSpotHolder queue_spot_holder;
        };
        std::deque<QueuedFrame> frame_queue;  // Under <queue_lock>.
        std::mutex queue_lock;