Try harder to keep the playhead at the same *content* frame after

trim (#737).

diff --git a/ChangeLog b/ChangeLog
index 79b4ff37bf336342273cd136dd4d0eb87a88f4bb..f8a96ffa5d20d1cad2d3688941ca085f6163d5d4 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
 2015-11-10  Carl Hetherington  <cth@carlh.net>
 
+       * Further adjustments to playhead behaviour during trim (#737).
+
        * Version 2.5.1 released.
 
 2015-11-10  Carl Hetherington  <cth@carlh.net>

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index e3f76cbc75960ea94ca3f1218c09b29c61b329ed..5ca147b747b932be49c035c51b6e9477bffac12e 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -70,7 +70,8 @@ FilmViewer::FilmViewer (wxWindow* p)
        , _frame_number (new wxStaticText (this, wxID_ANY, wxT("")))
        , _timecode (new wxStaticText (this, wxID_ANY, wxT("")))
        , _play_button (new wxToggleButton (this, wxID_ANY, _("Play")))
-       , _ignore_player_changes (false)
+       , _coalesce_player_changes (false)
+       , _pending_player_change (false)
        , _last_get_accurate (true)
 {
 #ifndef __WXOSX__
@@ -432,7 +433,12 @@ FilmViewer::forward_clicked ()
 void
 FilmViewer::player_changed (bool frequent)
 {
-       if (frequent || _ignore_player_changes) {
+       if (frequent) {
+               return;
+       }
+
+       if (_coalesce_player_changes) {
+               _pending_player_change = true;
                return;
        }
 
@@ -481,7 +487,15 @@ FilmViewer::set_position (DCPTime p)
 }
 
 void
-FilmViewer::set_ignore_player_changes (bool i)
+FilmViewer::set_coalesce_player_changes (bool c)
 {
-       _ignore_player_changes = i;
+       _coalesce_player_changes = c;
+
+       if (c) {
+               _pending_player_change = false;
+       } else {
+               if (_pending_player_change) {
+                       player_changed (false);
+               }
+       }
 }

diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h
index 4c7e3ba33c4d8a606c0a4ca92c1230311ce157b9..6abd3387f6e0ecc3742abe142a9a2f33774b57c1 100644 (file)
--- a/src/wx/film_viewer.h
+++ b/src/wx/film_viewer.h
@@ -45,7 +45,7 @@ public:
        }
 
        void set_position (DCPTime p);
-       void set_ignore_player_changes (bool i);
+       void set_coalesce_player_changes (bool c);
 
        void refresh ();
 
@@ -83,7 +83,8 @@ private:
        wxStaticText* _timecode;
        wxToggleButton* _play_button;
        wxTimer _timer;
-       bool _ignore_player_changes;
+       bool _coalesce_player_changes;
+       bool _pending_player_change;
 
        boost::shared_ptr<const Image> _frame;
        DCPTime _position;

diff --git a/src/wx/timing_panel.cc b/src/wx/timing_panel.cc
index 9f5aa78ead191be6e0434205d20e0834fdb3a99d..c7567455b762c393d86624aca6df8a4fe136d191 100644 (file)
--- a/src/wx/timing_panel.cc
+++ b/src/wx/timing_panel.cc
@@ -324,17 +324,49 @@ TimingPanel::full_length_changed ()
 void
 TimingPanel::trim_start_changed ()
 {
+       DCPTime const ph = _viewer->position ();
+
+       _viewer->set_coalesce_player_changes (true);
+
+       shared_ptr<Content> ref;
+       optional<FrameRateChange> ref_frc;
+       optional<DCPTime> ref_ph;
        BOOST_FOREACH (shared_ptr<Content> i, _parent->selected ()) {
+               if (i->position() <= ph && ph < i->end()) {
+                       /* The playhead is in i.  Use it as a reference to work out
+                          where to put the playhead post-trim; we're trying to keep the playhead
+                          at the same frame of content that we're looking at pre-trim.
+                       */
+                       ref = i;
+                       ref_frc = _parent->film()->active_frame_rate_change (i->position ());
+                       ref_ph = ph - i->position() + DCPTime (i->trim_start(), ref_frc.get());
+               }
+
                i->set_trim_start (_trim_start->get (_parent->film()->video_frame_rate ()));
        }
+
+       if (ref) {
+               _viewer->set_position (max (DCPTime(), ref_ph.get() + ref->position() - DCPTime (ref->trim_start(), ref_frc.get())));
+       }
+
+       _viewer->set_coalesce_player_changes (false);
 }
 
 void
 TimingPanel::trim_end_changed ()
 {
+       _viewer->set_coalesce_player_changes (true);
+
        BOOST_FOREACH (shared_ptr<Content> i, _parent->selected ()) {
                i->set_trim_end (_trim_end->get (_parent->film()->video_frame_rate ()));
        }
+
+       /* XXX: maybe playhead-off-the-end-of-the-film should be handled elsewhere */
+       if (_viewer->position() >= _parent->film()->length()) {
+               _viewer->set_position (_parent->film()->length() - DCPTime::from_frames (1, _parent->film()->video_frame_rate()));
+       }
+
+       _viewer->set_coalesce_player_changes (true);
 }
 
 void
@@ -401,7 +433,7 @@ TimingPanel::trim_start_to_playhead_clicked ()
        DCPTime const ph = _viewer->position ();
        optional<DCPTime> new_ph;
 
-       _viewer->set_ignore_player_changes (true);
+       _viewer->set_coalesce_player_changes (true);
 
        BOOST_FOREACH (shared_ptr<Content> i, _parent->selected ()) {
                if (i->position() < ph && ph < i->end ()) {
@@ -411,11 +443,11 @@ TimingPanel::trim_start_to_playhead_clicked ()
                }
        }
 
-       _viewer->set_ignore_player_changes (false);
-
        if (new_ph) {
                _viewer->set_position (new_ph.get());
        }
+
+       _viewer->set_coalesce_player_changes (false);
 }
 
 void