From: Carl Hetherington Date: Wed, 23 Dec 2020 00:52:21 +0000 (+0100) Subject: Various tweaks to fix playback at the end of a film (#1858). X-Git-Tag: 11.0-build-runs~31 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=6516fb170b0e7a83c582a858fb54d1f83f751cc0 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 7adf2ba2d..920ca3a10 100644 --- 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 e7125501e..0d79a7562 100644 --- 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 1f9ed08ea..675b324fc 100644 --- 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 808f885f8..2b86ea688 100644 --- 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 323047ada..93b9c6e3d 100644 --- 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 image) { diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 36dbef93a..7805a1fb3 100644 --- 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 = _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 2b4bd4717..e5fbb671d 100644 --- 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 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 time_until_next_frame () const; dcpomatic::DCPTime one_video_frame () const;