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

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index f04b324cdea4cc29e02bbac2a964d5c27c898c4b..3a75e67d75ae56f4a14207e5d4d093360d153e4d 100644 (file)
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -65,9 +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));
+       _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 6a13ed2d72548ebcec849a926b51a4a4f379f602..513176821e8f455c46f312d5458534e0809b2e5e 100644 (file)
--- 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<PlayerVideo> 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> _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 8fd36a1d8abf5c8a115ba22600ce20845b3ac19a..ae0b08dccc241f24ba5f4d43f6cfadd658e5b583 100644 (file)
--- a/src/lib/content.cc
+++ b/src/lib/content.cc
@@ -179,10 +179,14 @@ Content::examine (shared_ptr<Job> 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 c9edf6e22628fcdca99af1c1c16ed2dbc3caa2dd..e8710ccb7405b82a837eececb05e770457839f0a 100644 (file)
--- a/src/lib/content.h
+++ b/src/lib/content.h
@@ -178,14 +178,8 @@ public:
 
        std::list<UserProperty> user_properties () const;
 
-       /* May be emitted from any thread */
-       boost::signals2::signal<void ()> MayChange;
-
-       /* Emitted from the GUI thread */
-       boost::signals2::signal<void (boost::weak_ptr<Content>, int, bool)> Changed;
-
-       /* May be emitted from any thread */
-       boost::signals2::signal<void ()> NotChanged;
+       /* CHANGE_PENDING and CHANGE_CANCELLED may be emitted from any thread; CHANGE_DONE always from GUI thread */
+       boost::signals2::signal<void (ChangeType, boost::weak_ptr<Content>, int, bool)> Change;
 
        boost::shared_ptr<VideoContent> video;
        boost::shared_ptr<AudioContent> 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 c286f80eba029e8905d556e4350a6675924f48d6..6e390dc7664a114ba7d46a682857cafb6383bdbd 100644 (file)
--- 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 ba97f833ea39b7df3d6a8bd903dedb3767a942d8..846e8ac51cd2c48f12c110a2c4c98d0249fc929b 100644 (file)
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -160,9 +160,9 @@ Film::Film (optional<boost::filesystem::path> 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<Content> c, int p, bool frequent)
+Film::playlist_content_change (ChangeType type, weak_ptr<Content> 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<Content> 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 20a1e2ca6551ad9f7c6ddaf9ddb4e2409e51c453..c6c8403cb793d9245a53b39e7c8ae388c7aa0b08 100644 (file)
--- a/src/lib/film.h
+++ b/src/lib/film.h
@@ -327,7 +327,7 @@ public:
        mutable boost::signals2::signal<void (Property)> Changed;
 
        /** Emitted when some property of our content has changed */
-       mutable boost::signals2::signal<void (boost::weak_ptr<Content>, int, bool)> ContentChanged;
+       mutable boost::signals2::signal<void (ChangeType, boost::weak_ptr<Content>, 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<Content>, int, bool frequent);
+       void playlist_content_change (ChangeType type, boost::weak_ptr<Content>, int, bool frequent);
        void maybe_add_content (boost::weak_ptr<Job>, boost::weak_ptr<Content>, bool disable_audio_analysis);
        void audio_analysis_finished ();
 
@@ -401,9 +401,9 @@ private:
        /** film being used as a template, or 0 */
        boost::shared_ptr<Film> _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<boost::signals2::connection> _job_connections;
        std::list<boost::signals2::connection> _audio_analysis_connections;
 

diff --git a/src/lib/player.cc b/src/lib/player.cc
index df8c47cf9ebf1b83f84055ab8cbe1e8a6415cdd7..0c3aea0288ef057f531ef315333a01277c945655 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -98,10 +98,8 @@ Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> 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<Content> 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<Content> 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<const AudioBuffers> audio, DCPTime time, DCPTi
 void
 Player::set_dcp_decode_reduction (optional<int> 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<int> reduction)
                setup_pieces_unlocked ();
        }
 
-       Changed (PlayerProperty::DCP_DECODE_REDUCTION, false);
+       Change (CHANGE_TYPE_DONE, PlayerProperty::DCP_DECODE_REDUCTION, false);
 }
 
 optional<DCPTime>

diff --git a/src/lib/player.h b/src/lib/player.h
index f7ab1000d37f7392fed01187cdd6b776b414a4f1..1658207996c2a903020bac44844eaf5d8b0546e8 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -88,19 +88,7 @@ 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;
-
-       /** 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;
+       boost::signals2::signal<void (ChangeType, int, bool)> Change;
 
        /** The change suggested by a MayChange did not happen */
        boost::signals2::signal<void ()> 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<Content>, int, bool);
-       void playlist_content_not_changed ();
+       void playlist_change (ChangeType);
+       void playlist_content_change (ChangeType, int, bool);
        std::list<PositionImage> transform_bitmap_texts (std::list<BitmapText>) const;
        Frame dcp_to_content_video (boost::shared_ptr<const Piece> piece, DCPTime t) const;
        DCPTime content_video_to_dcp (boost::shared_ptr<const Piece> piece, Frame f) const;
@@ -217,10 +203,8 @@ private:
        boost::shared_ptr<AudioProcessor> _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 1bb0ad0d6cda449354ed0eae1da435d59e649131..e4fc0f0723df7e100229dd1765d8ee831ba4c7da 100644 (file)
--- 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> 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> 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<Content> 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<Content> 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<Content> 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<Content> c)
 void
 Playlist::remove (ContentList c)
 {
+       Change (CHANGE_TYPE_PENDING);
+
        BOOST_FOREACH (shared_ptr<Content> 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<Content> 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<Content> 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 649887f7271f38b2c2fd9c4afdf0f63bb4f0d195..d55232dadb8898793417165ed3b955290a39db14 100644 (file)
--- 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<void ()> Changed;
+       mutable boost::signals2::signal<void (ChangeType)> Change;
        mutable boost::signals2::signal<void ()> OrderChanged;
 
-       mutable boost::signals2::signal<void ()> 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<void (boost::weak_ptr<Content>, int, bool)> ContentChanged;
-       mutable boost::signals2::signal<void ()> ContentNotChanged;
+       mutable boost::signals2::signal<void (ChangeType, boost::weak_ptr<Content>, int, bool)> ContentChange;
 
 private:
-       void content_may_change ();
-       void content_changed (boost::weak_ptr<Content>, int, bool);
-       void content_not_changed ();
+       void content_change (ChangeType, boost::weak_ptr<Content>, 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 42e2e3eecd2c8d6801e3b6563ecdcf53a5b116d4..22d652b3f74e47d42c4cce22a0eaa2bf6c706c11 100644 (file)
--- 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 8c015ab68fabf5d95496f1b98ce2ba9cbc48b6e9..e1f7c15a621f913959feb2fa1e83e073f43022aa 100644 (file)
--- a/src/wx/audio_dialog.cc
+++ b/src/wx/audio_dialog.cc
@@ -150,7 +150,7 @@ AudioDialog::AudioDialog (wxWindow* parent, shared_ptr<Film> film, shared_ptr<Co
        overall_sizer->Layout ();
        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 6d44285463ba0823081a73b6e53695add2a0c0f0..2c077c9d1207718ad5af2b61a79a5d488b63dc8a 100644 (file)
--- a/src/wx/audio_dialog.h
+++ b/src/wx/audio_dialog.h
@@ -38,7 +38,7 @@ public:
        void set_cursor (boost::optional<DCPTime> time, boost::optional<float> 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 e162aca4950718ac7fd709a21d59d2bca114b52b..e5125680bcfe59bfa607cf08af79044392190b0b 100644 (file)
--- 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 5380cbd9bed582cd7716ade2096b7c5c34dc2065..1c1c02f5b4cfc9532e79741750f731894d1f6ba4 100644 (file)
--- 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> 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 576cd5ba1b251bab083dec3cab431049bda02ca4..cd2180b3c3d297cae87019f6835b4c86c28393ee 100644 (file)
--- 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<std::string>);

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 0e0fc4315ad7a58ff339d32c8cf4df4590718952..463854992f16347c875e738565d74221501b65e1 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -225,7 +225,7 @@ FilmViewer::set_film (shared_ptr<Film> 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 a41500acee992ba0020608ca6f8a41b1e1ce47e1..d6d9a99a82b38e95dd7622f470ef2ded524f7656 100644 (file)
--- 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 2c02a4f72a12ff74478838839741a79da999014a..3872ea8ae2a281772df3ee667b4fecbf075b67a7 100644 (file)
--- a/src/wx/hints_dialog.cc
+++ b/src/wx/hints_dialog.cc
@@ -76,7 +76,7 @@ HintsDialog::HintsDialog (wxWindow* parent, boost::weak_ptr<Film> film, bool ok)
        boost::shared_ptr<Film> 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 06f979a74fde34d575dc07afed42354ce759dd12..83510643a3be2264a54766efafbe7549a098aad2 100644 (file)
--- a/src/wx/hints_dialog.h
+++ b/src/wx/hints_dialog.h
@@ -18,6 +18,7 @@
 
 */
 
+#include "lib/types.h"
 #include <wx/wx.h>
 #include <boost/weak_ptr.hpp>
 #include <boost/signals2.hpp>
@@ -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 cab473c187d494bb3be751557aa8a681084c9269..6473cd68bbbf738be99effaaaed733985c5b2408 100644 (file)
--- 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 6cced717bae1cbcf5bb6cbbdcf584beef1c69285..80ef584704a17e2f3453aedc3ccca6ec90e55a25 100644 (file)
--- 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 02f8be0597637b77b4fad5ab64aa525c3e691994..7353baf82c7b8b94ffc5279d4b681bae0ef99bda 100644 (file)
--- a/src/wx/timeline.cc
+++ b/src/wx/timeline.cc
@@ -110,7 +110,7 @@ Timeline::Timeline (wxWindow* parent, ContentPanel* cp, shared_ptr<Film> 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 2214ee13fddb1ba5192b86e35b0e2ab5dc1a6cc1..9f3a4a47b6a7ead9aa7f4ac003cabc70fadc21f3 100644 (file)
--- a/src/wx/timeline.h
+++ b/src/wx/timeline.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2018 Carl Hetherington <cth@carlh.net>
 
     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 bf22e0156d7d739f4ecc0f6d47f24685ac042f53..12691f661d34d163eff989173d62bb1254f63ab0 100644 (file)
--- a/src/wx/timeline_content_view.cc
+++ b/src/wx/timeline_content_view.cc
@@ -33,7 +33,7 @@ TimelineContentView::TimelineContentView (Timeline& tl, shared_ptr<Content> 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<int>
@@ -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 b5b000bdb9cc14ece8451c1506cd8ff6d0bfdabb..27cfed53eef989373eee4db604886d4890c3facb 100644 (file)
--- 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 <wx/wx.h>
 #include <boost/signals2.hpp>
@@ -57,7 +58,7 @@ private:
 
        void do_paint (wxGraphicsContext* gc, std::list<dcpomatic::Rect<int> > overlaps);
        int y_pos (int t) const;
-       void content_changed (int p);
+       void content_change (ChangeType type, int p);
 
        boost::optional<int> _track;
        bool _selected;