Initial work on removing storage of subtitle times.

11 files changed, 139 insertions, 357 deletions

diff --git a/src/lib/content_subtitle.h b/src/lib/content_subtitle.h
index 8c0f29f4e..8751d56cb 100644
--- a/src/lib/content_subtitle.h
+++ b/src/lib/content_subtitle.h
@@ -32,23 +32,23 @@ class Image;
 class ContentSubtitle
 {
 public:
-	ContentSubtitle (ContentTimePeriod p)
-		: _period (p)
+	ContentSubtitle (ContentTime f)
+		: _from (f)
 	{}
 
-	ContentTimePeriod period () const {
-		return _period;
+	ContentTime from () const {
+		return _from;
 	}
 
 private:
-	ContentTimePeriod _period;
+	ContentTime _from;
 };
 
 class ContentImageSubtitle : public ContentSubtitle
 {
 public:
-	ContentImageSubtitle (ContentTimePeriod p, boost::shared_ptr<Image> im, dcpomatic::Rect<double> r)
-		: ContentSubtitle (p)
+	ContentImageSubtitle (ContentTime f, boost::shared_ptr<Image> im, dcpomatic::Rect<double> r)
+		: ContentSubtitle (f)
 		, sub (im, r)
 	{}
 
@@ -63,8 +63,8 @@ public:
 class ContentTextSubtitle : public ContentSubtitle
 {
 public:
-	ContentTextSubtitle (ContentTimePeriod p, std::list<dcp::SubtitleString> s)
-		: ContentSubtitle (p)
+	ContentTextSubtitle (ContentTime f, std::list<dcp::SubtitleString> s)
+		: ContentSubtitle (f)
 		, subs (s)
 	{}
 
diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index 4635cd5a3..1c886284b 100644
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -81,6 +81,7 @@ using dcp::Size;
 FFmpegDecoder::FFmpegDecoder (shared_ptr<const FFmpegContent> c, shared_ptr<Log> log)
 	: FFmpeg (c)
 	, _log (log)
+	, _have_current_subtitle (false)
 {
 	if (c->video) {
 		video.reset (new VideoDecoder (this, c, log));
@@ -346,6 +347,8 @@ FFmpegDecoder::seek (ContentTime time, bool accurate)
 	if (subtitle_codec_context ()) {
 		avcodec_flush_buffers (subtitle_codec_context ());
 	}
+
+	_have_current_subtitle = false;
 }
 
 void
@@ -477,10 +480,18 @@ FFmpegDecoder::decode_subtitle_packet ()
 		return;
 	}
 
+	/* Stop any current subtitle, either at the time it was supposed to stop, or now if now is sooner */
+	if (_have_current_subtitle) {
+		if (_current_subtitle_to) {
+			subtitle->emit_stop (min(*_current_subtitle_to, subtitle_period(sub).from + _pts_offset));
+		} else {
+			subtitle->emit_stop (subtitle_period(sub).from + _pts_offset);
+		}
+		_have_current_subtitle = false;
+	}
+
 	if (sub.num_rects <= 0) {
-		/* Sometimes we get an empty AVSubtitle, which is used by some codecs to
-		   indicate that the previous subtitle should stop.  We can ignore it here.
-		*/
+		/* Nothing new in this subtitle */
 		return;
 	}
 
@@ -488,13 +499,12 @@ FFmpegDecoder::decode_subtitle_packet ()
 	   source that we may have chopped off for the DCP).
 	*/
 	FFmpegSubtitlePeriod sub_period = subtitle_period (sub);
-	ContentTimePeriod period;
-	period.from = sub_period.from + _pts_offset;
-	/* We can't trust the `to' time from sub_period as there are some decoders which
-	   give a sub_period time for `to' which is subsequently overridden by a `stop' subtitle;
-	   see also FFmpegExaminer.
-	*/
-	period.to = ffmpeg_content()->subtitle_stream()->find_subtitle_to (subtitle_id (sub));
+	ContentTime from;
+	from = sub_period.from + _pts_offset;
+	_have_current_subtitle = true;
+	if (sub_period.to) {
+		_current_subtitle_to = *sub_period.to + _pts_offset;
+	}
 
 	for (unsigned int i = 0; i < sub.num_rects; ++i) {
 		AVSubtitleRect const * rect = sub.rects[i];
@@ -503,13 +513,13 @@ FFmpegDecoder::decode_subtitle_packet ()
 		case SUBTITLE_NONE:
 			break;
 		case SUBTITLE_BITMAP:
-			decode_bitmap_subtitle (rect, period);
+			decode_bitmap_subtitle (rect, from);
 			break;
 		case SUBTITLE_TEXT:
 			cout << "XXX: SUBTITLE_TEXT " << rect->text << "\n";
 			break;
 		case SUBTITLE_ASS:
-			decode_ass_subtitle (rect->ass, period);
+			decode_ass_subtitle (rect->ass, from);
 			break;
 		}
 	}
@@ -518,7 +528,7 @@ FFmpegDecoder::decode_subtitle_packet ()
 }
 
 void
-FFmpegDecoder::decode_bitmap_subtitle (AVSubtitleRect const * rect, ContentTimePeriod period)
+FFmpegDecoder::decode_bitmap_subtitle (AVSubtitleRect const * rect, ContentTime from)
 {
 	/* Note RGBA is expressed little-endian, so the first byte in the word is R, second
 	   G, third B, fourth A.
@@ -587,11 +597,11 @@ FFmpegDecoder::decode_bitmap_subtitle (AVSubtitleRect const * rect, ContentTimeP
 		static_cast<double> (rect->h) / target_height
 		);
 
-	subtitle->emit_image (period, image, scaled_rect);
+	subtitle->emit_image_start (from, image, scaled_rect);
 }
 
 void
-FFmpegDecoder::decode_ass_subtitle (string ass, ContentTimePeriod period)
+FFmpegDecoder::decode_ass_subtitle (string ass, ContentTime from)
 {
 	/* We have no styles and no Format: line, so I'm assuming that FFmpeg
 	   produces a single format of Dialogue: lines...
@@ -607,6 +617,6 @@ FFmpegDecoder::decode_ass_subtitle (string ass, ContentTimePeriod period)
 	list<sub::RawSubtitle> raw = sub::SSAReader::parse_line (base, bits[9]);
 
 	BOOST_FOREACH (sub::Subtitle const & i, sub::collect<list<sub::Subtitle> > (raw)) {
-		subtitle->emit_text (period, i);
+		subtitle->emit_text_start (from, i);
 	}
 }
diff --git a/src/lib/ffmpeg_decoder.h b/src/lib/ffmpeg_decoder.h
index 65e4cf46a..bd7ba98b8 100644
--- a/src/lib/ffmpeg_decoder.h
+++ b/src/lib/ffmpeg_decoder.h
@@ -61,8 +61,8 @@ private:
 	void decode_audio_packet ();
 	void decode_subtitle_packet ();
 
-	void decode_bitmap_subtitle (AVSubtitleRect const * rect, ContentTimePeriod period);
-	void decode_ass_subtitle (std::string ass, ContentTimePeriod period);
+	void decode_bitmap_subtitle (AVSubtitleRect const * rect, ContentTime from);
+	void decode_ass_subtitle (std::string ass, ContentTime from);
 
 	void maybe_add_subtitle ();
 	boost::shared_ptr<AudioBuffers> deinterleave_audio (boost::shared_ptr<FFmpegAudioStream> stream) const;
@@ -73,4 +73,6 @@ private:
 	boost::mutex _filter_graphs_mutex;
 
 	ContentTime _pts_offset;
+	boost::optional<ContentTime> _current_subtitle_to;
+	bool _have_current_subtitle;
 };
diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc
index eb02ba17b..a145ae25c 100644
--- a/src/lib/ffmpeg_examiner.cc
+++ b/src/lib/ffmpeg_examiner.cc
@@ -91,24 +91,13 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 		}
 	}
 
-	if (job) {
-		if (_need_video_length) {
-			job->sub (_("Finding length and subtitles"));
-		} else if (!_subtitle_streams.empty()) {
-			job->sub (_("Finding subtitles"));
-		} else {
-			job->sub (_("Finding length"));
-		}
+	if (job && _need_video_length) {
+		job->sub (_("Finding length"));
 	}
 
 	/* Run through until we find:
 	 *   - the first video.
 	 *   - the first audio for each stream.
-	 *   - the subtitle periods for each stream.
-	 *
-	 * We have to note subtitle periods as otherwise we have no way of knowing
-	 * where we should look for subtitles (video and audio are always present,
-	 * so they are ok).
 	 */
 
 	int64_t const len = _file_group.length ();
@@ -143,41 +132,13 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 			}
 		}
 
-		for (size_t i = 0; i < _subtitle_streams.size(); ++i) {
-			if (_subtitle_streams[i]->uses_index (_format_context, _packet.stream_index)) {
-				subtitle_packet (context, _subtitle_streams[i]);
-			}
-		}
-
 		av_packet_unref (&_packet);
 
-		if (_first_video && got_all_audio && _subtitle_streams.empty ()) {
+		if (_first_video && got_all_audio) {
 			/* All done */
 			break;
 		}
 	}
-
-	/* Finish off any hanging subtitles at the end */
-	for (LastSubtitleMap::const_iterator i = _last_subtitle_start.begin(); i != _last_subtitle_start.end(); ++i) {
-		if (i->second) {
-			if (i->first->unknown_to (i->second->id)) {
-				i->first->set_subtitle_to (
-					i->second->id,
-					ContentTime::from_frames (video_length(), video_frame_rate().get_value_or (24))
-					);
-			}
-		}
-	}
-
-	/* We just added subtitles to our streams without taking the PTS offset into account;
-	   this is because we might not know the PTS offset when the first subtitle is seen.
-	   Now we know the PTS offset so we can apply it to those subtitles.
-	*/
-	if (has_video() && video_frame_rate()) {
-		BOOST_FOREACH (shared_ptr<FFmpegSubtitleStream> i, _subtitle_streams) {
-			i->add_offset (pts_offset (_audio_streams, _first_video, video_frame_rate().get()));
-		}
-	}
 }
 
 void
@@ -215,70 +176,6 @@ FFmpegExaminer::audio_packet (AVCodecContext* context, shared_ptr<FFmpegAudioStr
 	}
 }
 
-void
-FFmpegExaminer::subtitle_packet (AVCodecContext* context, shared_ptr<FFmpegSubtitleStream> stream)
-{
-	int frame_finished;
-	AVSubtitle sub;
-	if (avcodec_decode_subtitle2 (context, &sub, &frame_finished, &_packet) >= 0 && frame_finished) {
-		string id = subtitle_id (sub);
-		FFmpegSubtitlePeriod const period = subtitle_period (sub);
-		bool const starts_image = subtitle_starts_image (sub);
-
-		/* Some streams (notably DVB streams) have subtitles which have a specified end time
-		   but which are then stopped earlier than this by a zero-num_rect subtitle.
-		*/
-
-		LastSubtitleMap::iterator last = _last_subtitle_start.find (stream);
-		if (sub.num_rects == 0 && last != _last_subtitle_start.end() && last->second) {
-			/* Set (or fix) the `to' time for the last subtitle */
-			stream->set_subtitle_to (last->second->id, period.from);
-			_last_subtitle_start[stream] = optional<SubtitleStart> ();
-		} else if (sub.num_rects > 0) {
-			/* Add a subtitle; if we don't know the `to' time we set it to the from time and fix it later */
-			if (starts_image) {
-				stream->add_image_subtitle (id, ContentTimePeriod (period.from, period.to.get_value_or (period.from)));
-			} else {
-				stream->add_text_subtitle (id, ContentTimePeriod (period.from, period.to.get_value_or (period.from)));
-			}
-
-			_last_subtitle_start[stream] = SubtitleStart (id, starts_image, period.from);
-		}
-
-		for (unsigned int i = 0; i < sub.num_rects; ++i) {
-			if (sub.rects[i]->type == SUBTITLE_BITMAP) {
-#ifdef DCPOMATIC_HAVE_AVSUBTITLERECT_PICT
-				uint32_t* palette = (uint32_t *) sub.rects[i]->pict.data[1];
-				for (int j = 0; j < sub.rects[i]->nb_colors; ++j) {
-					RGBA rgba  (
-						(palette[j] & 0x00ff0000) >> 16,
-						(palette[j] & 0x0000ff00) >> 8,
-						(palette[j] & 0x000000ff) >> 0,
-						(palette[j] & 0xff000000) >> 24
-						);
-
-					stream->set_colour (rgba, rgba);
-				}
-#else
-				uint32_t* palette = (uint32_t *) sub.rects[i]->data[1];
-				for (int j = 0; j < sub.rects[i]->nb_colors; ++j) {
-					RGBA rgba  (
-						(palette[j] & 0x00ff0000) >> 16,
-						(palette[j] & 0x0000ff00) >> 8,
-						(palette[j] & 0x000000ff) >> 0,
-						(palette[j] & 0xff000000) >> 24
-						);
-
-					stream->set_colour (rgba, rgba);
-				}
-#endif
-			}
-		}
-
-		avsubtitle_free (&sub);
-	}
-}
-
 optional<ContentTime>
 FFmpegExaminer::frame_time (AVStream* s) const
 {
diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h
index c3dde84fe..a511a2fc1 100644
--- a/src/lib/ffmpeg_examiner.h
+++ b/src/lib/ffmpeg_examiner.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2017 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -74,7 +74,6 @@ public:
 private:
 	void video_packet (AVCodecContext *);
 	void audio_packet (AVCodecContext *, boost::shared_ptr<FFmpegAudioStream>);
-	void subtitle_packet (AVCodecContext *, boost::shared_ptr<FFmpegSubtitleStream>);
 
 	std::string stream_name (AVStream* s) const;
 	std::string subtitle_stream_name (AVStream* s) const;
diff --git a/src/lib/ffmpeg_subtitle_stream.cc b/src/lib/ffmpeg_subtitle_stream.cc
index 3935e23a9..c3c6c1f86 100644
--- a/src/lib/ffmpeg_subtitle_stream.cc
+++ b/src/lib/ffmpeg_subtitle_stream.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2017 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -38,54 +38,7 @@ using dcp::raw_convert;
 FFmpegSubtitleStream::FFmpegSubtitleStream (cxml::ConstNodePtr node, int version)
 	: FFmpegStream (node)
 {
-	if (version == 32) {
-		BOOST_FOREACH (cxml::NodePtr i, node->node_children ("Period")) {
-			/* In version 32 we assumed that from times were unique, so they were
-			   used as identifiers.  All subtitles were image subtitles.
-			*/
-			add_image_subtitle (
-				raw_convert<string> (i->string_child ("From")),
-				ContentTimePeriod (
-					ContentTime (i->number_child<ContentTime::Type> ("From")),
-					ContentTime (i->number_child<ContentTime::Type> ("To"))
-					)
-				);
-		}
-	} else {
-		/* In version 33 we use a hash of various parts of the subtitle as the id.
-		   <Subtitle> was initially used for image subtitles; later we have
-		   <ImageSubtitle> and <TextSubtitle>
-		*/
-		BOOST_FOREACH (cxml::NodePtr i, node->node_children ("Subtitle")) {
-			add_image_subtitle (
-				raw_convert<string> (i->string_child ("Id")),
-				ContentTimePeriod (
-					ContentTime (i->number_child<ContentTime::Type> ("From")),
-					ContentTime (i->number_child<ContentTime::Type> ("To"))
-					)
-				);
-		}
-
-		BOOST_FOREACH (cxml::NodePtr i, node->node_children ("ImageSubtitle")) {
-			add_image_subtitle (
-				raw_convert<string> (i->string_child ("Id")),
-				ContentTimePeriod (
-					ContentTime (i->number_child<ContentTime::Type> ("From")),
-					ContentTime (i->number_child<ContentTime::Type> ("To"))
-					)
-				);
-		}
-
-		BOOST_FOREACH (cxml::NodePtr i, node->node_children ("TextSubtitle")) {
-			add_text_subtitle (
-				raw_convert<string> (i->string_child ("Id")),
-				ContentTimePeriod (
-					ContentTime (i->number_child<ContentTime::Type> ("From")),
-					ContentTime (i->number_child<ContentTime::Type> ("To"))
-					)
-				);
-		}
-
+	if (version >= 33) {
 		BOOST_FOREACH (cxml::NodePtr i, node->node_children ("Colour")) {
 			_colours[RGBA(i->node_child("From"))] = RGBA (i->node_child("To"));
 		}
@@ -97,9 +50,6 @@ FFmpegSubtitleStream::as_xml (xmlpp::Node* root) const
 {
 	FFmpegStream::as_xml (root);
 
-	as_xml (root, _image_subtitles, "ImageSubtitle");
-	as_xml (root, _text_subtitles, "TextSubtitle");
-
 	for (map<RGBA, RGBA>::const_iterator i = _colours.begin(); i != _colours.end(); ++i) {
 		xmlpp::Node* node = root->add_child("Colour");
 		i->first.as_xml (node->add_child("From"));
@@ -107,76 +57,6 @@ FFmpegSubtitleStream::as_xml (xmlpp::Node* root) const
 	}
 }
 
-void
-FFmpegSubtitleStream::as_xml (xmlpp::Node* root, PeriodMap const & subs, string node_name) const
-{
-	for (PeriodMap::const_iterator i = subs.begin(); i != subs.end(); ++i) {
-		xmlpp::Node* node = root->add_child (node_name);
-		node->add_child("Id")->add_child_text (i->first);
-		node->add_child("From")->add_child_text (raw_convert<string> (i->second.from.get ()));
-		node->add_child("To")->add_child_text (raw_convert<string> (i->second.to.get ()));
-	}
-}
-
-void
-FFmpegSubtitleStream::add_image_subtitle (string id, ContentTimePeriod period)
-{
-	DCPOMATIC_ASSERT (_image_subtitles.find (id) == _image_subtitles.end ());
-	_image_subtitles[id] = period;
-}
-
-void
-FFmpegSubtitleStream::add_text_subtitle (string id, ContentTimePeriod period)
-{
-	DCPOMATIC_ASSERT (_text_subtitles.find (id) == _text_subtitles.end ());
-	_text_subtitles[id] = period;
-}
-
-ContentTime
-FFmpegSubtitleStream::find_subtitle_to (string id) const
-{
-	PeriodMap::const_iterator i = _image_subtitles.find (id);
-	if (i != _image_subtitles.end ()) {
-		return i->second.to;
-	}
-
-	i = _text_subtitles.find (id);
-	DCPOMATIC_ASSERT (i != _text_subtitles.end ());
-	return i->second.to;
-}
-
-/** @param id Subtitle id.
- *  @return true if the `from' and `to' times for this id are equal, which indicates
- *  that the `to' time is unknown.
- */
-bool
-FFmpegSubtitleStream::unknown_to (string id) const
-{
-	PeriodMap::const_iterator i = _image_subtitles.find (id);
-	if (i != _image_subtitles.end ()) {
-		return i->second.from == i->second.to;
-	}
-
-	i = _text_subtitles.find (id);
-	DCPOMATIC_ASSERT (i != _text_subtitles.end ());
-	return i->second.from == i->second.to;
-}
-
-/** Add some offset to all the times in the stream */
-void
-FFmpegSubtitleStream::add_offset (ContentTime offset)
-{
-	for (PeriodMap::iterator i = _image_subtitles.begin(); i != _image_subtitles.end(); ++i) {
-		i->second.from += offset;
-		i->second.to += offset;
-	}
-
-	for (PeriodMap::iterator i = _text_subtitles.begin(); i != _text_subtitles.end(); ++i) {
-		i->second.from += offset;
-		i->second.to += offset;
-	}
-}
-
 map<RGBA, RGBA>
 FFmpegSubtitleStream::colours () const
 {
@@ -188,31 +68,3 @@ FFmpegSubtitleStream::set_colour (RGBA from, RGBA to)
 {
 	_colours[from] = to;
 }
-
-bool
-FFmpegSubtitleStream::has_text () const
-{
-	return !_text_subtitles.empty ();
-}
-
-bool
-FFmpegSubtitleStream::has_image () const
-{
-	return !_image_subtitles.empty ();
-}
-
-void
-FFmpegSubtitleStream::set_subtitle_to (string id, ContentTime to)
-{
-	PeriodMap::iterator i = _image_subtitles.find (id);
-	if (i != _image_subtitles.end ()) {
-		i->second.to = to;
-	} else {
-		i = _text_subtitles.find (id);
-		if (i != _text_subtitles.end ()) {
-			i->second.to = to;
-		} else {
-			DCPOMATIC_ASSERT (false);
-		}
-	}
-}
diff --git a/src/lib/ffmpeg_subtitle_stream.h b/src/lib/ffmpeg_subtitle_stream.h
index 8b0274a5d..064c72f8d 100644
--- a/src/lib/ffmpeg_subtitle_stream.h
+++ b/src/lib/ffmpeg_subtitle_stream.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2017 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -34,25 +34,9 @@ public:
 
 	void as_xml (xmlpp::Node *) const;
 
-	void add_image_subtitle (std::string id, ContentTimePeriod period);
-	void add_text_subtitle (std::string id, ContentTimePeriod period);
-	void set_subtitle_to (std::string id, ContentTime to);
-	ContentTime find_subtitle_to (std::string id) const;
-	bool unknown_to (std::string id) const;
-	void add_offset (ContentTime offset);
 	void set_colour (RGBA from, RGBA to);
 	std::map<RGBA, RGBA> colours () const;
 
-	bool has_text () const;
-	bool has_image () const;
-
 private:
-
-	typedef std::map<std::string, ContentTimePeriod> PeriodMap;
-
-	void as_xml (xmlpp::Node *, PeriodMap const & subs, std::string node) const;
-
-	PeriodMap _image_subtitles;
-	PeriodMap _text_subtitles;
 	std::map<RGBA, RGBA> _colours;
 };
diff --git a/src/lib/player.cc b/src/lib/player.cc
index a090b7c26..72922d70a 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -143,8 +143,9 @@ Player::setup_pieces ()
 		}
 
 		if (decoder->subtitle) {
-			decoder->subtitle->ImageData.connect (bind (&Player::image_subtitle, this, weak_ptr<Piece> (piece), _1));
-			decoder->subtitle->TextData.connect (bind (&Player::text_subtitle, this, weak_ptr<Piece> (piece), _1));
+			decoder->subtitle->ImageStart.connect (bind (&Player::image_subtitle_start, this, weak_ptr<Piece> (piece), _1));
+			decoder->subtitle->TextStart.connect (bind (&Player::text_subtitle_start, this, weak_ptr<Piece> (piece), _1));
+			decoder->subtitle->Stop.connect (bind (&Player::subtitle_stop, this, weak_ptr<Piece> (piece), _1));
 		}
 	}
 
@@ -632,21 +633,28 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
 
 	optional<PositionImage> subtitles;
 
-	for (list<pair<PlayerSubtitles, DCPTimePeriod> >::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
+	for (ActiveSubtitles::const_iterator i = _active_subtitles.begin(); i != _active_subtitles.end(); ++i) {
 
-		if (!i->second.overlap (period)) {
+		shared_ptr<Piece> sub_piece = i->first.lock ();
+		if (!sub_piece) {
 			continue;
 		}
 
+		if (!sub_piece->content->subtitle->use() || (!_always_burn_subtitles && !piece->content->subtitle->burn())) {
+			continue;
+		}
+
+		pair<PlayerSubtitles, DCPTime> sub = i->second;
+
 		list<PositionImage> sub_images;
 
 		/* Image subtitles */
-		list<PositionImage> c = transform_image_subtitles (i->first.image);
+		list<PositionImage> c = transform_image_subtitles (sub.first.image);
 		copy (c.begin(), c.end(), back_inserter (sub_images));
 
 		/* Text subtitles (rendered to an image) */
-		if (!i->first.text.empty ()) {
-			list<PositionImage> s = render_subtitles (i->first.text, i->first.fonts, _video_container_size, time);
+		if (!sub.first.text.empty ()) {
+			list<PositionImage> s = render_subtitles (sub.first.text, sub.first.fonts, _video_container_size, time);
 			copy (s.begin (), s.end (), back_inserter (sub_images));
 		}
 
@@ -683,19 +691,6 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
 	Video (_last_video, time);
 
 	_last_video_time = time + one_video_frame ();
-
-	/* Discard any subtitles we no longer need */
-
-	for (list<pair<PlayerSubtitles, DCPTimePeriod> >::iterator i = _subtitles.begin (); i != _subtitles.end(); ) {
-		list<pair<PlayerSubtitles, DCPTimePeriod> >::iterator tmp = i;
-		++tmp;
-
-		if (i->second.to < time) {
-			_subtitles.erase (i);
-		}
-
-		i = tmp;
-	}
 }
 
 void
@@ -825,7 +820,7 @@ Player::audio (weak_ptr<Piece> wp, AudioStreamPtr stream, ContentAudio content_a
 }
 
 void
-Player::image_subtitle (weak_ptr<Piece> wp, ContentImageSubtitle subtitle)
+Player::image_subtitle_start (weak_ptr<Piece> wp, ContentImageSubtitle subtitle)
 {
 	shared_ptr<Piece> piece = wp.lock ();
 	if (!piece) {
@@ -846,17 +841,13 @@ Player::image_subtitle (weak_ptr<Piece> wp, ContentImageSubtitle subtitle)
 
 	PlayerSubtitles ps;
 	ps.image.push_back (subtitle.sub);
-	DCPTimePeriod period (content_time_to_dcp (piece, subtitle.period().from), content_time_to_dcp (piece, subtitle.period().to));
+	DCPTime from (content_time_to_dcp (piece, subtitle.from()));
 
-	if (piece->content->subtitle->use() && (piece->content->subtitle->burn() || _always_burn_subtitles)) {
-		_subtitles.push_back (make_pair (ps, period));
-	} else {
-		Subtitle (ps, period);
-	}
+	_active_subtitles[wp] = make_pair (ps, from);
 }
 
 void
-Player::text_subtitle (weak_ptr<Piece> wp, ContentTextSubtitle subtitle)
+Player::text_subtitle_start (weak_ptr<Piece> wp, ContentTextSubtitle subtitle)
 {
 	shared_ptr<Piece> piece = wp.lock ();
 	if (!piece) {
@@ -864,7 +855,7 @@ Player::text_subtitle (weak_ptr<Piece> wp, ContentTextSubtitle subtitle)
 	}
 
 	PlayerSubtitles ps;
-	DCPTimePeriod const period (content_time_to_dcp (piece, subtitle.period().from), content_time_to_dcp (piece, subtitle.period().to));
+	DCPTime const from (content_time_to_dcp (piece, subtitle.from()));
 
 	BOOST_FOREACH (dcp::SubtitleString s, subtitle.subs) {
 		s.set_h_position (s.h_position() + piece->content->subtitle->x_offset ());
@@ -886,17 +877,31 @@ Player::text_subtitle (weak_ptr<Piece> wp, ContentTextSubtitle subtitle)
 			s.set_aspect_adjust (xs / ys);
 		}
 
-		s.set_in (dcp::Time(period.from.seconds(), 1000));
-		s.set_out (dcp::Time(period.to.seconds(), 1000));
+		s.set_in (dcp::Time(from.seconds(), 1000));
 		ps.text.push_back (SubtitleString (s, piece->content->subtitle->outline_width()));
 		ps.add_fonts (piece->content->subtitle->fonts ());
 	}
 
-	if (piece->content->subtitle->use() && (piece->content->subtitle->burn() || _always_burn_subtitles)) {
-		_subtitles.push_back (make_pair (ps, period));
-	} else {
-		Subtitle (ps, period);
+	_active_subtitles[wp] = make_pair (ps, from);
+}
+
+void
+Player::subtitle_stop (weak_ptr<Piece> wp, ContentTime to)
+{
+	if (_active_subtitles.find (wp) == _active_subtitles.end ()) {
+		return;
 	}
+
+	shared_ptr<Piece> piece = wp.lock ();
+	if (!piece) {
+		return;
+	}
+
+	if (piece->content->subtitle->use() && !_always_burn_subtitles && !piece->content->subtitle->burn()) {
+		Subtitle (_active_subtitles[wp].first, DCPTimePeriod (_active_subtitles[wp].second, content_time_to_dcp (piece, to)));
+	}
+
+	_active_subtitles.erase (wp);
 }
 
 void
@@ -912,6 +917,7 @@ Player::seek (DCPTime time, bool accurate)
 	}
 
 	_audio_merger.clear ();
+	_active_subtitles.clear ();
 
 	BOOST_FOREACH (shared_ptr<Piece> i, _pieces) {
 		i->done = false;
diff --git a/src/lib/player.h b/src/lib/player.h
index 684de53f5..eb6589b56 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -78,6 +78,9 @@ public:
 
 	boost::signals2::signal<void (boost::shared_ptr<PlayerVideo>, DCPTime)> Video;
 	boost::signals2::signal<void (boost::shared_ptr<AudioBuffers>, DCPTime)> Audio;
+	/** Emitted when a subtitle is ready.  This signal may be emitted considerably
+	 *  after the corresponding Video.
+	 */
 	boost::signals2::signal<void (PlayerSubtitles, DCPTimePeriod)> Subtitle;
 
 private:
@@ -104,15 +107,18 @@ private:
 	std::list<boost::shared_ptr<Piece> > overlaps (DCPTime from, DCPTime to, boost::function<bool (Content *)> valid);
 	void video (boost::weak_ptr<Piece>, ContentVideo);
 	void audio (boost::weak_ptr<Piece>, AudioStreamPtr, ContentAudio);
-	void image_subtitle (boost::weak_ptr<Piece>, ContentImageSubtitle);
-	void text_subtitle (boost::weak_ptr<Piece>, ContentTextSubtitle);
+	void image_subtitle_start (boost::weak_ptr<Piece>, ContentImageSubtitle);
+	void text_subtitle_start (boost::weak_ptr<Piece>, ContentTextSubtitle);
+	void subtitle_stop (boost::weak_ptr<Piece>, ContentTime);
 	boost::shared_ptr<Resampler> resampler (boost::shared_ptr<const AudioContent> content, AudioStreamPtr stream, bool create);
 	DCPTime one_video_frame () const;
 	void fill_video (DCPTimePeriod period);
 	void fill_audio (DCPTimePeriod period);
 	void audio_flush (boost::shared_ptr<Piece>, AudioStreamPtr stream);
 	void audio_transform (boost::shared_ptr<AudioContent> content, AudioStreamPtr stream, ContentAudio content_audio, DCPTime time);
-	std::pair<boost::shared_ptr<AudioBuffers>, DCPTime> discard_audio (boost::shared_ptr<const AudioBuffers> audio, DCPTime time, DCPTime discard_to) const;
+	std::pair<boost::shared_ptr<AudioBuffers>, DCPTime> discard_audio (
+		boost::shared_ptr<const AudioBuffers> audio, DCPTime time, DCPTime discard_to
+		) const;
 
 	boost::shared_ptr<const Film> _film;
 	boost::shared_ptr<const Playlist> _playlist;
@@ -164,7 +170,8 @@ private:
 	std::list<DCPTimePeriod> _no_video;
 	std::list<DCPTimePeriod> _no_audio;
 
-	std::list<std::pair<PlayerSubtitles, DCPTimePeriod> > _subtitles;
+	typedef std::map<boost::weak_ptr<Piece>, std::pair<PlayerSubtitles, DCPTime> > ActiveSubtitles;
+	ActiveSubtitles _active_subtitles;
 
 	boost::shared_ptr<AudioProcessor> _audio_processor;
 	typedef std::map<std::pair<boost::shared_ptr<const AudioContent>, AudioStreamPtr>, boost::shared_ptr<Resampler> > ResamplerMap;
diff --git a/src/lib/subtitle_decoder.cc b/src/lib/subtitle_decoder.cc
index 2a9434370..43ee4c457 100644
--- a/src/lib/subtitle_decoder.cc
+++ b/src/lib/subtitle_decoder.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2017 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -48,21 +48,21 @@ SubtitleDecoder::SubtitleDecoder (
 
 }
 
-/** Called by subclasses when an image subtitle is ready.
- *  @param period Period of the subtitle.
+/** Called by subclasses when an image subtitle is starting.
+ *  @param from From time of the subtitle.
  *  @param image Subtitle image.
  *  @param rect Area expressed as a fraction of the video frame that this subtitle
  *  is for (e.g. a width of 0.5 means the width of the subtitle is half the width
  *  of the video frame)
  */
 void
-SubtitleDecoder::emit_image (ContentTimePeriod period, shared_ptr<Image> image, dcpomatic::Rect<double> rect)
+SubtitleDecoder::emit_image_start (ContentTime from, shared_ptr<Image> image, dcpomatic::Rect<double> rect)
 {
-	ImageData (ContentImageSubtitle (period, image, rect));
+	ImageStart (ContentImageSubtitle (from, image, rect));
 }
 
 void
-SubtitleDecoder::emit_text (ContentTimePeriod period, list<dcp::SubtitleString> s)
+SubtitleDecoder::emit_text_start (ContentTime from, list<dcp::SubtitleString> s)
 {
 	/* We must escape < and > in strings, otherwise they might confuse our subtitle
 	   renderer (which uses some HTML-esque markup to do bold/italic etc.)
@@ -74,12 +74,12 @@ SubtitleDecoder::emit_text (ContentTimePeriod period, list<dcp::SubtitleString>
 		i.set_text (t);
 	}
 
-	TextData (ContentTextSubtitle (period, s));
-	_position = period.from;
+	TextStart (ContentTextSubtitle (from, s));
+	_position = from;
 }
 
 void
-SubtitleDecoder::emit_text (ContentTimePeriod period, sub::Subtitle const & subtitle)
+SubtitleDecoder::emit_text_start (ContentTime from, sub::Subtitle const & subtitle)
 {
 	/* See if our next subtitle needs to be placed on screen by us */
 	bool needs_placement = false;
@@ -170,8 +170,9 @@ SubtitleDecoder::emit_text (ContentTimePeriod period, sub::Subtitle const & subt
 					content()->colour(),
 					j.font_size.points (72 * 11),
 					1.0,
-					dcp::Time (period.from.seconds(), 1000),
-					dcp::Time (period.to.seconds(), 1000),
+					dcp::Time (from.seconds(), 1000),
+					/* XXX: hmm; this is a bit ugly (we don't know the to time yet) */
+					dcp::Time (),
 					0,
 					dcp::HALIGN_CENTER,
 					v_position,
@@ -187,5 +188,25 @@ SubtitleDecoder::emit_text (ContentTimePeriod period, sub::Subtitle const & subt
 		}
 	}
 
-	emit_text (period, out);
+	emit_text_start (from, out);
+}
+
+void
+SubtitleDecoder::emit_stop (ContentTime to)
+{
+	Stop (to);
+}
+
+void
+SubtitleDecoder::emit_text (ContentTimePeriod period, list<dcp::SubtitleString> s)
+{
+	emit_text_start (period.from, s);
+	emit_stop (period.to);
+}
+
+void
+SubtitleDecoder::emit_text (ContentTimePeriod period, sub::Subtitle const & s)
+{
+	emit_text_start (period.from, s);
+	emit_stop (period.to);
 }
diff --git a/src/lib/subtitle_decoder.h b/src/lib/subtitle_decoder.h
index 6d0479638..92a6266de 100644
--- a/src/lib/subtitle_decoder.h
+++ b/src/lib/subtitle_decoder.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2017 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -52,16 +52,20 @@ public:
 		return _position;
 	}
 
-	void emit_image (ContentTimePeriod period, boost::shared_ptr<Image> image, dcpomatic::Rect<double> rect);
+	void emit_image_start (ContentTime from, boost::shared_ptr<Image> image, dcpomatic::Rect<double> rect);
+	void emit_text_start (ContentTime from, std::list<dcp::SubtitleString> s);
+	void emit_text_start (ContentTime from, sub::Subtitle const & subtitle);
 	void emit_text (ContentTimePeriod period, std::list<dcp::SubtitleString> s);
 	void emit_text (ContentTimePeriod period, sub::Subtitle const & subtitle);
+	void emit_stop (ContentTime to);
 
 	boost::shared_ptr<const SubtitleContent> content () const {
 		return _content;
 	}
 
-	boost::signals2::signal<void (ContentImageSubtitle)> ImageData;
-	boost::signals2::signal<void (ContentTextSubtitle)> TextData;
+	boost::signals2::signal<void (ContentImageSubtitle)> ImageStart;
+	boost::signals2::signal<void (ContentTextSubtitle)> TextStart;
+	boost::signals2::signal<void (ContentTime)> Stop;
 
 private:
 	boost::shared_ptr<const SubtitleContent> _content;