Nicer protection of _player_video. Always run GL thread rather than starting/stoppin...
authorCarl Hetherington <cth@carlh.net>
Tue, 19 Nov 2019 22:57:14 +0000 (23:57 +0100)
committerCarl Hetherington <cth@carlh.net>
Wed, 8 Jan 2020 20:56:47 +0000 (21:56 +0100)
src/wx/gl_video_view.cc
src/wx/gl_video_view.h
src/wx/simple_video_view.cc
src/wx/video_view.h

index df45a14..3549cfe 100644 (file)
@@ -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<const Image> 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;
 }
 
index 44a4057..22b6d85 100644 (file)
@@ -25,6 +25,7 @@
 #include <dcp/util.h>
 #include <boost/shared_ptr.hpp>
 #include <boost/thread.hpp>
+#include <boost/thread/condition.hpp>
 #undef None
 #undef Success
 
@@ -60,4 +61,9 @@ private:
        boost::optional<dcp::Size> _size;
        bool _vsync_enabled;
        boost::thread* _thread;
+
+       boost::mutex _playing_mutex;
+       boost::condition _playing_condition;
+       bool _playing;
+       bool _one_shot;
 };
index dcf30cd..135892e 100644 (file)
@@ -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<Image>());
                _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 ();
 
index 0606713..8d76320 100644 (file)
@@ -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<boost::shared_ptr<PlayerVideo>, dcpomatic::DCPTime> _player_video;
+       std::pair<boost::shared_ptr<PlayerVideo>, 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<boost::shared_ptr<PlayerVideo>, dcpomatic::DCPTime> _player_video;
        int _video_frame_rate;
        dcpomatic::DCPTime _length;
 };