Various cleanups and thread-safety.

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index dcbfedc8194101fb2597dbf90c5adc7317a81edc..df66a8ade9120591b28cd086813ebd24988d0d18 100644 (file)
--- 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 ec481f34e7f10d46e1fd24b518335020ada0c590..60cde60d00139b10fb6a6546995e1eca66b50196 100644 (file)
--- 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<void (boost::weak_ptr<PlayerVideo>)> ImageChanged;
        boost::signals2::signal<void (dcpomatic::DCPTime)> Started;
        boost::signals2::signal<void (dcpomatic::DCPTime)> Stopped;
        /** While playing back we reached the end of the film (emitted from GUI thread) */
        boost::signals2::signal<void ()> Finished;
-       void emit_finished ();
 
        boost::signals2::signal<bool ()> 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 ce152787a971fc31cc7e3ecdc29c54350e19764e..869d555cb06ec967ca19a0a7c6c256ff73f6cfd2 100644 (file)
--- 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<int> 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<int> 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<int> 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 4f509049b3d4d954b60fd748f36f7188c24f9dff..827b12861c5d47ccb5caadadc668fdbb5b5222b2 100644 (file)
--- 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 5dd857fabc4a4089f8aa4ea08a98e09aeb55ba99..ee14f4a33523a6d428f2bbd0e633eabf595a4b99 100644 (file)
--- 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> 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 fd2684e4175a3d6a47e98f8dcc3d0affa2eca8d4..5d5d3316377c2c3e03a9af08d11254086bf479bd 100644 (file)
--- 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 <boost/shared_ptr.hpp>
 #include <boost/signals2.hpp>
 #include <boost/thread.hpp>
+#include <boost/noncopyable.hpp>
 
 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;