Various multi-reel fixes.

* _video_frames_enqueued -> _position
* some int -> Frame replacements
* Call Writer::ref_write when there is nothing to encode / write
  for one frame.
* Assume Encoder::encode is called once per output video frame.
* Possibly correct some off-by-1 overlap bugs.
* Don't reset _last_written on FAKE write as I believe there is no need.

7 files changed, 100 insertions, 70 deletions

diff --git a/src/lib/encoder.cc b/src/lib/encoder.cc
index f7a66708e..95a85f7e8 100644
--- a/src/lib/encoder.cc
+++ b/src/lib/encoder.cc
@@ -59,9 +59,7 @@ int const Encoder::_history_size = 25;
 Encoder::Encoder (shared_ptr<const Film> film, weak_ptr<Job> j, shared_ptr<Writer> writer)
 	: _film (film)
 	, _job (j)
-	, _video_frames_enqueued (0)
-	, _left_done (false)
-	, _right_done (false)
+	, _position (0)
 	, _terminate_enqueue (false)
 	, _terminate_encoding (false)
 	, _writer (writer)
@@ -155,7 +153,7 @@ int
 Encoder::video_frames_out () const
 {
 	boost::mutex::scoped_lock (_state_mutex);
-	return _video_frames_enqueued;
+	return _position;
 }
 
 /** Should be called when a frame has been encoded successfully.
@@ -174,10 +172,25 @@ Encoder::frame_done ()
 	}
 }
 
-/** Called in order, so each time this is called the supplied frame is the one
- *  after the previous one.
+/** Called to start encoding of the next video frame in the DCP.  This is called in order,
+ *  so each time the supplied frame is the one after the previous one.
+ *  pv represents one video frame, and could be empty if there is nothing to encode
+ *  for this DCP frame.
  */
 void
+Encoder::encode (list<shared_ptr<PlayerVideo> > pv)
+{
+	if (pv.empty ()) {
+		_writer->ref_write (_position);
+	} else {
+		BOOST_FOREACH (shared_ptr<PlayerVideo> i, pv) {
+			enqueue (i);
+		}
+	}
+	++_position;
+}
+
+void
 Encoder::enqueue (shared_ptr<PlayerVideo> pv)
 {
 	_waker.nudge ();
@@ -210,22 +223,22 @@ Encoder::enqueue (shared_ptr<PlayerVideo> pv)
 	*/
 	rethrow ();
 
-	if (_writer->can_fake_write (_video_frames_enqueued)) {
+	if (_writer->can_fake_write (_position)) {
 		/* We can fake-write this frame */
-		_writer->fake_write (_video_frames_enqueued, pv->eyes ());
+		_writer->fake_write (_position, pv->eyes ());
 		frame_done ();
 	} else if (pv->has_j2k ()) {
 		/* This frame already has JPEG2000 data, so just write it */
-		_writer->write (pv->j2k(), _video_frames_enqueued, pv->eyes ());
+		_writer->write (pv->j2k(), _position, pv->eyes ());
 	} else if (_last_player_video && pv->same (_last_player_video)) {
-		_writer->repeat (_video_frames_enqueued, pv->eyes ());
+		_writer->repeat (_position, pv->eyes ());
 	} else {
 		/* Queue this new frame for encoding */
 		LOG_TIMING ("add-frame-to-queue queue=%1", _queue.size ());
 		_queue.push_back (shared_ptr<DCPVideo> (
 					  new DCPVideo (
 						  pv,
-						  _video_frames_enqueued,
+						  _position,
 						  _film->video_frame_rate(),
 						  _film->j2k_bandwidth(),
 						  _film->resolution(),
@@ -239,25 +252,6 @@ Encoder::enqueue (shared_ptr<PlayerVideo> pv)
 		_empty_condition.notify_all ();
 	}
 
-	switch (pv->eyes ()) {
-	case EYES_BOTH:
-		++_video_frames_enqueued;
-		break;
-	case EYES_LEFT:
-		_left_done = true;
-		break;
-	case EYES_RIGHT:
-		_right_done = true;
-		break;
-	default:
-		break;
-	}
-
-	if (_left_done && _right_done) {
-		++_video_frames_enqueued;
-		_left_done = _right_done = false;
-	}
-
 	_last_player_video = pv;
 }
 
diff --git a/src/lib/encoder.h b/src/lib/encoder.h
index 76c87a9f7..3d0597cfa 100644
--- a/src/lib/encoder.h
+++ b/src/lib/encoder.h
@@ -59,10 +59,10 @@ public:
 	/** Called to indicate that a processing run is about to begin */
 	void begin ();
 
-	/** Call with a frame of video.
-	 *  @param f Video frame.
+	/** Called to pass in zero or more bits of video to be encoded
+	 *  as the next DCP frame.
 	 */
-	void enqueue (boost::shared_ptr<PlayerVideo> f);
+	void encode (std::list<boost::shared_ptr<PlayerVideo> > f);
 
 	/** Called when a processing run has finished */
 	void end ();
@@ -72,6 +72,7 @@ public:
 
 private:
 
+	void enqueue (boost::shared_ptr<PlayerVideo> f);
 	void frame_done ();
 
 	void encoder_thread (boost::optional<ServerDescription>);
@@ -90,10 +91,8 @@ private:
 	std::list<struct timeval> _time_history;
 	/** Number of frames that we should keep history for */
 	static int const _history_size;
-	/** Number of video frames enqueued so far */
-	int _video_frames_enqueued;
-	bool _left_done;
-	bool _right_done;
+	/** Current DCP frame index */
+	Frame _position;
 
 	/* XXX: probably should be atomic */
 	bool _terminate_enqueue;
diff --git a/src/lib/player.cc b/src/lib/player.cc
index 6c3d959fa..116410046 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -380,7 +380,7 @@ Player::get_video (DCPTime time, bool accurate)
 
 	list<shared_ptr<Piece> > ov = overlaps<VideoContent> (
 		time,
-		time + DCPTime::from_frames (1, _film->video_frame_rate ()) - DCPTime::delta()
+		time + DCPTime::from_frames (1, _film->video_frame_rate ())
 		);
 
 	list<shared_ptr<PlayerVideo> > pvf;
diff --git a/src/lib/player.h b/src/lib/player.h
index cd1423e9f..3106a3477 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -106,7 +106,7 @@ private:
 				continue;
 			}
 
-			if ((*i)->content->position() <= to && (*i)->content->end() >= from) {
+			if ((*i)->content->position() < to && (*i)->content->end() > from) {
 				overlaps.push_back (*i);
 			}
 		}
diff --git a/src/lib/transcoder.cc b/src/lib/transcoder.cc
index 20185e8c9..acff9c896 100644
--- a/src/lib/transcoder.cc
+++ b/src/lib/transcoder.cc
@@ -87,15 +87,8 @@ Transcoder::go ()
 	}
 
 	for (DCPTime t; t < length; t += frame) {
-		list<shared_ptr<PlayerVideo> > v = _player->get_video (t, true);
-		for (list<shared_ptr<PlayerVideo> >::const_iterator i = v.begin(); i != v.end(); ++i) {
-			_encoder->enqueue (*i);
-		}
-
-		shared_ptr<AudioBuffers> audio = _player->get_audio (t, frame, true);
-		if (audio) {
-			_writer->write (audio);
-		}
+		_encoder->encode (_player->get_video (t, true));
+		_writer->write (_player->get_audio (t, frame, true));
 
 		if (non_burnt_subtitles) {
 			_writer->write (_player->get_subtitles (t, frame, true, false));
diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 736dba78e..071cac4b6 100644
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -90,6 +90,7 @@ Writer::Writer (shared_ptr<const Film> film, weak_ptr<Job> j)
 	, _full_written (0)
 	, _fake_written (0)
 	, _repeat_written (0)
+	, _ref_written (0)
 	, _pushed_to_disk (0)
 {
 	/* Remove any old DCP */
@@ -178,8 +179,9 @@ Writer::~Writer ()
 	terminate_thread (false);
 }
 
+/** @param frame Frame within the DCP */
 void
-Writer::write (Data encoded, int frame, Eyes eyes)
+Writer::write (Data encoded, Frame frame, Eyes eyes)
 {
 	boost::mutex::scoped_lock lock (_state_mutex);
 
@@ -212,7 +214,7 @@ Writer::write (Data encoded, int frame, Eyes eyes)
 }
 
 void
-Writer::repeat (int frame, Eyes eyes)
+Writer::repeat (Frame frame, Eyes eyes)
 {
 	boost::mutex::scoped_lock lock (_state_mutex);
 
@@ -239,7 +241,7 @@ Writer::repeat (int frame, Eyes eyes)
 }
 
 void
-Writer::fake_write (int frame, Eyes eyes)
+Writer::fake_write (Frame frame, Eyes eyes)
 {
 	boost::mutex::scoped_lock lock (_state_mutex);
 
@@ -275,7 +277,29 @@ Writer::fake_write (int frame, Eyes eyes)
 	_empty_condition.notify_all ();
 }
 
-/** This method is not thread safe */
+void
+Writer::ref_write (Frame frame)
+{
+	boost::mutex::scoped_lock lock (_state_mutex);
+
+	while (_queued_full_in_memory > _maximum_frames_in_memory) {
+		/* The queue is too big; wait until that is sorted out */
+		_full_condition.wait (lock);
+	}
+
+	QueueItem qi;
+	qi.type = QueueItem::REF;
+	qi.frame = frame;
+	qi.eyes = EYES_BOTH;
+	_queue.push_back (qi);
+
+	/* Now there's something to do: wake anything wait()ing on _empty_condition */
+	_empty_condition.notify_all ();
+}
+
+/** Write one video frame's worth of audio frames to the DCP.
+ *  This method is not thread safe.
+ */
 void
 Writer::write (shared_ptr<const AudioBuffers> audio)
 {
@@ -283,10 +307,17 @@ Writer::write (shared_ptr<const AudioBuffers> audio)
 		return;
 	}
 
-	_audio_reel->sound_asset_writer->write (audio->data(), audio->frames());
+	if (audio) {
+		_audio_reel->sound_asset_writer->write (audio->data(), audio->frames());
+	}
+
+	++_audio_reel->written;
+
+	cout << "(written " << _audio_reel->written << "); period is " << _audio_reel->period.duration() << "\n";
 
-	/* SoundAsset's `frames' are video frames, not audio frames */
-	if (_audio_reel->sound_asset_writer->frames_written() >= _audio_reel->period.duration().frames_round (_film->video_frame_rate())) {
+	/* written is in video frames, not audio frames */
+	if (_audio_reel->written >= _audio_reel->period.duration().frames_round (_film->video_frame_rate())) {
+		cout << "NEXT AUDIO REEL!\n";
 		++_audio_reel;
 	}
 }
@@ -416,10 +447,10 @@ try
 			case QueueItem::FAKE:
 				LOG_DEBUG_ENCODE (N_("Writer FAKE-writes %1"), qi.frame);
 				reel.picture_asset_writer->fake_write (qi.size);
-				_last_written[qi.eyes].reset ();
 				++_fake_written;
 				break;
 			case QueueItem::REPEAT:
+			{
 				LOG_DEBUG_ENCODE (N_("Writer REPEAT-writes %1"), qi.frame);
 				dcp::FrameInfo fin = reel.picture_asset_writer->write (
 					_last_written[qi.eyes]->data().get(),
@@ -429,6 +460,11 @@ try
 				++_repeat_written;
 				break;
 			}
+			case QueueItem::REF:
+				LOG_DEBUG_ENCODE (N_("Writer REF-writes %1"), qi.frame);
+				++_ref_written;
+				break;
+			}
 
 			lock.lock ();
 
@@ -445,7 +481,7 @@ try
 				total *= 2;
 			}
 			if (total) {
-				job->set_progress (float (_full_written + _fake_written + _repeat_written) / total);
+				job->set_progress (float (_full_written + _fake_written + _repeat_written + _ref_written) / total);
 			}
 		}
 
@@ -527,13 +563,16 @@ Writer::finish ()
 	BOOST_FOREACH (Reel& i, _reels) {
 
 		if (!i.picture_asset_writer->finalize ()) {
-		/* Nothing was written to the picture asset */
+			/* Nothing was written to the picture asset */
 			i.picture_asset.reset ();
 		}
 
 		if (i.sound_asset_writer && !i.sound_asset_writer->finalize ()) {
 			/* Nothing was written to the sound asset */
+			cout << "nothing written to reel @ " << i.period << "\n";
 			i.sound_asset.reset ();
+		} else {
+			cout << "something written to reel @ " << i.period << "\n";
 		}
 
 		/* Hard-link any video asset file into the DCP */
@@ -718,7 +757,7 @@ Writer::finish ()
 	dcp.write_xml (_film->interop () ? dcp::INTEROP : dcp::SMPTE, meta, signer);
 
 	LOG_GENERAL (
-		N_("Wrote %1 FULL, %2 FAKE, %3 REPEAT, %4 pushed to disk"), _full_written, _fake_written, _repeat_written, _pushed_to_disk
+		N_("Wrote %1 FULL, %2 FAKE, %3 REPEAT, %4 REF, %5 pushed to disk"), _full_written, _fake_written, _repeat_written, _ref_written, _pushed_to_disk
 		);
 }
 
@@ -791,22 +830,21 @@ Writer::check_existing_picture_asset (Reel& reel)
 	fclose (info_file);
 }
 
-/** @param frame Frame index.
+/** @param frame Frame index within the whole DCP.
  *  @return true if we can fake-write this frame.
  */
 bool
-Writer::can_fake_write (int frame) const
+Writer::can_fake_write (Frame frame) const
 {
 	/* We have to do a proper write of the first frame so that we can set up the JPEG2000
 	   parameters in the asset writer.
 	*/
 
-	/* XXX: need to correct frame to be relative to the reel, perhaps?
-	   and clarify whether first_nonexistant_frame is relative to the start
-	   of the DCP or of the reel.
-	*/
+	Reel const & reel = video_reel (frame);
 
-	return (frame != 0 && frame < video_reel(frame).first_nonexistant_frame);
+	/* Make frame relative to the start of the reel */
+	frame -= reel.period.from.frames_floor (_film->video_frame_rate());
+	return (frame != 0 && frame < reel.first_nonexistant_frame);
 }
 
 void
diff --git a/src/lib/writer.h b/src/lib/writer.h
index 8f1e71278..95530a9d9 100644
--- a/src/lib/writer.h
+++ b/src/lib/writer.h
@@ -68,6 +68,7 @@ public:
 		*/
 		FAKE,
 		REPEAT,
+		REF,
 	} type;
 
 	/** encoded data for FULL */
@@ -76,6 +77,7 @@ public:
 	int size;
 	/** frame index */
 	int frame;
+	/** eyes for FULL, FAKE and REPEAT */
 	Eyes eyes;
 };
 
@@ -101,11 +103,12 @@ public:
 
 	void start ();
 
-	bool can_fake_write (int) const;
+	bool can_fake_write (Frame) const;
 
-	void write (Data, int, Eyes);
-	void fake_write (int, Eyes);
-	void repeat (int, Eyes);
+	void write (Data, Frame, Eyes);
+	void fake_write (Frame, Eyes);
+	void ref_write (Frame);
+	void repeat (Frame, Eyes);
 	void write (boost::shared_ptr<const AudioBuffers>);
 	void write (PlayerSubtitles subs);
 	void write (std::list<boost::shared_ptr<Font> > fonts);
@@ -120,11 +123,13 @@ private:
 	public:
 		Reel ()
 			: first_nonexistant_frame (0)
+			, written (0)
 		{}
 
 		DCPTimePeriod period;
 		/** the first frame index that does not already exist in our MXF */
 		int first_nonexistant_frame;
+		Frame written;
 
 		boost::shared_ptr<dcp::PictureAsset> picture_asset;
 		boost::shared_ptr<dcp::PictureAssetWriter> picture_asset_writer;
@@ -178,6 +183,7 @@ private:
 	/** number of FAKE written frames */
 	int _fake_written;
 	int _repeat_written;
+	int _ref_written;
 	/** number of frames pushed to disk and then recovered
 	    due to the limit of frames to be held in memory.
 	*/