Add some tests and hopefully clarify the DCPFrameRate class.

5 files changed, 108 insertions, 32 deletions

diff --git a/src/lib/config.cc b/src/lib/config.cc
index 65d01bf00..307b96844 100644
--- a/src/lib/config.cc
+++ b/src/lib/config.cc
@@ -44,6 +44,10 @@ Config::Config ()
 	, _tms_path (".")
 	, _sound_processor (SoundProcessor::from_id ("dolby_cp750"))
 {
+	_allowed_dcp_frame_rates.push_back (24);
+	_allowed_dcp_frame_rates.push_back (25);
+	_allowed_dcp_frame_rates.push_back (30);
+	
 	ifstream f (file().c_str ());
 	string line;
 	while (getline (f, line)) {
diff --git a/src/lib/config.h b/src/lib/config.h
index c84ce76b5..c41437efb 100644
--- a/src/lib/config.h
+++ b/src/lib/config.h
@@ -94,6 +94,10 @@ public:
 		return _sound_processor;
 	}
 
+	std::list<int> allowed_dcp_frame_rates () const {
+		return _allowed_dcp_frame_rates;
+	}
+	
 	/** @param n New number of local encoding threads */
 	void set_num_local_encoding_threads (int n) {
 		_num_local_encoding_threads = n;
@@ -140,7 +144,11 @@ public:
 	void set_tms_password (std::string p) {
 		_tms_password = p;
 	}
-	
+
+	void set_allowed_dcp_frame_rates (std::list<int> const & r) {
+		_allowed_dcp_frame_rates = r;
+	}
+
 	void write () const;
 
 	static Config* instance ();
@@ -172,6 +180,7 @@ private:
 	std::string _tms_password;
 	/** Our sound processor */
 	SoundProcessor const * _sound_processor;
+	std::list<int> _allowed_dcp_frame_rates;
 
 	/** Singleton instance, or 0 */
 	static Config* _instance;
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 17e14e544..83c60a5f5 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -255,7 +255,9 @@ Film::make_dcp (bool transcode)
 	}
 	
 	log()->log (String::compose ("Content is %1; type %2", content_path(), (content_type() == STILL ? "still" : "video")));
-	log()->log (String::compose ("Content length %1", length().get()));
+	if (length()) {
+		log()->log (String::compose ("Content length %1", length().get()));
+	}
 	log()->log (String::compose ("Content digest %1", content_digest()));
 	log()->log (String::compose ("%1 threads", Config::instance()->num_local_encoding_threads()));
 	log()->log (String::compose ("J2K bandwidth %1", j2k_bandwidth()));
@@ -707,13 +709,14 @@ Film::target_audio_sample_rate () const
 
 	DCPFrameRate dfr (frames_per_second ());
 
-	/* Compensate for the fact that video will be rounded to the
-	   nearest integer number of frames per second.
+	/* Compensate if the DCP is being run at a different frame rate
+	   to the source; that is, if the video is run such that it will
+	   look different in the DCP compared to the source (slower or faster).
+	   skip/repeat doesn't come into effect here.
 	*/
 
-	int const mult = dfr.skip ? 2 : 1;
-	if (dfr.run_fast) {
-		t *= _frames_per_second * mult / dfr.frames_per_second;
+	if (dfr.change_speed) {
+		t *= _frames_per_second * dfr.factor() / dfr.frames_per_second;
 	}
 
 	return rint (t);
diff --git a/src/lib/util.cc b/src/lib/util.cc
index 4228ce6cf..b500ddc2f 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -26,6 +26,7 @@
 #include <iomanip>
 #include <iostream>
 #include <fstream>
+#include <climits>
 #ifdef DVDOMATIC_POSIX
 #include <execinfo.h>
 #include <cxxabi.h>
@@ -58,6 +59,7 @@ extern "C" {
 #include "dcp_content_type.h"
 #include "filter.h"
 #include "sound_processor.h"
+#include "config.h"
 
 using namespace std;
 using namespace boost;
@@ -356,29 +358,74 @@ static bool about_equal (float a, float b)
 	return (fabs (a - b) < 1e-4);
 }
 
+class FrameRateCandidate
+{
+public:
+	FrameRateCandidate (float source_, int dcp_)
+		: source (source_)
+		, dcp (dcp_)
+	{}
+
+	bool skip () const {
+		return !about_equal (source, dcp) && source > dcp;
+	}
+
+	bool repeat () const {
+		return !about_equal (source, dcp) && source < dcp;
+	}
+
+	float source;
+	int dcp;
+};
+
 /** @param fps Arbitrary source frames-per-second value */
-DCPFrameRate::DCPFrameRate (float fps)
-	: frames_per_second (rint (fps))
-	, skip (false)
-	, repeat (false)
-	, run_fast (false)
-{
-	if (about_equal (fps, 50)) {
-		/* XXX: not sure about this; just run at 50?
-		   Ring Peter Jackson.
-		*/
-		frames_per_second = 25;
-		skip = true;
-	} else if (fps >= (27.5 / 2) && fps <= (32.5 / 2)) {
-		frames_per_second = 30;
-		repeat = true;
-	} else if (fps >= (24.5 / 2) && fps <= (27.5 / 2)) {
-		frames_per_second = 25;
-		repeat = true;
-	} else if (fps >= (20 / 2) && fps <= (24.5 / 2)) {
-		frames_per_second = 24;
-		repeat = true;
+/** XXX: this could be slow-ish */
+DCPFrameRate::DCPFrameRate (float source_fps)
+{
+	list<int> const allowed_dcp_frame_rates = Config::instance()->allowed_dcp_frame_rates ();
+
+	/* Work out what rates we could manage, including those achieved by using skip / repeat. */
+	list<FrameRateCandidate> candidates;
+
+	/* Start with the ones without skip / repeat so they will get matched in preference to skipped/repeated ones */
+	for (list<int>::const_iterator i = allowed_dcp_frame_rates.begin(); i != allowed_dcp_frame_rates.end(); ++i) {
+		candidates.push_back (FrameRateCandidate (*i, *i));
 	}
+
+	/* Then the skip/repeat ones */
+	for (list<int>::const_iterator i = allowed_dcp_frame_rates.begin(); i != allowed_dcp_frame_rates.end(); ++i) {
+		candidates.push_back (FrameRateCandidate (float (*i) / 2, *i));
+		candidates.push_back (FrameRateCandidate (float (*i) * 2, *i));
+	}
+
+	/* Pick the best one, bailing early if we hit an exact match */
+	float error = numeric_limits<float>::max ();
+	boost::optional<FrameRateCandidate> best;
+	list<FrameRateCandidate>::iterator i = candidates.begin();
+	while (i != candidates.end()) {
+		
+		if (about_equal (i->source, source_fps)) {
+			best = *i;
+			break;
+		}
+
+		float const e = fabs (i->source - source_fps);
+		if (e < error) {
+			error = e;
+			best = *i;
+		}
+
+		++i;
+	}
+
+	if (!best) {
+		throw EncodeError ("cannot find a suitable DCP frame rate for this source");
+	}
+
+	frames_per_second = best->dcp;
+	skip = best->skip ();
+	repeat = best->repeat ();
+	change_speed = !about_equal (source_fps * factor(), frames_per_second);
 }
 
 /** @param An arbitrary sampling rate.
diff --git a/src/lib/util.h b/src/lib/util.h
index 50e7410d6..1fd2c0150 100644
--- a/src/lib/util.h
+++ b/src/lib/util.h
@@ -62,6 +62,19 @@ typedef int SourceFrame;
 struct DCPFrameRate
 {
 	DCPFrameRate (float);
+
+	/** @return factor by which to multiply a source frame rate
+	    to get the effective rate after any skip or repeat has happened.
+	*/
+	float factor () const {
+		if (skip) {
+			return 0.5;
+		} else if (repeat) {
+			return 2;
+		}
+
+		return 1;
+	}
 	
 	/** frames per second for the DCP */
 	int frames_per_second;
@@ -69,16 +82,16 @@ struct DCPFrameRate
 	bool skip;
 	/** true to repeat every frame once */
 	bool repeat;
-	/** true if this DCP will run its video faster than the source
-	 *  without taking into account `skip' and `repeat'.
-	 *  (i.e. run_fast will be true if
+	/** true if this DCP will run its video faster or slower than the source
+	 *  without taking into account `repeat'.
+	 *  (e.g. change_speed will be true if
 	 *          source is 29.97fps, DCP is 30fps
 	 *          source is 14.50fps, DCP is 30fps
 	 *  but not if
 	 *          source is 15.00fps, DCP is 30fps
 	 *          source is 12.50fps, DCP is 25fps)
 	 */
-	bool run_fast;
+	bool change_speed;
 };
 
 enum ContentType {