From 8cc825a798d9a457b89f968a57d3b6a16f6cedfe Mon Sep 17 00:00:00 2001
From: Carl Hetherington <cth@carlh.net>
Date: Tue, 10 Nov 2015 23:26:41 +0000
Subject: [PATCH] Try harder to keep the playhead at the same *content* frame
 after trim (#737).

---
 ChangeLog              |  2 ++
 src/wx/film_viewer.cc  | 22 ++++++++++++++++++----
 src/wx/film_viewer.h   |  5 +++--
 src/wx/timing_panel.cc | 38 +++++++++++++++++++++++++++++++++++---
 4 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 79b4ff37b..f8a96ffa5 100644
--- 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 e3f76cbc7..5ca147b74 100644
--- 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 4c7e3ba33..6abd3387f 100644
--- 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 9f5aa78ea..c7567455b 100644
--- 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
-- 
2.30.2