Fix confusion between one-shot and playing causing surprising

[dcpomatic.git] / src / wx / gl_video_view.cc

diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc
index bfc611d3701dbfabbcf32f018fb25073b2e84608..786cee84a3a1afd9a8c19fc19640c7fc5ca433c6 100644 (file)
--- a/src/wx/gl_video_view.cc
+++ b/src/wx/gl_video_view.cc
@@ -55,8 +55,8 @@ using boost::optional;
 
 GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent)
        : VideoView (viewer)
+       , _have_storage (false)
        , _vsync_enabled (false)
-       , _thread (0)
        , _playing (false)
        , _one_shot (false)
 {
@@ -104,9 +104,12 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent)
 
 GLVideoView::~GLVideoView ()
 {
-       _thread->interrupt ();
-       _thread->join ();
-       delete _thread;
+       try {
+               _thread.interrupt ();
+               _thread.join ();
+       } catch (...) {
+
+       }
 
        glDeleteTextures (1, &_id);
 }
@@ -114,6 +117,10 @@ GLVideoView::~GLVideoView ()
 void
 GLVideoView::check_for_butler_errors ()
 {
+       if (!_viewer->butler()) {
+               return;
+       }
+
        try {
                _viewer->butler()->rethrow ();
        } catch (DecodeError& e) {
@@ -264,11 +271,21 @@ GLVideoView::set_image (shared_ptr<const Image> image)
        DCPOMATIC_ASSERT (image->pixel_format() == AV_PIX_FMT_RGB24);
        DCPOMATIC_ASSERT (!image->aligned());
 
+       if (image->size() != _size) {
+               _have_storage = false;
+       }
+
        _size = image->size ();
        glPixelStorei (GL_UNPACK_ALIGNMENT, 1);
        check_gl_error ("glPixelStorei");
-       glTexImage2D (GL_TEXTURE_2D, 0, GL_RGB8, _size->width, _size->height, 0, GL_RGB, GL_UNSIGNED_BYTE, image->data()[0]);
-       check_gl_error ("glTexImage2D");
+       if (_have_storage) {
+               glTexSubImage2D (GL_TEXTURE_2D, 0, 0, 0, _size->width, _size->height, GL_RGB, GL_UNSIGNED_BYTE, image->data()[0]);
+               check_gl_error ("glTexSubImage2D");
+       } else {
+               glTexImage2D (GL_TEXTURE_2D, 0, GL_RGB8, _size->width, _size->height, 0, GL_RGB, GL_UNSIGNED_BYTE, image->data()[0]);
+               _have_storage = true;
+               check_gl_error ("glTexImage2D");
+       }
 
        glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
        glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
@@ -286,7 +303,7 @@ GLVideoView::start ()
 
        boost::mutex::scoped_lock lm (_playing_mutex);
        _playing = true;
-       _playing_condition.notify_all ();
+       _thread_work_condition.notify_all ();
 }
 
 void
@@ -296,6 +313,44 @@ GLVideoView::stop ()
        _playing = false;
 }
 
+
+void
+GLVideoView::thread_playing ()
+{
+       if (length() != dcpomatic::DCPTime()) {
+               dcpomatic::DCPTime const next = position() + one_video_frame();
+
+               if (next >= length()) {
+                       _viewer->finished ();
+                       return;
+               }
+
+               get_next_frame (false);
+               set_image_and_draw ();
+       }
+
+       while (true) {
+               optional<int> n = time_until_next_frame();
+               if (!n || *n > 5) {
+                       break;
+               }
+               get_next_frame (true);
+               add_dropped ();
+       }
+}
+
+
+void
+GLVideoView::set_image_and_draw ()
+{
+       shared_ptr<PlayerVideo> pv = player_video().first;
+       if (pv) {
+               set_image (pv->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true));
+               draw (pv->inter_position(), pv->inter_size());
+       }
+}
+
+
 void
 GLVideoView::thread ()
 try
@@ -309,36 +364,19 @@ try
        while (true) {
                boost::mutex::scoped_lock lm (_playing_mutex);
                while (!_playing && !_one_shot) {
-                       _playing_condition.wait (lm);
+                       _thread_work_condition.wait (lm);
                }
-               _one_shot = false;
                lm.unlock ();
 
-               Position<int> inter_position;
-               dcp::Size inter_size;
-               if (length() != dcpomatic::DCPTime()) {
-                       dcpomatic::DCPTime const next = position() + one_video_frame();
-
-                       if (next >= length()) {
-                               _viewer->finished ();
-                               continue;
-                       }
-
-                       get_next_frame (false);
-                       shared_ptr<PlayerVideo> pv = player_video().first;
-                       set_image (pv->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true));
-                       inter_position = pv->inter_position();
-                       inter_size = pv->inter_size();
-               }
-               draw (inter_position, inter_size);
-
-               while (time_until_next_frame() < 5) {
-                       get_next_frame (true);
-                       add_dropped ();
+               if (_playing) {
+                       thread_playing ();
+               } else if (_one_shot) {
+                       _one_shot = false;
+                       set_image_and_draw ();
                }
 
                boost::this_thread::interruption_point ();
-               dcpomatic_sleep_milliseconds (time_until_next_frame());
+               dcpomatic_sleep_milliseconds (time_until_next_frame().get_value_or(0));
        }
 
        /* XXX: leaks _context, but that seems preferable to deleting it here
@@ -363,13 +401,13 @@ GLVideoView::request_one_shot ()
 {
        boost::mutex::scoped_lock lm (_playing_mutex);
        _one_shot = true;
-       _playing_condition.notify_all ();
+       _thread_work_condition.notify_all ();
 }
 
 void
 GLVideoView::create ()
 {
-       if (!_thread) {
-               _thread = new boost::thread (boost::bind(&GLVideoView::thread, this));
+       if (!_thread.joinable()) {
+               _thread = boost::thread (boost::bind(&GLVideoView::thread, this));
        }
 }