From 9a27d60ea7888d300a5a2414a477091428589b82 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 18 Aug 2018 22:49:46 +0100 Subject: [PATCH] Suspend butler on player may-change as otherwise there's a window between Player::_suspended being set to false and the the butler requesting a seek, during which the butler may call pass(). --- src/lib/butler.cc | 22 ++++++++++++++++++---- src/lib/butler.h | 3 +++ src/lib/player.cc | 11 +++++------ src/lib/player.h | 15 ++++++++++----- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/lib/butler.cc b/src/lib/butler.cc index 386e7f9db..f04b324cd 100644 --- a/src/lib/butler.cc +++ b/src/lib/butler.cc @@ -54,6 +54,7 @@ Butler::Butler (shared_ptr player, shared_ptr log, AudioMapping aud , _log (log) , _prepare_work (new boost::asio::io_service::work (_prepare_service)) , _pending_seek_accurate (false) + , _suspended (false) , _finished (false) , _died (false) , _stop_thread (false) @@ -64,6 +65,7 @@ Butler::Butler (shared_ptr player, shared_ptr log, AudioMapping aud _player_video_connection = _player->Video.connect (bind (&Butler::video, this, _1, _2)); _player_audio_connection = _player->Audio.connect (bind (&Butler::audio, this, _1, _2)); _player_text_connection = _player->Text.connect (bind (&Butler::text, this, _1, _2, _3)); + _player_may_change_connection = _player->MayChange.connect (bind (&Butler::suspend, this)); _player_changed_connection = _player->Changed.connect (bind (&Butler::return_seek, this, _2)); _player_not_changed_connection = _player->NotChanged.connect (bind (&Butler::return_seek, this, false)); _thread = new boost::thread (bind (&Butler::thread, this)); @@ -105,6 +107,10 @@ Butler::~Butler () bool Butler::should_run () const { + if (_suspended) { + return false; + } + if (_video.size() >= MAXIMUM_VIDEO_READAHEAD * 10) { /* This is way too big */ throw ProgrammingError @@ -257,8 +263,8 @@ Butler::video (shared_ptr video, DCPTime time) { boost::mutex::scoped_lock lm (_mutex); - if (_pending_seek_position) { - /* Don't store any video while a seek is pending */ + if (_pending_seek_position || _suspended) { + /* Don't store any video in these cases */ return; } @@ -273,8 +279,8 @@ Butler::audio (shared_ptr audio, DCPTime time) { { boost::mutex::scoped_lock lm (_mutex); - if (_pending_seek_position || _disable_audio) { - /* Don't store any audio while a seek is pending, or if audio is disabled */ + if (_pending_seek_position || _disable_audio || _suspended) { + /* Don't store any audio in these cases */ return; } } @@ -329,6 +335,7 @@ Butler::return_seek (bool frequent) } seek_unlocked (seek_to, true); + _suspended = false; _awaiting = seek_to; } @@ -342,3 +349,10 @@ Butler::text (PlayerText pt, TextType type, DCPTimePeriod period) boost::mutex::scoped_lock lm2 (_buffers_mutex); _closed_caption.put (make_pair(pt, period)); } + +void +Butler::suspend () +{ + boost::mutex::scoped_lock lm (_mutex); + _suspended = true; +} diff --git a/src/lib/butler.h b/src/lib/butler.h index 56374deeb..6a13ed2d7 100644 --- a/src/lib/butler.h +++ b/src/lib/butler.h @@ -56,6 +56,7 @@ private: void text (PlayerText pt, TextType type, DCPTimePeriod period); bool should_run () const; void prepare (boost::weak_ptr video) const; + void suspend (); void return_seek (bool frequent); void seek_unlocked (DCPTime position, bool accurate); @@ -83,6 +84,7 @@ private: boost::condition _arrived; boost::optional _pending_seek_position; bool _pending_seek_accurate; + bool _suspended; bool _finished; bool _died; bool _stop_thread; @@ -100,6 +102,7 @@ private: boost::signals2::scoped_connection _player_video_connection; boost::signals2::scoped_connection _player_audio_connection; boost::signals2::scoped_connection _player_text_connection; + boost::signals2::scoped_connection _player_may_change_connection; boost::signals2::scoped_connection _player_changed_connection; boost::signals2::scoped_connection _player_not_changed_connection; }; diff --git a/src/lib/player.cc b/src/lib/player.cc index 91f1cb517..df8c47cf9 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -87,7 +87,7 @@ int const PlayerProperty::DCP_DECODE_REDUCTION = 704; Player::Player (shared_ptr film, shared_ptr playlist) : _film (film) , _playlist (playlist) - , _can_run (false) + , _suspended (false) , _ignore_video (false) , _ignore_audio (false) , _ignore_text (false) @@ -220,7 +220,7 @@ Player::setup_pieces_unlocked () _last_video_time = DCPTime (); _last_video_eyes = EYES_BOTH; _last_audio_time = DCPTime (); - _can_run = true; + _suspended = false; } void @@ -232,7 +232,7 @@ Player::playlist_content_may_change () until that has happened and we've rebuilt our pieces. Stop pass() and seek() from working until then. */ - _can_run = false; + _suspended = true; } MayChange (); @@ -295,7 +295,6 @@ void Player::playlist_content_not_changed () { /* A possible content change did end up happening for some reason */ - setup_pieces (); NotChanged (); } @@ -613,7 +612,7 @@ Player::pass () { boost::mutex::scoped_lock lm (_mutex); - if (!_can_run) { + if (_suspended) { /* We can't pass in this state */ return false; } @@ -1039,7 +1038,7 @@ Player::seek (DCPTime time, bool accurate) { boost::mutex::scoped_lock lm (_mutex); - if (!_can_run) { + if (_suspended) { /* We can't seek in this state */ return; } diff --git a/src/lib/player.h b/src/lib/player.h index c43cfd40e..f7ab1000d 100644 --- a/src/lib/player.h +++ b/src/lib/player.h @@ -88,16 +88,21 @@ public: boost::optional content_time_to_dcp (boost::shared_ptr content, ContentTime t); + /* The player's internal state may be about to change such so + that its emissions from Video and Audio will suddenly be + from an undefined position. Listeners should prepare + themselves for this possibility. + */ boost::signals2::signal MayChange; - /** Emitted when something has changed such that if we went back and emitted - * the last frame again it would look different. This is not emitted after - * a seek. + /** The player's internal state has now changed. * * The first parameter is what changed. * The second parameter is true if these signals are currently likely to be frequent. */ boost::signals2::signal Changed; + + /** The change suggested by a MayChange did not happen */ boost::signals2::signal NotChanged; /** Emitted when a video frame is ready. These emissions happen in the correct order. */ @@ -156,8 +161,8 @@ private: boost::shared_ptr _film; boost::shared_ptr _playlist; - /** We can pass() and seek() */ - bool _can_run; + /** true if we are suspended (i.e. pass() and seek() do nothing */ + bool _suspended; std::list > _pieces; /** Size of the image in the DCP (e.g. 1990x1080 for flat) */ -- 2.30.2