Support skipping of multiple frames (so e.g. we can handle 120fps sources).

5 files changed, 48 insertions, 24 deletions

diff --git a/src/lib/frame_rate_change.cc b/src/lib/frame_rate_change.cc
index d4e51d669..6057f9ca3 100644
--- a/src/lib/frame_rate_change.cc
+++ b/src/lib/frame_rate_change.cc
@@ -46,7 +46,7 @@ FrameRateChange::FrameRateChange(double source, int dcp)
 		/* The difference between source and DCP frame rate will be lower
 		   (i.e. better) if we skip.
 		*/
-		_skip = true;
+		_skip = round(source / dcp) - 1;
 	} else if (fabs(_source * 2 - _dcp) < fabs(_source - _dcp)) {
 		/* The difference between source and DCP frame rate would be better
 		   if we repeated each frame once; it may be better still if we
@@ -84,11 +84,13 @@ FrameRateChange::description() const
 {
 	string description;
 
-	if (!_skip && _repeat == 1 && !_change_speed) {
+	if (_skip == 0 && _repeat == 1 && !_change_speed) {
 		description = _("Content and DCP have the same rate.\n");
 	} else {
-		if (_skip) {
+		if (_skip == 1) {
 			description = _("DCP will use every other frame of the content.\n");
+		} else if (_skip >= 2) {
+			description = fmt::format(_("DCP will contain 1 out of every {} frames of the content.\n"), _skip + 1);
 		} else if (_repeat == 2) {
 			description = _("Each content frame will be doubled in the DCP.\n");
 		} else if (_repeat > 2) {
diff --git a/src/lib/frame_rate_change.h b/src/lib/frame_rate_change.h
index 478cadeee..b58e499d8 100644
--- a/src/lib/frame_rate_change.h
+++ b/src/lib/frame_rate_change.h
@@ -43,8 +43,8 @@ public:
 	    to get the effective rate after any skip or repeat has happened.
 	*/
 	double factor() const {
-		if (_skip) {
-			return 0.5;
+		if (_skip > 0) {
+			return 1.0 / (_skip + 1);
 		}
 
 		return _repeat;
@@ -52,7 +52,7 @@ public:
 
 	std::string description() const;
 
-	bool skip() const {
+	int skip() const {
 		return _skip;
 	}
 
@@ -80,8 +80,10 @@ private:
 	double _source = 24;
 	int _dcp = 24;
 
-	/** true to skip every other frame */
-	bool _skip = false;
+	/** Frames to skip between each one to use, e.g.
+	 *  0 to skip no frames, 1 to skip every other one, 2 to skip 2 out of 3, etc.
+	 */
+	int _skip = 0;
 	/** number of times to use each frame (e.g. 1 is normal, 2 means repeat each frame once, and so on) */
 	int _repeat = 1;
 	/** true if this DCP will run its video faster or slower than the source
diff --git a/test/data b/test/data
-Subproject f93feeca2c9f44819a7e50f98262dc960f8d7e9
+Subproject 1b8cc12baa974939e806d70115a368b49d778af
diff --git a/test/frame_rate_test.cc b/test/frame_rate_test.cc
index 4cb78e76d..74f372c95 100644
--- a/test/frame_rate_test.cc
+++ b/test/frame_rate_test.cc
@@ -59,7 +59,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	int best = film->best_video_frame_rate();
 	auto frc = FrameRateChange(60, best);
 	BOOST_CHECK_EQUAL(best, 30);
-	BOOST_CHECK_EQUAL(frc.skip(), true);
+	BOOST_CHECK_EQUAL(frc.skip(), 1);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(50, best);
 	BOOST_CHECK_EQUAL(best, 25);
-	BOOST_CHECK_EQUAL(frc.skip(), true);
+	BOOST_CHECK_EQUAL(frc.skip(), 1);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(48, best);
 	BOOST_CHECK_EQUAL(best, 24);
-	BOOST_CHECK_EQUAL(frc.skip(), true);
+	BOOST_CHECK_EQUAL(frc.skip(), 1);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(30, best);
 	BOOST_CHECK_EQUAL(best, 30);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(29.97, best);
 	BOOST_CHECK_EQUAL(best, 30);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), true);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 30 / 29.97, 0.1);
@@ -104,7 +104,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(25, best);
 	BOOST_CHECK_EQUAL(best, 25);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -113,7 +113,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(24, best);
 	BOOST_CHECK_EQUAL(best, 24);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -122,7 +122,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(14.5, best);
 	BOOST_CHECK_EQUAL(best, 30);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 2);
 	BOOST_CHECK_EQUAL(frc.change_speed(), true);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 15 / 14.5, 0.1);
@@ -131,7 +131,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(12.6, best);
 	BOOST_CHECK_EQUAL(best, 25);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 2);
 	BOOST_CHECK_EQUAL(frc.change_speed(), true);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 25 / 25.2, 0.1);
@@ -140,7 +140,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(12.4, best);
 	BOOST_CHECK_EQUAL(best, 25);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 2);
 	BOOST_CHECK_EQUAL(frc.change_speed(), true);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 25 / 24.8, 0.1);
@@ -149,11 +149,20 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(12, best);
 	BOOST_CHECK_EQUAL(best, 24);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 2);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
 
+	content->_video_frame_rate = 120;
+	best = film->best_video_frame_rate();
+	frc = FrameRateChange(120, best);
+	BOOST_CHECK_EQUAL(best, 30);
+	BOOST_CHECK_EQUAL(frc.skip(), 3);
+	BOOST_CHECK_EQUAL(frc.repeat(), 1);
+	BOOST_CHECK_EQUAL(frc.change_speed(), false);
+	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
+
 	/* Now add some more rates and see if it will use them
 	   in preference to skip/repeat.
 	*/
@@ -167,7 +176,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->playlist()->best_video_frame_rate();
 	frc = FrameRateChange(60, best);
 	BOOST_CHECK_EQUAL(best, 60);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -176,7 +185,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->playlist()->best_video_frame_rate();
 	frc = FrameRateChange(50, best);
 	BOOST_CHECK_EQUAL(best, 50);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -185,7 +194,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->playlist()->best_video_frame_rate();
 	frc = FrameRateChange(48, best);
 	BOOST_CHECK_EQUAL(best, 48);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), false);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 1, 0.1);
@@ -193,7 +202,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	/* Check some out-there conversions (not the best) */
 
 	frc = FrameRateChange(14.99, 24);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 2);
 	BOOST_CHECK_EQUAL(frc.change_speed(), true);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 24 / (2 * 14.99), 0.1);
@@ -208,7 +217,7 @@ BOOST_AUTO_TEST_CASE(best_dcp_frame_rate_test_single)
 	best = film->best_video_frame_rate();
 	frc = FrameRateChange(25, best);
 	BOOST_CHECK_EQUAL(best, 24);
-	BOOST_CHECK_EQUAL(frc.skip(), false);
+	BOOST_CHECK_EQUAL(frc.skip(), 0);
 	BOOST_CHECK_EQUAL(frc.repeat(), 1);
 	BOOST_CHECK_EQUAL(frc.change_speed(), true);
 	BOOST_CHECK_CLOSE(frc.speed_up(), 24.0 / 25, 0.1);
diff --git a/test/hints_test.cc b/test/hints_test.cc
index 55bd9d77f..073415018 100644
--- a/test/hints_test.cc
+++ b/test/hints_test.cc
@@ -315,3 +315,14 @@ BOOST_AUTO_TEST_CASE(hints_mpeg2)
 		"encoded with JPEG2000 rather than MPEG2.  Make sure that your cinema really wants an old-style MPEG2 DCP."
 		);
 }
+
+
+BOOST_AUTO_TEST_CASE(hints_120fps)
+{
+	auto content = content_factory("test/data/numbered_120.mp4");
+	auto film = new_test_film("hints_120fps", content);
+	auto hints = get_hints(film);
+	for (auto hint: hints) {
+		BOOST_CHECK(hint.find("There is a large difference between the frame rate of your DCP and that of some of your content.") == std::string::npos);
+	}
+}