More fixes for errors / crashes / misbehaviour with content changes

and the butler.

Here we signal both before and after a change in content.  Before,
the player disables itself so that any pass()/seek() will be no-ops.
After, the player rebuilds its pieces and the butler re-seeks to
get back to where it was before the change.

diff --git a/src/lib/audio_content.cc b/src/lib/audio_content.cc
index 7f0ec1fd6511ff294d655a43f1c72683b6332f80..703696a44d7d7accf1b71b2fa236306645a72e6d 100644 (file)
--- a/src/lib/audio_content.cc
+++ b/src/lib/audio_content.cc
@@ -146,6 +146,8 @@ AudioContent::technical_summary () const
 void
 AudioContent::set_mapping (AudioMapping mapping)
 {
+       ContentChange cc (_parent, AudioContentProperty::STREAMS);
+
        int c = 0;
        BOOST_FOREACH (AudioStreamPtr i, streams ()) {
                AudioMapping stream_mapping (i->channels (), MAX_DCP_AUDIO_CHANNELS);
@@ -157,8 +159,6 @@ AudioContent::set_mapping (AudioMapping mapping)
                }
                i->set_mapping (stream_mapping);
        }
-
-       _parent->signal_changed (AudioContentProperty::STREAMS);
 }
 
 AudioMapping
@@ -341,12 +341,12 @@ AudioContent::add_properties (list<UserProperty>& p) const
 void
 AudioContent::set_streams (vector<AudioStreamPtr> streams)
 {
+       ContentChange cc (_parent, AudioContentProperty::STREAMS);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _streams = streams;
        }
-
-       _parent->signal_changed (AudioContentProperty::STREAMS);
 }
 
 AudioStreamPtr
@@ -360,24 +360,24 @@ AudioContent::stream () const
 void
 AudioContent::add_stream (AudioStreamPtr stream)
 {
+       ContentChange cc (_parent, AudioContentProperty::STREAMS);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _streams.push_back (stream);
        }
-
-       _parent->signal_changed (AudioContentProperty::STREAMS);
 }
 
 void
 AudioContent::set_stream (AudioStreamPtr stream)
 {
+       ContentChange cc (_parent, AudioContentProperty::STREAMS);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _streams.clear ();
                _streams.push_back (stream);
        }
-
-       _parent->signal_changed (AudioContentProperty::STREAMS);
 }
 
 void

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 6789d74a5b925858caa234600fd734dfd7e1100b..386e7f9db78a90b723bb6b5463f5bed802ab20b9 100644 (file)
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -64,7 +64,8 @@ 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_changed_connection = _player->Changed.connect (bind (&Butler::player_changed, this, _2));
+       _player_changed_connection = _player->Changed.connect (bind (&Butler::return_seek, this, _2));
+       _player_not_changed_connection = _player->NotChanged.connect (bind (&Butler::return_seek, this, false));
        _thread = new boost::thread (bind (&Butler::thread, this));
 #ifdef DCPOMATIC_LINUX
        pthread_setname_np (_thread->native_handle(), "butler");
@@ -309,7 +310,7 @@ Butler::memory_used () const
 }
 
 void
-Butler::player_changed (bool frequent)
+Butler::return_seek (bool frequent)
 {
        boost::mutex::scoped_lock lm (_mutex);
        if (_died || _pending_seek_position || frequent) {

diff --git a/src/lib/butler.h b/src/lib/butler.h
index 5b09f6e5f836bfa861e4681588608bad9e8f0d6e..56374deeb31a9c904b7f2dd175e64b82463ceffc 100644 (file)
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -56,7 +56,7 @@ private:
        void text (PlayerText pt, TextType type, DCPTimePeriod period);
        bool should_run () const;
        void prepare (boost::weak_ptr<PlayerVideo> video) const;
-       void player_changed (bool frequent);
+       void return_seek (bool frequent);
        void seek_unlocked (DCPTime position, bool accurate);
 
        boost::shared_ptr<Player> _player;
@@ -101,4 +101,5 @@ private:
        boost::signals2::scoped_connection _player_audio_connection;
        boost::signals2::scoped_connection _player_text_connection;
        boost::signals2::scoped_connection _player_changed_connection;
+       boost::signals2::scoped_connection _player_not_changed_connection;
 };

diff --git a/src/lib/content.cc b/src/lib/content.cc
index 9b16eff7fa548d19ff82a4ea529138e375dc12f2..3814956d581951c3057a1112b6d9bfef5d0824a0 100644 (file)
--- a/src/lib/content.cc
+++ b/src/lib/content.cc
@@ -182,7 +182,7 @@ void
 Content::signal_changed (int p)
 {
        try {
-               emit (boost::bind (boost::ref (Changed), shared_from_this (), p, _change_signals_frequent));
+               emit (boost::bind (boost::ref(Changed), shared_from_this(), p, _change_signals_frequent));
        } catch (boost::bad_weak_ptr) {
                /* This must be during construction; never mind */
        }
@@ -201,16 +201,17 @@ Content::set_position (DCPTime p)
                audio->modify_position (p);
        }
 
+       ContentChange cc (this, ContentProperty::POSITION);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                if (p == _position) {
+                       cc.abort ();
                        return;
                }
 
                _position = p;
        }
-
-       signal_changed (ContentProperty::POSITION);
 }
 
 void
@@ -226,23 +227,23 @@ Content::set_trim_start (ContentTime t)
                audio->modify_trim_start (t);
        }
 
+       ContentChange cc (this, ContentProperty::TRIM_START);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _trim_start = t;
        }
-
-       signal_changed (ContentProperty::TRIM_START);
 }
 
 void
 Content::set_trim_end (ContentTime t)
 {
+       ContentChange cc (this, ContentProperty::TRIM_END);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _trim_end = t;
        }
-
-       signal_changed (ContentProperty::TRIM_END);
 }
 
 
@@ -305,16 +306,16 @@ Content::paths_valid () const
 void
 Content::set_path (boost::filesystem::path path)
 {
+       ContentChange cc (this, ContentProperty::PATH);
        _paths.clear ();
        _paths.push_back (path);
-       signal_changed (ContentProperty::PATH);
 }
 
 void
 Content::set_paths (vector<boost::filesystem::path> paths)
 {
+       ContentChange cc (this, ContentProperty::PATH);
        _paths = paths;
-       signal_changed (ContentProperty::PATH);
 }
 
 string
@@ -364,13 +365,13 @@ Content::reel_split_points () const
 void
 Content::set_video_frame_rate (double r)
 {
+       ContentChange cc (this, ContentProperty::VIDEO_FRAME_RATE);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _video_frame_rate = r;
        }
 
-       signal_changed (ContentProperty::VIDEO_FRAME_RATE);
-
        /* Make sure things are still on frame boundaries */
        if (video) {
                set_position (position());
@@ -381,12 +382,12 @@ Content::set_video_frame_rate (double r)
 void
 Content::unset_video_frame_rate ()
 {
+       ContentChange cc (this, ContentProperty::VIDEO_FRAME_RATE);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _video_frame_rate = optional<double>();
        }
-
-       signal_changed (ContentProperty::VIDEO_FRAME_RATE);
 }
 
 double

diff --git a/src/lib/content.h b/src/lib/content.h
index 23afa2243046d85db3ccbb9ffe823ea58d6905b2..c9edf6e22628fcdca99af1c1c16ed2dbc3caa2dd 100644 (file)
--- a/src/lib/content.h
+++ b/src/lib/content.h
@@ -178,8 +178,15 @@ 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;
+
        boost::shared_ptr<VideoContent> video;
        boost::shared_ptr<AudioContent> audio;
        std::list<boost::shared_ptr<TextContent> > text;
@@ -187,8 +194,6 @@ public:
        boost::shared_ptr<TextContent> only_text () const;
        boost::shared_ptr<TextContent> text_of_original_type (TextType type) const;
 
-       void signal_changed (int);
-
 protected:
 
        virtual void add_properties (std::list<UserProperty> &) const;
@@ -208,6 +213,9 @@ private:
        friend struct best_dcp_frame_rate_test_single;
        friend struct best_dcp_frame_rate_test_double;
        friend struct audio_sampling_rate_test;
+       friend class ContentChange;
+
+       void signal_changed (int);
 
        std::string _digest;
        DCPTime _position;

diff --git a/src/lib/content_part.h b/src/lib/content_part.h
index 4f6e9e396bbb70325505cfd7f7cce191ead95e3b..58833655fb5dcb9d503b1e0fc2f87ef8f62c1f67 100644 (file)
--- a/src/lib/content_part.h
+++ b/src/lib/content_part.h
@@ -23,6 +23,7 @@
 #define DCPOMATIC_CONTENT_PART_H
 
 #include "content.h"
+#include "content_change.h"
 #include <boost/weak_ptr.hpp>
 #include <boost/thread/mutex.hpp>
 
@@ -41,28 +42,30 @@ protected:
        void
        maybe_set (T& member, T new_value, int property) const
        {
+               ContentChange cc (_parent, property);
                {
                        boost::mutex::scoped_lock lm (_mutex);
                        if (member == new_value) {
+                               cc.abort ();
                                return;
                        }
                        member = new_value;
                }
-               _parent->signal_changed (property);
        }
 
        template <class T>
        void
        maybe_set (boost::optional<T>& member, T new_value, int property) const
        {
+               ContentChange cc (_parent, property);
                {
                        boost::mutex::scoped_lock lm (_mutex);
                        if (member && member.get() == new_value) {
+                               cc.abort ();
                                return;
                        }
                        member = new_value;
                }
-               _parent->signal_changed (property);
        }
 
        Content* _parent;

diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index b6a66f90ac8423650af4c82fc33381c48bdb43f7..a527b1f809605a8435ebcece3738aebb01ec8983 100644 (file)
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -61,6 +61,7 @@ int const DCPContentProperty::REFERENCE_AUDIO    = 603;
 int const DCPContentProperty::REFERENCE_TEXT     = 604;
 int const DCPContentProperty::NAME               = 605;
 int const DCPContentProperty::TEXTS              = 606;
+int const DCPContentProperty::CPL                = 607;
 
 DCPContent::DCPContent (shared_ptr<const Film> film, boost::filesystem::path p)
        : Content (film)
@@ -158,6 +159,12 @@ DCPContent::examine (shared_ptr<Job> job)
        string const old_name = name ();
        int const old_texts = text.size ();
 
+       ContentChange cc_texts (this, DCPContentProperty::TEXTS);
+       ContentChange cc_assets (this, DCPContentProperty::NEEDS_ASSETS);
+       ContentChange cc_kdm (this, DCPContentProperty::NEEDS_KDM);
+       ContentChange cc_name (this, DCPContentProperty::NAME);
+       ContentChange cc_streams (this, AudioContentProperty::STREAMS);
+
        if (job) {
                job->set_progress_unknown ();
        }
@@ -175,6 +182,7 @@ DCPContent::examine (shared_ptr<Job> job)
        }
 
        if (examiner->has_audio()) {
+               ContentChange cc (this, AudioContentProperty::STREAMS);
                {
                        boost::mutex::scoped_lock lm (_mutex);
                        audio.reset (new AudioContent (this));
@@ -184,7 +192,6 @@ DCPContent::examine (shared_ptr<Job> job)
                AudioMapping m = as->mapping ();
                film()->make_audio_mapping_default (m);
                as->set_mapping (m);
-               signal_changed (AudioContentProperty::STREAMS);
        }
 
        int texts = 0;
@@ -206,24 +213,22 @@ DCPContent::examine (shared_ptr<Job> job)
                _reel_lengths = examiner->reel_lengths ();
        }
 
-       if (old_texts != texts) {
-               signal_changed (DCPContentProperty::TEXTS);
+       if (old_texts == texts) {
+               cc_texts.abort ();
        }
 
-       if (needed_assets != needs_assets ()) {
-               signal_changed (DCPContentProperty::NEEDS_ASSETS);
+       if (needed_assets == needs_assets()) {
+               cc_assets.abort ();
        }
 
-       if (needed_kdm != needs_kdm ()) {
-               signal_changed (DCPContentProperty::NEEDS_KDM);
+       if (needed_kdm == needs_kdm()) {
+               cc_kdm.abort ();
        }
 
-       if (old_name != name ()) {
-               signal_changed (DCPContentProperty::NAME);
+       if (old_name == name()) {
+               cc_name.abort ();
        }
 
-       signal_changed (AudioContentProperty::STREAMS);
-
        if (video) {
                video->set_frame_type (_three_d ? VIDEO_FRAME_TYPE_3D : VIDEO_FRAME_TYPE_2D);
        }
@@ -396,34 +401,34 @@ DCPContent::set_default_colour_conversion ()
 void
 DCPContent::set_reference_video (bool r)
 {
+       ContentChange cc (this, DCPContentProperty::REFERENCE_VIDEO);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _reference_video = r;
        }
-
-       signal_changed (DCPContentProperty::REFERENCE_VIDEO);
 }
 
 void
 DCPContent::set_reference_audio (bool r)
 {
+       ContentChange cc (this, DCPContentProperty::REFERENCE_AUDIO);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _reference_audio = r;
        }
-
-       signal_changed (DCPContentProperty::REFERENCE_AUDIO);
 }
 
 void
 DCPContent::set_reference_text (TextType type, bool r)
 {
+       ContentChange cc (this, DCPContentProperty::REFERENCE_TEXT);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _reference_text[type] = r;
        }
-
-       signal_changed (DCPContentProperty::REFERENCE_TEXT);
 }
 
 list<DCPTimePeriod>
@@ -644,6 +649,10 @@ DCPContent::take_settings_from (shared_ptr<const Content> c)
 void
 DCPContent::set_cpl (string id)
 {
-       boost::mutex::scoped_lock lm (_mutex);
-       _cpl = id;
+       ContentChange cc (this, DCPContentProperty::CPL);
+
+       {
+               boost::mutex::scoped_lock lm (_mutex);
+               _cpl = id;
+       }
 }

diff --git a/src/lib/dcp_content.h b/src/lib/dcp_content.h
index cccf127c31732d00f1003bdf82fc2d6d78bc3097..43476e729c7f8ca3495bb26dcde21988b96dc4e5 100644 (file)
--- a/src/lib/dcp_content.h
+++ b/src/lib/dcp_content.h
@@ -39,6 +39,7 @@ public:
        static int const REFERENCE_TEXT;
        static int const NAME;
        static int const TEXTS;
+       static int const CPL;
 };
 
 class ContentPart;

diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index ebcbcc47ea008bcf1245f020af6483b4cde77cbb..05b6ae8ea8a9105952c858d3ae3bc5a116be2030 100644 (file)
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -251,6 +251,9 @@ FFmpegContent::as_xml (xmlpp::Node* node, bool with_paths) const
 void
 FFmpegContent::examine (shared_ptr<Job> job)
 {
+       ContentChange cc1 (this, FFmpegContentProperty::SUBTITLE_STREAMS);
+       ContentChange cc2 (this, FFmpegContentProperty::SUBTITLE_STREAM);
+
        job->set_progress_unknown ();
 
        Content::examine (job);
@@ -313,9 +316,6 @@ FFmpegContent::examine (shared_ptr<Job> job)
        if (examiner->has_video ()) {
                set_default_colour_conversion ();
        }
-
-       signal_changed (FFmpegContentProperty::SUBTITLE_STREAMS);
-       signal_changed (FFmpegContentProperty::SUBTITLE_STREAM);
 }
 
 string
@@ -369,12 +369,12 @@ FFmpegContent::technical_summary () const
 void
 FFmpegContent::set_subtitle_stream (shared_ptr<FFmpegSubtitleStream> s)
 {
+       ContentChange cc (this, FFmpegContentProperty::SUBTITLE_STREAM);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _subtitle_stream = s;
        }
-
-       signal_changed (FFmpegContentProperty::SUBTITLE_STREAM);
 }
 
 bool
@@ -410,12 +410,12 @@ FFmpegContent::full_length () const
 void
 FFmpegContent::set_filters (vector<Filter const *> const & filters)
 {
+       ContentChange cc (this, FFmpegContentProperty::FILTERS);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _filters = filters;
        }
-
-       signal_changed (FFmpegContentProperty::FILTERS);
 }
 
 string
@@ -629,7 +629,8 @@ FFmpegContent::add_properties (list<UserProperty>& p) const
 void
 FFmpegContent::signal_subtitle_stream_changed ()
 {
-       signal_changed (FFmpegContentProperty::SUBTITLE_STREAM);
+       /* XXX: this is too late; really it should be before the change */
+       ContentChange cc (this, FFmpegContentProperty::SUBTITLE_STREAM);
 }
 
 vector<shared_ptr<FFmpegAudioStream> >

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 4ee7d93596706abeafd8e7d4b28c056e98b023ad..91f1cb517c3f6d9fb1db980ad944ebc9807b1278 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -87,7 +87,7 @@ int const PlayerProperty::DCP_DECODE_REDUCTION = 704;
 Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist)
        : _film (film)
        , _playlist (playlist)
-       , _have_valid_pieces (false)
+       , _can_run (false)
        , _ignore_video (false)
        , _ignore_audio (false)
        , _ignore_text (false)
@@ -99,11 +99,14 @@ Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist
 {
        _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_changed_connection = _playlist->ContentChanged.connect (bind (&Player::playlist_content_changed, this, _1, _2, _3));
+       _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));
        set_video_container_size (_film->frame_size ());
 
        film_changed (Film::AUDIO_PROCESSOR);
 
+       setup_pieces ();
        seek (DCPTime (), true);
 }
 
@@ -114,6 +117,13 @@ Player::~Player ()
 
 void
 Player::setup_pieces ()
+{
+       boost::mutex::scoped_lock lm (_mutex);
+       setup_pieces_unlocked ();
+}
+
+void
+Player::setup_pieces_unlocked ()
 {
        _pieces.clear ();
 
@@ -210,17 +220,38 @@ Player::setup_pieces ()
        _last_video_time = DCPTime ();
        _last_video_eyes = EYES_BOTH;
        _last_audio_time = DCPTime ();
-       _have_valid_pieces = true;
+       _can_run = true;
+}
+
+void
+Player::playlist_content_may_change ()
+{
+       {
+               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.
+               */
+               _can_run = false;
+       }
+
+       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 ||
@@ -232,21 +263,12 @@ Player::playlist_content_changed (weak_ptr<Content> w, int property, bool freque
                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
-               ) {
-
-               {
-                       boost::mutex::scoped_lock lm (_mutex);
-                       _have_valid_pieces = false;
-               }
-
-               Changed (property, frequent);
-
-       } else if (
+               property == FFmpegContentProperty::FILTERS ||
                property == TextContentProperty::LINE_SPACING ||
                property == TextContentProperty::OUTLINE_WIDTH ||
                property == TextContentProperty::Y_SCALE ||
@@ -269,6 +291,14 @@ Player::playlist_content_changed (weak_ptr<Content> w, int property, bool freque
        }
 }
 
+void
+Player::playlist_content_not_changed ()
+{
+       /* A possible content change did end up happening for some reason */
+       setup_pieces ();
+       NotChanged ();
+}
+
 void
 Player::set_video_container_size (dcp::Size s)
 {
@@ -291,11 +321,7 @@ Player::set_video_container_size (dcp::Size s)
 void
 Player::playlist_changed ()
 {
-       {
-               boost::mutex::scoped_lock lm (_mutex);
-               _have_valid_pieces = false;
-       }
-
+       setup_pieces ();
        Changed (PlayerProperty::PLAYLIST, false);
 }
 
@@ -313,10 +339,7 @@ Player::film_changed (Film::Property p)
                /* Pieces contain a FrameRateChange which contains the DCP frame rate,
                   so we need new pieces here.
                */
-               {
-                       boost::mutex::scoped_lock lm (_mutex);
-                       _have_valid_pieces = false;
-               }
+               setup_pieces ();
                Changed (PlayerProperty::FILM_VIDEO_FRAME_RATE, false);
        } else if (p == Film::AUDIO_PROCESSOR) {
                if (_film->audio_processor ()) {
@@ -441,11 +464,7 @@ Player::content_time_to_dcp (shared_ptr<const Piece> piece, ContentTime t) const
 list<shared_ptr<Font> >
 Player::get_subtitle_fonts ()
 {
-       /* Does not require a lock on _mutex as it's only called from DCPEncoder */
-
-       if (!_have_valid_pieces) {
-               setup_pieces ();
-       }
+       boost::mutex::scoped_lock lm (_mutex);
 
        list<shared_ptr<Font> > fonts;
        BOOST_FOREACH (shared_ptr<Piece> i, _pieces) {
@@ -467,7 +486,7 @@ Player::set_ignore_video ()
 {
        boost::mutex::scoped_lock lm (_mutex);
        _ignore_video = true;
-       setup_pieces ();
+       setup_pieces_unlocked ();
 }
 
 void
@@ -475,7 +494,7 @@ Player::set_ignore_audio ()
 {
        boost::mutex::scoped_lock lm (_mutex);
        _ignore_audio = true;
-       setup_pieces ();
+       setup_pieces_unlocked ();
 }
 
 void
@@ -483,7 +502,7 @@ Player::set_ignore_text ()
 {
        boost::mutex::scoped_lock lm (_mutex);
        _ignore_text = true;
-       setup_pieces ();
+       setup_pieces_unlocked ();
 }
 
 /** Set the player to always burn open texts into the image regardless of the content settings */
@@ -500,7 +519,7 @@ Player::set_fast ()
 {
        boost::mutex::scoped_lock lm (_mutex);
        _fast = true;
-       setup_pieces ();
+       setup_pieces_unlocked ();
 }
 
 void
@@ -508,7 +527,7 @@ Player::set_play_referenced ()
 {
        boost::mutex::scoped_lock lm (_mutex);
        _play_referenced = true;
-       setup_pieces ();
+       setup_pieces_unlocked ();
 }
 
 list<ReferencedReelAsset>
@@ -594,13 +613,8 @@ Player::pass ()
 {
        boost::mutex::scoped_lock lm (_mutex);
 
-       if (!_have_valid_pieces) {
-               /* This should only happen when we are under the control of the butler.  In this case, _have_valid_pieces
-                  will be false if something in the Player has changed and we are waiting for the butler to notice
-                  and do a seek back to the place we were at before.  During this time we don't want pass() to do anything,
-                  as just after setup_pieces the new decoders will be back to time 0 until the seek has gone through.  Just do nothing
-                  here and assume that the seek will be forthcoming.
-               */
+       if (!_can_run) {
+               /* We can't pass in this state */
                return false;
        }
 
@@ -1025,8 +1039,9 @@ Player::seek (DCPTime time, bool accurate)
 {
        boost::mutex::scoped_lock lm (_mutex);
 
-       if (!_have_valid_pieces) {
-               setup_pieces ();
+       if (!_can_run) {
+               /* We can't seek in this state */
+               return;
        }
 
        if (_shuffler) {
@@ -1181,27 +1196,23 @@ Player::set_dcp_decode_reduction (optional<int> reduction)
                }
 
                _dcp_decode_reduction = reduction;
-               _have_valid_pieces = false;
+               setup_pieces_unlocked ();
        }
 
        Changed (PlayerProperty::DCP_DECODE_REDUCTION, false);
 }
 
-DCPTime
+optional<DCPTime>
 Player::content_time_to_dcp (shared_ptr<Content> content, ContentTime t)
 {
        boost::mutex::scoped_lock lm (_mutex);
 
-       if (_have_valid_pieces) {
-               setup_pieces ();
-       }
-
        BOOST_FOREACH (shared_ptr<Piece> i, _pieces) {
                if (i->content == content) {
                        return content_time_to_dcp (i, t);
                }
        }
 
-       DCPOMATIC_ASSERT (false);
-       return DCPTime ();
+       /* We couldn't find this content; perhaps things are being changed over */
+       return optional<DCPTime>();
 }

diff --git a/src/lib/player.h b/src/lib/player.h
index 20b68ef774243d632d0048747fce5b264802b289..c43cfd40e1eca4d0dc0b154c8aeb71089475c8bc 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -86,7 +86,9 @@ public:
        void set_play_referenced ();
        void set_dcp_decode_reduction (boost::optional<int> reduction);
 
-       DCPTime content_time_to_dcp (boost::shared_ptr<Content> content, ContentTime t);
+       boost::optional<DCPTime> content_time_to_dcp (boost::shared_ptr<Content> content, ContentTime t);
+
+       boost::signals2::signal<void ()> MayChange;
 
        /** Emitted when something has changed such that if we went back and emitted
         *  the last frame again it would look different.  This is not emitted after
@@ -96,6 +98,7 @@ public:
         *  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 ()> NotChanged;
 
        /** Emitted when a video frame is ready.  These emissions happen in the correct order. */
        boost::signals2::signal<void (boost::shared_ptr<PlayerVideo>, DCPTime)> Video;
@@ -114,10 +117,13 @@ private:
        friend struct player_subframe_test;
 
        void setup_pieces ();
+       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 ();
        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;
@@ -150,8 +156,8 @@ private:
        boost::shared_ptr<const Film> _film;
        boost::shared_ptr<const Playlist> _playlist;
 
-       /** Our pieces are ready to go; if this is false the pieces must be (re-)created before they are used */
-       bool _have_valid_pieces;
+       /** We can pass() and seek() */
+       bool _can_run;
        std::list<boost::shared_ptr<Piece> > _pieces;
 
        /** Size of the image in the DCP (e.g. 1990x1080 for flat) */
@@ -207,7 +213,9 @@ private:
 
        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;
 };
 
 #endif

diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc
index c830e5e1e49c3b8e83a3de969f47551853bec826..1bb0ad0d6cda449354ed0eae1da435d59e649131 100644 (file)
--- a/src/lib/playlist.cc
+++ b/src/lib/playlist.cc
@@ -64,6 +64,18 @@ Playlist::~Playlist ()
        reconnect ();
 }
 
+void
+Playlist::content_may_change ()
+{
+       ContentMayChange ();
+}
+
+void
+Playlist::content_not_changed ()
+{
+       ContentNotChanged ();
+}
+
 void
 Playlist::content_changed (weak_ptr<Content> content, int property, bool frequent)
 {
@@ -350,7 +362,9 @@ Playlist::reconnect ()
        _content_connections.clear ();
 
        BOOST_FOREACH (shared_ptr<Content> i, _content) {
-               _content_connections.push_back (i->Changed.connect (bind (&Playlist::content_changed, this, _1, _2, _3)));
+               _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)));
        }
 }
 

diff --git a/src/lib/playlist.h b/src/lib/playlist.h
index 4ee1201989f5a3842d1d9e1a9a7b83bd816fe6a3..649887f7271f38b2c2fd9c4afdf0f63bb4f0d195 100644 (file)
--- a/src/lib/playlist.h
+++ b/src/lib/playlist.h
@@ -77,14 +77,19 @@ public:
        /** 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 ()> 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;
 
 private:
+       void content_may_change ();
        void content_changed (boost::weak_ptr<Content>, int, bool);
+       void content_not_changed ();
        void reconnect ();
 
        /** List of content.  Kept sorted in position order. */

diff --git a/src/lib/text_content.cc b/src/lib/text_content.cc
index ee29ae8c4d28122c6dfeafc2838906a48bf4c0d8..35dee525adf9c94827842e2376411e875279f5a1 100644 (file)
--- a/src/lib/text_content.cc
+++ b/src/lib/text_content.cc
@@ -426,7 +426,8 @@ TextContent::connect_to_fonts ()
 void
 TextContent::font_changed ()
 {
-       _parent->signal_changed (TextContentProperty::FONTS);
+       /* XXX: too late */
+       ContentChange cc (_parent, TextContentProperty::FONTS);
 }
 
 void

diff --git a/src/lib/video_content.cc b/src/lib/video_content.cc
index d3ba6c1ab1f3d50d89806290b8eb8ed12b2b54aa..81f4138fbef98248cf52aa16a7a8375c7d9a5adc 100644 (file)
--- a/src/lib/video_content.cc
+++ b/src/lib/video_content.cc
@@ -234,6 +234,10 @@ VideoContent::take_from_examiner (shared_ptr<VideoExaminer> d)
        optional<double> const ar = d->sample_aspect_ratio ();
        bool const yuv = d->yuv ();
 
+       ContentChange cc1 (_parent, VideoContentProperty::SIZE);
+       ContentChange cc2 (_parent, VideoContentProperty::SCALE);
+       ContentChange cc3 (_parent, ContentProperty::LENGTH);
+
        {
                boost::mutex::scoped_lock lm (_mutex);
                _size = vs;
@@ -256,10 +260,6 @@ VideoContent::take_from_examiner (shared_ptr<VideoExaminer> d)
        if (d->video_frame_rate()) {
                _parent->set_video_frame_rate (d->video_frame_rate().get());
        }
-
-       _parent->signal_changed (VideoContentProperty::SIZE);
-       _parent->signal_changed (VideoContentProperty::SCALE);
-       _parent->signal_changed (ContentProperty::LENGTH);
 }
 
 /** @return string which includes everything about how this content looks */

diff --git a/src/lib/wscript b/src/lib/wscript
index 0cc3d782361fd3648b13bed645f69eb115e67c60..68e44ccd053608d7c79dd57e7bb8313237803b31 100644 (file)
--- a/src/lib/wscript
+++ b/src/lib/wscript
@@ -47,6 +47,7 @@ sources = """
           colour_conversion.cc
           config.cc
           content.cc
+          content_change.cc
           content_factory.cc
           cross.cc
           curl_uploader.cc

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index d00c0bfafb0148e995649fcb8e5ac6c503d9f2ee..0e0fc4315ad7a58ff339d32c8cf4df4590718952 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -767,7 +767,10 @@ FilmViewer::set_position (DCPTime p)
 void
 FilmViewer::set_position (shared_ptr<Content> content, ContentTime t)
 {
-       set_position (_player->content_time_to_dcp (content, t));
+       optional<DCPTime> dt = _player->content_time_to_dcp (content, t);
+       if (dt) {
+               set_position (*dt);
+       }
 }
 
 void