From f77529bdfa01ae13f889442900988fc401b63c62 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 21 Nov 2019 23:43:23 +0100 Subject: [PATCH] Various cleanups and thread-safety. --- src/wx/film_viewer.cc | 20 ++++++++++++++++--- src/wx/film_viewer.h | 3 ++- src/wx/gl_video_view.cc | 43 ++++++++++++++++++++++++++--------------- src/wx/gl_video_view.h | 4 ++++ src/wx/video_view.cc | 15 +++++++++----- src/wx/video_view.h | 10 +++++++++- 6 files changed, 69 insertions(+), 26 deletions(-) diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index dcbfedc81..df66a8ade 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -334,6 +334,8 @@ FilmViewer::stop () _playing = false; _video_view->stop (); Stopped (position()); + + _video_view->rethrow (); return true; } @@ -379,6 +381,8 @@ FilmViewer::film_change (ChangeType type, Film::Property p) recreate_butler (); } else if (p == Film::VIDEO_FRAME_RATE) { _video_view->set_video_frame_rate (_film->video_frame_rate()); + } else if (p == Film::THREE_D) { + _video_view->set_three_d (_film->three_d()); } } @@ -636,11 +640,21 @@ FilmViewer::set_pad_black (bool p) _pad_black = p; } -/* May be called from a non-UI thread */ +/** Called when a player has finished the current film. + * May be called from a non-UI thread. + */ void -FilmViewer::emit_finished () +FilmViewer::finished () { - emit (boost::bind(boost::ref(Finished))); + emit (boost::bind(&FilmViewer::ui_finished, this)); +} + +/** Called by finished() in the UI thread */ +void +FilmViewer::ui_finished () +{ + stop (); + Finished (); } int diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index ec481f34e..60cde60d0 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -131,13 +131,13 @@ public: ClosedCaptionsDialog* closed_captions_dialog () const { return _closed_captions_dialog; } + void finished (); boost::signals2::signal)> ImageChanged; boost::signals2::signal Started; boost::signals2::signal Stopped; /** While playing back we reached the end of the film (emitted from GUI thread) */ boost::signals2::signal Finished; - void emit_finished (); boost::signals2::signal PlaybackPermitted; @@ -152,6 +152,7 @@ private: void recreate_butler (); void config_changed (Config::Property); void film_length_change (); + void ui_finished (); dcpomatic::DCPTime uncorrected_time () const; Frame average_latency () const; diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index ce152787a..869d555cb 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -133,8 +133,11 @@ check_gl_error (char const * last) void GLVideoView::update () { - if (!_canvas->IsShownOnScreen()) { - return; + { + boost::mutex::scoped_lock lm (_canvas_mutex); + if (!_canvas->IsShownOnScreen()) { + return; + } } request_one_shot (); } @@ -155,16 +158,22 @@ GLVideoView::draw (Position inter_position, dcp::Size inter_size) check_gl_error ("glDisable GL_DEPTH_TEST"); glBlendFunc (GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); - if (_canvas->GetSize().x < 64 || _canvas->GetSize().y < 0) { + wxSize canvas_size; + { + boost::mutex::scoped_lock lm (_canvas_mutex); + canvas_size = _canvas->GetSize (); + } + + if (canvas_size.GetWidth() < 64 || canvas_size.GetHeight() < 0) { return; } - glViewport (0, 0, _canvas->GetSize().x, _canvas->GetSize().y); + glViewport (0, 0, canvas_size.GetWidth(), canvas_size.GetHeight()); check_gl_error ("glViewport"); glMatrixMode (GL_PROJECTION); glLoadIdentity (); - gluOrtho2D (0, _canvas->GetSize().x, _canvas->GetSize().y, 0); + gluOrtho2D (0, canvas_size.GetWidth(), canvas_size.GetHeight(), 0); check_gl_error ("gluOrtho2d"); glMatrixMode (GL_MODELVIEW); glLoadIdentity (); @@ -197,8 +206,6 @@ GLVideoView::draw (Position inter_position, dcp::Size inter_size) glEnd (); } - wxSize const canvas_size = _canvas->GetSize (); - if (!_viewer->pad_black() && out_size.width < canvas_size.GetWidth()) { glBegin (GL_QUADS); /* XXX: these colours are right for GNOME; may need adjusting for other OS */ @@ -241,6 +248,8 @@ GLVideoView::draw (Position inter_position, dcp::Size inter_size) } glFlush(); + + boost::mutex::scoped_lock lm (_canvas_mutex); _canvas->SwapBuffers(); } @@ -291,9 +300,11 @@ void GLVideoView::thread () try { - /* XXX_b: check all calls and signal emissions in this method & protect them if necessary */ - _context = new wxGLContext (_canvas); - _canvas->SetCurrent (*_context); + { + boost::mutex::scoped_lock lm (_canvas_mutex); + _context = new wxGLContext (_canvas); //local + _canvas->SetCurrent (*_context); + } while (true) { boost::mutex::scoped_lock lm (_playing_mutex); @@ -309,12 +320,12 @@ try dcpomatic::DCPTime const next = position() + one_video_frame(); if (next >= length()) { - _viewer->stop (); - _viewer->emit_finished (); + _viewer->finished (); continue; } get_next_frame (false); + //-- set_image (player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); inter_position = player_video().first->inter_position(); inter_size = player_video().first->inter_size(); @@ -330,13 +341,13 @@ try dcpomatic_sleep_milliseconds (time_until_next_frame()); } - delete _context; + /* XXX: leaks _context, but that seems preferable to deleting it here + * without also deleting the wxGLCanvas. + */ } catch (boost::thread_interrupted& e) { - /* XXX_b: store exceptions here */ - delete _context; - return; + store_current (); } bool diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index 4f509049b..827b12861 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -57,6 +57,10 @@ private: void create (); void check_for_butler_errors (); + /* Mutex for use of _canvas; it's only contended when our ::thread + is started up so this may be overkill. + */ + boost::mutex _canvas_mutex; wxGLCanvas* _canvas; wxGLContext* _context; diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 5dd857fab..ee14f4a33 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -23,6 +23,8 @@ #include "film_viewer.h" #include "lib/butler.h" +using boost::shared_ptr; + VideoView::VideoView (FilmViewer* viewer) : _viewer (viewer) #ifdef DCPOMATIC_VARIANT_SWAROOP @@ -31,6 +33,7 @@ VideoView::VideoView (FilmViewer* viewer) , _state_timer ("viewer") , _video_frame_rate (0) , _eyes (EYES_LEFT) + , _three_d (false) , _dropped (0) , _gets (0) { @@ -45,30 +48,32 @@ VideoView::clear () _player_video.second = dcpomatic::DCPTime (); } -/** @param non_blocking true to return false quickly if no video is available quickly. +/** Could be called from any thread. + * @param non_blocking true to return false quickly if no video is available quickly. * @return false if we gave up because it would take too long, otherwise true. */ bool VideoView::get_next_frame (bool non_blocking) { - if (_length == dcpomatic::DCPTime()) { + if (length() == dcpomatic::DCPTime()) { return true; } - DCPOMATIC_ASSERT (_viewer->butler()); + shared_ptr butler = _viewer->butler (); + DCPOMATIC_ASSERT (butler); add_get (); boost::mutex::scoped_lock lm (_mutex); do { Butler::Error e; - _player_video = _viewer->butler()->get_video (!non_blocking, &e); + _player_video = butler->get_video (!non_blocking, &e); if (!_player_video.first && e == Butler::AGAIN) { return false; } } while ( _player_video.first && - _viewer->film()->three_d() && + _three_d && _eyes != _player_video.first->eyes() && _player_video.first->eyes() != EYES_BOTH ); diff --git a/src/wx/video_view.h b/src/wx/video_view.h index fd2684e41..5d5d33163 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -24,16 +24,18 @@ #include "lib/dcpomatic_time.h" #include "lib/timer.h" #include "lib/types.h" +#include "lib/exception_store.h" #include #include #include +#include class Image; class wxWindow; class FilmViewer; class PlayerVideo; -class VideoView +class VideoView : public ExceptionStore, public boost::noncopyable { public: VideoView (FilmViewer* viewer); @@ -93,6 +95,11 @@ public: _eyes = eyes; } + void set_three_d (bool t) { + boost::mutex::scoped_lock lm (_mutex); + _three_d = t; + } + protected: /* XXX_b: to remove */ friend class FilmViewer; @@ -145,6 +152,7 @@ private: /** length of the film we are playing, or 0 if there is none */ dcpomatic::DCPTime _length; Eyes _eyes; + bool _three_d; int _dropped; int _gets; -- 2.30.2