Fix non-frame-aligned trims when using trim-to-playhead.

Reimplement Time::ceil and add a corresponding Time::floor with tests.
ceil returns slightly different results to previously with non-integer
frame rates.

Then use floor to round the playhead position when trimming.

diff --git a/ChangeLog b/ChangeLog
index efc8f7d5ccd751b9d8088e06abed1929e9cdcf73..3b631c2af7c3588de9925844a41ab6bd6c0ed74c 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2017-01-04  Carl Hetherington  <cth@carlh.net>
+
+       * Fix non frame-aligned trims when using the to-playhead
+       buttons.
+
 2016-12-25  Carl Hetherington  <cth@carlh.net>
 
        * Updated fr_FR translation from Thierry Journet.

diff --git a/src/lib/dcpomatic_time.h b/src/lib/dcpomatic_time.h
index 6834ee099616a94c273175fa8323378f8372fd14..16d93ca28bc83abc01c8273c6ece6cd092b42359 100644 (file)
--- a/src/lib/dcpomatic_time.h
+++ b/src/lib/dcpomatic_time.h
@@ -119,9 +119,11 @@ public:
         *  @param r Sampling rate.
         */
        Time<S, O> ceil (float r) const {
-               Type const n = llrintf (HZ / r);
-               Type const a = _t + n - 1;
-               return Time<S, O> (a - (a % n));
+               return Time<S, O> (llrint (HZ * frames_ceil(r) / double(r)));
+       }
+
+       Time<S, O> floor (float r) const {
+               return Time<S, O> (llrint (HZ * frames_floor(r) / double(r)));
        }
 
        double seconds () const {
@@ -143,7 +145,7 @@ public:
 
        template <typename T>
        int64_t frames_floor (T r) const {
-               return floor (_t * r / HZ);
+               return ::floor (_t * r / HZ);
        }
 
        template <typename T>
@@ -212,6 +214,7 @@ public:
 
 private:
        friend struct dcptime_ceil_test;
+       friend struct dcptime_floor_test;
 
        Type _t;
        static const int HZ = 96000;

diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h
index a67820a8ca21229e2b7ef8f40dfaaba7cf617596..aa588b92607d9a3922c1d7a03f70a19a348d603b 100644 (file)
--- a/src/wx/film_viewer.h
+++ b/src/wx/film_viewer.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2017 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -41,6 +41,7 @@ public:
 
        void set_film (boost::shared_ptr<Film>);
 
+       /** @return our `playhead' position; this may not lie exactly on a frame boundary */
        DCPTime position () const {
                return _position;
        }

diff --git a/src/wx/timing_panel.cc b/src/wx/timing_panel.cc
index bd4177ffbaaae51f21926b9c3ebaedd31be44dfd..d7ed46f467eca85192d6b6c355cf375830a72d9b 100644 (file)
--- a/src/wx/timing_panel.cc
+++ b/src/wx/timing_panel.cc
@@ -441,14 +441,15 @@ TimingPanel::film_changed (Film::Property p)
 void
 TimingPanel::trim_start_to_playhead_clicked ()
 {
-       DCPTime const ph = _viewer->position ();
+       shared_ptr<const Film> film = _parent->film ();
+       DCPTime const ph = _viewer->position().floor (film->video_frame_rate ());
        optional<DCPTime> new_ph;
 
        _viewer->set_coalesce_player_changes (true);
 
        BOOST_FOREACH (shared_ptr<Content> i, _parent->selected ()) {
                if (i->position() < ph && ph < i->end ()) {
-                       FrameRateChange const frc = _parent->film()->active_frame_rate_change (i->position ());
+                       FrameRateChange const frc = film->active_frame_rate_change (i->position ());
                        i->set_trim_start (i->trim_start() + ContentTime (ph - i->position (), frc));
                        new_ph = i->position ();
                }
@@ -464,10 +465,11 @@ TimingPanel::trim_start_to_playhead_clicked ()
 void
 TimingPanel::trim_end_to_playhead_clicked ()
 {
-       DCPTime const ph = _viewer->position ();
+       shared_ptr<const Film> film = _parent->film ();
+       DCPTime const ph = _viewer->position().floor (film->video_frame_rate ());
        BOOST_FOREACH (shared_ptr<Content> i, _parent->selected ()) {
                if (i->position() < ph && ph < i->end ()) {
-                       FrameRateChange const frc = _parent->film()->active_frame_rate_change (i->position ());
+                       FrameRateChange const frc = film->active_frame_rate_change (i->position ());
                        i->set_trim_end (ContentTime (i->position() + i->full_length() - ph - DCPTime::from_frames (1, frc.dcp), frc) - i->trim_start());
                }
        }

diff --git a/test/util_test.cc b/test/util_test.cc
index c7bf9944d926e90fe037c58f89340a4daf44789e..334811cb7a9db4caa0219513d32b7ec007c8e129 100644 (file)
--- a/test/util_test.cc
+++ b/test/util_test.cc
@@ -65,7 +65,24 @@ BOOST_AUTO_TEST_CASE (dcptime_ceil_test)
        BOOST_CHECK_EQUAL (DCPTime(43).ceil(DCPTime::HZ / 42).get(), 84);
 
        /* Check that rounding up to non-integer frame rates works */
-       BOOST_CHECK_EQUAL (DCPTime(45312).ceil(29.976).get(), 48045);
+       BOOST_CHECK_EQUAL (DCPTime(45312).ceil(29.976).get(), 48038);
+}
+
+/* Straightforward test of DCPTime::floor */
+BOOST_AUTO_TEST_CASE (dcptime_floor_test)
+{
+       BOOST_CHECK_EQUAL (DCPTime(0).floor(DCPTime::HZ / 2).get(), 0);
+       BOOST_CHECK_EQUAL (DCPTime(1).floor(DCPTime::HZ / 2).get(), 0);
+       BOOST_CHECK_EQUAL (DCPTime(2).floor(DCPTime::HZ / 2).get(), 2);
+       BOOST_CHECK_EQUAL (DCPTime(3).floor(DCPTime::HZ / 2).get(), 2);
+
+       BOOST_CHECK_EQUAL (DCPTime(0).floor(DCPTime::HZ / 42).get(), 0);
+       BOOST_CHECK_EQUAL (DCPTime(1).floor(DCPTime::HZ / 42).get(), 0);
+       BOOST_CHECK_EQUAL (DCPTime(42).floor(DCPTime::HZ / 42.0).get(), 42);
+       BOOST_CHECK_EQUAL (DCPTime(43).floor(DCPTime::HZ / 42.0).get(), 42);
+
+       /* Check that rounding down to non-integer frame rates works */
+       BOOST_CHECK_EQUAL (DCPTime(45312).floor(29.976).get(), 44836);
 }
 
 BOOST_AUTO_TEST_CASE (timecode_test)