WIP: untested main part of changes.

7 files changed, 104 insertions, 79 deletions

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 67fbd8738..98481b77a 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -344,7 +344,9 @@ Player::black_player_video_frame (Eyes eyes) const
 			PART_WHOLE,
 			PresetColourConversion::all().front().conversion,
 			VIDEO_RANGE_FULL,
-			boost::weak_ptr<Content>()
+			boost::weak_ptr<Content>(),
+			optional<dcpomatic::DCPTime>(),
+			_film->video_frame_rate()
 		)
 	);
 }
@@ -713,52 +715,6 @@ Player::pass ()
 	return done;
 }
 
-/** @return Open subtitles for the frame at the given time, converted to images */
-optional<PositionImage>
-Player::open_subtitles_for_frame (DCPTime time) const
-{
-	list<PositionImage> captions;
-	int const vfr = _film->video_frame_rate();
-
-	BOOST_FOREACH (
-		PlayerText j,
-		_active_texts[TEXT_OPEN_SUBTITLE].get_burnt(DCPTimePeriod(time, time + DCPTime::from_frames(1, vfr)), _always_burn_open_subtitles)
-		) {
-
-		/* Bitmap subtitles */
-		BOOST_FOREACH (BitmapText i, j.bitmap) {
-			if (!i.image) {
-				continue;
-			}
-
-			/* i.image will already have been scaled to fit _video_container_size */
-			dcp::Size scaled_size (i.rectangle.width * _video_container_size.width, i.rectangle.height * _video_container_size.height);
-
-			captions.push_back (
-				PositionImage (
-					i.image,
-					Position<int> (
-						lrint (_video_container_size.width * i.rectangle.x),
-						lrint (_video_container_size.height * i.rectangle.y)
-						)
-					)
-				);
-		}
-
-		/* String subtitles (rendered to an image) */
-		if (!j.string.empty ()) {
-			list<PositionImage> s = render_text (j.string, j.fonts, _video_container_size, time, vfr);
-			copy (s.begin(), s.end(), back_inserter (captions));
-		}
-	}
-
-	if (captions.empty ()) {
-		return optional<PositionImage> ();
-	}
-
-	return merge (captions);
-}
-
 void
 Player::video (weak_ptr<Piece> wp, ContentVideo video)
 {
@@ -846,7 +802,9 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
 			video.part,
 			piece->content->video->colour_conversion(),
 			piece->content->video->range(),
-			piece->content
+			piece->content,
+			time,
+			_film->video_frame_rate()
 			)
 		);
 
@@ -1125,10 +1083,7 @@ Player::do_emit_video (shared_ptr<PlayerVideo> pv, DCPTime time)
 		}
 	}
 
-	optional<PositionImage> subtitles = open_subtitles_for_frame (time);
-	if (subtitles) {
-		pv->set_text (subtitles.get ());
-	}
+	pv->set_text (_active_texts[TEXT_OPEN_SUBTITLE].get_burnt(DCPTimePeriod(time, time + DCPTime::from_frames(1, _film->video_frame_rate())), _always_burn_open_subtitles));
 
 	Video (pv, time);
 }
diff --git a/src/lib/player.h b/src/lib/player.h
index e99c345bb..37b3f6c1b 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -135,7 +135,6 @@ private:
 	std::pair<boost::shared_ptr<AudioBuffers>, dcpomatic::DCPTime> discard_audio (
 		boost::shared_ptr<const AudioBuffers> audio, dcpomatic::DCPTime time, dcpomatic::DCPTime discard_to
 		) const;
-	boost::optional<PositionImage> open_subtitles_for_frame (dcpomatic::DCPTime time) const;
 	void emit_video (boost::shared_ptr<PlayerVideo> pv, dcpomatic::DCPTime time);
 	void do_emit_video (boost::shared_ptr<PlayerVideo> pv, dcpomatic::DCPTime time);
 	void emit_audio (boost::shared_ptr<AudioBuffers> data, dcpomatic::DCPTime time);
diff --git a/src/lib/player_text.cc b/src/lib/player_text.cc
index 429f85ddf..d9c153416 100644
--- a/src/lib/player_text.cc
+++ b/src/lib/player_text.cc
@@ -48,3 +48,9 @@ operator== (PlayerText const & a, PlayerText const & b)
 {
 	return deep_equals(a.fonts, b.fonts) && deep_equals(a.bitmap, b.bitmap) && deep_equals(a.string, b.string);
 }
+
+bool
+operator!= (PlayerText const & a, PlayerText const & b)
+{
+	return !(a == b);
+}
diff --git a/src/lib/player_text.h b/src/lib/player_text.h
index fb1d846d8..3c571b324 100644
--- a/src/lib/player_text.h
+++ b/src/lib/player_text.h
@@ -41,4 +41,7 @@ public:
 	std::list<StringText> string;
 };
 
+extern bool operator== (PlayerText const & a, PlayerText const & b);
+extern bool operator!= (PlayerText const & a, PlayerText const & b);
+
 #endif
diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index 74692b054..ca0845d2b 100644
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2018 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2019 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -25,6 +25,7 @@
 #include "image_proxy.h"
 #include "j2k_image_proxy.h"
 #include "film.h"
+#include "render_text.h"
 #include <dcp/raw_convert.h>
 extern "C" {
 #include <libavutil/pixfmt.h>
@@ -34,6 +35,7 @@ extern "C" {
 
 using std::string;
 using std::cout;
+using std::list;
 using std::pair;
 using boost::shared_ptr;
 using boost::weak_ptr;
@@ -53,7 +55,9 @@ PlayerVideo::PlayerVideo (
 	Part part,
 	optional<ColourConversion> colour_conversion,
 	VideoRange video_range,
-	weak_ptr<Content> content
+	weak_ptr<Content> content,
+	optional<dcpomatic::DCPTime> time,
+	int video_frame_rate
 	)
 	: _in (in)
 	, _crop (crop)
@@ -65,6 +69,8 @@ PlayerVideo::PlayerVideo (
 	, _colour_conversion (colour_conversion)
 	, _video_range (video_range)
 	, _content (content)
+	, _time (time)
+	, _video_frame_rate (video_frame_rate)
 {
 
 }
@@ -80,6 +86,7 @@ PlayerVideo::PlayerVideo (shared_ptr<cxml::Node> node, shared_ptr<Socket> socket
 	_eyes = (Eyes) node->number_child<int> ("Eyes");
 	_part = (Part) node->number_child<int> ("Part");
 	_video_range = (VideoRange) node->number_child<int>("VideoRange");
+	_video_frame_rate = node->number_child<int>("VideoFrameRate");
 
 	/* Assume that the ColourConversion uses the current state version */
 	_colour_conversion = ColourConversion::from_xml (node, Film::current_state_version);
@@ -94,14 +101,16 @@ PlayerVideo::PlayerVideo (shared_ptr<cxml::Node> node, shared_ptr<Socket> socket
 
 		image->read_from_socket (socket);
 
+		/* XXX_c
 		_text = PositionImage (image, Position<int> (node->number_child<int> ("SubtitleX"), node->number_child<int> ("SubtitleY")));
+		*/
 	}
 }
 
 void
-PlayerVideo::set_text (PositionImage image)
+PlayerVideo::set_text (list<PlayerText> text)
 {
-	_text = image;
+	_text = text;
 }
 
 shared_ptr<Image>
@@ -166,8 +175,41 @@ PlayerVideo::make_image (function<AVPixelFormat (AVPixelFormat)> pixel_format, b
 		total_crop, _inter_size, _out_size, yuv_to_rgb, _video_range, pixel_format (im->pixel_format()), aligned, fast
 		);
 
-	if (_text) {
-		_image->alpha_blend (Image::ensure_aligned (_text->image), _text->position);
+	list<PositionImage> subtitles;
+
+	BOOST_FOREACH (PlayerText i, _text) {
+
+		/* Bitmap subtitles */
+		BOOST_FOREACH (BitmapText j, i.bitmap) {
+			if (!j.image) {
+				continue;
+			}
+
+			/* i.image will already have been scaled to fit _out_size */
+			dcp::Size scaled_size (j.rectangle.width * _out_size.width, j.rectangle.height * _out_size.height);
+
+			subtitles.push_back (
+				PositionImage (
+					j.image,
+					Position<int> (
+						lrint(_out_size.width * j.rectangle.x),
+						lrint(_out_size.height * j.rectangle.y)
+						)
+					)
+				);
+		}
+
+		/* String subtitles (rendered to an image) */
+		if (!i.string.empty ()) {
+			DCPOMATIC_ASSERT (_time);
+			list<PositionImage> s = render_text (i.string, i.fonts, _out_size, *_time, _video_frame_rate);
+			copy (s.begin(), s.end(), back_inserter(subtitles));
+		}
+	}
+
+	if (!subtitles.empty()) {
+		PositionImage p = merge (subtitles);
+		_image->alpha_blend (Image::ensure_aligned(p.image), p.position);
 	}
 
 	if (_fade) {
@@ -192,24 +234,29 @@ PlayerVideo::add_metadata (xmlpp::Node* node) const
 	node->add_child("Eyes")->add_child_text (raw_convert<string> (static_cast<int> (_eyes)));
 	node->add_child("Part")->add_child_text (raw_convert<string> (static_cast<int> (_part)));
 	node->add_child("VideoRange")->add_child_text(raw_convert<string>(static_cast<int>(_video_range)));
+	node->add_child("VideoFrameRate")->add_child_text(raw_convert<string>(_video_frame_rate));
 	if (_colour_conversion) {
 		_colour_conversion.get().as_xml (node);
 	}
+	/* XXX_c
 	if (_text) {
 		node->add_child ("SubtitleWidth")->add_child_text (raw_convert<string> (_text->image->size().width));
 		node->add_child ("SubtitleHeight")->add_child_text (raw_convert<string> (_text->image->size().height));
 		node->add_child ("SubtitleX")->add_child_text (raw_convert<string> (_text->position.x));
 		node->add_child ("SubtitleY")->add_child_text (raw_convert<string> (_text->position.y));
 	}
+	*/
 }
 
 void
 PlayerVideo::send_binary (shared_ptr<Socket> socket) const
 {
 	_in->send_binary (socket);
+	/* XXX_c
 	if (_text) {
 		_text->image->write_to_socket (socket);
 	}
+	*/
 }
 
 bool
@@ -222,7 +269,7 @@ PlayerVideo::has_j2k () const
 		return false;
 	}
 
-	return _crop == Crop () && _out_size == j2k->size() && !_text && !_fade && !_colour_conversion;
+	return _crop == Crop () && _out_size == j2k->size() && _text.empty() && !_fade && !_colour_conversion;
 }
 
 Data
@@ -254,18 +301,10 @@ PlayerVideo::definitely_equal (shared_ptr<const PlayerVideo> other) const
 		return false;
 	}
 
-	if ((!_text && other->_text) || (_text && !other->_text)) {
-		/* One has a text and the other doesn't */
+	if (!deep_equals(_text, other->_text)) {
 		return false;
 	}
 
-	if (_text && other->_text && !_text->definitely_equal(other->_text.get())) {
-		/* They both have texts but they are different */
-		return false;
-	}
-
-	/* Now neither has subtitles */
-
 	return _in->definitely_equal (other->_in);
 }
 
@@ -312,7 +351,9 @@ PlayerVideo::shallow_copy () const
 			_part,
 			_colour_conversion,
 			_video_range,
-			_content
+			_content,
+			_time,
+			_video_frame_rate
 			)
 		);
 }
diff --git a/src/lib/player_video.h b/src/lib/player_video.h
index cefb5cdb8..6cb8a3140 100644
--- a/src/lib/player_video.h
+++ b/src/lib/player_video.h
@@ -26,6 +26,7 @@
 #include "dcpomatic_time.h"
 #include "colour_conversion.h"
 #include "position_image.h"
+#include "player_text.h"
 extern "C" {
 #include <libavutil/pixfmt.h>
 }
@@ -55,14 +56,16 @@ public:
 		Part,
 		boost::optional<ColourConversion>,
 		VideoRange video_range,
-		boost::weak_ptr<Content>
+		boost::weak_ptr<Content>,
+		boost::optional<dcpomatic::DCPTime>,
+		int video_frame_rate
 		);
 
 	PlayerVideo (boost::shared_ptr<cxml::Node>, boost::shared_ptr<Socket>);
 
 	boost::shared_ptr<PlayerVideo> shallow_copy () const;
 
-	void set_text (PositionImage);
+	void set_text (std::list<PlayerText>);
 
 	void prepare (boost::function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast);
 	boost::shared_ptr<Image> image (boost::function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const;
@@ -118,11 +121,13 @@ private:
 	Part _part;
 	boost::optional<ColourConversion> _colour_conversion;
 	VideoRange _video_range;
-	boost::optional<PositionImage> _text;
+	std::list<PlayerText> _text;
 	/** Content that we came from.  This is so that reset_metadata() can work, and also
 	 *  for variant:swaroop's non-skippable ads.
 	 */
 	boost::weak_ptr<Content> _content;
+	boost::optional<dcpomatic::DCPTime> _time;
+	int _video_frame_rate;
 
 	mutable boost::mutex _mutex;
 	mutable boost::shared_ptr<Image> _image;
diff --git a/test/client_server_test.cc b/test/client_server_test.cc
index 77a66ccad..41f7997ee 100644
--- a/test/client_server_test.cc
+++ b/test/client_server_test.cc
@@ -98,11 +98,17 @@ BOOST_AUTO_TEST_CASE (client_server_test_rgb)
 			PART_WHOLE,
 			ColourConversion(),
 			VIDEO_RANGE_FULL,
-			weak_ptr<Content>()
+			weak_ptr<Content>(),
+			optional<dcpomatic::DCPTime>(),
+			24
 			)
 		);
 
-	pvf->set_text (PositionImage (sub_image, Position<int> (50, 60)));
+	PlayerText pt;
+	pt.bitmap.push_back (BitmapText(sub_image, dcpomatic::Rect<double>(50, 60, 100, 200)));
+	list<PlayerText> ptl;
+	ptl.push_back (pt);
+	pvf->set_text (ptl);
 
 	shared_ptr<DCPVideo> frame (
 		new DCPVideo (
@@ -182,11 +188,17 @@ BOOST_AUTO_TEST_CASE (client_server_test_yuv)
 			PART_WHOLE,
 			ColourConversion(),
 			VIDEO_RANGE_FULL,
-			weak_ptr<Content>()
+			weak_ptr<Content>(),
+			boost::optional<dcpomatic::DCPTime>(),
+			24
 			)
 		);
 
-	pvf->set_text (PositionImage (sub_image, Position<int> (50, 60)));
+	PlayerText pt;
+	pt.bitmap.push_back (BitmapText(sub_image, dcpomatic::Rect<double>(50, 60, 100, 200)));
+	list<PlayerText> ptl;
+	ptl.push_back (pt);
+	pvf->set_text (ptl);
 
 	shared_ptr<DCPVideo> frame (
 		new DCPVideo (
@@ -253,7 +265,9 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k)
 			PART_WHOLE,
 			ColourConversion(),
 			VIDEO_RANGE_FULL,
-			weak_ptr<Content>()
+			weak_ptr<Content>(),
+			boost::optional<dcpomatic::DCPTime>(),
+			24
 			)
 		);
 
@@ -280,7 +294,9 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k)
 			PART_WHOLE,
 			PresetColourConversion::all().front().conversion,
 			VIDEO_RANGE_FULL,
-			weak_ptr<Content>()
+			weak_ptr<Content>(),
+			boost::optional<dcpomatic::DCPTime>(),
+			24
 			)
 		);