Add some tests and hopefully clarify the DCPFrameRate class.

diff --git a/src/lib/config.cc b/src/lib/config.cc
index 65d01bf00b64654e0e96762d6fb0f43f43e2aec6..307b9684414359ffb823a937f3febc515711db84 100644 (file)
--- 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 c84ce76b514bf64cf6e370c8231871857832c534..c41437efbe290f6aacad37ec5387fd21dc4d99db 100644 (file)
--- 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 17e14e544339e3b4546f73657f00a98173e18f80..83c60a5f5308915c41919a762af9577fcaa1e367 100644 (file)
--- 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 4228ce6cfef66eccbcd1f4bc0c8f681b58734993..b500ddc2f9ae4068cd2f06a4da26cbabd6308822 100644 (file)
--- 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 50e7410d67fb2047935b4aea47fe94cc92858c01..1fd2c0150e3457391b296f54be5056d41bc090c6 100644 (file)
--- 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 {

diff --git a/test/test.cc b/test/test.cc
index c393aac5e4d485c5a01e59e6410335db9ef7c013..45a80f0243570922581436cb045deda2cbd5fd65 100644 (file)
--- a/test/test.cc
+++ b/test/test.cc
@@ -429,6 +429,111 @@ BOOST_AUTO_TEST_CASE (make_dcp_with_range_test)
        BOOST_CHECK_EQUAL (JobManager::instance()->errors(), false);
 }
 
+/* Test the constructor of DCPFrameRate */
+BOOST_AUTO_TEST_CASE (dcp_frame_rate_test)
+{
+       /* 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);
+
+       DCPFrameRate dfr = DCPFrameRate (60);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 30);
+       BOOST_CHECK_EQUAL (dfr.skip, true);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+       
+       dfr = DCPFrameRate (50);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 25);
+       BOOST_CHECK_EQUAL (dfr.skip, true);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+
+       dfr = DCPFrameRate (48);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 24);
+       BOOST_CHECK_EQUAL (dfr.skip, true);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+       
+       dfr = DCPFrameRate (30);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 30);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+
+       dfr = DCPFrameRate (29.97);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 30);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, true);
+       
+       dfr = DCPFrameRate (25);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 25);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+
+       dfr = DCPFrameRate (24);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 24);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+
+       dfr = DCPFrameRate (14.5);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 30);
+       BOOST_CHECK_EQUAL (dfr.repeat, true);
+       BOOST_CHECK_EQUAL (dfr.change_speed, true);
+
+       dfr = DCPFrameRate (12.6);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 25);
+       BOOST_CHECK_EQUAL (dfr.repeat, true);
+       BOOST_CHECK_EQUAL (dfr.change_speed, true);
+
+       dfr = DCPFrameRate (12.4);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 25);
+       BOOST_CHECK_EQUAL (dfr.repeat, true);
+       BOOST_CHECK_EQUAL (dfr.change_speed, true);
+
+       dfr = DCPFrameRate (12);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 24);
+       BOOST_CHECK_EQUAL (dfr.repeat, true);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+
+       /* Now add some more rates and see if it will use them
+          in preference to skip/repeat.
+       */
+
+       afr.push_back (48);
+       afr.push_back (50);
+       afr.push_back (60);
+       Config::instance()->set_allowed_dcp_frame_rates (afr);
+
+       dfr = DCPFrameRate (60);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 60);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+       
+       dfr = DCPFrameRate (50);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 50);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+
+       dfr = DCPFrameRate (48);
+       BOOST_CHECK_EQUAL (dfr.frames_per_second, 48);
+       BOOST_CHECK_EQUAL (dfr.skip, false);
+       BOOST_CHECK_EQUAL (dfr.repeat, false);
+       BOOST_CHECK_EQUAL (dfr.change_speed, false);
+}
+
 BOOST_AUTO_TEST_CASE (audio_sampling_rate_test)
 {
        shared_ptr<Film> f = new_test_film ("audio_sampling_rate_test");