Optimise the feel of some GUI functions by doing the seek after
authorCarl Hetherington <cth@carlh.net>
Wed, 24 Jul 2019 19:42:50 +0000 (20:42 +0100)
committerCarl Hetherington <cth@carlh.net>
Wed, 24 Jul 2019 19:42:50 +0000 (20:42 +0100)
many content changes in an idle handler, rather than blocking
the UI update until the seek and image redisplay have finished.

src/lib/butler.cc
src/lib/butler.h
src/lib/ffmpeg_encoder.cc
src/wx/film_viewer.cc
src/wx/film_viewer.h
test/butler_test.cc
test/dcp_playback_test.cc
test/player_test.cc

index 8c46d51..2d6c46c 100644 (file)
@@ -216,12 +216,16 @@ try
        _arrived.notify_all ();
 }
 
+/** @param blocking true if we should block until video is available.  If blocking is false
+ *  and no video is immediately available the method will return a 0 PlayerVideo and the error AGAIN.
+ *  @param e if non-0 this is filled with an error code (if an error occurs) or is untouched if no error occurs.
+ */
 pair<shared_ptr<PlayerVideo>, DCPTime>
-Butler::get_video (Error* e)
+Butler::get_video (bool blocking, Error* e)
 {
        boost::mutex::scoped_lock lm (_mutex);
 
-       if (_suspended) {
+       if (_suspended || (_video.empty() && !blocking)) {
                if (e) {
                        *e = AGAIN;
                }
index 09c182f..e5581cc 100644 (file)
@@ -54,7 +54,7 @@ public:
                AGAIN
        };
 
-       std::pair<boost::shared_ptr<PlayerVideo>, dcpomatic::DCPTime> get_video (Error* e = 0);
+       std::pair<boost::shared_ptr<PlayerVideo>, dcpomatic::DCPTime> get_video (bool blocking, Error* e = 0);
        boost::optional<dcpomatic::DCPTime> get_audio (float* out, Frame frames);
        boost::optional<TextRingBuffers::Data> get_closed_caption ();
 
index 7c641f5..572e7ae 100644 (file)
@@ -150,7 +150,7 @@ FFmpegEncoder::go ()
                }
 
                for (int j = 0; j < gets_per_frame; ++j) {
-                       pair<shared_ptr<PlayerVideo>, DCPTime> v = _butler->get_video ();
+                       pair<shared_ptr<PlayerVideo>, DCPTime> v = _butler->get_video (true, 0);
                        encoder->get(v.first->eyes())->video(v.first, v.second);
                }
 
index a2b0559..edb451e 100644 (file)
@@ -231,17 +231,22 @@ FilmViewer::refresh_view ()
        _state_timer.unset ();
 }
 
+/** @param lazy true if it is *not* important that the display be updated as quickly as possible.
+ *  If lazy is true we will try to return from this method quickly and postpone any time-consuming
+ *  work until the UI is next idle.  Otherwise we will block here and try to get the image on
+ *  screen as soon as possible.
+ */
 void
-FilmViewer::get ()
+FilmViewer::get (bool lazy)
 {
        DCPOMATIC_ASSERT (_butler);
        ++_gets;
 
        do {
                Butler::Error e;
-               _player_video = _butler->get_video (&e);
+               _player_video = _butler->get_video (!lazy, &e);
                if (!_player_video.first && e == Butler::AGAIN) {
-                       signal_manager->when_idle (boost::bind(&FilmViewer::get, this));
+                       signal_manager->when_idle (boost::bind(&FilmViewer::get, this, lazy));
                        return;
                }
        } while (
@@ -257,7 +262,11 @@ FilmViewer::get ()
                error_dialog (_video_view->get(), e.what());
        }
 
-       display_player_video ();
+       if (lazy) {
+               signal_manager->when_idle (boost::bind(&FilmViewer::display_player_video, this));
+       } else {
+               display_player_video ();
+       }
 }
 
 void
@@ -322,7 +331,7 @@ FilmViewer::timer ()
                return;
        }
 
-       get ();
+       get (false);
        PositionChanged ();
        DCPTime const next = _video_position + one_video_frame();
 
@@ -541,7 +550,7 @@ FilmViewer::seek (DCPTime t, bool accurate)
 
        _closed_captions_dialog->clear ();
        _butler->seek (t, accurate);
-       get ();
+       get (true);
 
        if (was_running) {
                start ();
index 5ddb12b..298e9dd 100644 (file)
@@ -151,7 +151,7 @@ private:
        void timer ();
        void calculate_sizes ();
        void player_change (ChangeType type, int, bool);
-       void get ();
+       void get (bool lazy);
        void display_player_video ();
        void film_change (ChangeType, Film::Property);
        void recreate_butler ();
index cc28412..7aeba78 100644 (file)
@@ -54,9 +54,9 @@ BOOST_AUTO_TEST_CASE (butler_test1)
 
        Butler butler (shared_ptr<Player>(new Player(film, film->playlist())), map, 6, bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, false);
 
-       BOOST_CHECK (butler.get_video().second == DCPTime());
-       BOOST_CHECK (butler.get_video().second == DCPTime::from_frames(1, 24));
-       BOOST_CHECK (butler.get_video().second == DCPTime::from_frames(2, 24));
+       BOOST_CHECK (butler.get_video(true, 0).second == DCPTime());
+       BOOST_CHECK (butler.get_video(true, 0).second == DCPTime::from_frames(1, 24));
+       BOOST_CHECK (butler.get_video(true, 0).second == DCPTime::from_frames(2, 24));
        /* XXX: check the frame contents */
 
        float buffer[256 * 6];
index 9fef180..1dda667 100644 (file)
@@ -49,7 +49,7 @@ BOOST_AUTO_TEST_CASE (dcp_playback_test)
                );
        float* audio_buffer = new float[2000*6];
        while (true) {
-               pair<shared_ptr<PlayerVideo>, DCPTime> p = butler->get_video ();
+               pair<shared_ptr<PlayerVideo>, DCPTime> p = butler->get_video (true, 0);
                if (!p.first) {
                        break;
                }
index 7b65783..af40002 100644 (file)
@@ -220,7 +220,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test)
        for (int i = 0; i < 10; ++i) {
                DCPTime t = DCPTime::from_frames (i, 24);
                butler->seek (t, true);
-               pair<shared_ptr<PlayerVideo>, DCPTime> video = butler->get_video();
+               pair<shared_ptr<PlayerVideo>, DCPTime> video = butler->get_video(true, 0);
                BOOST_CHECK_EQUAL(video.second.get(), t.get());
                write_image(video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true), String::compose("build/test/player_seek_test_%1.png", i), "RGB");
                /* This 0.055 is empirically chosen (hopefully) to accept changes in rendering between the reference and a test machine
@@ -253,7 +253,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test2)
        for (int i = 0; i < 10; ++i) {
                DCPTime t = DCPTime::from_seconds(5) + DCPTime::from_frames (i, 24);
                butler->seek (t, true);
-               pair<shared_ptr<PlayerVideo>, DCPTime> video = butler->get_video();
+               pair<shared_ptr<PlayerVideo>, DCPTime> video = butler->get_video(true, 0);
                BOOST_CHECK_EQUAL(video.second.get(), t.get());
                write_image(video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true), String::compose("build/test/player_seek_test2_%1.png", i), "RGB");
                check_image(String::compose("test/data/player_seek_test2_%1.png", i), String::compose("build/test/player_seek_test2_%1.png", i), 0.055);