Change how video timing is done.

This commit changes the approach with video timing.  Previously,
we would (more-or-less) try to use every video frame from the content
in the output, hoping that they come at a constant frame rate.

This is not always the case, however.  Here we preserve the PTS
of video frames, and then when one arrives we output whatever
DCP video frames we can (at the regular DCP frame rate).

Hopefully this will solve a range of sync problems, but it
could also introduce new ones.

1 files changed, 110 insertions, 152 deletions

diff --git a/src/lib/player.cc b/src/lib/player.cc
index ef7ecde5a..9debbaf7b 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -71,10 +71,8 @@ using std::dynamic_pointer_cast;
 using std::list;
 using std::make_pair;
 using std::make_shared;
-using std::make_shared;
 using std::max;
 using std::min;
-using std::min;
 using std::pair;
 using std::shared_ptr;
 using std::vector;
@@ -410,7 +408,6 @@ Player::setup_pieces ()
 	_silent = Empty(film, playlist(), bind(&have_audio, _1), _playback_length);
 
 	_next_video_time = boost::none;
-	_next_video_eyes = Eyes::BOTH;
 	_next_audio_time = boost::none;
 }
 
@@ -523,7 +520,7 @@ Player::black_player_video_frame (Eyes eyes) const
 	boost::mutex::scoped_lock lm(_black_image_mutex);
 
 	return std::make_shared<PlayerVideo> (
-		std::make_shared<const RawImageProxy>(_black_image),
+		make_shared<const RawImageProxy>(_black_image),
 		Crop(),
 		optional<double>(),
 		_video_container_size,
@@ -533,7 +530,7 @@ Player::black_player_video_frame (Eyes eyes) const
 		PresetColourConversion::all().front().conversion,
 		VideoRange::FULL,
 		std::weak_ptr<Content>(),
-		boost::optional<Frame>(),
+		boost::optional<dcpomatic::ContentTime>(),
 		false
 	);
 }
@@ -701,7 +698,7 @@ Player::pass ()
 
 	if (_playback_length.load() == DCPTime() || !film) {
 		/* Special; just give one black frame */
-		emit_video (black_player_video_frame(Eyes::BOTH), DCPTime());
+		use_video(black_player_video_frame(Eyes::BOTH), DCPTime(), one_video_frame());
 		return true;
 	}
 
@@ -759,22 +756,27 @@ Player::pass ()
 		LOG_DEBUG_PLAYER ("Calling pass() on %1", earliest_content->content->path(0));
 		earliest_content->done = earliest_content->decoder->pass ();
 		auto dcp = dynamic_pointer_cast<DCPContent>(earliest_content->content);
-		if (dcp && !_play_referenced && dcp->reference_audio()) {
-			/* We are skipping some referenced DCP audio content, so we need to update _next_audio_time
-			   to `hide' the fact that no audio was emitted during the referenced DCP (though
-			   we need to behave as though it was).
-			*/
-			_next_audio_time = dcp->end(film);
+		if (dcp && !_play_referenced) {
+			if (dcp->reference_video()) {
+				_next_video_time = dcp->end(film);
+			}
+			if (dcp->reference_audio()) {
+				/* We are skipping some referenced DCP audio content, so we need to update _next_audio_time
+				   to `hide' the fact that no audio was emitted during the referenced DCP (though
+				   we need to behave as though it was).
+				*/
+				_next_audio_time = dcp->end(film);
+			}
 		}
 		break;
 	}
 	case BLACK:
 		LOG_DEBUG_PLAYER ("Emit black for gap at %1", to_string(_black.position()));
 		if (film->three_d()) {
-			emit_video(black_player_video_frame(Eyes::LEFT), _black.position());
-			emit_video(black_player_video_frame(Eyes::RIGHT), _black.position());
+			use_video(black_player_video_frame(Eyes::LEFT), _black.position(), _black.period_at_position().to);
+			use_video(black_player_video_frame(Eyes::RIGHT), _black.position(), _black.period_at_position().to);
 		} else {
-			emit_video(black_player_video_frame(Eyes::BOTH), _black.position());
+			use_video(black_player_video_frame(Eyes::BOTH), _black.position(), _black.period_at_position().to);
 		}
 		_black.set_position (_black.position() + one_video_frame());
 		break;
@@ -877,24 +879,13 @@ Player::pass ()
 	}
 
 	if (done) {
+		emit_video_until(film->length());
+
 		if (_shuffler) {
 			_shuffler->flush ();
 		}
 		for (auto const& i: _delay) {
-			do_emit_video(i.first, i.second);
-		}
-
-		/* Perhaps we should have Empty entries for both eyes in the 3D case (somehow).
-		 * However, if we have L and R video files, and one is shorter than the other,
-		 * the fill code in ::video mostly takes care of filling in the gaps.
-		 * However, since it fills at the point when it knows there is more video coming
-		 * at time t (so it should fill any gap up to t) it can't do anything right at the
-		 * end.  This is particularly bad news if the last frame emitted is a LEFT
-		 * eye, as the MXF writer will complain about the 3D sequence being wrong.
-		 * Here's a hack to workaround that particular case.
-		 */
-		if (_next_video_eyes && _next_video_time && *_next_video_eyes == Eyes::RIGHT) {
-			do_emit_video (black_player_video_frame(Eyes::RIGHT), *_next_video_time);
+			emit_video(i.first, i.second);
 		}
 	}
 
@@ -954,15 +945,60 @@ Player::open_subtitles_for_frame (DCPTime time) const
 }
 
 
-static
-Eyes
-increment_eyes (Eyes e)
+void
+Player::emit_video_until(DCPTime time)
 {
-	if (e == Eyes::LEFT) {
-		return Eyes::RIGHT;
-	}
+	auto frame = [this](shared_ptr<PlayerVideo> pv, DCPTime time) {
+		/* We need a delay to give a little wiggle room to ensure that relevant subtitles arrive at the
+		   player before the video that requires them.
+		*/
+		_delay.push_back(make_pair(pv, time));
+
+		if (pv->eyes() == Eyes::BOTH || pv->eyes() == Eyes::RIGHT) {
+			_next_video_time = time + one_video_frame();
+		}
+
+		if (_delay.size() < 3) {
+			return;
+		}
 
-	return Eyes::LEFT;
+		auto to_do = _delay.front();
+		_delay.pop_front();
+		emit_video(to_do.first, to_do.second);
+	};
+
+	auto const age_threshold = one_video_frame() * 2;
+
+	while (_next_video_time.get_value_or({}) < time) {
+		auto left = _last_video[Eyes::LEFT];
+		auto right = _last_video[Eyes::RIGHT];
+		auto both = _last_video[Eyes::BOTH];
+
+		auto const next = _next_video_time.get_value_or({});
+
+		if (
+			left.first &&
+			right.first &&
+			(!both.first || (left.second >= both.second && right.second >= both.second)) &&
+			(left.second - next) < age_threshold &&
+			(right.second - next) < age_threshold
+		   ) {
+			frame(left.first, next);
+			frame(right.first, next);
+		} else if (both.first && (both.second - next) < age_threshold) {
+			frame(both.first, next);
+			LOG_DEBUG_PLAYER("Content %1 selected for DCP %2 (age %3)", to_string(both.second), to_string(next), to_string(both.second - next));
+		} else {
+			auto film = _film.lock();
+			if (film && film->three_d()) {
+				frame(black_player_video_frame(Eyes::LEFT), next);
+				frame(black_player_video_frame(Eyes::RIGHT), next);
+			} else {
+				frame(black_player_video_frame(Eyes::BOTH), next);
+			}
+			LOG_DEBUG_PLAYER("Black selected for DCP %1", to_string(next));
+		}
+	}
 }
 
 
@@ -987,11 +1023,6 @@ Player::video (weak_ptr<Piece> weak_piece, ContentVideo video)
 		return;
 	}
 
-	FrameRateChange frc(film, piece->content);
-	if (frc.skip && (video.frame % 2) == 1) {
-		return;
-	}
-
 	vector<Eyes> eyes_to_emit;
 
 	if (!film->three_d()) {
@@ -1014,15 +1045,11 @@ Player::video (weak_ptr<Piece> weak_piece, ContentVideo video)
 		eyes_to_emit = { video.eyes };
 	}
 
-	/* Time of the first frame we will emit */
-	DCPTime const time = content_video_to_dcp (piece, video.frame);
-	LOG_DEBUG_PLAYER("Received video frame %1 at %2", video.frame, to_string(time));
+	/* Time of the frame we just received within the DCP */
+	auto const time = content_time_to_dcp(piece, video.time);
+	LOG_DEBUG_PLAYER("Received video frame %1 %2 eyes %3", to_string(video.time), to_string(time), static_cast<int>(video.eyes));
 
-	/* Discard if it's before the content's period or the last accurate seek.  We can't discard
-	   if it's after the content's period here as in that case we still need to fill any gap between
-	   `now' and the end of the content's period.
-	*/
-	if (time < piece->content->position() || (_next_video_time && time < *_next_video_time)) {
+	if (time < piece->content->position()) {
 		return;
 	}
 
@@ -1035,88 +1062,45 @@ Player::video (weak_ptr<Piece> weak_piece, ContentVideo video)
 		return;
 	}
 
-	/* Fill gaps that we discover now that we have some video which needs to be emitted.
-	   This is where we need to fill to.
-	*/
-	DCPTime fill_to = min(time, piece->content->end(film));
-
-	if (_next_video_time) {
-		DCPTime fill_from = max (*_next_video_time, piece->content->position());
-
-		/* Fill if we have more than half a frame to do */
-		if ((fill_to - fill_from) > one_video_frame() / 2) {
-			auto last = _last_video.find (weak_piece);
-			if (film->three_d()) {
-				auto fill_to_eyes = eyes_to_emit[0];
-				if (fill_to_eyes == Eyes::BOTH) {
-					fill_to_eyes = Eyes::LEFT;
-				}
-				if (fill_to == piece->content->end(film)) {
-					/* Don't fill after the end of the content */
-					fill_to_eyes = Eyes::LEFT;
-				}
-				auto j = fill_from;
-				auto eyes = _next_video_eyes.get_value_or(Eyes::LEFT);
-				if (eyes == Eyes::BOTH) {
-					eyes = Eyes::LEFT;
-				}
-				while (j < fill_to || eyes != fill_to_eyes) {
-					if (last != _last_video.end()) {
-						LOG_DEBUG_PLAYER("Fill using last video at %1 in 3D mode", to_string(j));
-						auto copy = last->second->shallow_copy();
-						copy->set_eyes (eyes);
-						emit_video (copy, j);
-					} else {
-						LOG_DEBUG_PLAYER("Fill using black at %1 in 3D mode", to_string(j));
-						emit_video (black_player_video_frame(eyes), j);
-					}
-					if (eyes == Eyes::RIGHT) {
-						j += one_video_frame();
-					}
-					eyes = increment_eyes (eyes);
-				}
-			} else {
-				for (DCPTime j = fill_from; j < fill_to; j += one_video_frame()) {
-					if (last != _last_video.end()) {
-						emit_video (last->second, j);
-					} else {
-						emit_video (black_player_video_frame(Eyes::BOTH), j);
-					}
-				}
-			}
-		}
+	if (!_next_video_time) {
+		_next_video_time = time.round(film->video_frame_rate());
 	}
 
 	auto const content_video = piece->content->video;
 
 	for (auto eyes: eyes_to_emit) {
-		_last_video[weak_piece] = std::make_shared<PlayerVideo>(
-			video.image,
-			content_video->actual_crop(),
-			content_video->fade(film, video.frame),
-			scale_for_display(
-				content_video->scaled_size(film->frame_size()),
+		use_video(
+			std::make_shared<PlayerVideo>(
+				video.image,
+				content_video->actual_crop(),
+				content_video->fade(film, video.time),
+				scale_for_display(
+					content_video->scaled_size(film->frame_size()),
+					_video_container_size,
+					film->frame_size(),
+					content_video->pixel_quanta()
+					),
 				_video_container_size,
-				film->frame_size(),
-				content_video->pixel_quanta()
+				eyes,
+				video.part,
+				content_video->colour_conversion(),
+				content_video->range(),
+				piece->content,
+				video.time,
+				false
 				),
-			_video_container_size,
-			eyes,
-			video.part,
-			content_video->colour_conversion(),
-			content_video->range(),
-			piece->content,
-			video.frame,
-			false
+			time,
+			piece->content->end(film)
 			);
+	}
+}
 
-		DCPTime t = time;
-		for (int i = 0; i < frc.repeat; ++i) {
-			if (t < piece->content->end(film)) {
-				emit_video (_last_video[weak_piece], t);
-			}
-			t += one_video_frame ();
-		}
+void
+Player::use_video(shared_ptr<PlayerVideo> pv, DCPTime time, DCPTime end)
+{
+	_last_video[pv->eyes()] = { pv, time };
+	if (pv->eyes() != Eyes::LEFT) {
+		emit_video_until(std::min(time + one_video_frame() / 2, end));
 	}
 }
 
@@ -1398,18 +1382,18 @@ Player::seek (DCPTime time, bool accurate)
 
 	if (accurate) {
 		_next_video_time = time;
-		_next_video_eyes = Eyes::LEFT;
 		_next_audio_time = time;
 	} else {
 		_next_video_time = boost::none;
-		_next_video_eyes = boost::none;
 		_next_audio_time = boost::none;
 	}
 
 	_black.set_position (time);
 	_silent.set_position (time);
 
-	_last_video.clear ();
+	_last_video[Eyes::LEFT] = {};
+	_last_video[Eyes::RIGHT] = {};
+	_last_video[Eyes::BOTH] = {};
 
 	for (auto& state: _stream_states) {
 		state.second.last_push_end = boost::none;
@@ -1418,33 +1402,7 @@ Player::seek (DCPTime time, bool accurate)
 
 
 void
-Player::emit_video (shared_ptr<PlayerVideo> pv, DCPTime time)
-{
-	auto film = _film.lock();
-	DCPOMATIC_ASSERT(film);
-
-	/* We need a delay to give a little wiggle room to ensure that relevant subtitles arrive at the
-	   player before the video that requires them.
-	*/
-	_delay.push_back (make_pair (pv, time));
-
-	if (pv->eyes() == Eyes::BOTH || pv->eyes() == Eyes::RIGHT) {
-		_next_video_time = time + one_video_frame();
-	}
-	_next_video_eyes = increment_eyes (pv->eyes());
-
-	if (_delay.size() < 3) {
-		return;
-	}
-
-	auto to_do = _delay.front();
-	_delay.pop_front();
-	do_emit_video (to_do.first, to_do.second);
-}
-
-
-void
-Player::do_emit_video (shared_ptr<PlayerVideo> pv, DCPTime time)
+Player::emit_video(shared_ptr<PlayerVideo> pv, DCPTime time)
 {
 	if (pv->eyes() == Eyes::BOTH || pv->eyes() == Eyes::RIGHT) {
 		std::for_each(_active_texts.begin(), _active_texts.end(), [time](ActiveText& a) { a.clear_before(time); });