From 4fe1a062eb31d680b8b4ac0191b9e2fc2d6aaec3 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 2 Aug 2018 23:22:49 +0100 Subject: [PATCH] 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. --- src/lib/butler.cc | 51 +++++++++++++++++++++++++++++++++++++++++-- src/lib/butler.h | 12 ++++++++++ src/lib/player.cc | 20 ++++++++++++++--- src/wx/film_viewer.cc | 2 -- 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/src/lib/butler.cc b/src/lib/butler.cc index 112778e25..63fe729ae 100644 --- a/src/lib/butler.cc +++ b/src/lib/butler.cc @@ -63,6 +63,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)); + _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 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(video))); + + boost::mutex::scoped_lock lm2 (_video_audio_mutex); _video.put (video, time); } @@ -254,6 +268,7 @@ Butler::audio (shared_ptr 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 a61011f40..a8b38ef2e 100644 --- a/src/lib/butler.h +++ b/src/lib/butler.h @@ -53,11 +53,17 @@ private: void audio (boost::shared_ptr audio); bool should_run () const; void prepare (boost::weak_ptr video) const; + void player_changed (int); + void seek_unlocked (DCPTime position, bool accurate); boost::shared_ptr _player; boost::shared_ptr _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 _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 bb0aef458..408e791cd 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -239,7 +239,11 @@ Player::playlist_content_changed (weak_ptr 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 8c1a79113..ef28018e9 100644 --- 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 (); -- 2.30.2