From: Steinar H. Gunderson Date: Sat, 27 Oct 2018 19:46:55 +0000 (+0200) Subject: Move to RAII queue counting, to fix some underflows. X-Git-Tag: 1.8.0~76^2~32 X-Git-Url: https://git.sesse.net/?p=nageru;a=commitdiff_plain;h=5c3d31147e398494e8ea8be386f7d533b2b3baaa Move to RAII queue counting, to fix some underflows. --- diff --git a/player.cpp b/player.cpp index 1432f04..dd37143 100644 --- a/player.cpp +++ b/player.cpp @@ -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 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 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 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 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 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 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 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 lock(queue_state_mu); + ++num_queued_frames; +} + +void Player::release_queue_spot() +{ + unique_lock lock(queue_state_mu); + assert(num_queued_frames > 0); + --num_queued_frames; + new_clip_changed.notify_all(); +} diff --git a/player.h b/player.h index f5adcf7..fb9f4a0 100644 --- 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 @@ -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 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 index 0000000..b9dee06 --- /dev/null +++ b/queue_spot_holder.h @@ -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) diff --git a/video_stream.cpp b/video_stream.cpp index 49458f8..bc8cada 100644 --- a/video_stream.cpp +++ b/video_stream.cpp @@ -290,6 +290,7 @@ void VideoStream::clear_queue() void VideoStream::schedule_original_frame(steady_clock::time_point local_pts, int64_t output_pts, function &&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 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 &&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 &&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 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 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 &&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 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 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 &&display_func) + int64_t output_pts, function &&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 lock(queue_lock); frame_queue.push_back(move(qf)); diff --git a/video_stream.h b/video_stream.h index 8038c7b..8e0b0ab 100644 --- a/video_stream.h +++ b/video_stream.h @@ -10,6 +10,7 @@ extern "C" { #include "jpeg_frame_view.h" #include "ref_counted_gl_sync.h" +#include "queue_spot_holder.h" #include #include @@ -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 &&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 &&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 &&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 &&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 &&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 &&display_func); + std::function &&display_func, + QueueSpotHolder &&queue_spot_holder); private: void encode_thread_func(); @@ -103,6 +107,8 @@ private: JPEGID id; std::function display_func; // Called when the image is done decoding. + + QueueSpotHolder queue_spot_holder; }; std::deque frame_queue; // Under . std::mutex queue_lock;