From e1ec5b2c81ec2e15d4c1d97cce8252fa34c7116a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 24 Feb 2016 00:17:26 +0000 Subject: [PATCH] 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. --- src/lib/film.cc | 30 +++++++++++++-------- src/lib/film.h | 10 +++---- src/lib/playlist.cc | 47 ++++++++++++++++++++++++++------- src/lib/playlist.h | 11 ++++---- src/wx/content_panel.cc | 9 +++++++ src/wx/content_panel.h | 1 + src/wx/dcp_panel.cc | 2 +- src/wx/timeline.cc | 21 +++++++-------- src/wx/timeline.h | 2 +- src/wx/timeline_content_view.cc | 9 ++----- src/wx/timeline_content_view.h | 2 +- src/wx/timeline_dialog.cc | 18 ++++++------- src/wx/timeline_dialog.h | 6 ++--- test/black_fill_test.cc | 3 +-- test/time_calculation_test.cc | 6 ++--- 15 files changed, 108 insertions(+), 69 deletions(-) diff --git a/src/lib/film.cc b/src/lib/film.cc index f4d983260..3ea7545df 100644 --- 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 (_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 j, weak_ptr c) void Film::add_content (shared_ptr c) { - /* Add video content after any existing content */ + /* Add {video,subtitle} content after any existing {video,subtitle} content */ if (dynamic_pointer_cast (c)) { c->set_position (_playlist->video_end ()); + } else if (dynamic_pointer_cast (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 a33c0238e..b34e4bcfa 100644 --- 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 b5faec567..4c07a0d52 100644 --- 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, 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, 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 i, _content) { + shared_ptr sc = dynamic_pointer_cast (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 i, _content) { + if (dynamic_pointer_cast (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 3af17bb6c..0ad70b524 100644 --- a/src/lib/playlist.h +++ b/src/lib/playlist.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2013 Carl Hetherington + Copyright (C) 2013-2016 Carl Hetherington 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 _content_connections; }; diff --git a/src/wx/content_panel.cc b/src/wx/content_panel.cc index 59a43fa37..cefb5b0d9 100644 --- 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 5f6faa0a3..e356b5a49 100644 --- 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; FilmViewer* _film_viewer; diff --git a/src/wx/dcp_panel.cc b/src/wx/dcp_panel.cc index c46dcba18..c8f1c0563 100644 --- a/src/wx/dcp_panel.cc +++ b/src/wx/dcp_panel.cc @@ -503,7 +503,7 @@ DCPPanel::set_film (shared_ptr 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 1516202bc..a964ea98d 100644 --- a/src/wx/timeline.cc +++ b/src/wx/timeline.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2015 Carl Hetherington + Copyright (C) 2013-2016 Carl Hetherington 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) 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.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 e0e4dfb2a..c0a18ab0d 100644 --- 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 989be3fad..ed7e1d3d0 100644 --- a/src/wx/timeline_content_view.cc +++ b/src/wx/timeline_content_view.cc @@ -31,7 +31,7 @@ TimelineContentView::TimelineContentView (Timeline& tl, shared_ptr 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 @@ -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 676e4792b..07bfb2575 100644 --- 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; boost::optional _track; diff --git a/src/wx/timeline_dialog.cc b/src/wx/timeline_dialog.cc index f054763c8..ac17cf6db 100644 --- a/src/wx/timeline_dialog.cc +++ b/src/wx/timeline_dialog.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2013 Carl Hetherington + Copyright (C) 2013-2016 Carl Hetherington 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) 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) _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.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 bc5b7228b..ce7c9113e 100644 --- a/src/wx/timeline_dialog.h +++ b/src/wx/timeline_dialog.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2013 Carl Hetherington + Copyright (C) 2013-2016 Carl Hetherington 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; 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 e6f4b69ee..6da0b037a 100644 --- 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 contentA (new ImageContent (film, "test/data/simple_testcard_640x480.png")); shared_ptr 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 f11f0dc28..272d3edfe 100644 --- 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 notes; shared_ptr 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 (new Player (film, film->playlist ())); @@ -357,7 +357,7 @@ BOOST_AUTO_TEST_CASE (player_time_calculation_test2) list notes; shared_ptr 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 (new Player (film, film->playlist ())); @@ -534,7 +534,7 @@ BOOST_AUTO_TEST_CASE (player_time_calculation_test3) list notes; shared_ptr 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 (new Player (film, film->playlist ())); -- 2.30.2