An unfortunately large set of timeline-related changes:

- Rename sequence_video to sequence, and sequence subtitle content
like we do video content (i.e. adding multiple subtitle contents
will result in them sequenced in time rather than overlaid).
- Stop doing selection-changed related stuff in ContentPanel
if no selection change has actually happened.
- Attempt to tidy up event handling in the timeline a bit.

diff --git a/src/lib/film.cc b/src/lib/film.cc
index f4d983260b03daa69baf91f46dc8689a6f81d1cf..3ea7545dfcb2fe3661a40e9fbd762c7d1a9b3a33 100644 (file)
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -127,7 +127,7 @@ Film::Film (boost::filesystem::path dir, bool log)
        , _video_frame_rate (24)
        , _audio_channels (6)
        , _three_d (false)
-       , _sequence_video (true)
+       , _sequence (true)
        , _interop (Config::instance()->default_interop ())
        , _audio_processor (0)
        , _reel_type (REELTYPE_SINGLE)
@@ -168,7 +168,7 @@ Film::Film (boost::filesystem::path dir, bool log)
                _log.reset (new NullLog);
        }
 
-       _playlist->set_sequence_video (_sequence_video);
+       _playlist->set_sequence (_sequence);
 }
 
 Film::~Film ()
@@ -347,7 +347,7 @@ Film::metadata () const
        root->add_child("ISDCFDate")->add_child_text (boost::gregorian::to_iso_string (_isdcf_date));
        root->add_child("AudioChannels")->add_child_text (raw_convert<string> (_audio_channels));
        root->add_child("ThreeD")->add_child_text (_three_d ? "1" : "0");
-       root->add_child("SequenceVideo")->add_child_text (_sequence_video ? "1" : "0");
+       root->add_child("Sequence")->add_child_text (_sequence ? "1" : "0");
        root->add_child("Interop")->add_child_text (_interop ? "1" : "0");
        root->add_child("Signed")->add_child_text (_signed ? "1" : "0");
        root->add_child("Encrypted")->add_child_text (_encrypted ? "1" : "0");
@@ -430,7 +430,13 @@ Film::read_metadata ()
        } else if ((_audio_channels % 2) == 1) {
                _audio_channels++;
        }
-       _sequence_video = f.bool_child ("SequenceVideo");
+
+       if (f.optional_bool_child("SequenceVideo")) {
+               _sequence = f.bool_child("SequenceVideo");
+       } else {
+               _sequence = f.bool_child("Sequence");
+       }
+
        _three_d = f.bool_child ("ThreeD");
        _interop = f.bool_child ("Interop");
        _key = dcp::Key (f.string_child ("Key"));
@@ -886,8 +892,8 @@ Film::signal_changed (Property p)
                set_video_frame_rate (_playlist->best_dcp_frame_rate ());
                break;
        case Film::VIDEO_FRAME_RATE:
-       case Film::SEQUENCE_VIDEO:
-               _playlist->maybe_sequence_video ();
+       case Film::SEQUENCE:
+               _playlist->maybe_sequence ();
                break;
        default:
                break;
@@ -1040,9 +1046,11 @@ Film::maybe_add_content (weak_ptr<Job> j, weak_ptr<Content> c)
 void
 Film::add_content (shared_ptr<Content> c)
 {
-       /* Add video content after any existing content */
+       /* Add {video,subtitle} content after any existing {video,subtitle} content */
        if (dynamic_pointer_cast<VideoContent> (c)) {
                c->set_position (_playlist->video_end ());
+       } else if (dynamic_pointer_cast<SubtitleContent> (c)) {
+               c->set_position (_playlist->subtitle_end ());
        }
 
        _playlist->add (c);
@@ -1126,11 +1134,11 @@ Film::audio_frame_rate () const
 }
 
 void
-Film::set_sequence_video (bool s)
+Film::set_sequence (bool s)
 {
-       _sequence_video = s;
-       _playlist->set_sequence_video (s);
-       signal_changed (SEQUENCE_VIDEO);
+       _sequence = s;
+       _playlist->set_sequence (s);
+       signal_changed (SEQUENCE);
 }
 
 /** @return Size of the largest possible image in whatever resolution we are using */

diff --git a/src/lib/film.h b/src/lib/film.h
index a33c0238ec956666e85b039d0f91fd1765c54416..b34e4bcfa03883c39359d840e3a7d22cf468b7b7 100644 (file)
--- a/src/lib/film.h
+++ b/src/lib/film.h
@@ -177,7 +177,7 @@ public:
                AUDIO_CHANNELS,
                /** The setting of _three_d has changed */
                THREE_D,
-               SEQUENCE_VIDEO,
+               SEQUENCE,
                INTEROP,
                AUDIO_PROCESSOR,
                REEL_TYPE,
@@ -246,8 +246,8 @@ public:
                return _three_d;
        }
 
-       bool sequence_video () const {
-               return _sequence_video;
+       bool sequence () const {
+               return _sequence;
        }
 
        bool interop () const {
@@ -294,7 +294,7 @@ public:
        void set_audio_channels (int);
        void set_three_d (bool);
        void set_isdcf_date_today ();
-       void set_sequence_video (bool);
+       void set_sequence (bool);
        void set_interop (bool);
        void set_audio_processor (AudioProcessor const * processor);
        void set_reel_type (ReelType);
@@ -358,7 +358,7 @@ private:
            This will be regardless of what content is on the playlist.
        */
        bool _three_d;
-       bool _sequence_video;
+       bool _sequence;
        bool _interop;
        AudioProcessor const * _audio_processor;
        ReelType _reel_type;

diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc
index b5faec5679ed97eb435e59812ca9c8736261e2f5..4c07a0d52fdae49adc318250aed6a09f57d13a72 100644 (file)
--- a/src/lib/playlist.cc
+++ b/src/lib/playlist.cc
@@ -51,8 +51,8 @@ using boost::weak_ptr;
 using boost::dynamic_pointer_cast;
 
 Playlist::Playlist ()
-       : _sequence_video (true)
-       , _sequencing_video (false)
+       : _sequence (true)
+       , _sequencing (false)
 {
 
 }
@@ -72,7 +72,7 @@ Playlist::content_changed (weak_ptr<Content> content, int property, bool frequen
                   - any other position changes will be timeline drags which should not result in content
                   being sequenced.
                */
-               maybe_sequence_video ();
+               maybe_sequence ();
        }
 
        if (
@@ -92,13 +92,15 @@ Playlist::content_changed (weak_ptr<Content> content, int property, bool frequen
 }
 
 void
-Playlist::maybe_sequence_video ()
+Playlist::maybe_sequence ()
 {
-       if (!_sequence_video || _sequencing_video) {
+       if (!_sequence || _sequencing) {
                return;
        }
 
-       _sequencing_video = true;
+       _sequencing = true;
+
+       /* Video */
 
        DCPTime next_left;
        DCPTime next_right;
@@ -117,9 +119,23 @@ Playlist::maybe_sequence_video ()
                }
        }
 
+       /* Subtitles */
+
+       DCPTime next;
+       BOOST_FOREACH (shared_ptr<Content> i, _content) {
+               shared_ptr<SubtitleContent> sc = dynamic_pointer_cast<SubtitleContent> (i);
+               if (!sc) {
+                       continue;
+               }
+
+               sc->set_position (next);
+               next = sc->end();
+       }
+
+
        /* This won't change order, so it does not need a sort */
 
-       _sequencing_video = false;
+       _sequencing = false;
 }
 
 string
@@ -333,6 +349,19 @@ Playlist::video_end () const
        return end;
 }
 
+DCPTime
+Playlist::subtitle_end () const
+{
+       DCPTime end;
+       BOOST_FOREACH (shared_ptr<Content> i, _content) {
+               if (dynamic_pointer_cast<const SubtitleContent> (i)) {
+                       end = max (end, i->end ());
+               }
+       }
+
+       return end;
+}
+
 FrameRateChange
 Playlist::active_frame_rate_change (DCPTime t, int dcp_video_frame_rate) const
 {
@@ -354,9 +383,9 @@ Playlist::active_frame_rate_change (DCPTime t, int dcp_video_frame_rate) const
 }
 
 void
-Playlist::set_sequence_video (bool s)
+Playlist::set_sequence (bool s)
 {
-       _sequence_video = s;
+       _sequence = s;
 }
 
 bool

diff --git a/src/lib/playlist.h b/src/lib/playlist.h
index 3af17bb6c4794cf3114016eedfba312dcab5576b..0ad70b52421ece5984664373b62e0ccc52428cbd 100644 (file)
--- a/src/lib/playlist.h
+++ b/src/lib/playlist.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -63,10 +63,11 @@ public:
 
        int best_dcp_frame_rate () const;
        DCPTime video_end () const;
+       DCPTime subtitle_end () const;
        FrameRateChange active_frame_rate_change (DCPTime, int dcp_frame_rate) const;
 
-       void set_sequence_video (bool);
-       void maybe_sequence_video ();
+       void set_sequence (bool);
+       void maybe_sequence ();
 
        void repeat (ContentList, int);
 
@@ -85,8 +86,8 @@ private:
 
        /** List of content.  Kept sorted in position order. */
        ContentList _content;
-       bool _sequence_video;
-       bool _sequencing_video;
+       bool _sequence;
+       bool _sequencing;
        std::list<boost::signals2::connection> _content_connections;
 };
 

diff --git a/src/wx/content_panel.cc b/src/wx/content_panel.cc
index 59a43fa37827d850e808e7c601f636a899d63ff6..cefb5b0d90dc795f1bc9a09362490baf031033f3 100644 (file)
--- a/src/wx/content_panel.cc
+++ b/src/wx/content_panel.cc
@@ -228,6 +228,15 @@ ContentPanel::film_changed (Film::Property p)
 void
 ContentPanel::selection_changed ()
 {
+       if (_last_selected == selected()) {
+               /* This was triggered by a re-build of the view but the selection
+                  did not really change.
+               */
+               return;
+       }
+
+       _last_selected = selected ();
+
        setup_sensitivity ();
 
        BOOST_FOREACH (ContentSubPanel* i, _panels) {

diff --git a/src/wx/content_panel.h b/src/wx/content_panel.h
index 5f6faa0a36e47a3d269c6a90f178d237813e98cb..e356b5a49ed24fc093c4b15e1b9c73e3ab97715d 100644 (file)
--- a/src/wx/content_panel.h
+++ b/src/wx/content_panel.h
@@ -99,6 +99,7 @@ private:
        ContentMenu* _menu;
        TimelineDialog* _timeline_dialog;
        wxNotebook* _parent;
+       ContentList _last_selected;
 
        boost::shared_ptr<Film> _film;
        FilmViewer* _film_viewer;

diff --git a/src/wx/dcp_panel.cc b/src/wx/dcp_panel.cc
index c46dcba18e4333e5acb6e12061db9a3660aa9deb..c8f1c05634b3d02f1158d024870047dc848b67d8 100644 (file)
--- a/src/wx/dcp_panel.cc
+++ b/src/wx/dcp_panel.cc
@@ -503,7 +503,7 @@ DCPPanel::set_film (shared_ptr<Film> film)
        film_changed (Film::ISDCF_METADATA);
        film_changed (Film::VIDEO_FRAME_RATE);
        film_changed (Film::AUDIO_CHANNELS);
-       film_changed (Film::SEQUENCE_VIDEO);
+       film_changed (Film::SEQUENCE);
        film_changed (Film::THREE_D);
        film_changed (Film::INTEROP);
        film_changed (Film::AUDIO_PROCESSOR);

diff --git a/src/wx/timeline.cc b/src/wx/timeline.cc
index 1516202bcbb286719ff00e96b4b12fffeeb04386..a964ea98daeb507320436991212c100f6e3fcd4f 100644 (file)
--- a/src/wx/timeline.cc
+++ b/src/wx/timeline.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -76,7 +76,7 @@ Timeline::Timeline (wxWindow* parent, ContentPanel* cp, shared_ptr<Film> film)
        SetMinSize (wxSize (640, tracks() * track_height() + 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));
+       _film_content_changed_connection = film->ContentChanged.connect (bind (&Timeline::film_content_changed, this, _2, _3));
 }
 
 void
@@ -103,11 +103,6 @@ Timeline::film_changed (Film::Property p)
                ensure_ui_thread ();
                recreate_views ();
        } else if (p == Film::CONTENT_ORDER) {
-               assign_tracks ();
-               if (!_left_down) {
-                       /* Only do this if we are not dragging, as it's confusing otherwise */
-                       setup_pixels_per_second ();
-               }
                Refresh ();
        }
 }
@@ -146,12 +141,15 @@ Timeline::recreate_views ()
 }
 
 void
-Timeline::film_content_changed (int property)
+Timeline::film_content_changed (int property, bool frequent)
 {
        ensure_ui_thread ();
 
        if (property == AudioContentProperty::AUDIO_STREAMS) {
                recreate_views ();
+       } else if (!frequent) {
+               setup_pixels_per_second ();
+               Refresh ();
        }
 }
 
@@ -311,9 +309,8 @@ Timeline::left_up (wxMouseEvent& ev)
 
        set_position_from_event (ev);
 
-       /* We don't do this during drag, and set_position_from_event above
-          might not have changed the position, so do it now.
-       */
+       /* Clear up up the stuff we don't do during drag */
+       assign_tracks ();
        setup_pixels_per_second ();
        Refresh ();
 
@@ -419,7 +416,7 @@ Timeline::set_position_from_event (wxMouseEvent& ev)
 
        shared_ptr<Film> film = _film.lock ();
        DCPOMATIC_ASSERT (film);
-       film->set_sequence_video (false);
+       film->set_sequence (false);
 }
 
 void

diff --git a/src/wx/timeline.h b/src/wx/timeline.h
index e0e4dfb2a416e4b1668a151606d788ea9d1f0eb8..c0a18ab0dd107065be62902aae3e40458f28771d 100644 (file)
--- a/src/wx/timeline.h
+++ b/src/wx/timeline.h
@@ -83,7 +83,7 @@ private:
        void right_down (wxMouseEvent &);
        void mouse_moved (wxMouseEvent &);
        void film_changed (Film::Property);
-       void film_content_changed (int);
+       void film_content_changed (int, bool frequent);
        void resized ();
        void assign_tracks ();
        void set_position_from_event (wxMouseEvent &);

diff --git a/src/wx/timeline_content_view.cc b/src/wx/timeline_content_view.cc
index 989be3fad8d0d07da5570c1329a84db9684e7e6b..ed7e1d3d04ac4c13abd145f881360b861dc55593 100644 (file)
--- a/src/wx/timeline_content_view.cc
+++ b/src/wx/timeline_content_view.cc
@@ -31,7 +31,7 @@ TimelineContentView::TimelineContentView (Timeline& tl, shared_ptr<Content> c)
        , _content (c)
        , _selected (false)
 {
-       _content_connection = c->Changed.connect (bind (&TimelineContentView::content_changed, this, _2, _3));
+       _content_connection = c->Changed.connect (bind (&TimelineContentView::content_changed, this, _2));
 }
 
 dcpomatic::Rect<int>
@@ -152,16 +152,11 @@ TimelineContentView::y_pos (int t) const
 }
 
 void
-TimelineContentView::content_changed (int p, bool frequent)
+TimelineContentView::content_changed (int p)
 {
        ensure_ui_thread ();
 
        if (p == ContentProperty::POSITION || p == ContentProperty::LENGTH) {
                force_redraw ();
        }
-
-       if (!frequent) {
-               _timeline.setup_pixels_per_second ();
-               _timeline.Refresh ();
-       }
 }

diff --git a/src/wx/timeline_content_view.h b/src/wx/timeline_content_view.h
index 676e4792bbc95687f1ea1f27f1a9a45375f96452..07bfb25759a4e73839e98778597c3d9026dc6209 100644 (file)
--- a/src/wx/timeline_content_view.h
+++ b/src/wx/timeline_content_view.h
@@ -50,7 +50,7 @@ private:
 
        void do_paint (wxGraphicsContext* gc);
        int y_pos (int t) const;
-       void content_changed (int p, bool frequent);
+       void content_changed (int p);
 
        boost::weak_ptr<Content> _content;
        boost::optional<int> _track;

diff --git a/src/wx/timeline_dialog.cc b/src/wx/timeline_dialog.cc
index f054763c864a5676ca11c03fd9db52d7051c9384..ac17cf6dbe28873ff93f6596da01b5020a153c3b 100644 (file)
--- a/src/wx/timeline_dialog.cc
+++ b/src/wx/timeline_dialog.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -40,8 +40,8 @@ TimelineDialog::TimelineDialog (ContentPanel* cp, shared_ptr<Film> film)
        wxBoxSizer* controls = new wxBoxSizer (wxHORIZONTAL);
        _snap = new wxCheckBox (this, wxID_ANY, _("Snap"));
        controls->Add (_snap);
-       _sequence_video = new wxCheckBox (this, wxID_ANY, _("Keep video in sequence"));
-       controls->Add (_sequence_video, 1, wxLEFT, 12);
+       _sequence = new wxCheckBox (this, wxID_ANY, _("Keep video and subtitles in sequence"));
+       controls->Add (_sequence, 1, wxLEFT, 12);
 
        sizer->Add (controls, 0, wxALL, 12);
        sizer->Add (&_timeline, 1, wxEXPAND | wxALL, 12);
@@ -59,8 +59,8 @@ TimelineDialog::TimelineDialog (ContentPanel* cp, shared_ptr<Film> film)
 
        _snap->SetValue (_timeline.snap ());
        _snap->Bind (wxEVT_COMMAND_CHECKBOX_CLICKED, boost::bind (&TimelineDialog::snap_toggled, this));
-       film_changed (Film::SEQUENCE_VIDEO);
-       _sequence_video->Bind (wxEVT_COMMAND_CHECKBOX_CLICKED, boost::bind (&TimelineDialog::sequence_video_toggled, this));
+       film_changed (Film::SEQUENCE);
+       _sequence->Bind (wxEVT_COMMAND_CHECKBOX_CLICKED, boost::bind (&TimelineDialog::sequence_toggled, this));
 
        _film_changed_connection = film->Changed.connect (bind (&TimelineDialog::film_changed, this, _1));
 }
@@ -72,14 +72,14 @@ TimelineDialog::snap_toggled ()
 }
 
 void
-TimelineDialog::sequence_video_toggled ()
+TimelineDialog::sequence_toggled ()
 {
        shared_ptr<Film> film = _film.lock ();
        if (!film) {
                return;
        }
 
-       film->set_sequence_video (_sequence_video->GetValue ());
+       film->set_sequence (_sequence->GetValue ());
 }
 
 void
@@ -90,8 +90,8 @@ TimelineDialog::film_changed (Film::Property p)
                return;
        }
 
-       if (p == Film::SEQUENCE_VIDEO) {
-               _sequence_video->SetValue (film->sequence_video ());
+       if (p == Film::SEQUENCE) {
+               _sequence->SetValue (film->sequence ());
        }
 }
 

diff --git a/src/wx/timeline_dialog.h b/src/wx/timeline_dialog.h
index bc5b7228bc01d087b651aabdb22e1a25efb26fc8..ce7c9113ee3948b33ec7bf5b66d7ae7917b95286 100644 (file)
--- a/src/wx/timeline_dialog.h
+++ b/src/wx/timeline_dialog.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -33,12 +33,12 @@ public:
 
 private:
        void snap_toggled ();
-       void sequence_video_toggled ();
+       void sequence_toggled ();
        void film_changed (Film::Property);
 
        boost::weak_ptr<Film> _film;
        Timeline _timeline;
        wxCheckBox* _snap;
-       wxCheckBox* _sequence_video;
+       wxCheckBox* _sequence;
        boost::signals2::scoped_connection _film_changed_connection;
 };

diff --git a/test/black_fill_test.cc b/test/black_fill_test.cc
index e6f4b69eea64b857cd0477e4ab95ae88832cf730..6da0b037a1164a35923c0c14ee849e0cd6f7232c 100644 (file)
--- a/test/black_fill_test.cc
+++ b/test/black_fill_test.cc
@@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE (black_fill_test)
        film->set_dcp_content_type (DCPContentType::from_isdcf_name ("FTR"));
        film->set_name ("black_fill_test");
        film->set_container (Ratio::from_id ("185"));
-       film->set_sequence_video (false);
+       film->set_sequence (false);
        shared_ptr<ImageContent> contentA (new ImageContent (film, "test/data/simple_testcard_640x480.png"));
        shared_ptr<ImageContent> contentB (new ImageContent (film, "test/data/simple_testcard_640x480.png"));
 
@@ -68,4 +68,3 @@ BOOST_AUTO_TEST_CASE (black_fill_test)
 
        check_dcp (ref.string(), check.string());
 }
-

diff --git a/test/time_calculation_test.cc b/test/time_calculation_test.cc
index f11f0dc28f5213d1dbedc248e8bd31fc6472f421..272d3edfefb951b56ad52039e0813c8a5497f711 100644 (file)
--- a/test/time_calculation_test.cc
+++ b/test/time_calculation_test.cc
@@ -152,7 +152,7 @@ BOOST_AUTO_TEST_CASE (player_time_calculation_test1)
 
        list<string> notes;
        shared_ptr<FFmpegContent> content (new FFmpegContent (film, doc, film->state_version(), notes));
-       film->set_sequence_video (false);
+       film->set_sequence (false);
        film->add_content (content);
 
        shared_ptr<Player> player (new Player (film, film->playlist ()));
@@ -357,7 +357,7 @@ BOOST_AUTO_TEST_CASE (player_time_calculation_test2)
 
        list<string> notes;
        shared_ptr<FFmpegContent> content (new FFmpegContent (film, doc, film->state_version(), notes));
-       film->set_sequence_video (false);
+       film->set_sequence (false);
        film->add_content (content);
 
        shared_ptr<Player> player (new Player (film, film->playlist ()));
@@ -534,7 +534,7 @@ BOOST_AUTO_TEST_CASE (player_time_calculation_test3)
        list<string> notes;
        shared_ptr<FFmpegContent> content (new FFmpegContent (film, doc, film->state_version(), notes));
        AudioStreamPtr stream = content->audio_streams().front();
-       film->set_sequence_video (false);
+       film->set_sequence (false);
        film->add_content (content);
 
        shared_ptr<Player> player (new Player (film, film->playlist ()));