Get closed caption view data from the butler, rather than the player.

You can't introduce the butler (so that the player is ahead of time)
and then ask the player what should be in the frame that is being
displayed "now"; the player will already have moved on.

10 files changed, 95 insertions, 80 deletions

diff --git a/src/lib/active_text.cc b/src/lib/active_text.cc
index 5f4440728..2988c04a3 100644
--- a/src/lib/active_text.cc
+++ b/src/lib/active_text.cc
@@ -30,38 +30,6 @@ using boost::weak_ptr;
 using boost::shared_ptr;
 using boost::optional;
 
-void
-ActiveText::add (DCPTimePeriod period, list<PlayerText>& pc, list<Period> p) const
-{
-	BOOST_FOREACH (Period i, p) {
-		DCPTimePeriod test (i.from, i.to.get_value_or(DCPTime::max()));
-		optional<DCPTimePeriod> overlap = period.overlap (test);
-		if (overlap && overlap->duration() > DCPTime(period.duration().get() / 2)) {
-			pc.push_back (i.subs);
-		}
-	}
-}
-
-list<PlayerText>
-ActiveText::get (DCPTimePeriod period) const
-{
-	boost::mutex::scoped_lock lm (_mutex);
-
-	list<PlayerText> ps;
-
-	for (Map::const_iterator i = _data.begin(); i != _data.end(); ++i) {
-
-		shared_ptr<const TextContent> caption = i->first.lock ();
-		if (!caption || !caption->use()) {
-			continue;
-		}
-
-		add (period, ps, i->second);
-	}
-
-	return ps;
-}
-
 /** Get the open captions that should be burnt into a given period.
  *  @param period Period of interest.
  *  @param always_burn_captions Always burn captions even if their content is not set to burn.
@@ -85,7 +53,13 @@ ActiveText::get_burnt (DCPTimePeriod period, bool always_burn_captions) const
 			continue;
 		}
 
-		add (period, ps, i->second);
+		BOOST_FOREACH (Period j, i->second) {
+			DCPTimePeriod test (j.from, j.to.get_value_or(DCPTime::max()));
+			optional<DCPTimePeriod> overlap = period.overlap (test);
+			if (overlap && overlap->duration() > DCPTime(period.duration().get() / 2)) {
+				ps.push_back (j.subs);
+			}
+		}
 	}
 
 	return ps;
diff --git a/src/lib/active_text.h b/src/lib/active_text.h
index 1dab7a867..ff4a1bd2f 100644
--- a/src/lib/active_text.h
+++ b/src/lib/active_text.h
@@ -37,7 +37,6 @@ class TextContent;
 class ActiveText : public boost::noncopyable
 {
 public:
-	std::list<PlayerText> get (DCPTimePeriod period) const;
 	std::list<PlayerText> get_burnt (DCPTimePeriod period, bool always_burn_captions) const;
 	void clear_before (DCPTime time);
 	void clear ();
@@ -63,8 +62,6 @@ private:
 
 	typedef std::map<boost::weak_ptr<const TextContent>, std::list<Period> > Map;
 
-	void add (DCPTimePeriod period, std::list<PlayerText>& pc, std::list<Period> p) const;
-
 	mutable boost::mutex _mutex;
 	Map _data;
 };
diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 6a1cc68fc..45cd5a9db 100644
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -63,6 +63,7 @@ Butler::Butler (shared_ptr<Player> player, shared_ptr<Log> log, AudioMapping aud
 {
 	_player_video_connection = _player->Video.connect (bind (&Butler::video, this, _1, _2));
 	_player_audio_connection = _player->Audio.connect (bind (&Butler::audio, this, _1, _2));
+	_player_text_connection = _player->Text.connect (bind (&Butler::text, this, _1, _2, _3));
 	_player_changed_connection = _player->Changed.connect (bind (&Butler::player_changed, this));
 	_thread = new boost::thread (bind (&Butler::thread, this));
 #ifdef DCPOMATIC_LINUX
@@ -203,6 +204,13 @@ Butler::get_video ()
 	return r;
 }
 
+optional<pair<PlayerText, DCPTimePeriod> >
+Butler::get_closed_caption ()
+{
+	boost::mutex::scoped_lock lm (_mutex);
+	return _closed_caption.get ();
+}
+
 void
 Butler::seek (DCPTime position, bool accurate)
 {
@@ -218,9 +226,10 @@ Butler::seek_unlocked (DCPTime position, bool accurate)
 	}
 
 	{
-		boost::mutex::scoped_lock lm (_video_audio_mutex);
+		boost::mutex::scoped_lock lm (_buffers_mutex);
 		_video.clear ();
 		_audio.clear ();
+		_closed_caption.clear ();
 	}
 
 	_finished = false;
@@ -253,7 +262,7 @@ Butler::video (shared_ptr<PlayerVideo> video, DCPTime time)
 
 	_prepare_service.post (bind (&Butler::prepare, this, weak_ptr<PlayerVideo>(video)));
 
-	boost::mutex::scoped_lock lm2 (_video_audio_mutex);
+	boost::mutex::scoped_lock lm2 (_buffers_mutex);
 	_video.put (video, time);
 }
 
@@ -268,7 +277,7 @@ Butler::audio (shared_ptr<AudioBuffers> audio, DCPTime time)
 		}
 	}
 
-	boost::mutex::scoped_lock lm2 (_video_audio_mutex);
+	boost::mutex::scoped_lock lm2 (_buffers_mutex);
 	_audio.put (remap (audio, _audio_channels, _audio_mapping), time);
 }
 
@@ -318,9 +327,10 @@ Butler::player_changed ()
 	}
 
 	{
-		boost::mutex::scoped_lock lm (_video_audio_mutex);
+		boost::mutex::scoped_lock lm (_buffers_mutex);
 		_video.clear ();
 		_audio.clear ();
+		_closed_caption.clear ();
 	}
 
 	_finished = false;
@@ -329,3 +339,14 @@ Butler::player_changed ()
 	seek_unlocked (seek_to, true);
 	_awaiting = seek_to;
 }
+
+void
+Butler::text (PlayerText pt, TextType type, DCPTimePeriod period)
+{
+	if (type != TEXT_CLOSED_CAPTION) {
+		return;
+	}
+
+	boost::mutex::scoped_lock lm2 (_buffers_mutex);
+	_closed_caption.put (make_pair(pt, period));
+}
diff --git a/src/lib/butler.h b/src/lib/butler.h
index 7e97bd3c0..0b926047a 100644
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -20,6 +20,7 @@
 
 #include "video_ring_buffers.h"
 #include "audio_ring_buffers.h"
+#include "text_ring_buffers.h"
 #include "audio_mapping.h"
 #include "exception_store.h"
 #include <boost/shared_ptr.hpp>
@@ -42,6 +43,7 @@ public:
 	void seek (DCPTime position, bool accurate);
 	std::pair<boost::shared_ptr<PlayerVideo>, DCPTime> get_video ();
 	boost::optional<DCPTime> get_audio (float* out, Frame frames);
+	boost::optional<std::pair<PlayerText, DCPTimePeriod> > get_closed_caption ();
 
 	void disable_audio ();
 
@@ -51,6 +53,7 @@ private:
 	void thread ();
 	void video (boost::shared_ptr<PlayerVideo> video, DCPTime time);
 	void audio (boost::shared_ptr<AudioBuffers> audio, DCPTime time);
+	void text (PlayerText pt, TextType type, DCPTimePeriod period);
 	bool should_run () const;
 	void prepare (boost::weak_ptr<PlayerVideo> video) const;
 	void player_changed ();
@@ -60,12 +63,15 @@ private:
 	boost::shared_ptr<Log> _log;
 	boost::thread* _thread;
 
-	/** mutex to protect _video and _audio for when we are clearing them and they both need to be
-	    cleared together without any data being inserted in the interim.
+	/** mutex to protect _video, _audio and _closed_caption for when we are clearing them and they all need to be
+	    cleared together without any data being inserted in the interim;
+	    XXX: is this necessary now that all butler output data is timestamped? Perhaps the locked clear-out
+	    is only required if we guarantee that get_video() and get_audio() calls are in sync.
 	*/
-	boost::mutex _video_audio_mutex;
+	boost::mutex _buffers_mutex;
 	VideoRingBuffers _video;
 	AudioRingBuffers _audio;
+	TextRingBuffers _closed_caption;
 
 	boost::thread_group _prepare_pool;
 	boost::asio::io_service _prepare_service;
@@ -93,5 +99,6 @@ private:
 
 	boost::signals2::scoped_connection _player_video_connection;
 	boost::signals2::scoped_connection _player_audio_connection;
+	boost::signals2::scoped_connection _player_text_connection;
 	boost::signals2::scoped_connection _player_changed_connection;
 };
diff --git a/src/lib/player.cc b/src/lib/player.cc
index 68b3365ea..c1a6c0f3c 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -724,15 +724,6 @@ Player::pass ()
 	return done;
 }
 
-list<PlayerText>
-Player::closed_captions_for_frame (DCPTime time) const
-{
-	boost::mutex::scoped_lock _lm (_mutex);
-	return _active_texts[TEXT_CLOSED_CAPTION].get (
-		DCPTimePeriod(time, time + DCPTime::from_frames(1, _film->video_frame_rate()))
-		);
-}
-
 /** @return Open subtitles for the frame at the given time, converted to images */
 optional<PositionImage>
 Player::open_subtitles_for_frame (DCPTime time) const
diff --git a/src/lib/player.h b/src/lib/player.h
index a4fd80c16..20b68ef77 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -88,8 +88,6 @@ public:
 
 	DCPTime content_time_to_dcp (boost::shared_ptr<Content> content, ContentTime t);
 
-	std::list<PlayerText> closed_captions_for_frame (DCPTime time) const;
-
 	/** Emitted when something has changed such that if we went back and emitted
 	 *  the last frame again it would look different.  This is not emitted after
 	 *  a seek.
diff --git a/src/lib/wscript b/src/lib/wscript
index a9ed0c621..0cc3d7823 100644
--- a/src/lib/wscript
+++ b/src/lib/wscript
@@ -137,6 +137,7 @@ sources = """
           string_text_file.cc
           string_text_file_content.cc
           string_text_file_decoder.cc
+          text_ring_buffers.cc
           timer.cc
           transcode_job.cc
           types.cc
diff --git a/src/wx/closed_captions_dialog.cc b/src/wx/closed_captions_dialog.cc
index a504cade9..c07961cc8 100644
--- a/src/wx/closed_captions_dialog.cc
+++ b/src/wx/closed_captions_dialog.cc
@@ -20,14 +20,17 @@
 
 #include "closed_captions_dialog.h"
 #include "lib/string_text.h"
+#include "lib/butler.h"
 #include <boost/bind.hpp>
 
 using std::list;
 using std::max;
 using std::cout;
+using std::pair;
 using std::make_pair;
 using boost::shared_ptr;
 using boost::weak_ptr;
+using boost::optional;
 
 ClosedCaptionsDialog::ClosedCaptionsDialog (wxWindow* parent)
 	: wxDialog (parent, wxID_ANY, _("Closed captions"), wxDefaultPosition, wxDefaultSize,
@@ -40,7 +43,7 @@ ClosedCaptionsDialog::ClosedCaptionsDialog (wxWindow* parent)
 		wxDEFAULT_FRAME_STYLE | wxRESIZE_BORDER | wxFULL_REPAINT_ON_RESIZE | wxFRAME_FLOAT_ON_PARENT
 #endif
 		)
-
+	, _current_in_lines (false)
 {
 	_lines.resize (CLOSED_CAPTION_LINES);
 	Bind (wxEVT_PAINT, boost::bind (&ClosedCaptionsDialog::paint, this));
@@ -100,40 +103,62 @@ private:
 void
 ClosedCaptionsDialog::update (DCPTime time)
 {
-	shared_ptr<Player> player = _player.lock ();
-	DCPOMATIC_ASSERT (player);
-	list<StringText> to_show;
-	BOOST_FOREACH (PlayerText i, player->closed_captions_for_frame(time)) {
-		BOOST_FOREACH (StringText j, i.string) {
-			to_show.push_back (j);
+	if (_current_in_lines && _current->second.to > time) {
+		/* Current one is fine */
+		return;
+	}
+
+	if (_current && _current->second.to < time) {
+		/* Current one has finished; clear out */
+		for (int j = 0; j < CLOSED_CAPTION_LINES; ++j) {
+			_lines[j] = "";
 		}
+		Refresh ();
+		_current = optional<pair<PlayerText, DCPTimePeriod> >();
 	}
 
-	for (int j = 0; j < CLOSED_CAPTION_LINES; ++j) {
-		_lines[j] = "";
+	if (!_current) {
+		/* We have no current one: get another */
+		shared_ptr<Butler> butler = _butler.lock ();
+		DCPOMATIC_ASSERT (butler);
+		_current = butler->get_closed_caption ();
+		_current_in_lines = false;
 	}
 
-	to_show.sort (ClosedCaptionSorter());
+	if (_current && _current->second.contains(time)) {
+		/* We need to set this new one up */
 
-	list<StringText>::const_iterator j = to_show.begin();
-	int k = 0;
-	while (j != to_show.end() && k < CLOSED_CAPTION_LINES) {
-		_lines[k] = j->text();
-		++j;
-		++k;
-	}
+		list<StringText> to_show = _current->first.string;
 
-	Refresh ();
+		for (int j = 0; j < CLOSED_CAPTION_LINES; ++j) {
+			_lines[j] = "";
+		}
+
+		to_show.sort (ClosedCaptionSorter());
+
+		list<StringText>::const_iterator j = to_show.begin();
+		int k = 0;
+		while (j != to_show.end() && k < CLOSED_CAPTION_LINES) {
+			_lines[k] = j->text();
+			++j;
+			++k;
+		}
+
+		Refresh ();
+		_current_in_lines = true;
+	}
 }
 
 void
 ClosedCaptionsDialog::clear ()
 {
+	_current = optional<pair<PlayerText, DCPTimePeriod> >();
+	_current_in_lines = false;
 	Refresh ();
 }
 
 void
-ClosedCaptionsDialog::set_player (weak_ptr<Player> player)
+ClosedCaptionsDialog::set_butler (weak_ptr<Butler> butler)
 {
-	_player = player;
+	_butler = butler;
 }
diff --git a/src/wx/closed_captions_dialog.h b/src/wx/closed_captions_dialog.h
index e3f13f62f..3da7f6522 100644
--- a/src/wx/closed_captions_dialog.h
+++ b/src/wx/closed_captions_dialog.h
@@ -22,7 +22,7 @@
 #include "lib/player.h"
 #include <wx/wx.h>
 
-class Player;
+class Butler;
 
 class ClosedCaptionsDialog : public wxDialog
 {
@@ -31,11 +31,13 @@ public:
 
 	void update (DCPTime);
 	void clear ();
-	void set_player (boost::weak_ptr<Player>);
+	void set_butler (boost::weak_ptr<Butler>);
 
 private:
 	void paint ();
 
+	boost::optional<std::pair<PlayerText, DCPTimePeriod> > _current;
+	bool _current_in_lines;
 	std::vector<wxString> _lines;
-	boost::weak_ptr<Player> _player;
+	boost::weak_ptr<Butler> _butler;
 };
diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index ce34b06b7..d00c0bfaf 100644
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -203,7 +203,6 @@ FilmViewer::set_film (shared_ptr<Film> film)
 
 	if (!_film) {
 		_player.reset ();
-		_closed_captions_dialog->set_player (_player);
 		recreate_butler ();
 		_frame.reset ();
 		refresh_panel ();
@@ -222,8 +221,6 @@ FilmViewer::set_film (shared_ptr<Film> film)
 		return;
 	}
 
-	_closed_captions_dialog->set_player (_player);
-
 	_player->set_always_burn_open_subtitles ();
 	_player->set_play_referenced ();
 
@@ -277,6 +274,8 @@ FilmViewer::recreate_butler ()
 		_butler->disable_audio ();
 	}
 
+	_closed_captions_dialog->set_butler (_butler);
+
 	if (was_running) {
 		start ();
 	}