Make film hold its DCP frame rate.

12 files changed, 91 insertions, 61 deletions

diff --git a/src/lib/ab_transcoder.cc b/src/lib/ab_transcoder.cc
index f47a99fda..4ed5d02ca 100644
--- a/src/lib/ab_transcoder.cc
+++ b/src/lib/ab_transcoder.cc
@@ -60,7 +60,7 @@ ABTranscoder::ABTranscoder (
 
 	if (_film_a->audio_stream()) {
 		shared_ptr<AudioStream> st = _film_a->audio_stream();
-		_matcher.reset (new Matcher (_film_a->log(), st->sample_rate(), _film_a->frames_per_second()));
+		_matcher.reset (new Matcher (_film_a->log(), st->sample_rate(), _film_a->source_frame_rate()));
 		_delay_line.reset (new DelayLine (_film_a->log(), st->channels(), _film_a->audio_delay() * st->sample_rate() / 1000));
 		_gain.reset (new Gain (_film_a->log(), _film_a->audio_gain()));
 	}
diff --git a/src/lib/analyse_audio_job.cc b/src/lib/analyse_audio_job.cc
index ca316f70e..41f918f34 100644
--- a/src/lib/analyse_audio_job.cc
+++ b/src/lib/analyse_audio_job.cc
@@ -66,7 +66,7 @@ AnalyseAudioJob::run ()
 	decoders.audio->set_audio_stream (_film->audio_stream ());
 	decoders.audio->Audio.connect (bind (&AnalyseAudioJob::audio, this, _1));
 
-	int64_t total_audio_frames = video_frames_to_audio_frames (_film->length().get(), _film->audio_stream()->sample_rate(), _film->frames_per_second());
+	int64_t total_audio_frames = video_frames_to_audio_frames (_film->length().get(), _film->audio_stream()->sample_rate(), _film->source_frame_rate());
 	_samples_per_point = total_audio_frames / _num_points;
 
 	_current.resize (_film->audio_stream()->channels ());
diff --git a/src/lib/dcp_video_frame.cc b/src/lib/dcp_video_frame.cc
index 098d222cd..67617c63c 100644
--- a/src/lib/dcp_video_frame.cc
+++ b/src/lib/dcp_video_frame.cc
@@ -80,7 +80,7 @@ using libdcp::Size;
 DCPVideoFrame::DCPVideoFrame (
 	shared_ptr<const Image> yuv, shared_ptr<Subtitle> sub,
 	Size out, int p, int subtitle_offset, float subtitle_scale,
-	Scaler const * s, int f, float fps, string pp, int clut, int bw, Log* l
+	Scaler const * s, int f, int dcp_fps, string pp, int clut, int bw, Log* l
 	)
 	: _input (yuv)
 	, _subtitle (sub)
@@ -90,7 +90,7 @@ DCPVideoFrame::DCPVideoFrame (
 	, _subtitle_scale (subtitle_scale)
 	, _scaler (s)
 	, _frame (f)
-	, _frames_per_second (DCPFrameRate(fps).frames_per_second)
+	, _frames_per_second (dcp_fps)
 	, _post_process (pp)
 	, _colour_lut (clut)
 	, _j2k_bandwidth (bw)
diff --git a/src/lib/dcp_video_frame.h b/src/lib/dcp_video_frame.h
index ab458b58f..6794765ac 100644
--- a/src/lib/dcp_video_frame.h
+++ b/src/lib/dcp_video_frame.h
@@ -107,7 +107,7 @@ class DCPVideoFrame
 public:
 	DCPVideoFrame (
 		boost::shared_ptr<const Image>, boost::shared_ptr<Subtitle>, libdcp::Size,
-		int, int, float, Scaler const *, int, float, std::string, int, int, Log *
+		int, int, float, Scaler const *, int, int, std::string, int, int, Log *
 		);
 	
 	virtual ~DCPVideoFrame ();
@@ -130,7 +130,7 @@ private:
 	float _subtitle_scale;
 	Scaler const * _scaler;          ///< scaler to use
 	int _frame;                      ///< frame index within the DCP's intrinsic duration
-	int _frames_per_second;          ///< Frames per second that we will use for the DCP (rounded)
+	int _frames_per_second;          ///< Frames per second that we will use for the DCP
 	std::string _post_process;       ///< FFmpeg post-processing string to use
 	int _colour_lut;                 ///< Colour look-up table to use
 	int _j2k_bandwidth;              ///< J2K bandwidth to use
diff --git a/src/lib/encoder.cc b/src/lib/encoder.cc
index 3cc643cd6..687dfdd2b 100644
--- a/src/lib/encoder.cc
+++ b/src/lib/encoder.cc
@@ -233,9 +233,9 @@ Encoder::frame_done ()
 void
 Encoder::process_video (shared_ptr<Image> image, bool same, boost::shared_ptr<Subtitle> sub)
 {
-	DCPFrameRate dfr (_film->frames_per_second ());
+	FrameRateConversion frc (_film->source_frame_rate(), _film->dcp_frame_rate());
 	
-	if (dfr.skip && (_video_frames_in % 2)) {
+	if (frc.skip && (_video_frames_in % 2)) {
 		++_video_frames_in;
 		return;
 	}
@@ -273,7 +273,7 @@ Encoder::process_video (shared_ptr<Image> image, bool same, boost::shared_ptr<Su
 					  new DCPVideoFrame (
 						  image, sub, _film->format()->dcp_size(), _film->format()->dcp_padding (_film),
 						  _film->subtitle_offset(), _film->subtitle_scale(),
-						  _film->scaler(), _video_frames_out, _film->frames_per_second(), s.second,
+						  _film->scaler(), _video_frames_out, _film->dcp_frame_rate(), s.second,
 						  _film->colour_lut(), _film->j2k_bandwidth(),
 						  _film->log()
 						  )
@@ -286,7 +286,7 @@ Encoder::process_video (shared_ptr<Image> image, bool same, boost::shared_ptr<Su
 	++_video_frames_in;
 	++_video_frames_out;
 
-	if (dfr.repeat) {
+	if (frc.repeat) {
 		_writer->repeat (_video_frames_out);
 		++_video_frames_out;
 		frame_done ();
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 510158e94..a661adcc6 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -74,7 +74,7 @@ using boost::starts_with;
 using boost::optional;
 using libdcp::Size;
 
-int const Film::state_version = 3;
+int const Film::state_version = 4;
 
 /** Construct a Film object in a given directory, reading any metadata
  *  file that exists in that directory.  An exception will be thrown if
@@ -103,7 +103,8 @@ Film::Film (string d, bool must_exist)
 	, _colour_lut (0)
 	, _j2k_bandwidth (200000000)
 	, _dci_metadata (Config::instance()->default_dci_metadata ())
-	, _frames_per_second (0)
+	, _dcp_frame_rate (0)
+	, _source_frame_rate (0)
 	, _dirty (false)
 {
 	set_dci_date_today ();
@@ -175,6 +176,7 @@ Film::Film (Film const & o)
 	, _j2k_bandwidth     (o._j2k_bandwidth)
 	, _dci_metadata      (o._dci_metadata)
 	, _dci_date          (o._dci_date)
+	, _dcp_frame_rate    (o._dcp_frame_rate)
 	, _size              (o._size)
 	, _length            (o._length)
 	, _dcp_intrinsic_duration (o._dcp_intrinsic_duration)
@@ -182,7 +184,7 @@ Film::Film (Film const & o)
 	, _content_audio_streams (o._content_audio_streams)
 	, _external_audio_stream (o._external_audio_stream)
 	, _subtitle_streams  (o._subtitle_streams)
-	, _frames_per_second (o._frames_per_second)
+	, _source_frame_rate (o._source_frame_rate)
 	, _dirty             (o._dirty)
 {
 
@@ -204,6 +206,7 @@ Film::video_state_identifier () const
 	s << format()->id()
 	  << N_("_") << content_digest()
 	  << N_("_") << crop().left << N_("_") << crop().right << N_("_") << crop().top << N_("_") << crop().bottom
+	  << N_("_") << _dcp_frame_rate
 	  << N_("_") << f.first << N_("_") << f.second
 	  << N_("_") << scaler()->id()
 	  << N_("_") << j2k_bandwidth()
@@ -441,6 +444,7 @@ Film::write_metadata () const
 	f << N_("j2k_bandwidth ") << _j2k_bandwidth << N_("\n");
 	_dci_metadata.write (f);
 	f << N_("dci_date ") << boost::gregorian::to_iso_string (_dci_date) << N_("\n");
+	f << N_("dcp_frame_rate ") << _dcp_frame_rate << "\n";
 	f << N_("width ") << _size.width << N_("\n");
 	f << N_("height ") << _size.height << N_("\n");
 	f << N_("length ") << _length.get_value_or(0) << N_("\n");
@@ -457,7 +461,7 @@ Film::write_metadata () const
 		f << N_("subtitle_stream ") << (*i)->to_string () << N_("\n");
 	}
 
-	f << N_("frames_per_second ") << _frames_per_second << N_("\n");
+	f << N_("source_frame_rate ") << _source_frame_rate << N_("\n");
 	
 	_dirty = false;
 }
@@ -569,6 +573,8 @@ Film::read_metadata ()
 			_j2k_bandwidth = atoi (v.c_str ());
 		} else if (k == N_("dci_date")) {
 			_dci_date = boost::gregorian::from_undelimited_string (v);
+		} else if (k == "dcp_frame_rate") {
+			_dcp_frame_rate = atoi (v.c_str ());
 		}
 
 		_dci_metadata.read (k, v);
@@ -596,8 +602,8 @@ Film::read_metadata ()
 			_external_audio_stream = audio_stream_factory (v, version);
 		} else if (k == N_("subtitle_stream")) {
 			_subtitle_streams.push_back (subtitle_stream_factory (v, version));
-		} else if (k == N_("frames_per_second")) {
-			_frames_per_second = atof (v.c_str ());
+		} else if (k == N_("source_frame_rate") || (version < 4 && k == "frames_per_second")) {
+			_source_frame_rate = atof (v.c_str ());
 		}
 	}
 
@@ -707,7 +713,7 @@ Film::target_audio_sample_rate () const
 	/* Resample to a DCI-approved sample rate */
 	double t = dcp_audio_sample_rate (audio_stream()->sample_rate());
 
-	DCPFrameRate dfr (frames_per_second ());
+	FrameRateConversion frc (source_frame_rate(), dcp_frame_rate());
 
 	/* 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
@@ -715,8 +721,8 @@ Film::target_audio_sample_rate () const
 	   skip/repeat doesn't come into effect here.
 	*/
 
-	if (dfr.change_speed) {
-		t *= _frames_per_second * dfr.factor() / dfr.frames_per_second;
+	if (frc.change_speed) {
+		t *= source_frame_rate() * frc.factor() / dcp_frame_rate();
 	}
 
 	return rint (t);
@@ -725,7 +731,7 @@ Film::target_audio_sample_rate () const
 int
 Film::still_duration_in_frames () const
 {
-	return still_duration() * frames_per_second();
+	return still_duration() * source_frame_rate();
 }
 
 /** @return a DCI-compliant name for a DCP of this film */
@@ -898,7 +904,7 @@ Film::set_content (string c)
 		Decoders d = decoder_factory (shared_from_this(), DecodeOptions());
 		
 		set_size (d.video->native_size ());
-		set_frames_per_second (d.video->frames_per_second ());
+		set_source_frame_rate (d.video->frames_per_second ());
 		set_subtitle_streams (d.video->subtitle_streams ());
 		if (d.audio) {
 			set_content_audio_streams (d.audio->audio_streams ());
@@ -1236,6 +1242,17 @@ Film::set_dci_metadata (DCIMetadata m)
 	signal_changed (DCI_METADATA);
 }
 
+
+void
+Film::set_dcp_frame_rate (int f)
+{
+	{
+		boost::mutex::scoped_lock lm (_state_mutex);
+		_dcp_frame_rate = f;
+	}
+	signal_changed (DCP_FRAME_RATE);
+}
+
 void
 Film::set_size (libdcp::Size s)
 {
@@ -1307,13 +1324,13 @@ Film::set_subtitle_streams (vector<shared_ptr<SubtitleStream> > s)
 }
 
 void
-Film::set_frames_per_second (float f)
+Film::set_source_frame_rate (float f)
 {
 	{
 		boost::mutex::scoped_lock lm (_state_mutex);
-		_frames_per_second = f;
+		_source_frame_rate = f;
 	}
-	signal_changed (FRAMES_PER_SECOND);
+	signal_changed (SOURCE_FRAME_RATE);
 }
 	
 void
diff --git a/src/lib/film.h b/src/lib/film.h
index 847ab434e..9921acbb4 100644
--- a/src/lib/film.h
+++ b/src/lib/film.h
@@ -148,7 +148,8 @@ public:
 		DCP_INTRINSIC_DURATION,
 		CONTENT_AUDIO_STREAMS,
 		SUBTITLE_STREAMS,
-		FRAMES_PER_SECOND,
+		SOURCE_FRAME_RATE,
+		DCP_FRAME_RATE
 	};
 
 
@@ -285,6 +286,11 @@ public:
 		boost::mutex::scoped_lock lm (_state_mutex);
 		return _dci_metadata;
 	}
+
+	int dcp_frame_rate () const {
+		boost::mutex::scoped_lock lm (_state_mutex);
+		return _dcp_frame_rate;
+	}
 	
 	libdcp::Size size () const {
 		boost::mutex::scoped_lock lm (_state_mutex);
@@ -311,13 +317,13 @@ public:
 		return _subtitle_streams;
 	}
 	
-	float frames_per_second () const {
+	float source_frame_rate () const {
 		boost::mutex::scoped_lock lm (_state_mutex);
 		if (content_type() == STILL) {
 			return 24;
 		}
 		
-		return _frames_per_second;
+		return _source_frame_rate;
 	}
 
 	boost::shared_ptr<AudioStream> audio_stream () const;
@@ -355,6 +361,7 @@ public:
 	void set_colour_lut (int);
 	void set_j2k_bandwidth (int);
 	void set_dci_metadata (DCIMetadata);
+	void set_dcp_frame_rate (int);
 	void set_size (libdcp::Size);
 	void set_length (SourceFrame);
 	void unset_length ();
@@ -362,7 +369,7 @@ public:
 	void set_content_digest (std::string);
 	void set_content_audio_streams (std::vector<boost::shared_ptr<AudioStream> >);
 	void set_subtitle_streams (std::vector<boost::shared_ptr<SubtitleStream> >);
-	void set_frames_per_second (float);
+	void set_source_frame_rate (float);
 
 	/** Emitted when some property has changed */
 	mutable boost::signals2::signal<void (Property)> Changed;
@@ -461,6 +468,8 @@ private:
 	DCIMetadata _dci_metadata;
 	/** The date that we should use in a DCI name */
 	boost::gregorian::date _dci_date;
+	/** Frames per second to run our DCP at */
+	int _dcp_frame_rate;
 
 	/* Data which are cached to speed things up */
 
@@ -478,7 +487,7 @@ private:
 	/** the subtitle streams that we can use */
 	std::vector<boost::shared_ptr<SubtitleStream> > _subtitle_streams;
 	/** Frames per second of the source */
-	float _frames_per_second;
+	float _source_frame_rate;
 
 	/** true if our state has changed since we last saved it */
 	mutable bool _dirty;
diff --git a/src/lib/transcode_job.cc b/src/lib/transcode_job.cc
index 61fad2e2b..f7cc500fe 100644
--- a/src/lib/transcode_job.cc
+++ b/src/lib/transcode_job.cc
@@ -120,8 +120,8 @@ TranscodeJob::remaining_time () const
 
 	/* Compute approximate proposed length here, as it's only here that we need it */
 	int length = _film->length().get();
-	DCPFrameRate const dfr (_film->frames_per_second ());
-	if (dfr.skip) {
+	FrameRateConversion const frc (_film->source_frame_rate(), _film->dcp_frame_rate());
+	if (frc.skip) {
 		length /= 2;
 	}
 	/* If we are repeating it shouldn't affect transcode time, so don't take it into account */
diff --git a/src/lib/transcoder.cc b/src/lib/transcoder.cc
index 959fac857..9720ca56a 100644
--- a/src/lib/transcoder.cc
+++ b/src/lib/transcoder.cc
@@ -57,7 +57,7 @@ Transcoder::Transcoder (shared_ptr<Film> f, DecodeOptions o, Job* j, shared_ptr<
 
 	if (f->audio_stream()) {
 		shared_ptr<AudioStream> st = f->audio_stream();
-		_matcher.reset (new Matcher (f->log(), st->sample_rate(), f->frames_per_second()));
+		_matcher.reset (new Matcher (f->log(), st->sample_rate(), f->source_frame_rate()));
 		_delay_line.reset (new DelayLine (f->log(), st->channels(), f->audio_delay() * st->sample_rate() / 1000));
 		_gain.reset (new Gain (f->log(), f->audio_gain()));
 	}
diff --git a/src/lib/util.cc b/src/lib/util.cc
index de69636da..85a04ed17 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -375,21 +375,12 @@ public:
 		, 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 */
-/** XXX: this could be slow-ish */
-DCPFrameRate::DCPFrameRate (float source_fps)
+int
+best_dcp_frame_rate (float source_fps)
 {
 	list<int> const allowed_dcp_frame_rates = Config::instance()->allowed_dcp_frame_rates ();
 
@@ -427,14 +418,8 @@ DCPFrameRate::DCPFrameRate (float source_fps)
 		++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);
+	assert (best);
+	return best->dcp;
 }
 
 /** @param An arbitrary sampling rate.
@@ -962,3 +947,17 @@ AudioMapping::dcp_channels () const
 
 	return _source_channels;
 }
+
+FrameRateConversion::FrameRateConversion (float source, int dcp)
+	: skip (false)
+	, repeat (false)
+	, change_speed (false)
+{
+	if (fabs (source / 2.0 - dcp) < (fabs (source - dcp))) {
+		skip = true;
+	} else if (fabs (source * 2 - dcp) < fabs (source - dcp)) {
+		repeat = true;
+	}
+
+	change_speed = !about_equal (source * factor(), dcp);
+}
diff --git a/src/lib/util.h b/src/lib/util.h
index 22c6ea95b..103907151 100644
--- a/src/lib/util.h
+++ b/src/lib/util.h
@@ -62,9 +62,9 @@ extern std::string audio_channel_name (int);
 
 typedef int SourceFrame;
 
-struct DCPFrameRate
+struct FrameRateConversion
 {
-	DCPFrameRate (float);
+	FrameRateConversion (float, int);
 
 	/** @return factor by which to multiply a source frame rate
 	    to get the effective rate after any skip or repeat has happened.
@@ -79,14 +79,12 @@ struct DCPFrameRate
 		return 1;
 	}
 	
-	/** frames per second for the DCP */
-	int frames_per_second;
 	/** true to skip every other frame */
 	bool skip;
 	/** true to repeat every frame once */
 	bool repeat;
 	/** true if this DCP will run its video faster or slower than the source
-	 *  without taking into account `repeat'.
+	 *  without taking into account `repeat' nor `skip'.
 	 *  (e.g. change_speed will be true if
 	 *          source is 29.97fps, DCP is 30fps
 	 *          source is 14.50fps, DCP is 30fps
@@ -97,6 +95,8 @@ struct DCPFrameRate
 	bool change_speed;
 };
 
+int best_dcp_frame_rate (float);
+
 enum ContentType {
 	STILL, ///< content is still images
 	VIDEO  ///< content is a video
diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 334ecec65..5a2f7c9a9 100644
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -67,8 +67,8 @@ Writer::Writer (shared_ptr<Film> f)
 		new libdcp::MonoPictureAsset (
 			_film->video_mxf_dir (),
 			_film->video_mxf_filename (),
-			DCPFrameRate (_film->frames_per_second()).frames_per_second,
-			_film->format()->dcp_size()
+			_film->dcp_frame_rate (),
+			_film->format()->dcp_size ()
 			)
 		);
 
@@ -81,7 +81,7 @@ Writer::Writer (shared_ptr<Film> f)
 			new libdcp::SoundAsset (
 				_film->dir (_film->dcp_name()),
 				N_("audio.mxf"),
-				DCPFrameRate (_film->frames_per_second()).frames_per_second,
+				_film->dcp_frame_rate (),
 				m.dcp_channels (),
 				dcp_audio_sample_rate (_film->audio_stream()->sample_rate())
 				)
@@ -289,10 +289,15 @@ Writer::finish ()
 	}
 	
 	libdcp::DCP dcp (_film->dir (_film->dcp_name()));
-	DCPFrameRate dfr (_film->frames_per_second ());
 
 	shared_ptr<libdcp::CPL> cpl (
-		new libdcp::CPL (_film->dir (_film->dcp_name()), _film->dcp_name(), _film->dcp_content_type()->libdcp_kind (), frames, dfr.frames_per_second)
+		new libdcp::CPL (
+			_film->dir (_film->dcp_name()),
+			_film->dcp_name(),
+			_film->dcp_content_type()->libdcp_kind (),
+			frames,
+			_film->dcp_frame_rate ()
+			)
 		);
 	
 	dcp.add_cpl (cpl);