From 798819f74c6d194b95d3458f88b7ad60ef5f282c Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 19 Nov 2019 23:57:14 +0100 Subject: [PATCH] Nicer protection of _player_video. Always run GL thread rather than starting/stopping it. --- src/wx/gl_video_view.cc | 41 ++++++++++++++++++++++--------------- src/wx/gl_video_view.h | 6 ++++++ src/wx/simple_video_view.cc | 12 +++++------ src/wx/video_view.h | 15 ++++++++++---- 4 files changed, 47 insertions(+), 27 deletions(-) diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index df45a143f..3549cfe8f 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -54,7 +54,8 @@ using boost::optional; GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent) : VideoView (viewer) , _vsync_enabled (false) - , _thread (0) + , _playing (false) + , _one_shot (false) { _canvas = new wxGLCanvas (parent, wxID_ANY, 0, wxDefaultPosition, wxDefaultSize, wxFULL_REPAINT_ON_RESIZE); _canvas->Bind (wxEVT_PAINT, boost::bind(&GLVideoView::paint, this)); @@ -91,14 +92,14 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent) glGenTextures (1, &_id); glBindTexture (GL_TEXTURE_2D, _id); glPixelStorei (GL_UNPACK_ALIGNMENT, 1); + + _thread = new boost::thread (boost::bind(&GLVideoView::thread, this)); } GLVideoView::~GLVideoView () { - if (_thread) { - _thread->interrupt (); - _thread->join (); - } + _thread->interrupt (); + _thread->join (); delete _thread; glDeleteTextures (1, &_id); @@ -263,18 +264,16 @@ GLVideoView::set_image (shared_ptr image) void GLVideoView::start () { - _thread = new boost::thread (boost::bind(&GLVideoView::thread, this)); + boost::mutex::scoped_lock lm (_playing_mutex); + _playing = true; + _playing_condition.notify_all (); } void GLVideoView::stop () { - if (_thread) { - _thread->interrupt (); - _thread->join (); - } - delete _thread; - _thread = 0; + boost::mutex::scoped_lock lm (_playing_mutex); + _playing = false; } void @@ -288,6 +287,13 @@ try std::cout << "Here we go " << video_frame_rate() << " " << to_string(length()) << "\n"; while (true) { + boost::mutex::scoped_lock lm (_playing_mutex); + while (!_playing || !_one_shot) { + _playing_condition.wait (lm); + } + _one_shot = false; + lm.unlock (); + dcpomatic::DCPTime const next = position() + one_video_frame(); if (next >= length()) { @@ -297,10 +303,7 @@ try } get_next_frame (false); - { - boost::mutex::scoped_lock lm (_mutex); - set_image (_player_video.first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); - } + set_image (player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); draw (); while (time_until_next_frame() < 5) { @@ -323,6 +326,10 @@ catch (boost::thread_interrupted& e) bool GLVideoView::display_next_frame (bool non_blocking) { - return get_next_frame (non_blocking); + bool const r = get_next_frame (non_blocking); + boost::mutex::scoped_lock lm (_playing_mutex); + _one_shot = true; + _playing_condition.notify_all (); + return r; } diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index 44a4057fd..22b6d8513 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -25,6 +25,7 @@ #include #include #include +#include #undef None #undef Success @@ -60,4 +61,9 @@ private: boost::optional _size; bool _vsync_enabled; boost::thread* _thread; + + boost::mutex _playing_mutex; + boost::condition _playing_condition; + bool _playing; + bool _one_shot; }; diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index dcf30cd1a..135892e07 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -200,13 +200,13 @@ SimpleVideoView::display_next_frame (bool non_blocking) void SimpleVideoView::display_player_video () { - if (!_player_video.first) { + if (!player_video().first) { set_image (shared_ptr()); _viewer->refresh_view (); return; } - if (_viewer->playing() && (_viewer->time() - _player_video.second) > _viewer->one_video_frame()) { + if (_viewer->playing() && (_viewer->time() - player_video().second) > _viewer->one_video_frame()) { /* Too late; just drop this frame before we try to get its image (which will be the time-consuming part if this frame is J2K). */ @@ -235,15 +235,15 @@ SimpleVideoView::display_player_video () _viewer->_state_timer.set ("get image"); set_image ( - _player_video.first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true) + player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true) ); _viewer->_state_timer.set ("ImageChanged"); - _viewer->ImageChanged (_player_video.first); + _viewer->ImageChanged (player_video().first); _viewer->_state_timer.unset (); - _viewer->_inter_position = _player_video.first->inter_position (); - _viewer->_inter_size = _player_video.first->inter_size (); + _viewer->_inter_position = player_video().first->inter_position (); + _viewer->_inter_size = player_video().first->inter_size (); _viewer->refresh_view (); diff --git a/src/wx/video_view.h b/src/wx/video_view.h index 06067130c..8d763204c 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -84,20 +84,23 @@ protected: bool get_next_frame (bool non_blocking); int time_until_next_frame () const; dcpomatic::DCPTime one_video_frame () const; + int video_frame_rate () const { boost::mutex::scoped_lock lm (_mutex); return _video_frame_rate; } + dcpomatic::DCPTime length () const { boost::mutex::scoped_lock lm (_mutex); return _length; } - FilmViewer* _viewer; - std::pair, dcpomatic::DCPTime> _player_video; + std::pair, dcpomatic::DCPTime> player_video () const { + boost::mutex::scoped_lock lm (_mutex); + return _player_video; + } - /** Mutex protecting all the state in VideoView */ - mutable boost::mutex _mutex; + FilmViewer* _viewer; #ifdef DCPOMATIC_VARIANT_SWAROOP bool _in_watermark; @@ -106,6 +109,10 @@ protected: #endif private: + /** Mutex protecting all the state in VideoView */ + mutable boost::mutex _mutex; + + std::pair, dcpomatic::DCPTime> _player_video; int _video_frame_rate; dcpomatic::DCPTime _length; }; -- 2.30.2