From a64b7a670a29674aa4a6cb2abe2f5a29f6cc14bc Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Wed, 12 Dec 2018 22:30:51 +0100 Subject: [PATCH] Fix some Futatabi shutdown problems. --- futatabi/mainwindow.cpp | 12 ++++++++++-- futatabi/mainwindow.h | 4 +++- futatabi/player.cpp | 35 +++++++++++++++++++++++++++++------ futatabi/player.h | 5 +++++ futatabi/video_stream.cpp | 4 +++- futatabi/video_stream.h | 2 ++ 6 files changed, 52 insertions(+), 10 deletions(-) diff --git a/futatabi/mainwindow.cpp b/futatabi/mainwindow.cpp index 1412e25..24a9d5b 100644 --- a/futatabi/mainwindow.cpp +++ b/futatabi/mainwindow.cpp @@ -126,8 +126,8 @@ MainWindow::MainWindow() this, &MainWindow::playlist_selection_changed); playlist_selection_changed(); // First time set-up. - preview_player = new Player(ui->preview_display, /*also_output_to_stream=*/false); - live_player = new Player(ui->live_display, /*also_output_to_stream=*/true); + preview_player.reset(new Player(ui->preview_display, /*also_output_to_stream=*/false)); + live_player.reset(new Player(ui->live_display, /*also_output_to_stream=*/true)); live_player->set_done_callback([this]{ post_to_main_thread([this]{ live_player_clip_done(); @@ -149,6 +149,11 @@ MainWindow::MainWindow() this, &MainWindow::clip_list_selection_changed); } +MainWindow::~MainWindow() +{ + // Empty so that we can forward-declare Player in the .h file. +} + void MainWindow::cue_in_clicked() { if (!cliplist_clips->empty() && cliplist_clips->back()->pts_out < 0) { @@ -374,6 +379,9 @@ pair MainWindow::live_player_get_next_clip() { // playlist_clips can only be accessed on the main thread. // Hopefully, we won't have to wait too long for this to come back. + // + // TODO: If MainWindow is in the process of being destroyed and waiting + // for Player to shut down, we could have a deadlock here. promise> clip_promise; future> clip = clip_promise.get_future(); post_to_main_thread([this, &clip_promise] { diff --git a/futatabi/mainwindow.h b/futatabi/mainwindow.h index 4e3800e..57814b3 100644 --- a/futatabi/mainwindow.h +++ b/futatabi/mainwindow.h @@ -5,6 +5,7 @@ #include "db.h" #include "state.pb.h" +#include #include #include #include @@ -24,6 +25,7 @@ class MainWindow : public QMainWindow { public: MainWindow(); + ~MainWindow(); // HTTP callback. TODO: Does perhaps not belong to MainWindow? std::pair get_queue_status() const; @@ -33,7 +35,7 @@ public: private: QLabel *disk_free_label; - Player *preview_player, *live_player; + std::unique_ptr preview_player, live_player; DB db; // State when doing a scrub operation on a timestamp with the mouse. diff --git a/futatabi/player.cpp b/futatabi/player.cpp index d926974..e20a43c 100644 --- a/futatabi/player.cpp +++ b/futatabi/player.cpp @@ -54,7 +54,7 @@ void Player::thread_func(bool also_output_to_stream) bool got_next_clip = false; double next_clip_fade_time = -1.0; - for ( ;; ) { + while (!should_quit) { wait_for_clip: bool clip_ready; steady_clock::time_point before_sleep = steady_clock::now(); @@ -63,8 +63,11 @@ wait_for_clip: { unique_lock lock(queue_state_mu); clip_ready = new_clip_changed.wait_for(lock, milliseconds(100), [this] { - return new_clip_ready && current_clip.pts_in != -1; + return should_quit || (new_clip_ready && current_clip.pts_in != -1); }); + if (should_quit) { + return; + } new_clip_ready = false; playing = true; } @@ -114,7 +117,7 @@ got_clip: int64_t in_pts_start_next_clip = -1; steady_clock::time_point next_frame_start; - for (int frameno = 0; ; ++frameno) { // Ends when the clip ends. + for (int frameno = 0; !should_quit; ++frameno) { // Ends when the clip ends. double out_pts = out_pts_origin + TIMEBASE * frameno / output_framerate; next_frame_start = origin + microseconds(lrint((out_pts - out_pts_origin) * 1e6 / TIMEBASE)); @@ -198,8 +201,11 @@ got_clip: if (video_stream == nullptr) { // No queue, just wait until the right time and then show the frame. new_clip_changed.wait_until(lock, next_frame_start, [this]{ - return new_clip_ready || override_stream_idx != -1; + return should_quit || new_clip_ready || override_stream_idx != -1; }); + if (should_quit) { + return; + } } else { // If the queue is full (which is really the state we'd like to be in), // wait until there's room for one more frame (ie., one was output from @@ -211,9 +217,12 @@ got_clip: if (num_queued_frames < max_queued_frames) { return true; } - return new_clip_ready || override_stream_idx != -1; + return should_quit || new_clip_ready || override_stream_idx != -1; }); } + if (should_quit) { + return; + } if (new_clip_ready) { if (video_stream != nullptr) { lock.unlock(); // Urg. @@ -307,6 +316,10 @@ got_clip: } } + if (should_quit) { + return; + } + // The clip ended. // Last-ditch effort to get the next clip (if e.g. the fade time was zero seconds). @@ -373,7 +386,17 @@ bool Player::find_surrounding_frames(int64_t pts, int stream_idx, FrameOnDisk *f Player::Player(JPEGFrameView *destination, bool also_output_to_stream) : destination(destination) { - thread(&Player::thread_func, this, also_output_to_stream).detach(); + player_thread = thread(&Player::thread_func, this, also_output_to_stream); +} + +Player::~Player() +{ + should_quit = true; + if (video_stream != nullptr) { + video_stream->stop(); + } + new_clip_changed.notify_all(); + player_thread.join(); } void Player::play_clip(const Clip &clip, size_t clip_idx, unsigned stream_idx) diff --git a/futatabi/player.h b/futatabi/player.h index c7f8e07..b57bada 100644 --- a/futatabi/player.h +++ b/futatabi/player.h @@ -12,6 +12,7 @@ extern "C" { #include #include #include +#include class JPEGFrameView; class VideoStream; @@ -21,6 +22,7 @@ class QSurfaceFormat; class Player : public QueueInterface { public: Player(JPEGFrameView *destination, bool also_output_to_stream); + ~Player(); void play_clip(const Clip &clip, size_t clip_idx, unsigned stream_idx); void override_angle(unsigned stream_idx); // For the current clip only. @@ -55,6 +57,9 @@ private: // Returns false if pts is after the last frame. bool find_surrounding_frames(int64_t pts, int stream_idx, FrameOnDisk *frame_lower, FrameOnDisk *frame_upper); + std::thread player_thread; + std::atomic should_quit{false}; + JPEGFrameView *destination; done_callback_func done_callback; next_clip_callback_func next_clip_callback; diff --git a/futatabi/video_stream.cpp b/futatabi/video_stream.cpp index c4cf52e..6749f0d 100644 --- a/futatabi/video_stream.cpp +++ b/futatabi/video_stream.cpp @@ -269,6 +269,8 @@ void VideoStream::start() void VideoStream::stop() { + should_quit = true; + clear_queue(); encode_thread.join(); } @@ -552,7 +554,7 @@ void VideoStream::encode_thread_func() exit(1); } - for ( ;; ) { + while (!should_quit) { QueuedFrame qf; { unique_lock lock(queue_lock); diff --git a/futatabi/video_stream.h b/futatabi/video_stream.h index d0634e0..d4cb18e 100644 --- a/futatabi/video_stream.h +++ b/futatabi/video_stream.h @@ -13,6 +13,7 @@ extern "C" { #include "shared/ref_counted_gl_sync.h" #include "queue_spot_holder.h" +#include #include #include #include @@ -66,6 +67,7 @@ private: void encode_thread_func(); std::thread encode_thread; + std::atomic should_quit{false}; static int write_packet2_thunk(void *opaque, uint8_t *buf, int buf_size, AVIODataMarkerType type, int64_t time); int write_packet2(uint8_t *buf, int buf_size, AVIODataMarkerType type, int64_t time); -- 2.39.2