From 0e896f9f37db001f34c876ed5fc50e874f96ae09 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 15 Oct 2021 22:26:47 +0200 Subject: [PATCH 1/1] Use an enum instead of a bool to specify blocking/non-blocking. --- src/lib/butler.cc | 6 +++--- src/lib/butler.h | 7 ++++++- src/lib/ffmpeg_encoder.cc | 2 +- src/wx/video_view.cc | 2 +- test/butler_test.cc | 6 +++--- test/dcp_playback_test.cc | 2 +- test/player_test.cc | 4 ++-- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/lib/butler.cc b/src/lib/butler.cc index 6f7349b5f..f19e1e080 100644 --- a/src/lib/butler.cc +++ b/src/lib/butler.cc @@ -238,12 +238,12 @@ try } -/** @param blocking true if we should block until video is available. If blocking is false +/** @param behaviour BLOCKING if we should block until video is available. If behaviour is NON_BLOCKING * 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, DCPTime> -Butler::get_video (bool blocking, Error* e) +Butler::get_video (Behaviour behaviour, Error* e) { boost::mutex::scoped_lock lm (_mutex); @@ -260,7 +260,7 @@ Butler::get_video (bool blocking, Error* e) } }; - if (_video.empty() && (_finished || _died || (_suspended && !blocking))) { + if (_video.empty() && (_finished || _died || (_suspended && behaviour == Behaviour::NON_BLOCKING))) { setup_error (e, Error::Code::AGAIN); return make_pair(shared_ptr(), DCPTime()); } diff --git a/src/lib/butler.h b/src/lib/butler.h index 498af8d86..529b7383d 100644 --- a/src/lib/butler.h +++ b/src/lib/butler.h @@ -72,7 +72,12 @@ public: std::string summary () const; }; - std::pair, dcpomatic::DCPTime> get_video (bool blocking, Error* e = 0); + enum class Behaviour { + BLOCKING, + NON_BLOCKING + }; + + std::pair, dcpomatic::DCPTime> get_video (Behaviour behaviour, Error* e = nullptr); boost::optional get_audio (float* out, Frame frames); boost::optional get_closed_caption (); diff --git a/src/lib/ffmpeg_encoder.cc b/src/lib/ffmpeg_encoder.cc index 5db3e31a6..e1081f518 100644 --- a/src/lib/ffmpeg_encoder.cc +++ b/src/lib/ffmpeg_encoder.cc @@ -176,7 +176,7 @@ FFmpegEncoder::go () for (int j = 0; j < gets_per_frame; ++j) { Butler::Error e; - auto v = _butler->get_video (true, &e); + auto v = _butler->get_video (Butler::Behaviour::BLOCKING, &e); _butler->rethrow (); if (v.first) { auto fe = encoder->get (v.first->eyes()); diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 331095253..1c645f11f 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -75,7 +75,7 @@ VideoView::get_next_frame (bool non_blocking) do { Butler::Error e; - auto pv = butler->get_video (!non_blocking, &e); + auto pv = butler->get_video (non_blocking ? Butler::Behaviour::NON_BLOCKING : Butler::Behaviour::BLOCKING, &e); if (e.code == Butler::Error::Code::DIED) { LOG_ERROR ("Butler died with %1", e.summary()); } diff --git a/test/butler_test.cc b/test/butler_test.cc index 787d1c324..e57779334 100644 --- a/test/butler_test.cc +++ b/test/butler_test.cc @@ -61,9 +61,9 @@ BOOST_AUTO_TEST_CASE (butler_test1) Butler butler (film, make_shared(film, Image::Alignment::COMPACT), map, 6, bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, false, false); - 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)); + BOOST_CHECK (butler.get_video(Butler::Behaviour::BLOCKING, 0).second == DCPTime()); + BOOST_CHECK (butler.get_video(Butler::Behaviour::BLOCKING, 0).second == DCPTime::from_frames(1, 24)); + BOOST_CHECK (butler.get_video(Butler::Behaviour::BLOCKING, 0).second == DCPTime::from_frames(2, 24)); /* XXX: check the frame contents */ float buffer[256 * 6]; diff --git a/test/dcp_playback_test.cc b/test/dcp_playback_test.cc index 2ab7eaec3..0e57acae4 100644 --- a/test/dcp_playback_test.cc +++ b/test/dcp_playback_test.cc @@ -57,7 +57,7 @@ BOOST_AUTO_TEST_CASE (dcp_playback_test) auto audio_buffer = new float[2000 * 6]; while (true) { - auto p = butler->get_video (true, 0); + auto p = butler->get_video (Butler::Behaviour::BLOCKING, 0); if (!p.first) { break; } diff --git a/test/player_test.cc b/test/player_test.cc index 5ed772193..cafb14586 100644 --- a/test/player_test.cc +++ b/test/player_test.cc @@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test) for (int i = 0; i < 10; ++i) { auto t = DCPTime::from_frames (i, 24); butler->seek (t, true); - auto video = butler->get_video(true, 0); + auto video = butler->get_video(Butler::Behaviour::BLOCKING, 0); BOOST_CHECK_EQUAL(video.second.get(), t.get()); write_image(video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, true), String::compose("build/test/player_seek_test_%1.png", i)); /* This 14.08 is empirically chosen (hopefully) to accept changes in rendering between the reference and a test machine @@ -273,7 +273,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test2) for (int i = 0; i < 10; ++i) { auto t = DCPTime::from_seconds(5) + DCPTime::from_frames (i, 24); butler->seek (t, true); - auto video = butler->get_video(true, 0); + auto video = butler->get_video(Butler::Behaviour::BLOCKING, 0); BOOST_CHECK_EQUAL(video.second.get(), t.get()); write_image( video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, true), String::compose("build/test/player_seek_test2_%1.png", i) -- 2.30.2