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().

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 386e7f9db78a90b723bb6b5463f5bed802ab20b9..f04b324cdea4cc29e02bbac2a964d5c27c898c4b 100644 (file)
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -54,6 +54,7 @@ Butler::Butler (shared_ptr<Player> player, shared_ptr<Log> 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> player, shared_ptr<Log> 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<PlayerVideo> 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<AudioBuffers> 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 56374deeb31a9c904b7f2dd175e64b82463ceffc..6a13ed2d72548ebcec849a926b51a4a4f379f602 100644 (file)
--- 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<PlayerVideo> 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<DCPTime> _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 91f1cb517c3f6d9fb1db980ad944ebc9807b1278..df8c47cf9ebf1b83f84055ab8cbe1e8a6415cdd7 100644 (file)
--- 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<const Film> film, shared_ptr<const Playlist> 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 c43cfd40e1eca4d0dc0b154c8aeb71089475c8bc..f7ab1000d37f7392fed01187cdd6b776b414a4f1 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -88,16 +88,21 @@ public:
 
        boost::optional<DCPTime> content_time_to_dcp (boost::shared_ptr<Content> 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<void ()> 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<void (int, bool)> Changed;
+
+       /** The change suggested by a MayChange did not happen */
        boost::signals2::signal<void ()> NotChanged;
 
        /** Emitted when a video frame is ready.  These emissions happen in the correct order. */
@@ -156,8 +161,8 @@ private:
        boost::shared_ptr<const Film> _film;
        boost::shared_ptr<const Playlist> _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<boost::shared_ptr<Piece> > _pieces;
 
        /** Size of the image in the DCP (e.g. 1990x1080 for flat) */