Decide best DCP rate based on the largest difference between a particular content's frame rate and the DCP one, rather than a sum of all the differences.

3 files changed, 34 insertions, 5 deletions

diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc
index 9e7f7f5f5..703a14663 100644
--- a/src/lib/playlist.cc
+++ b/src/lib/playlist.cc
@@ -221,7 +221,7 @@ Playlist::best_dcp_frame_rate () const
 		candidates.push_back (FrameRateCandidate (float (*i) * 2, *i));
 	}
 
-	/* Pick the best one, bailing early if we hit an exact match */
+	/* Pick the best one */
 	float error = std::numeric_limits<float>::max ();
 	optional<FrameRateCandidate> best;
 	list<FrameRateCandidate>::iterator i = candidates.begin();
@@ -234,7 +234,8 @@ Playlist::best_dcp_frame_rate () const
 				continue;
 			}
 
-			this_error += fabs (i->source - vc->video_frame_rate ());
+			/* Use the largest difference between DCP and source as the "error" */
+			this_error = max (this_error, float (fabs (i->source - vc->video_frame_rate ())));
 		}
 
 		if (this_error < error) {
diff --git a/src/lib/video_content.h b/src/lib/video_content.h
index 5b9a17b57..697a0ecc3 100644
--- a/src/lib/video_content.h
+++ b/src/lib/video_content.h
@@ -88,7 +88,8 @@ protected:
 
 private:
 	friend class ffmpeg_pts_offset_test;
-	friend class best_dcp_frame_rate_test;
+	friend class best_dcp_frame_rate_test_single;
+	friend class best_dcp_frame_rate_test_double;
 	friend class audio_sampling_rate_test;
 	
 	libdcp::Size _video_size;
diff --git a/test/frame_rate_test.cc b/test/frame_rate_test.cc
index b7e8a3ce4..89a74e086 100644
--- a/test/frame_rate_test.cc
+++ b/test/frame_rate_test.cc
@@ -20,9 +20,9 @@
 /* Test Playlist::best_dcp_frame_rate and FrameRateConversion
    with a single piece of content.
 */
-BOOST_AUTO_TEST_CASE (best_dcp_frame_rate_test)
+BOOST_AUTO_TEST_CASE (best_dcp_frame_rate_test_single)
 {
-	shared_ptr<Film> film = new_test_film ("best_dcp_frame_rate_test");
+	shared_ptr<Film> film = new_test_film ("best_dcp_frame_rate_test_single");
 	/* Get any piece of content, it doesn't matter what */
 	shared_ptr<FFmpegContent> content (new FFmpegContent (film, "test/data/test.mp4"));
 	film->add_content (content);
@@ -179,6 +179,33 @@ BOOST_AUTO_TEST_CASE (best_dcp_frame_rate_test)
 	BOOST_CHECK_EQUAL (frc.change_speed, true);
 }
 
+/* Test Playlist::best_dcp_frame_rate and FrameRateConversion
+   with two pieces of content.
+*/
+BOOST_AUTO_TEST_CASE (best_dcp_frame_rate_test_double)
+{
+	shared_ptr<Film> film = new_test_film ("best_dcp_frame_rate_test_double");
+	/* Get any old content, it doesn't matter what */
+	shared_ptr<FFmpegContent> A (new FFmpegContent (film, "test/data/test.mp4"));
+	film->add_content (A);
+	shared_ptr<FFmpegContent> B (new FFmpegContent (film, "test/data/test.mp4"));
+	film->add_content (B);
+	wait_for_jobs ();
+
+	/* Run some tests with a limited range of allowed rates */
+	
+	std::list<int> afr;
+	afr.push_back (24);
+	afr.push_back (25);
+	afr.push_back (30);
+	Config::instance()->set_allowed_dcp_frame_rates (afr);
+
+	A->_video_frame_rate = 30;
+	B->_video_frame_rate = 24;
+	BOOST_CHECK_EQUAL (film->playlist()->best_dcp_frame_rate(), 25);
+}
+
+
 BOOST_AUTO_TEST_CASE (audio_sampling_rate_test)
 {
 	shared_ptr<Film> film = new_test_film ("audio_sampling_rate_test");