From 6c7489e5d778d3e71065d88a094a7383ba2c117d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 19 Aug 2018 01:04:59 +0100 Subject: [PATCH] Replace May/Done/NotDone signal sets with one signal and extend this treatment to anything that caused Player::setup_pieces. This should fix out-of-sequence Player emissions caused by setup_pieces being called by one thread while the butler is calling pass(). --- src/lib/butler.cc | 59 ++++++++--------- src/lib/butler.h | 7 +- src/lib/content.cc | 8 ++- src/lib/content.h | 12 +--- src/lib/content_change.cc | 6 +- src/lib/film.cc | 20 ++++-- src/lib/film.h | 10 +-- src/lib/player.cc | 96 +++++++--------------------- src/lib/player.h | 26 ++------ src/lib/playlist.cc | 83 ++++++++++++------------ src/lib/playlist.h | 14 +--- src/lib/types.h | 7 ++ src/wx/audio_dialog.cc | 8 ++- src/wx/audio_dialog.h | 2 +- src/wx/content_widget.h | 6 +- src/wx/film_editor.cc | 8 ++- src/wx/film_editor.h | 2 +- src/wx/film_viewer.cc | 8 +-- src/wx/film_viewer.h | 2 +- src/wx/hints_dialog.cc | 10 ++- src/wx/hints_dialog.h | 2 + src/wx/subtitle_appearance_dialog.cc | 10 ++- src/wx/subtitle_appearance_dialog.h | 1 + src/wx/timeline.cc | 8 ++- src/wx/timeline.h | 6 +- src/wx/timeline_content_view.cc | 8 ++- src/wx/timeline_content_view.h | 3 +- 27 files changed, 195 insertions(+), 237 deletions(-) diff --git a/src/lib/butler.cc b/src/lib/butler.cc index f04b324cd..3a75e67d7 100644 --- a/src/lib/butler.cc +++ b/src/lib/butler.cc @@ -65,9 +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)); + _player_change_connection = _player->Change.connect (bind (&Butler::player_change, this, _1, _3)); _thread = new boost::thread (bind (&Butler::thread, this)); #ifdef DCPOMATIC_LINUX pthread_setname_np (_thread->native_handle(), "butler"); @@ -107,10 +105,6 @@ 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 @@ -131,7 +125,7 @@ Butler::should_run () const LOG_WARNING ("Butler audio buffers reached %1 frames (video is %2)", _audio.size(), _video.size()); } - if (_stop_thread || _finished || _died) { + if (_stop_thread || _finished || _died || _suspended) { /* Definitely do not run */ return false; } @@ -198,7 +192,7 @@ Butler::get_video () boost::mutex::scoped_lock lm (_mutex); /* Wait for data if we have none */ - while (_video.empty() && !_finished && !_died) { + while (_video.empty() && !_finished && !_died && !_suspended) { _arrived.wait (lm); } @@ -316,27 +310,33 @@ Butler::memory_used () const } void -Butler::return_seek (bool frequent) +Butler::player_change (ChangeType type, bool frequent) { boost::mutex::scoped_lock lm (_mutex); - if (_died || _pending_seek_position || frequent) { - 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; - } + if (type == CHANGE_TYPE_PENDING) { + _suspended = true; + } else if (type == CHANGE_TYPE_DONE) { + + if (_died || _pending_seek_position || frequent) { + 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; + } - seek_unlocked (seek_to, true); - _suspended = false; - _awaiting = seek_to; + seek_unlocked (seek_to, true); + _suspended = false; + _awaiting = seek_to; + } } void @@ -349,10 +349,3 @@ 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 6a13ed2d7..513176821 100644 --- a/src/lib/butler.h +++ b/src/lib/butler.h @@ -56,8 +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 player_change (ChangeType type, bool frequent); void seek_unlocked (DCPTime position, bool accurate); boost::shared_ptr _player; @@ -102,7 +101,5 @@ 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; + boost::signals2::scoped_connection _player_change_connection; }; diff --git a/src/lib/content.cc b/src/lib/content.cc index 8fd36a1d8..ae0b08dcc 100644 --- a/src/lib/content.cc +++ b/src/lib/content.cc @@ -179,10 +179,14 @@ Content::examine (shared_ptr job) } void -Content::signal_changed (int p) +Content::signal_change (ChangeType c, int p) { try { - emit (boost::bind (boost::ref(Changed), shared_from_this(), p, _change_signals_frequent)); + if (c == CHANGE_TYPE_PENDING || c == CHANGE_TYPE_CANCELLED) { + Change (c, shared_from_this(), p, _change_signals_frequent); + } else { + emit (boost::bind (boost::ref(Change), c, shared_from_this(), p, _change_signals_frequent)); + } } catch (boost::bad_weak_ptr) { /* This must be during construction; never mind */ } diff --git a/src/lib/content.h b/src/lib/content.h index c9edf6e22..e8710ccb7 100644 --- a/src/lib/content.h +++ b/src/lib/content.h @@ -178,14 +178,8 @@ public: std::list user_properties () const; - /* May be emitted from any thread */ - boost::signals2::signal MayChange; - - /* Emitted from the GUI thread */ - boost::signals2::signal, int, bool)> Changed; - - /* May be emitted from any thread */ - boost::signals2::signal NotChanged; + /* CHANGE_PENDING and CHANGE_CANCELLED may be emitted from any thread; CHANGE_DONE always from GUI thread */ + boost::signals2::signal, int, bool)> Change; boost::shared_ptr video; boost::shared_ptr audio; @@ -215,7 +209,7 @@ private: friend struct audio_sampling_rate_test; friend class ContentChange; - void signal_changed (int); + void signal_change (ChangeType, int); std::string _digest; DCPTime _position; diff --git a/src/lib/content_change.cc b/src/lib/content_change.cc index c286f80eb..6e390dc76 100644 --- a/src/lib/content_change.cc +++ b/src/lib/content_change.cc @@ -26,16 +26,16 @@ ContentChange::ContentChange (Content* c, int p) , _property (p) , _done (true) { - _content->MayChange (); + _content->signal_change (CHANGE_TYPE_PENDING, _property); } ContentChange::~ContentChange () { if (_done) { - _content->signal_changed (_property); + _content->signal_change (CHANGE_TYPE_DONE, _property); } else { - _content->NotChanged (); + _content->signal_change (CHANGE_TYPE_CANCELLED, _property); } } diff --git a/src/lib/film.cc b/src/lib/film.cc index ba97f833e..846e8ac51 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -160,9 +160,9 @@ Film::Film (optional dir) { set_isdcf_date_today (); - _playlist_changed_connection = _playlist->Changed.connect (bind (&Film::playlist_changed, this)); + _playlist_change_connection = _playlist->Change.connect (bind (&Film::playlist_change, this, _1)); _playlist_order_changed_connection = _playlist->OrderChanged.connect (bind (&Film::playlist_order_changed, this)); - _playlist_content_changed_connection = _playlist->ContentChanged.connect (bind (&Film::playlist_content_changed, this, _1, _2, _3)); + _playlist_content_change_connection = _playlist->ContentChange.connect (bind (&Film::playlist_content_change, this, _1, _2, _3, _4)); if (dir) { /* Make state.directory a complete path without ..s (where possible) @@ -1154,8 +1154,12 @@ Film::active_frame_rate_change (DCPTime t) const } void -Film::playlist_content_changed (weak_ptr c, int p, bool frequent) +Film::playlist_content_change (ChangeType type, weak_ptr c, int p, bool frequent) { + if (type != CHANGE_TYPE_DONE) { + return; + } + _dirty = true; if (p == ContentProperty::VIDEO_FRAME_RATE) { @@ -1164,14 +1168,16 @@ Film::playlist_content_changed (weak_ptr c, int p, bool frequent) signal_changed (NAME); } - emit (boost::bind (boost::ref (ContentChanged), c, p, frequent)); + emit (boost::bind (boost::ref (ContentChange), type, c, p, frequent)); } void -Film::playlist_changed () +Film::playlist_change (ChangeType type) { - signal_changed (CONTENT); - signal_changed (NAME); + if (type == CHANGE_TYPE_DONE) { + signal_changed (CONTENT); + signal_changed (NAME); + } } void diff --git a/src/lib/film.h b/src/lib/film.h index 20a1e2ca6..c6c8403cb 100644 --- a/src/lib/film.h +++ b/src/lib/film.h @@ -327,7 +327,7 @@ public: mutable boost::signals2::signal Changed; /** Emitted when some property of our content has changed */ - mutable boost::signals2::signal, int, bool)> ContentChanged; + mutable boost::signals2::signal, int, bool)> ContentChange; /** Current version number of the state file */ static int const current_state_version; @@ -338,9 +338,9 @@ private: void signal_changed (Property); std::string video_identifier () const; - void playlist_changed (); + void playlist_change (ChangeType); void playlist_order_changed (); - void playlist_content_changed (boost::weak_ptr, int, bool frequent); + void playlist_content_change (ChangeType type, boost::weak_ptr, int, bool frequent); void maybe_add_content (boost::weak_ptr, boost::weak_ptr, bool disable_audio_analysis); void audio_analysis_finished (); @@ -401,9 +401,9 @@ private: /** film being used as a template, or 0 */ boost::shared_ptr _template_film; - boost::signals2::scoped_connection _playlist_changed_connection; + boost::signals2::scoped_connection _playlist_change_connection; boost::signals2::scoped_connection _playlist_order_changed_connection; - boost::signals2::scoped_connection _playlist_content_changed_connection; + boost::signals2::scoped_connection _playlist_content_change_connection; std::list _job_connections; std::list _audio_analysis_connections; diff --git a/src/lib/player.cc b/src/lib/player.cc index df8c47cf9..0c3aea028 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -98,10 +98,8 @@ Player::Player (shared_ptr film, shared_ptr playlist , _shuffler (0) { _film_changed_connection = _film->Changed.connect (bind (&Player::film_changed, this, _1)); - _playlist_changed_connection = _playlist->Changed.connect (bind (&Player::playlist_changed, this)); - _playlist_content_may_change_connection = _playlist->ContentMayChange.connect (bind(&Player::playlist_content_may_change, this)); - _playlist_content_changed_connection = _playlist->ContentChanged.connect (bind(&Player::playlist_content_changed, this, _1, _2, _3)); - _playlist_content_not_changed_connection = _playlist->ContentNotChanged.connect (bind(&Player::playlist_content_not_changed, this)); + _playlist_change_connection = _playlist->Change.connect (bind (&Player::playlist_change, this, _1)); + _playlist_content_change_connection = _playlist->ContentChange.connect (bind(&Player::playlist_content_change, this, _1, _3, _4)); set_video_container_size (_film->frame_size ()); film_changed (Film::AUDIO_PROCESSOR); @@ -224,78 +222,21 @@ Player::setup_pieces_unlocked () } void -Player::playlist_content_may_change () +Player::playlist_content_change (ChangeType type, int property, bool frequent) { - { + if (type == CHANGE_TYPE_PENDING) { boost::mutex::scoped_lock lm (_mutex); /* The player content is probably about to change, so we can't carry on until that has happened and we've rebuilt our pieces. Stop pass() and seek() from working until then. */ _suspended = true; + } else if (type == CHANGE_TYPE_DONE) { + /* A change in our content has gone through. Re-build our pieces. */ + setup_pieces (); } - MayChange (); -} - -void -Player::playlist_content_changed (weak_ptr w, int property, bool frequent) -{ - /* A change in our content has gone through. Re-build our pieces and signal - it to anybody that is interested. - */ - - shared_ptr c = w.lock (); - if (!c) { - return; - } - - setup_pieces (); - - if ( - property == ContentProperty::POSITION || - property == ContentProperty::LENGTH || - property == ContentProperty::TRIM_START || - property == ContentProperty::TRIM_END || - property == ContentProperty::PATH || - property == VideoContentProperty::FRAME_TYPE || - property == VideoContentProperty::COLOUR_CONVERSION || - property == AudioContentProperty::STREAMS || - property == DCPContentProperty::NEEDS_ASSETS || - property == DCPContentProperty::NEEDS_KDM || - property == DCPContentProperty::CPL || - property == TextContentProperty::COLOUR || - property == TextContentProperty::EFFECT || - property == TextContentProperty::EFFECT_COLOUR || - property == FFmpegContentProperty::SUBTITLE_STREAM || - property == FFmpegContentProperty::FILTERS || - property == TextContentProperty::LINE_SPACING || - property == TextContentProperty::OUTLINE_WIDTH || - property == TextContentProperty::Y_SCALE || - property == TextContentProperty::FADE_IN || - property == TextContentProperty::FADE_OUT || - property == ContentProperty::VIDEO_FRAME_RATE || - property == TextContentProperty::USE || - property == TextContentProperty::X_OFFSET || - property == TextContentProperty::Y_OFFSET || - property == TextContentProperty::X_SCALE || - property == TextContentProperty::FONTS || - property == TextContentProperty::TYPE || - property == VideoContentProperty::CROP || - property == VideoContentProperty::SCALE || - property == VideoContentProperty::FADE_IN || - property == VideoContentProperty::FADE_OUT - ) { - - Changed (property, frequent); - } -} - -void -Player::playlist_content_not_changed () -{ - /* A possible content change did end up happening for some reason */ - NotChanged (); + Change (type, property, frequent); } void @@ -314,14 +255,16 @@ Player::set_video_container_size (dcp::Size s) _black_image->make_black (); } - Changed (PlayerProperty::VIDEO_CONTAINER_SIZE, false); + Change (CHANGE_TYPE_DONE, PlayerProperty::VIDEO_CONTAINER_SIZE, false); } void -Player::playlist_changed () +Player::playlist_change (ChangeType type) { - setup_pieces (); - Changed (PlayerProperty::PLAYLIST, false); + if (type == CHANGE_TYPE_DONE) { + setup_pieces (); + } + Change (type, PlayerProperty::PLAYLIST, false); } void @@ -333,13 +276,14 @@ Player::film_changed (Film::Property p) */ if (p == Film::CONTAINER) { - Changed (PlayerProperty::FILM_CONTAINER, false); + Change (CHANGE_TYPE_PENDING, PlayerProperty::FILM_CONTAINER, false); } else if (p == Film::VIDEO_FRAME_RATE) { /* Pieces contain a FrameRateChange which contains the DCP frame rate, so we need new pieces here. */ + /* XXX: missing PENDING! */ setup_pieces (); - Changed (PlayerProperty::FILM_VIDEO_FRAME_RATE, false); + Change (CHANGE_TYPE_DONE, PlayerProperty::FILM_VIDEO_FRAME_RATE, false); } else if (p == Film::AUDIO_PROCESSOR) { if (_film->audio_processor ()) { boost::mutex::scoped_lock lm (_mutex); @@ -1187,10 +1131,14 @@ Player::discard_audio (shared_ptr audio, DCPTime time, DCPTi void Player::set_dcp_decode_reduction (optional reduction) { + Change (CHANGE_TYPE_PENDING, PlayerProperty::DCP_DECODE_REDUCTION, false); + { boost::mutex::scoped_lock lm (_mutex); if (reduction == _dcp_decode_reduction) { + lm.unlock (); + Change (CHANGE_TYPE_CANCELLED, PlayerProperty::DCP_DECODE_REDUCTION, false); return; } @@ -1198,7 +1146,7 @@ Player::set_dcp_decode_reduction (optional reduction) setup_pieces_unlocked (); } - Changed (PlayerProperty::DCP_DECODE_REDUCTION, false); + Change (CHANGE_TYPE_DONE, PlayerProperty::DCP_DECODE_REDUCTION, false); } optional diff --git a/src/lib/player.h b/src/lib/player.h index f7ab1000d..165820799 100644 --- a/src/lib/player.h +++ b/src/lib/player.h @@ -88,19 +88,7 @@ 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; - - /** 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; + boost::signals2::signal Change; /** The change suggested by a MayChange did not happen */ boost::signals2::signal NotChanged; @@ -125,10 +113,8 @@ private: void setup_pieces_unlocked (); void flush (); void film_changed (Film::Property); - void playlist_changed (); - void playlist_content_may_change (); - void playlist_content_changed (boost::weak_ptr, int, bool); - void playlist_content_not_changed (); + void playlist_change (ChangeType); + void playlist_content_change (ChangeType, int, bool); std::list transform_bitmap_texts (std::list) const; Frame dcp_to_content_video (boost::shared_ptr piece, DCPTime t) const; DCPTime content_video_to_dcp (boost::shared_ptr piece, Frame f) const; @@ -217,10 +203,8 @@ private: boost::shared_ptr _audio_processor; boost::signals2::scoped_connection _film_changed_connection; - boost::signals2::scoped_connection _playlist_changed_connection; - boost::signals2::scoped_connection _playlist_content_may_change_connection; - boost::signals2::scoped_connection _playlist_content_changed_connection; - boost::signals2::scoped_connection _playlist_content_not_changed_connection; + boost::signals2::scoped_connection _playlist_change_connection; + boost::signals2::scoped_connection _playlist_content_change_connection; }; #endif diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc index 1bb0ad0d6..e4fc0f072 100644 --- a/src/lib/playlist.cc +++ b/src/lib/playlist.cc @@ -65,48 +65,38 @@ Playlist::~Playlist () } void -Playlist::content_may_change () +Playlist::content_change (ChangeType type, weak_ptr content, int property, bool frequent) { - ContentMayChange (); -} + if (type == CHANGE_TYPE_DONE) { + if ( + property == ContentProperty::TRIM_START || + property == ContentProperty::TRIM_END || + property == ContentProperty::LENGTH || + property == VideoContentProperty::FRAME_TYPE + ) { + /* Don't respond to position changes here, as: + - sequencing after earlier/later changes is handled by move_earlier/move_later + - any other position changes will be timeline drags which should not result in content + being sequenced. + */ + maybe_sequence (); + } -void -Playlist::content_not_changed () -{ - ContentNotChanged (); -} + if ( + property == ContentProperty::POSITION || + property == ContentProperty::LENGTH || + property == ContentProperty::TRIM_START || + property == ContentProperty::TRIM_END) { -void -Playlist::content_changed (weak_ptr content, int property, bool frequent) -{ - if ( - property == ContentProperty::TRIM_START || - property == ContentProperty::TRIM_END || - property == ContentProperty::LENGTH || - property == VideoContentProperty::FRAME_TYPE - ) { - /* Don't respond to position changes here, as: - - sequencing after earlier/later changes is handled by move_earlier/move_later - - any other position changes will be timeline drags which should not result in content - being sequenced. - */ - maybe_sequence (); - } - - if ( - property == ContentProperty::POSITION || - property == ContentProperty::LENGTH || - property == ContentProperty::TRIM_START || - property == ContentProperty::TRIM_END) { - - ContentList old = _content; - sort (_content.begin(), _content.end(), ContentSorter ()); - if (_content != old) { - OrderChanged (); + ContentList old = _content; + sort (_content.begin(), _content.end(), ContentSorter ()); + if (_content != old) { + OrderChanged (); + } } } - ContentChanged (content, property, frequent); + ContentChange (type, content, property, frequent); } void @@ -215,15 +205,18 @@ Playlist::as_xml (xmlpp::Node* node, bool with_content_paths) void Playlist::add (shared_ptr c) { + Change (CHANGE_TYPE_PENDING); _content.push_back (c); sort (_content.begin(), _content.end(), ContentSorter ()); reconnect (); - Changed (); + Change (CHANGE_TYPE_DONE); } void Playlist::remove (shared_ptr c) { + Change (CHANGE_TYPE_PENDING); + ContentList::iterator i = _content.begin (); while (i != _content.end() && *i != c) { ++i; @@ -231,7 +224,9 @@ Playlist::remove (shared_ptr c) if (i != _content.end ()) { _content.erase (i); - Changed (); + Change (CHANGE_TYPE_DONE); + } else { + Change (CHANGE_TYPE_CANCELLED); } /* This won't change order, so it does not need a sort */ @@ -240,6 +235,8 @@ Playlist::remove (shared_ptr c) void Playlist::remove (ContentList c) { + Change (CHANGE_TYPE_PENDING); + BOOST_FOREACH (shared_ptr i, c) { ContentList::iterator j = _content.begin (); while (j != _content.end() && *j != i) { @@ -253,7 +250,7 @@ Playlist::remove (ContentList c) /* This won't change order, so it does not need a sort */ - Changed (); + Change (CHANGE_TYPE_DONE); } class FrameRateCandidate @@ -362,9 +359,7 @@ Playlist::reconnect () _content_connections.clear (); BOOST_FOREACH (shared_ptr i, _content) { - _content_connections.push_back (i->MayChange.connect(boost::bind(&Playlist::content_may_change, this))); - _content_connections.push_back (i->Changed.connect(boost::bind(&Playlist::content_changed, this, _1, _2, _3))); - _content_connections.push_back (i->NotChanged.connect(boost::bind(&Playlist::content_not_changed, this))); + _content_connections.push_back (i->Change.connect(boost::bind(&Playlist::content_change, this, _1, _2, _3, _4))); } } @@ -461,6 +456,8 @@ Playlist::repeat (ContentList c, int n) range.second = max (range.second, i->end ()); } + Change (CHANGE_TYPE_PENDING); + DCPTime pos = range.second; for (int i = 0; i < n; ++i) { BOOST_FOREACH (shared_ptr j, c) { @@ -474,7 +471,7 @@ Playlist::repeat (ContentList c, int n) sort (_content.begin(), _content.end(), ContentSorter ()); reconnect (); - Changed (); + Change (CHANGE_TYPE_DONE); } void diff --git a/src/lib/playlist.h b/src/lib/playlist.h index 649887f72..d55232dad 100644 --- a/src/lib/playlist.h +++ b/src/lib/playlist.h @@ -75,21 +75,13 @@ public: void repeat (ContentList, int); /** Emitted when content has been added to or removed from the playlist; implies OrderChanged */ - mutable boost::signals2::signal Changed; + mutable boost::signals2::signal Change; mutable boost::signals2::signal OrderChanged; - mutable boost::signals2::signal ContentMayChange; - /** Emitted when something about a piece of our content has changed; - * these emissions include when the position of the content changes. - * Third parameter is true if signals are currently being emitted frequently. - */ - mutable boost::signals2::signal, int, bool)> ContentChanged; - mutable boost::signals2::signal ContentNotChanged; + mutable boost::signals2::signal, int, bool)> ContentChange; private: - void content_may_change (); - void content_changed (boost::weak_ptr, int, bool); - void content_not_changed (); + void content_change (ChangeType, boost::weak_ptr, int, bool); void reconnect (); /** List of content. Kept sorted in position order. */ diff --git a/src/lib/types.h b/src/lib/types.h index 42e2e3eec..22d652b3f 100644 --- a/src/lib/types.h +++ b/src/lib/types.h @@ -129,6 +129,13 @@ enum ReelType REELTYPE_BY_LENGTH }; +enum ChangeType +{ + CHANGE_TYPE_PENDING, + CHANGE_TYPE_DONE, + CHANGE_TYPE_CANCELLED +}; + /** Type of captions. * * The generally accepted definitions seem to be: diff --git a/src/wx/audio_dialog.cc b/src/wx/audio_dialog.cc index 8c015ab68..e1f7c15a6 100644 --- a/src/wx/audio_dialog.cc +++ b/src/wx/audio_dialog.cc @@ -150,7 +150,7 @@ AudioDialog::AudioDialog (wxWindow* parent, shared_ptr film, shared_ptrLayout (); overall_sizer->SetSizeHints (this); - _film_connection = film->ContentChanged.connect (boost::bind (&AudioDialog::content_changed, this, _2)); + _film_connection = film->ContentChange.connect (boost::bind (&AudioDialog::content_change, this, _1, _3)); DCPOMATIC_ASSERT (film->directory()); SetTitle(wxString::Format(_("DCP-o-matic audio - %s"), std_to_wx(film->directory().get().string()))); @@ -282,8 +282,12 @@ AudioDialog::channel_clicked (wxCommandEvent& ev) } void -AudioDialog::content_changed (int p) +AudioDialog::content_change (ChangeType type, int p) { + if (type != CHANGE_TYPE_DONE) { + return; + } + if (p == AudioContentProperty::STREAMS) { try_to_load_analysis (); } else if (p == AudioContentProperty::GAIN) { diff --git a/src/wx/audio_dialog.h b/src/wx/audio_dialog.h index 6d4428546..2c077c9d1 100644 --- a/src/wx/audio_dialog.h +++ b/src/wx/audio_dialog.h @@ -38,7 +38,7 @@ public: void set_cursor (boost::optional time, boost::optional db); private: - void content_changed (int); + void content_change (ChangeType, int); void channel_clicked (wxCommandEvent &); void type_clicked (wxCommandEvent &); void smoothing_changed (); diff --git a/src/wx/content_widget.h b/src/wx/content_widget.h index e162aca49..e5125680b 100644 --- a/src/wx/content_widget.h +++ b/src/wx/content_widget.h @@ -105,7 +105,7 @@ public: update_from_model (); for (typename List::iterator i = _content.begin(); i != _content.end(); ++i) { - _connections.push_back ((*i)->Changed.connect (boost::bind (&ContentWidget::model_changed, this, _2))); + _connections.push_back ((*i)->Change.connect (boost::bind (&ContentWidget::model_changed, this, _1, _3))); } } @@ -185,9 +185,9 @@ private: } } - void model_changed (int property) + void model_changed (ChangeType type, int property) { - if (property == _property && !_ignore_model_changes) { + if (type == CHANGE_TYPE_DONE && property == _property && !_ignore_model_changes) { update_from_model (); } } diff --git a/src/wx/film_editor.cc b/src/wx/film_editor.cc index 5380cbd9b..1c1c02f5b 100644 --- a/src/wx/film_editor.cc +++ b/src/wx/film_editor.cc @@ -86,8 +86,12 @@ FilmEditor::film_changed (Film::Property p) } void -FilmEditor::film_content_changed (int property) +FilmEditor::film_content_change (ChangeType type, int property) { + if (type != CHANGE_TYPE_DONE) { + return; + } + ensure_ui_thread (); if (!_film) { @@ -118,7 +122,7 @@ FilmEditor::set_film (shared_ptr film) if (_film) { _film->Changed.connect (bind (&FilmEditor::film_changed, this, _1)); - _film->ContentChanged.connect (bind (&FilmEditor::film_content_changed, this, _2)); + _film->ContentChange.connect (bind (&FilmEditor::film_content_change, this, _1, _3)); } if (_film && _film->directory()) { diff --git a/src/wx/film_editor.h b/src/wx/film_editor.h index 576cd5ba1..cd2180b3c 100644 --- a/src/wx/film_editor.h +++ b/src/wx/film_editor.h @@ -57,7 +57,7 @@ public: /* Handle changes to the model */ void film_changed (Film::Property); - void film_content_changed (int); + void film_content_change (ChangeType type, int); void set_general_sensitivity (bool); void active_jobs_changed (boost::optional); diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index 0e0fc4315..463854992 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -225,7 +225,7 @@ FilmViewer::set_film (shared_ptr film) _player->set_play_referenced (); _film->Changed.connect (boost::bind (&FilmViewer::film_changed, this, _1)); - _player->Changed.connect (boost::bind (&FilmViewer::player_changed, this, _1, _2)); + _player->Change.connect (boost::bind (&FilmViewer::player_change, this, _1, _2, _3)); /* Keep about 1 second's worth of history samples */ _latency_history_count = _film->audio_frame_rate() / _audio_block_size; @@ -665,9 +665,9 @@ FilmViewer::forward_clicked (wxKeyboardState& ev) } void -FilmViewer::player_changed (int property, bool frequent) +FilmViewer::player_change (ChangeType type, int property, bool frequent) { - if (frequent) { + if (type != CHANGE_TYPE_DONE || frequent) { return; } @@ -780,7 +780,7 @@ FilmViewer::set_coalesce_player_changes (bool c) if (!c) { BOOST_FOREACH (int i, _pending_player_changes) { - player_changed (i, false); + player_change (CHANGE_TYPE_DONE, i, false); } _pending_player_changes.clear (); } diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index a41500ace..d6d9a99a8 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -97,7 +97,7 @@ private: void rewind_clicked (wxMouseEvent &); void back_clicked (wxKeyboardState& s); void forward_clicked (wxKeyboardState &); - void player_changed (int, bool); + void player_change (ChangeType type, int, bool); void update_position_label (); void update_position_slider (); void get (); diff --git a/src/wx/hints_dialog.cc b/src/wx/hints_dialog.cc index 2c02a4f72..3872ea8ae 100644 --- a/src/wx/hints_dialog.cc +++ b/src/wx/hints_dialog.cc @@ -76,7 +76,7 @@ HintsDialog::HintsDialog (wxWindow* parent, boost::weak_ptr film, bool ok) boost::shared_ptr locked_film = _film.lock (); if (locked_film) { _film_changed_connection = locked_film->Changed.connect (boost::bind (&HintsDialog::film_changed, this)); - _film_content_changed_connection = locked_film->ContentChanged.connect (boost::bind (&HintsDialog::film_changed, this)); + _film_content_changed_connection = locked_film->ContentChange.connect (boost::bind (&HintsDialog::film_content_change, this, _1)); } _hints->Hint.connect (bind (&HintsDialog::hint, this, _1)); @@ -106,6 +106,14 @@ HintsDialog::film_changed () _hints->start (); } +void +HintsDialog::film_content_change (ChangeType type) +{ + if (type == CHANGE_TYPE_DONE) { + film_changed (); + } +} + void HintsDialog::update () { diff --git a/src/wx/hints_dialog.h b/src/wx/hints_dialog.h index 06f979a74..83510643a 100644 --- a/src/wx/hints_dialog.h +++ b/src/wx/hints_dialog.h @@ -18,6 +18,7 @@ */ +#include "lib/types.h" #include #include #include @@ -33,6 +34,7 @@ public: private: void film_changed (); + void film_content_change (ChangeType type); void shut_up (wxCommandEvent& ev); void update (); void hint (std::string text); diff --git a/src/wx/subtitle_appearance_dialog.cc b/src/wx/subtitle_appearance_dialog.cc index cab473c18..6473cd68b 100644 --- a/src/wx/subtitle_appearance_dialog.cc +++ b/src/wx/subtitle_appearance_dialog.cc @@ -185,11 +185,19 @@ SubtitleAppearanceDialog::SubtitleAppearanceDialog (wxWindow* parent, shared_ptr _force_fade_in->Bind (wxEVT_CHECKBOX, bind (&SubtitleAppearanceDialog::setup_sensitivity, this)); _force_fade_out->Bind (wxEVT_CHECKBOX, bind (&SubtitleAppearanceDialog::setup_sensitivity, this)); _effect->Bind (wxEVT_CHOICE, bind (&SubtitleAppearanceDialog::setup_sensitivity, this)); - _content_connection = _content->Changed.connect (bind (&SubtitleAppearanceDialog::setup_sensitivity, this)); + _content_connection = _content->Change.connect (bind (&SubtitleAppearanceDialog::content_change, this, _1)); setup_sensitivity (); } +void +SubtitleAppearanceDialog::content_change (ChangeType type) +{ + if (type == CHANGE_TYPE_DONE) { + setup_sensitivity (); + } +} + wxCheckBox* SubtitleAppearanceDialog::set_to (wxWindow* w, int& r) { diff --git a/src/wx/subtitle_appearance_dialog.h b/src/wx/subtitle_appearance_dialog.h index 6cced717b..80ef58470 100644 --- a/src/wx/subtitle_appearance_dialog.h +++ b/src/wx/subtitle_appearance_dialog.h @@ -44,6 +44,7 @@ private: void setup_sensitivity (); void restore (); wxCheckBox* set_to (wxWindow* w, int& r); + void content_change (ChangeType type); wxCheckBox* _force_colour; wxColourPickerCtrl* _colour; diff --git a/src/wx/timeline.cc b/src/wx/timeline.cc index 02f8be059..7353baf82 100644 --- a/src/wx/timeline.cc +++ b/src/wx/timeline.cc @@ -110,7 +110,7 @@ Timeline::Timeline (wxWindow* parent, ContentPanel* cp, shared_ptr film) SetMinSize (wxSize (640, 4 * pixels_per_track() + 96)); _film_changed_connection = film->Changed.connect (bind (&Timeline::film_changed, this, _1)); - _film_content_changed_connection = film->ContentChanged.connect (bind (&Timeline::film_content_changed, this, _2, _3)); + _film_content_change_connection = film->ContentChange.connect (bind (&Timeline::film_content_change, this, _1, _3, _4)); setup_scrollbars (); _labels_canvas->ShowScrollbars (wxSHOW_SB_NEVER, wxSHOW_SB_NEVER); @@ -243,8 +243,12 @@ Timeline::recreate_views () } void -Timeline::film_content_changed (int property, bool frequent) +Timeline::film_content_change (ChangeType type, int property, bool frequent) { + if (type != CHANGE_TYPE_DONE) { + return; + } + ensure_ui_thread (); if (property == AudioContentProperty::STREAMS) { diff --git a/src/wx/timeline.h b/src/wx/timeline.h index 2214ee13f..9f3a4a47b 100644 --- a/src/wx/timeline.h +++ b/src/wx/timeline.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2015 Carl Hetherington + Copyright (C) 2013-2018 Carl Hetherington This file is part of DCP-o-matic. @@ -92,7 +92,7 @@ private: void mouse_moved_select (wxMouseEvent &); void mouse_moved_zoom (wxMouseEvent &); void film_changed (Film::Property); - void film_content_changed (int, bool frequent); + void film_content_change (ChangeType type, int, bool frequent); void resized (); void assign_tracks (); void set_position_from_event (wxMouseEvent &); @@ -139,5 +139,5 @@ private: static int const _minimum_pixels_per_track; boost::signals2::scoped_connection _film_changed_connection; - boost::signals2::scoped_connection _film_content_changed_connection; + boost::signals2::scoped_connection _film_content_change_connection; }; diff --git a/src/wx/timeline_content_view.cc b/src/wx/timeline_content_view.cc index bf22e0156..12691f661 100644 --- a/src/wx/timeline_content_view.cc +++ b/src/wx/timeline_content_view.cc @@ -33,7 +33,7 @@ TimelineContentView::TimelineContentView (Timeline& tl, shared_ptr c) , _content (c) , _selected (false) { - _content_connection = c->Changed.connect (bind (&TimelineContentView::content_changed, this, _2)); + _content_connection = c->Change.connect (bind (&TimelineContentView::content_change, this, _1, _3)); } dcpomatic::Rect @@ -160,8 +160,12 @@ TimelineContentView::y_pos (int t) const } void -TimelineContentView::content_changed (int p) +TimelineContentView::content_change (ChangeType type, int p) { + if (type != CHANGE_TYPE_DONE) { + return; + } + ensure_ui_thread (); if (p == ContentProperty::POSITION || p == ContentProperty::LENGTH) { diff --git a/src/wx/timeline_content_view.h b/src/wx/timeline_content_view.h index b5b000bdb..27cfed53e 100644 --- a/src/wx/timeline_content_view.h +++ b/src/wx/timeline_content_view.h @@ -21,6 +21,7 @@ #ifndef DCPOMATIC_TIMELINE_CONTENT_VIEW_H #define DCPOMATIC_TIMELINE_CONTENT_VIEW_H +#include "lib/types.h" #include "timeline_view.h" #include #include @@ -57,7 +58,7 @@ private: void do_paint (wxGraphicsContext* gc, std::list > overlaps); int y_pos (int t) const; - void content_changed (int p); + void content_change (ChangeType type, int p); boost::optional _track; bool _selected; -- 2.30.2