Various tweaks to fix playback at the end of a film (#1858).

The most questionable change here is probably how
SimpleVideoView::display_next_frame no longer re-schedules
itself if the call to get_next_frame returned AGAIN; it seems
wrong to do that when FilmViewer::idle_handler() also reschedules
itself when display_next_frame() returns AGAIN.

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 7adf2ba2d16b49716536c179eb16319fb4f22c01..920ca3a10bce5c874c4b420056fca527347f8f32 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -137,11 +137,11 @@ FilmViewer::idle_handler ()
                return;
        }
 
-       if (_video_view->display_next_frame(true)) {
-               _idle_get = false;
-       } else {
+       if (_video_view->display_next_frame(true) == VideoView::AGAIN) {
                /* get() could not complete quickly so we'll try again later */
                signal_manager->when_idle (boost::bind(&FilmViewer::idle_handler, this));
+       } else {
+               _idle_get = false;
        }
 }
 
@@ -346,8 +346,11 @@ FilmViewer::start ()
        }
 
        _playing = true;
-       _video_view->start ();
+       /* Calling start() below may directly result in Stopped being emitted, and if that
+        * happens we want it to come after the Started signal, so do that first.
+        */
        Started (position());
+       _video_view->start ();
 }
 
 bool
@@ -496,7 +499,7 @@ FilmViewer::seek (DCPTime t, bool accurate)
                /* We're going to start playing again straight away
                   so wait for the seek to finish.
                */
-               while (!_video_view->display_next_frame(false)) {}
+               while (_video_view->display_next_frame(false) == VideoView::AGAIN) {}
        }
 
        resume ();

diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc
index e7125501eb469677d5061cac0310f074d7dc4d3b..0d79a75627973d9f44d03515ecbb490eb0d93969 100644 (file)
--- a/src/wx/gl_video_view.cc
+++ b/src/wx/gl_video_view.cc
@@ -405,14 +405,16 @@ catch (boost::thread_interrupted& e)
        store_current ();
 }
 
-bool
+
+VideoView::NextFrameResult
 GLVideoView::display_next_frame (bool non_blocking)
 {
-       bool const r = get_next_frame (non_blocking);
+       NextFrameResult const r = get_next_frame (non_blocking);
        request_one_shot ();
        return r;
 }
 
+
 void
 GLVideoView::request_one_shot ()
 {

diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h
index 1f9ed08ea75e78e2ef53bbbe48ca151389ea3977..675b324fc4d4095c9df248bea2a859edbfbb6c67 100644 (file)
--- a/src/wx/gl_video_view.h
+++ b/src/wx/gl_video_view.h
@@ -48,7 +48,7 @@ public:
        void start ();
        void stop ();
 
-       bool display_next_frame (bool);
+       NextFrameResult display_next_frame (bool);
 
        bool vsync_enabled () const {
                return _vsync_enabled;

diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc
index 808f885f8b6fea97e2bd83fd6317910251ae1a32..2b86ea68884194b0711ee75e3a95bdf9d0b5f254 100644 (file)
--- a/src/wx/simple_video_view.cc
+++ b/src/wx/simple_video_view.cc
@@ -156,19 +156,12 @@ SimpleVideoView::start ()
  *  false to ask the butler to block until it has video (unless it is suspended).
  *  @return true on success, false if we did nothing because it would have taken too long.
  */
-bool
+VideoView::NextFrameResult
 SimpleVideoView::display_next_frame (bool non_blocking)
 {
-       bool r = get_next_frame (non_blocking);
-       if (!r) {
-               if (non_blocking) {
-                       /* No video available; return saying we failed */
-                       return false;
-               } else {
-                       /* Player was suspended; come back later */
-                       signal_manager->when_idle (boost::bind(&SimpleVideoView::display_next_frame, this, false));
-                       return false;
-               }
+       NextFrameResult const r = get_next_frame (non_blocking);
+       if (r != SUCCESS) {
+               return r;
        }
 
        update ();
@@ -179,7 +172,7 @@ SimpleVideoView::display_next_frame (bool non_blocking)
                error_dialog (get(), e.what());
        }
 
-       return true;
+       return SUCCESS;
 }
 
 void

diff --git a/src/wx/simple_video_view.h b/src/wx/simple_video_view.h
index 323047adaca9182fcc237c0dfa6a330aa3f6cfd7..93b9c6e3d604862ad39f087c57c95dc17756f3b7 100644 (file)
--- a/src/wx/simple_video_view.h
+++ b/src/wx/simple_video_view.h
@@ -39,7 +39,7 @@ public:
 
        void update ();
        void start ();
-       bool display_next_frame (bool non_blocking);
+       NextFrameResult display_next_frame (bool non_blocking);
 
 private:
        void set_image (boost::shared_ptr<const Image> image) {

diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc
index 36dbef93a6bc73aab5daeb4f2e7f14bc584b71d4..7805a1fb3e9204085932b9db4f43a507c279121b 100644 (file)
--- a/src/wx/video_view.cc
+++ b/src/wx/video_view.cc
@@ -52,18 +52,19 @@ VideoView::clear ()
 
 /** 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.
+ *  @return FAIL if there's no frame, AGAIN if the method should be called again, or SUCCESS
+ *  if there is a frame.
  */
-bool
+VideoView::NextFrameResult
 VideoView::get_next_frame (bool non_blocking)
 {
        if (length() == dcpomatic::DCPTime()) {
-               return true;
+               return FAIL;
        }
 
        shared_ptr<Butler> butler = _viewer->butler ();
        if (!butler) {
-               return false;
+               return FAIL;
        }
        add_get ();
 
@@ -75,8 +76,8 @@ VideoView::get_next_frame (bool non_blocking)
                if (e.code == Butler::Error::DIED) {
                        LOG_ERROR ("Butler died with %1", e.summary());
                }
-               if (!pv.first && e.code == Butler::Error::AGAIN) {
-                       return false;
+               if (!pv.first) {
+                       return e.code == Butler::Error::AGAIN ? AGAIN : FAIL;
                }
                _player_video = pv;
        } while (
@@ -90,7 +91,7 @@ VideoView::get_next_frame (bool non_blocking)
                ++_errored;
        }
 
-       return true;
+       return SUCCESS;
 }
 
 dcpomatic::DCPTime

diff --git a/src/wx/video_view.h b/src/wx/video_view.h
index 2b4bd47179ae861c4bc5e68055a98a339cb47b7e..e5fbb671dd20e7f770fee744a5c8c676fd45d509 100644 (file)
--- a/src/wx/video_view.h
+++ b/src/wx/video_view.h
@@ -50,8 +50,15 @@ public:
        virtual void start ();
        /** Called when playback stops */
        virtual void stop () {}
+
+       enum NextFrameResult {
+               FAIL,
+               AGAIN,
+               SUCCESS
+       };
+
        /** Get the next frame and display it; used after seek */
-       virtual bool display_next_frame (bool) = 0;
+       virtual NextFrameResult display_next_frame (bool) = 0;
 
        void clear ();
        bool reset_metadata (boost::shared_ptr<const Film> film, dcp::Size player_video_container_size);
@@ -112,7 +119,7 @@ public:
        }
 
 protected:
-       bool get_next_frame (bool non_blocking);
+       NextFrameResult get_next_frame (bool non_blocking);
        boost::optional<int> time_until_next_frame () const;
        dcpomatic::DCPTime one_video_frame () const;