8 files changed, 65 insertions, 25 deletions

diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc
index 636cc8ed2..ad7d3e112 100644
--- a/src/lib/dcp_decoder.cc
+++ b/src/lib/dcp_decoder.cc
@@ -80,8 +80,11 @@ DCPDecoder::DCPDecoder (shared_ptr<const Film> film, shared_ptr<const DCPContent
 			audio = make_shared<AudioDecoder>(this, content->audio, fast);
 		}
 		for (auto i: content->text) {
-			/* XXX: this time here should be the time of the first subtitle, not 0 */
-			text.push_back (make_shared<TextDecoder>(this, i, ContentTime()));
+			text.push_back (make_shared<TextDecoder>(this, i));
+			/* We should really call maybe_set_position() on this TextDecoder to set the time
+			 * of the first subtitle, but it probably doesn't matter since we'll always
+			 * have regularly occurring video (and maybe audio) content.
+			 */
 		}
 		if (content->atmos) {
 			atmos = make_shared<AtmosDecoder>(this, content);
diff --git a/src/lib/dcp_subtitle_decoder.cc b/src/lib/dcp_subtitle_decoder.cc
index 83faa1e93..4743ecf42 100644
--- a/src/lib/dcp_subtitle_decoder.cc
+++ b/src/lib/dcp_subtitle_decoder.cc
@@ -46,11 +46,8 @@ DCPSubtitleDecoder::DCPSubtitleDecoder (shared_ptr<const Film> film, shared_ptr<
 	_subtitles = c->subtitles ();
 	_next = _subtitles.begin ();
 
-	ContentTime first;
-	if (_next != _subtitles.end()) {
-		first = content_time_period(*_next).from;
-	}
-	text.push_back (make_shared<TextDecoder>(this, content->only_text(), first));
+	text.push_back (make_shared<TextDecoder>(this, content->only_text()));
+	update_position();
 }
 
 
@@ -64,6 +61,8 @@ DCPSubtitleDecoder::seek (ContentTime time, bool accurate)
 	while (i != _subtitles.end() && ContentTime::from_seconds ((*_next)->in().as_seconds()) < time) {
 		++i;
 	}
+
+	update_position();
 }
 
 
@@ -104,6 +103,9 @@ DCPSubtitleDecoder::pass ()
 	}
 
 	only_text()->emit_plain (p, s);
+
+	update_position();
+
 	return false;
 }
 
@@ -129,3 +131,14 @@ DCPSubtitleDecoder::first () const
 	return ContentTime::from_seconds(_subtitles[0]->in().as_seconds());
 }
 
+
+void
+DCPSubtitleDecoder::update_position()
+{
+	if (_next != _subtitles.end()) {
+		only_text()->maybe_set_position(
+			ContentTime::from_seconds((*_next)->in().as_seconds())
+			);
+	}
+}
+
diff --git a/src/lib/dcp_subtitle_decoder.h b/src/lib/dcp_subtitle_decoder.h
index 4f5964c6f..16f89b530 100644
--- a/src/lib/dcp_subtitle_decoder.h
+++ b/src/lib/dcp_subtitle_decoder.h
@@ -38,6 +38,7 @@ public:
 
 private:
 	dcpomatic::ContentTimePeriod content_time_period (std::shared_ptr<const dcp::Subtitle> s) const;
+	void update_position();
 
 	std::vector<std::shared_ptr<const dcp::Subtitle>> _subtitles;
 	std::vector<std::shared_ptr<const dcp::Subtitle>>::const_iterator _next;
diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index bc7d28d7d..82eccb576 100644
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -93,8 +93,10 @@ FFmpegDecoder::FFmpegDecoder (shared_ptr<const Film> film, shared_ptr<const FFmp
 	}
 
 	if (c->only_text()) {
-		/* XXX: this time here should be the time of the first subtitle, not 0 */
-		text.push_back (make_shared<TextDecoder>(this, c->only_text(), ContentTime()));
+		text.push_back (make_shared<TextDecoder>(this, c->only_text()));
+		/* XXX: we should be calling maybe_set_position() on this TextDecoder, but we can't easily find
+		 * the time of the first subtitle at this point.
+		 */
 	}
 
 	for (auto i: c->ffmpeg_audio_streams()) {
diff --git a/src/lib/string_text_file_decoder.cc b/src/lib/string_text_file_decoder.cc
index d5f320a27..2ec0ec1cb 100644
--- a/src/lib/string_text_file_decoder.cc
+++ b/src/lib/string_text_file_decoder.cc
@@ -40,11 +40,8 @@ StringTextFileDecoder::StringTextFileDecoder (shared_ptr<const Film> film, share
 	, StringTextFile (content)
 	, _next (0)
 {
-	ContentTime first;
-	if (!_subtitles.empty()) {
-		first = content_time_period(_subtitles[0]).from;
-	}
-	text.push_back (make_shared<TextDecoder>(this, content->only_text(), first));
+	text.push_back (make_shared<TextDecoder>(this, content->only_text()));
+	update_position();
 }
 
 
@@ -65,6 +62,8 @@ StringTextFileDecoder::seek (ContentTime time, bool accurate)
 	while (_next < _subtitles.size() && ContentTime::from_seconds (_subtitles[_next].from.all_as_seconds ()) < time) {
 		++_next;
 	}
+
+	update_position();
 }
 
 
@@ -79,6 +78,9 @@ StringTextFileDecoder::pass ()
 	only_text()->emit_plain (p, _subtitles[_next]);
 
 	++_next;
+
+	update_position();
+
 	return false;
 }
 
@@ -91,3 +93,15 @@ StringTextFileDecoder::content_time_period (sub::Subtitle s) const
 		ContentTime::from_seconds (s.to.all_as_seconds())
 		);
 }
+
+
+void
+StringTextFileDecoder::update_position ()
+{
+	if (_next < _subtitles.size()) {
+		only_text()->maybe_set_position(
+			ContentTime::from_seconds(_subtitles[_next].from.all_as_seconds())
+			);
+	}
+}
+
diff --git a/src/lib/string_text_file_decoder.h b/src/lib/string_text_file_decoder.h
index 8fbf09608..c58021acb 100644
--- a/src/lib/string_text_file_decoder.h
+++ b/src/lib/string_text_file_decoder.h
@@ -36,6 +36,7 @@ public:
 
 private:
 	dcpomatic::ContentTimePeriod content_time_period (sub::Subtitle s) const;
+	void update_position();
 
 	size_t _next;
 };
diff --git a/src/lib/text_decoder.cc b/src/lib/text_decoder.cc
index e3b7d9958..b1b6cbcc4 100644
--- a/src/lib/text_decoder.cc
+++ b/src/lib/text_decoder.cc
@@ -41,12 +41,10 @@ using namespace dcpomatic;
 
 TextDecoder::TextDecoder (
 	Decoder* parent,
-	shared_ptr<const TextContent> content,
-	ContentTime first
+	shared_ptr<const TextContent> content
 	)
 	: DecoderPart (parent)
 	, _content (content)
-	, _position (first)
 {
 
 }
@@ -63,7 +61,7 @@ void
 TextDecoder::emit_bitmap_start (ContentBitmapText const& bitmap)
 {
 	BitmapStart (bitmap);
-	_position = bitmap.from();
+	maybe_set_position(bitmap.from());
 }
 
 
@@ -116,7 +114,7 @@ TextDecoder::emit_plain_start (ContentTime from, vector<dcp::SubtitleString> sub
 	}
 
 	PlainStart(ContentStringText(from, string_texts));
-	_position = from;
+	maybe_set_position(from);
 }
 
 
@@ -274,7 +272,7 @@ TextDecoder::emit_plain_start (ContentTime from, sub::Subtitle const & sub_subti
 	}
 
 	PlainStart(ContentStringText(from, string_texts));
-	_position = from;
+	maybe_set_position(from);
 }
 
 
@@ -318,3 +316,13 @@ TextDecoder::seek ()
 {
 	_position = ContentTime ();
 }
+
+
+void
+TextDecoder::maybe_set_position (dcpomatic::ContentTime position)
+{
+	if (!_position || position > *_position) {
+		_position = position;
+	}
+}
+
diff --git a/src/lib/text_decoder.h b/src/lib/text_decoder.h
index 607d9e194..5f01c5b1f 100644
--- a/src/lib/text_decoder.h
+++ b/src/lib/text_decoder.h
@@ -42,11 +42,7 @@ class Image;
 class TextDecoder : public DecoderPart
 {
 public:
-	TextDecoder (
-		Decoder* parent,
-		std::shared_ptr<const TextContent>,
-		dcpomatic::ContentTime first
-		);
+	TextDecoder (Decoder* parent, std::shared_ptr<const TextContent>);
 
 	boost::optional<dcpomatic::ContentTime> position (std::shared_ptr<const Film>) const override {
 		return _position;
@@ -60,6 +56,8 @@ public:
 	void emit_plain (dcpomatic::ContentTimePeriod period, sub::Subtitle const & subtitle);
 	void emit_stop (dcpomatic::ContentTime to);
 
+	void maybe_set_position (dcpomatic::ContentTime position);
+
 	void seek () override;
 
 	std::shared_ptr<const TextContent> content () const {