Add _last_written to Writer, containing the last written frame and eyes

to each reel.  This is updated when things are popped off the queue, with
_state_mutex_held, and used in preference to the ones in ReelWriter
which were previously being updated during the time the _state_mutex
lock is unlocked in the body of Writer::thread().  This was not
thread safe (thanks, valgrind!)

4 files changed, 51 insertions, 31 deletions

diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index 49e09663f..06d20e7c8 100644
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -71,8 +71,6 @@ ReelWriter::ReelWriter (
 	)
 	: _film (film)
 	, _period (period)
-	, _last_written_video_frame (-1)
-	, _last_written_eyes (EYES_RIGHT)
 	, _reel_index (reel_index)
 	, _reel_count (reel_count)
 	, _content_summary (content_summary)
@@ -266,12 +264,10 @@ ReelWriter::write (optional<Data> encoded, Frame frame, Eyes eyes)
 	dcp::FrameInfo fin = _picture_asset_writer->write (encoded->data().get (), encoded->size());
 	write_frame_info (frame, eyes, fin);
 	_last_written[eyes] = encoded;
-	_last_written_video_frame = frame;
-	_last_written_eyes = eyes;
 }
 
 void
-ReelWriter::fake_write (Frame frame, Eyes eyes, int size)
+ReelWriter::fake_write (int size)
 {
 	if (!_picture_asset_writer) {
 		/* We're not writing any data */
@@ -279,8 +275,6 @@ ReelWriter::fake_write (Frame frame, Eyes eyes, int size)
 	}
 
 	_picture_asset_writer->fake_write (size);
-	_last_written_video_frame = frame;
-	_last_written_eyes = eyes;
 }
 
 void
@@ -296,8 +290,6 @@ ReelWriter::repeat_write (Frame frame, Eyes eyes)
 		_last_written[eyes]->size()
 		);
 	write_frame_info (frame, eyes, fin);
-	_last_written_video_frame = frame;
-	_last_written_eyes = eyes;
 }
 
 void
diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h
index d241c0fac..0b5a3dad5 100644
--- a/src/lib/reel_writer.h
+++ b/src/lib/reel_writer.h
@@ -64,7 +64,7 @@ public:
 		);
 
 	void write (boost::optional<dcp::Data> encoded, Frame frame, Eyes eyes);
-	void fake_write (Frame frame, Eyes eyes, int size);
+	void fake_write (int size);
 	void repeat_write (Frame frame, Eyes eyes);
 	void write (boost::shared_ptr<const AudioBuffers> audio);
 	void write (PlayerText text, TextType type, boost::optional<DCPTextTrack> track, dcpomatic::DCPTimePeriod period);
@@ -79,14 +79,6 @@ public:
 		return _period;
 	}
 
-	int last_written_video_frame () const {
-		return _last_written_video_frame;
-	}
-
-	Eyes last_written_eyes () const {
-		return _last_written_eyes;
-	}
-
 	int first_nonexistant_frame () const {
 		return _first_nonexistant_frame;
 	}
@@ -109,9 +101,6 @@ private:
 	int _first_nonexistant_frame;
 	/** the data of the last written frame, if there is one */
 	boost::optional<dcp::Data> _last_written[EYES_COUNT];
-	/** the index of the last written video frame within the reel */
-	int _last_written_video_frame;
-	Eyes _last_written_eyes;
 	/** index of this reel within the DCP (starting from 0) */
 	int _reel_index;
 	/** number of reels in the DCP */
diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 6d1886406..d346c4a4f 100644
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -87,6 +87,8 @@ Writer::Writer (shared_ptr<const Film> film, weak_ptr<Job> j)
 		_reels.push_back (ReelWriter (film, p, job, reel_index++, reels.size(), _film->content_summary(p)));
 	}
 
+	_last_written.resize (reels.size());
+
 	/* We can keep track of the current audio, subtitle and closed caption reels easily because audio
 	   and captions arrive to the Writer in sequence.  This is not so for video.
 	*/
@@ -300,7 +302,8 @@ Writer::write (shared_ptr<const AudioBuffers> audio, DCPTime const time)
 	}
 }
 
-/** This must be called from Writer::thread() with an appropriate lock held */
+
+/** Caller must hold a lock on _state_mutex */
 bool
 Writer::have_sequenced_image_at_queue_head ()
 {
@@ -309,30 +312,41 @@ Writer::have_sequenced_image_at_queue_head ()
 	}
 
 	_queue.sort ();
-
 	QueueItem const & f = _queue.front();
-	ReelWriter const & reel = _reels[f.reel];
+	return _last_written[f.reel].next(f);
+}
 
-	/* The queue should contain only EYES_LEFT/EYES_RIGHT pairs or EYES_BOTH */
 
-	if (f.eyes == EYES_BOTH) {
+bool
+Writer::LastWritten::next (QueueItem qi) const
+{
+	if (qi.eyes == EYES_BOTH) {
 		/* 2D */
-		return f.frame == (reel.last_written_video_frame() + 1);
+		return qi.frame == (_frame + 1);
 	}
 
 	/* 3D */
 
-	if (reel.last_written_eyes() == EYES_LEFT && f.frame == reel.last_written_video_frame() && f.eyes == EYES_RIGHT) {
+	if (_eyes == EYES_LEFT && qi.frame == _frame && qi.eyes == EYES_RIGHT) {
 		return true;
 	}
 
-	if (reel.last_written_eyes() == EYES_RIGHT && f.frame == (reel.last_written_video_frame() + 1) && f.eyes == EYES_LEFT) {
+	if (_eyes == EYES_RIGHT && qi.frame == (_frame + 1) && qi.eyes == EYES_LEFT) {
 		return true;
 	}
 
 	return false;
 }
 
+
+void
+Writer::LastWritten::update (QueueItem qi)
+{
+	_frame = qi.frame;
+	_eyes = qi.eyes;
+}
+
+
 void
 Writer::thread ()
 try
@@ -381,6 +395,7 @@ try
 		/* Write any frames that we can write; i.e. those that are in sequence. */
 		while (have_sequenced_image_at_queue_head ()) {
 			QueueItem qi = _queue.front ();
+			_last_written[qi.reel].update (qi);
 			_queue.pop_front ();
 			if (qi.type == QueueItem::FULL && qi.encoded) {
 				--_queued_full_in_memory;
@@ -401,7 +416,7 @@ try
 				break;
 			case QueueItem::FAKE:
 				LOG_DEBUG_ENCODE (N_("Writer FAKE-writes %1"), qi.frame);
-				reel.fake_write (qi.frame, qi.eyes, qi.size);
+				reel.fake_write (qi.size);
 				++_fake_written;
 				break;
 			case QueueItem::REPEAT:
@@ -430,7 +445,7 @@ try
 			DCPOMATIC_ASSERT (i != _queue.rend());
 			++_pushed_to_disk;
 			/* For the log message below */
-			int const awaiting = _reels[_queue.front().reel].last_written_video_frame() + 1;
+			int const awaiting = _last_written[_queue.front().reel].frame() + 1;
 			lock.unlock ();
 
 			/* i is valid here, even though we don't hold a lock on the mutex,
diff --git a/src/lib/writer.h b/src/lib/writer.h
index d304133dc..d09b06264 100644
--- a/src/lib/writer.h
+++ b/src/lib/writer.h
@@ -151,6 +151,30 @@ private:
 	int _maximum_frames_in_memory;
 	unsigned int _maximum_queue_size;
 
+	class LastWritten
+	{
+	public:
+		LastWritten()
+			: _frame(-1)
+			, _eyes(EYES_RIGHT)
+		{}
+
+		/** @return true if qi is the next item after this one */
+		bool next (QueueItem qi) const;
+		void update (QueueItem qi);
+
+		int frame () const {
+			return _frame;
+		}
+
+	private:
+		int _frame;
+		Eyes _eyes;
+	};
+
+	/** The last frame written to each reel */
+	std::vector<LastWritten> _last_written;
+
 	/** number of FULL written frames */
 	int _full_written;
 	/** number of FAKE written frames */