A variety of changes to improve (but not entirely fix) behaviour

when moving content (or otherwise changing the playlist) while playing.

This commit refills the butler when things change in certain ways,
and improves locking to cope with Player methods being called from
the GUI and butler threads at the same time.

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 112778e25184794f7f72e20dd1f790192854c7be..63fe729aecea5f2c43557022e3a90769576e9c11 100644 (file)
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -63,6 +63,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));
+       _player_changed_connection = _player->Changed.connect (bind (&Butler::player_changed, this, _1));
        _thread = new boost::thread (bind (&Butler::thread, this));
 #ifdef DCPOMATIC_LINUX
        pthread_setname_np (_thread->native_handle(), "butler");
@@ -206,12 +207,22 @@ void
 Butler::seek (DCPTime position, bool accurate)
 {
        boost::mutex::scoped_lock lm (_mutex);
+       seek_unlocked (position, accurate);
+}
+
+void
+Butler::seek_unlocked (DCPTime position, bool accurate)
+{
        if (_died) {
                return;
        }
 
-       _video.clear ();
-       _audio.clear ();
+       {
+               boost::mutex::scoped_lock lm (_video_audio_mutex);
+               _video.clear ();
+               _audio.clear ();
+       }
+
        _finished = false;
        _pending_seek_position = position;
        _pending_seek_accurate = accurate;
@@ -234,12 +245,15 @@ void
 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 */
                return;
        }
 
        _prepare_service.post (bind (&Butler::prepare, this, weak_ptr<PlayerVideo>(video)));
+
+       boost::mutex::scoped_lock lm2 (_video_audio_mutex);
        _video.put (video, time);
 }
 
@@ -254,6 +268,7 @@ Butler::audio (shared_ptr<AudioBuffers> audio)
                }
        }
 
+       boost::mutex::scoped_lock lm2 (_video_audio_mutex);
        _audio.put (remap (audio, _audio_channels, _audio_mapping));
 }
 
@@ -282,3 +297,35 @@ Butler::memory_used () const
        /* XXX: should also look at _audio.memory_used() */
        return _video.memory_used();
 }
+
+void
+Butler::player_changed (int what)
+{
+       boost::mutex::scoped_lock lm (_mutex);
+       if (_died || _pending_seek_position) {
+               return;
+       }
+
+       DCPTime seek_to;
+       DCPTime next = _video.get().second;
+       if (_awaiting && _awaiting > next) {
+               /* We have recently done a player_changed seek and our buffers haven't been refilled yet,
+                  so assume that we're seeking to the same place as last time.
+               */
+               seek_to = *_awaiting;
+       } else {
+               seek_to = next;
+       }
+
+       {
+               boost::mutex::scoped_lock lm (_video_audio_mutex);
+               _video.clear ();
+               _audio.clear ();
+       }
+
+       _finished = false;
+       _summon.notify_all ();
+
+       seek_unlocked (seek_to, true);
+       _awaiting = seek_to;
+}

diff --git a/src/lib/butler.h b/src/lib/butler.h
index a61011f4019fa1ddcc5e6e01f184a9e9011a3fb0..a8b38ef2e83075e71464a426ff4e574b91467357 100644 (file)
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -53,11 +53,17 @@ private:
        void audio (boost::shared_ptr<AudioBuffers> audio);
        bool should_run () const;
        void prepare (boost::weak_ptr<PlayerVideo> video) const;
+       void player_changed (int);
+       void seek_unlocked (DCPTime position, bool accurate);
 
        boost::shared_ptr<Player> _player;
        boost::shared_ptr<Log> _log;
        boost::thread* _thread;
 
+       /** mutex to protect _video and _audio for when we are clearing them and they both need to be
+           cleared together without any data being inserted in the interim.
+       */
+       boost::mutex _video_audio_mutex;
        VideoRingBuffers _video;
        AudioRingBuffers _audio;
 
@@ -80,6 +86,12 @@ private:
 
        bool _disable_audio;
 
+       /** If we are waiting to be refilled following a seek, this is the time we were
+           seeking to.
+       */
+       boost::optional<DCPTime> _awaiting;
+
        boost::signals2::scoped_connection _player_video_connection;
        boost::signals2::scoped_connection _player_audio_connection;
+       boost::signals2::scoped_connection _player_changed_connection;
 };

diff --git a/src/lib/player.cc b/src/lib/player.cc
index bb0aef458153034bb9b26cc2cbdfaa3cec5ce857..408e791cd560952fd0ee673efe660e5138444e6c 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -239,7 +239,11 @@ Player::playlist_content_changed (weak_ptr<Content> w, int property, bool freque
                property == FFmpegContentProperty::FILTERS
                ) {
 
-               _have_valid_pieces = false;
+               {
+                       boost::mutex::scoped_lock lm (_mutex);
+                       _have_valid_pieces = false;
+               }
+
                Changed (property, frequent);
 
        } else if (
@@ -287,7 +291,11 @@ Player::set_video_container_size (dcp::Size s)
 void
 Player::playlist_changed ()
 {
-       _have_valid_pieces = false;
+       {
+               boost::mutex::scoped_lock lm (_mutex);
+               _have_valid_pieces = false;
+       }
+
        Changed (PlayerProperty::PLAYLIST, false);
 }
 
@@ -305,13 +313,18 @@ Player::film_changed (Film::Property p)
                /* Pieces contain a FrameRateChange which contains the DCP frame rate,
                   so we need new pieces here.
                */
-               _have_valid_pieces = false;
+               {
+                       boost::mutex::scoped_lock lm (_mutex);
+                       _have_valid_pieces = false;
+               }
                Changed (PlayerProperty::FILM_VIDEO_FRAME_RATE, false);
        } else if (p == Film::AUDIO_PROCESSOR) {
                if (_film->audio_processor ()) {
+                       boost::mutex::scoped_lock lm (_mutex);
                        _audio_processor = _film->audio_processor()->clone (_film->audio_frame_rate ());
                }
        } else if (p == Film::AUDIO_CHANNELS) {
+               boost::mutex::scoped_lock lm (_mutex);
                _audio_merger.clear ();
        }
 }
@@ -460,6 +473,7 @@ Player::set_ignore_video ()
 void
 Player::set_ignore_audio ()
 {
+       boost::mutex::scoped_lock lm (_mutex);
        _ignore_audio = true;
        _have_valid_pieces = false;
 }

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 8c1a79113c732d33d2ca054307569eba039b1b90..ef28018e96b0a960623ba665d4cc6344851f07b0 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -529,8 +529,6 @@ FilmViewer::start ()
                _audio.startStream ();
        }
 
-       cout << "start playback at " << to_string(_video_position) << "\n";
-
        _playing = true;
        _dropped = 0;
        timer ();