Nicer protection of _player_video. Always run GL thread rather than starting/stoppin...

diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc
index df45a143f42206fde7d1633aa656703bf2a56052..3549cfe8f78a06219ab719f0dab7bc6b78ebfa25 100644 (file)
--- 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<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;
 }
 

diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h
index 44a4057fd1f55cd816fe0b377b91d3b22de024e9..22b6d8513d18e4434be85e57887acf2c30a54ac8 100644 (file)
--- a/src/wx/gl_video_view.h
+++ b/src/wx/gl_video_view.h
@@ -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;
 };

diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc
index dcf30cd1a7bdadc0ad2e19805acb2f51d8d6255e..135892e07c8d3042c61c8c177d39858daec0b4e9 100644 (file)
--- 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<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 ();
 

diff --git a/src/wx/video_view.h b/src/wx/video_view.h
index 06067130c5a330d64a103b4bd47f0ac75eac514f..8d763204ce6ae24a31e5370f28fb74d82b718a55 100644 (file)
--- 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<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;
 };