Merge branch '2389-vpos'

Here we are trying to fix a variety of confusions related to vertical
subtitle position (#2389).

19 files changed, 202 insertions, 101 deletions

diff --git a/src/lib/analyse_subtitles_job.cc b/src/lib/analyse_subtitles_job.cc
index 7f1b8ad04..b41b65c3b 100644
--- a/src/lib/analyse_subtitles_job.cc
+++ b/src/lib/analyse_subtitles_job.cc
@@ -92,7 +92,7 @@ AnalyseSubtitlesJob::run ()
 
 
 void
-AnalyseSubtitlesJob::analyse (PlayerText text, TextType type)
+AnalyseSubtitlesJob::analyse(PlayerText const& text, TextType type)
 {
 	if (type != TextType::OPEN_SUBTITLE) {
 		return;
@@ -106,14 +106,34 @@ AnalyseSubtitlesJob::analyse (PlayerText text, TextType type)
 		}
 	}
 
-	if (!text.string.empty()) {
-		/* We can provide dummy values for time and frame rate here as they are only used to calculate fades */
-		dcp::Size const frame = _film->frame_size();
-		for (auto i: render_text(text.string, frame, dcpomatic::DCPTime(), 24)) {
+	if (text.string.empty()) {
+		return;
+	}
+
+	/* We can provide dummy values for time and frame rate here as they are only used to calculate fades */
+	dcp::Size const frame = _film->frame_size();
+	std::vector<dcp::SubtitleStandard> override_standard;
+	if (_film->interop()) {
+		/* Since the film is Interop there is only one way the vpositions in the subs can be interpreted
+		 * (we assume).
+		 */
+		override_standard.push_back(dcp::SubtitleStandard::INTEROP);
+	} else {
+		/* We're using the great new SMPTE standard, which means there are two different ways that vposition
+		 * could be interpreted; we will write SMPTE-2014 standard assets, but if the projection system uses
+		 * SMPTE 20{07,10} instead they won't be placed how we intended.  To show the user this, make the
+		 * bounding rectangle enclose both possibilities.
+		 */
+		override_standard.push_back(dcp::SubtitleStandard::SMPTE_2007);
+		override_standard.push_back(dcp::SubtitleStandard::SMPTE_2014);
+	}
+
+	for (auto standard: override_standard) {
+		for (auto i: bounding_box(text.string, frame, standard)) {
 			dcpomatic::Rect<double> rect (
-					double(i.position.x) / frame.width, double(i.position.y) / frame.height,
-					double(i.image->size().width) / frame.width, double(i.image->size().height) / frame.height
-					);
+				double(i.x) / frame.width, double(i.y) / frame.height,
+				double(i.width) / frame.width, double(i.height) / frame.height
+				);
 			if (!_bounding_box) {
 				_bounding_box = rect;
 			} else {
diff --git a/src/lib/analyse_subtitles_job.h b/src/lib/analyse_subtitles_job.h
index ef720ef09..c47117c57 100644
--- a/src/lib/analyse_subtitles_job.h
+++ b/src/lib/analyse_subtitles_job.h
@@ -42,7 +42,7 @@ public:
 	}
 
 private:
-	void analyse (PlayerText text, TextType type);
+	void analyse(PlayerText const& text, TextType type);
 
 	std::weak_ptr<Content> _content;
 	boost::filesystem::path _path;
diff --git a/src/lib/content.h b/src/lib/content.h
index 979680d6a..0ce87ed9b 100644
--- a/src/lib/content.h
+++ b/src/lib/content.h
@@ -208,7 +208,7 @@ public:
 
 	std::shared_ptr<VideoContent> video;
 	std::shared_ptr<AudioContent> audio;
-	std::list<std::shared_ptr<TextContent>> text;
+	std::vector<std::shared_ptr<TextContent>> text;
 	std::shared_ptr<AtmosContent> atmos;
 
 	std::shared_ptr<TextContent> only_text () const;
diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index cdf104f03..231a93bd0 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -258,7 +258,7 @@ DCPContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
 		atmos->set_length (examiner->atmos_length());
 	}
 
-	list<shared_ptr<TextContent>> new_text;
+	vector<shared_ptr<TextContent>> new_text;
 
 	for (int i = 0; i < examiner->text_count(TextType::OPEN_SUBTITLE); ++i) {
 		auto c = make_shared<TextContent>(this, TextType::OPEN_SUBTITLE, TextType::OPEN_SUBTITLE);
diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc
index 0f4e1afa1..e82e9e958 100644
--- a/src/lib/dcp_decoder.cc
+++ b/src/lib/dcp_decoder.cc
@@ -304,7 +304,7 @@ DCPDecoder::pass_texts (
 							ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.out().as_seconds())
 							),
 						strings,
-						_dcp_content->standard()
+						asset->subtitle_standard()
 						);
 					strings.clear ();
 				}
@@ -340,7 +340,7 @@ DCPDecoder::pass_texts (
 					ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.out().as_seconds())
 					),
 				strings,
-				_dcp_content->standard()
+				asset->subtitle_standard()
 				);
 			strings.clear ();
 		}
diff --git a/src/lib/dcp_subtitle_decoder.cc b/src/lib/dcp_subtitle_decoder.cc
index 617f7ec53..fa92193a5 100644
--- a/src/lib/dcp_subtitle_decoder.cc
+++ b/src/lib/dcp_subtitle_decoder.cc
@@ -47,11 +47,7 @@ DCPSubtitleDecoder::DCPSubtitleDecoder (shared_ptr<const Film> film, shared_ptr<
 	_subtitles = asset->subtitles ();
 	_next = _subtitles.begin ();
 
-	if (dynamic_pointer_cast<dcp::InteropSubtitleAsset>(asset)) {
-		_standard = dcp::Standard::INTEROP;
-	} else {
-		_standard = dcp::Standard::SMPTE;
-	}
+	_subtitle_standard = asset->subtitle_standard();
 
 	text.push_back (make_shared<TextDecoder>(this, content->only_text()));
 	update_position();
@@ -109,7 +105,7 @@ DCPSubtitleDecoder::pass ()
 		}
 	}
 
-	only_text()->emit_plain(p, s, _standard);
+	only_text()->emit_plain(p, s, _subtitle_standard);
 
 	update_position();
 
diff --git a/src/lib/dcp_subtitle_decoder.h b/src/lib/dcp_subtitle_decoder.h
index 3eed4ad24..45a4999dd 100644
--- a/src/lib/dcp_subtitle_decoder.h
+++ b/src/lib/dcp_subtitle_decoder.h
@@ -43,5 +43,5 @@ private:
 	std::vector<std::shared_ptr<const dcp::Subtitle>> _subtitles;
 	std::vector<std::shared_ptr<const dcp::Subtitle>>::const_iterator _next;
 
-	dcp::Standard _standard;
+	dcp::SubtitleStandard _subtitle_standard;
 };
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 8e409fc69..69d55c7c4 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -353,6 +353,7 @@ Film::subtitle_analysis_path (shared_ptr<const Content> content) const
 
 	Digester digester;
 	digester.add (content->digest());
+	digester.add(_interop ? "1" : "0");
 
 	if (!content->text.empty()) {
 		auto tc = content->text.front();
diff --git a/src/lib/job.cc b/src/lib/job.cc
index 6ec154c34..53527d265 100644
--- a/src/lib/job.cc
+++ b/src/lib/job.cc
@@ -344,6 +344,10 @@ Job::set_state (State s)
 
 	{
 		boost::mutex::scoped_lock lm (_state_mutex);
+		if (_state == s) {
+			return;
+		}
+
 		_state = s;
 
 		if (_state == FINISHED_OK || _state == FINISHED_ERROR || _state == FINISHED_CANCELLED) {
diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index e0279725b..47df4feb1 100644
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -892,9 +892,10 @@ ReelWriter::empty_text_asset (TextType type, optional<DCPTextTrack> track, bool
 
 
 float
-ReelWriter::convert_vertical_position(StringText const& subtitle, dcp::Standard to) const
+ReelWriter::convert_vertical_position(StringText const& subtitle, dcp::SubtitleStandard to) const
 {
-	if (subtitle.valign_standard == to) {
+	if (dcp::uses_baseline(subtitle.valign_standard) == dcp::uses_baseline(to)) {
+		/* The from and to standards use the same alignment reference */
 		return subtitle.v_position();
 	}
 
@@ -914,7 +915,7 @@ ReelWriter::convert_vertical_position(StringText const& subtitle, dcp::Standard
 		break;
 	}
 
-	return subtitle.v_position() + ((subtitle.valign_standard == dcp::Standard::SMPTE) ? correction : -correction);
+	return subtitle.v_position() + (dcp::uses_bounding_box(subtitle.valign_standard) ? correction : -correction);
 }
 
 
@@ -957,7 +958,7 @@ ReelWriter::write (PlayerText subs, TextType type, optional<DCPTextTrack> track,
 	for (auto i: subs.string) {
 		i.set_in  (dcp::Time(period.from.seconds() - _period.from.seconds(), tcr));
 		i.set_out (dcp::Time(period.to.seconds() - _period.from.seconds(), tcr));
-		i.set_v_position(convert_vertical_position(i, film()->interop() ? dcp::Standard::INTEROP : dcp::Standard::SMPTE));
+		i.set_v_position(convert_vertical_position(i, film()->interop() ? dcp::SubtitleStandard::INTEROP : dcp::SubtitleStandard::SMPTE_2014));
 		auto sub = make_shared<dcp::SubtitleString>(i);
 		if (type == TextType::OPEN_SUBTITLE) {
 			sub->set_font(fonts.get(i.font));
diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h
index 8ceef9f51..892d803a5 100644
--- a/src/lib/reel_writer.h
+++ b/src/lib/reel_writer.h
@@ -121,7 +121,7 @@ private:
 		std::set<DCPTextTrack> ensure_closed_captions
 		) const;
 	void create_reel_markers (std::shared_ptr<dcp::Reel> reel) const;
-	float convert_vertical_position(StringText const& subtitle, dcp::Standard to) const;
+	float convert_vertical_position(StringText const& subtitle, dcp::SubtitleStandard to) const;
 
 	dcpomatic::DCPTimePeriod _period;
 	/** the first picture frame index that does not already exist in our MXF */
diff --git a/src/lib/render_text.cc b/src/lib/render_text.cc
index 702f848ac..33e0c6a89 100644
--- a/src/lib/render_text.cc
+++ b/src/lib/render_text.cc
@@ -47,6 +47,7 @@ using std::min;
 using std::pair;
 using std::shared_ptr;
 using std::string;
+using boost::optional;
 using namespace dcpomatic;
 
 
@@ -55,7 +56,7 @@ using namespace dcpomatic;
  *  for the actual render.
  */
 static Glib::RefPtr<Pango::Layout>
-create_layout()
+create_layout(string font_name, string markup)
 {
 	auto c_font_map = pango_cairo_font_map_new ();
 	DCPOMATIC_ASSERT (c_font_map);
@@ -63,17 +64,14 @@ create_layout()
 	auto c_context = pango_font_map_create_context (c_font_map);
 	DCPOMATIC_ASSERT (c_context);
 	auto context = Glib::wrap (c_context);
-	return Pango::Layout::create (context);
-}
-
+	auto layout = Pango::Layout::create(context);
 
-static void
-setup_layout (Glib::RefPtr<Pango::Layout> layout, string font_name, string markup)
-{
 	layout->set_alignment (Pango::ALIGN_LEFT);
 	Pango::FontDescription font (font_name);
 	layout->set_font_description (font);
 	layout->set_markup (markup);
+
+	return layout;
 }
 
 
@@ -120,8 +118,7 @@ marked_up (list<StringText> subtitles, int target_height, float fade_factor, str
 			 * be written with letter_spacing either side.  This means that to get a horizontal space x we
 			 * need to write a " " with letter spacing (x - s) / 2, where s is the width of the " ".
 			 */
-			auto layout = create_layout();
-			setup_layout(layout, font_name, make_span(i, " ", {}));
+			auto layout = create_layout(font_name, make_span(i, " ", {}));
 			int space_width;
 			int dummy;
 			layout->get_pixel_size(space_width, dummy);
@@ -226,21 +223,21 @@ calculate_fade_factor (StringText const& first, DCPTime time, int frame_rate)
 
 
 static int
-x_position (StringText const& first, int target_width, int layout_width)
+x_position(dcp::HAlign align, float position, int target_width, int layout_width)
 {
 	int x = 0;
-	switch (first.h_align()) {
+	switch (align) {
 	case dcp::HAlign::LEFT:
 		/* h_position is distance between left of frame and left of subtitle */
-		x = first.h_position() * target_width;
+		x = position * target_width;
 		break;
 	case dcp::HAlign::CENTER:
 		/* h_position is distance between centre of frame and centre of subtitle */
-		x = (0.5 + first.h_position()) * target_width - layout_width / 2;
+		x = (0.5 + position) * target_width - layout_width / 2;
 		break;
 	case dcp::HAlign::RIGHT:
 		/* h_position is distance between right of frame and right of subtitle */
-		x = (1.0 - first.h_position()) * target_width - layout_width;
+		x = (1.0 - position) * target_width - layout_width;
 		break;
 	}
 
@@ -248,40 +245,50 @@ x_position (StringText const& first, int target_width, int layout_width)
 }
 
 
+/** @param align_standard Standard with which to interpret this subtitle's position.
+ *  @param align alignment.
+ *  @param position position (between 0 and 1)
+ *  @param target_height Height of the target screen (in pixels).
+ *  @param baseline_to_bottom Distance from text baseline to the bottom of the bounding box (in pixels).
+ *  @param layout_height Height of the subtitle bounding box (in pixels).
+ *  @return y position of the top of the subtitle bounding box (in pixels) from the top of the screen.
+ */
 static int
-y_position (StringText const& first, int target_height, int baseline_to_bottom, int layout_height)
+y_position(dcp::SubtitleStandard standard, dcp::VAlign align, float position, int target_height, int baseline_to_bottom, int layout_height)
 {
 	int y = 0;
-	switch (first.valign_standard) {
-	case dcp::Standard::INTEROP:
-		switch (first.v_align()) {
+	switch (standard) {
+	case dcp::SubtitleStandard::INTEROP:
+	case dcp::SubtitleStandard::SMPTE_2014:
+		switch (align) {
 		case dcp::VAlign::TOP:
-			/* v_position is distance from top of frame to subtitle baseline */
-			y = first.v_position() * target_height - (layout_height - baseline_to_bottom);
+			/* position is distance from top of frame to subtitle baseline */
+			y = position * target_height - (layout_height - baseline_to_bottom);
 			break;
 		case dcp::VAlign::CENTER:
-			/* v_position is distance from centre of frame to subtitle baseline */
-			y = (0.5 + first.v_position()) * target_height - (layout_height - baseline_to_bottom);
+			/* position is distance from centre of frame to subtitle baseline */
+			y = (0.5 + position) * target_height - (layout_height - baseline_to_bottom);
 			break;
 		case dcp::VAlign::BOTTOM:
-			/* v_position is distance from bottom of frame to subtitle baseline */
-			y = (1.0 - first.v_position()) * target_height - (layout_height - baseline_to_bottom);
+			/* position is distance from bottom of frame to subtitle baseline */
+			y = (1.0 - position) * target_height - (layout_height - baseline_to_bottom);
 			break;
 		}
 		break;
-	case dcp::Standard::SMPTE:
-		switch (first.v_align()) {
+	case dcp::SubtitleStandard::SMPTE_2007:
+	case dcp::SubtitleStandard::SMPTE_2010:
+		switch (align) {
 		case dcp::VAlign::TOP:
 			/* v_position is distance from top of frame to top of subtitle */
-			y = first.v_position() * target_height;
+			y = position * target_height;
 			break;
 		case dcp::VAlign::CENTER:
 			/* v_position is distance from centre of frame to centre of subtitle */
-			y = (0.5 + first.v_position()) * target_height - layout_height / 2;
+			y = (0.5 + position) * target_height - layout_height / 2;
 			break;
 		case dcp::VAlign::BOTTOM:
 			/* v_position is distance from bottom of frame to bottom of subtitle */
-			y = (1.0 - first.v_position()) * target_height - layout_height;
+			y = (1.0 - position) * target_height - layout_height;
 			break;
 		}
 	}
@@ -290,6 +297,32 @@ y_position (StringText const& first, int target_height, int baseline_to_bottom,
 }
 
 
+struct Layout
+{
+	Position<int> position;
+	dcp::Size size;
+	Glib::RefPtr<Pango::Layout> pango;
+};
+
+
+/** @param subtitles A list of subtitles that are all on the same line,
+ *  at the same time and with the same fade in/out.
+ */
+static Layout
+setup_layout(list<StringText> subtitles, dcp::Size target, DCPTime time, int frame_rate)
+{
+	DCPOMATIC_ASSERT(!subtitles.empty());
+	auto const& first = subtitles.front();
+
+	auto const font_name = setup_font(first.font);
+	auto const fade_factor = calculate_fade_factor(first, time, frame_rate);
+	auto const markup = marked_up(subtitles, target.height, fade_factor, font_name);
+	auto layout = create_layout(font_name, markup);
+	auto ink = layout->get_ink_extents();
+	return { { ink.get_x() / Pango::SCALE, ink.get_y() / Pango::SCALE }, { ink.get_width() / Pango::SCALE, ink.get_height() / Pango::SCALE }, layout };
+}
+
+
 /** @param subtitles A list of subtitles that are all on the same line,
  *  at the same time and with the same fade in/out.
  */
@@ -300,16 +333,11 @@ render_line (list<StringText> subtitles, dcp::Size target, DCPTime time, int fra
 	   nothing else yet.
 	*/
 
-	DCPOMATIC_ASSERT (!subtitles.empty ());
-	auto const& first = subtitles.front ();
+	DCPOMATIC_ASSERT(!subtitles.empty ());
+	auto const& first = subtitles.front();
+	auto const fade_factor = calculate_fade_factor(first, time, frame_rate);
 
-	auto const font_name = setup_font(first.font);
-	auto const fade_factor = calculate_fade_factor (first, time, frame_rate);
-	auto const markup = marked_up (subtitles, target.height, fade_factor, font_name);
-	auto layout = create_layout ();
-	setup_layout (layout, font_name, markup);
-	auto ink = layout->get_ink_extents();
-	dcp::Size size{ink.get_width() / Pango::SCALE, ink.get_height() / Pango::SCALE};
+	auto layout = setup_layout(subtitles, target, time, frame_rate);
 
 	/* Calculate x and y scale factors.  These are only used to stretch
 	   the font away from its normal aspect ratio.
@@ -327,29 +355,29 @@ render_line (list<StringText> subtitles, dcp::Size target, DCPTime time, int fra
 	}
 
 	auto const border_width = first.effect() == dcp::Effect::BORDER ? (first.outline_width * target.width / 2048.0) : 0;
-	size.width += 2 * ceil (border_width);
-	size.height += 2 * ceil (border_width);
+	layout.size.width += 2 * ceil (border_width);
+	layout.size.height += 2 * ceil (border_width);
 
-	size.width *= x_scale;
-	size.height *= y_scale;
+	layout.size.width *= x_scale;
+	layout.size.height *= y_scale;
 
 	/* Shuffle the subtitle over by the border width (if we have any) so it's not cut off */
-	int const x_offset = (-ink.get_x() / Pango::SCALE) + ceil(border_width);
-	int const y_offset = -ink.get_y() / Pango::SCALE + ceil(border_width);
+	int const x_offset = -layout.position.x + ceil(border_width);
+	int const y_offset = -layout.position.y + ceil(border_width);
 
-	auto image = create_image (size);
+	auto image = create_image(layout.size);
 	auto surface = create_surface (image);
 	auto context = Cairo::Context::create (surface);
 
 	context->set_line_width (1);
 	context->scale (x_scale, y_scale);
-	layout->update_from_cairo_context (context);
+	layout.pango->update_from_cairo_context(context);
 
 	if (first.effect() == dcp::Effect::SHADOW) {
 		/* Drop-shadow effect */
 		set_source_rgba (context, first.effect_colour(), fade_factor);
 		context->move_to (x_offset + 4, y_offset + 4);
-		layout->add_to_cairo_context (context);
+		layout.pango->add_to_cairo_context(context);
 		context->fill ();
 	}
 
@@ -359,7 +387,7 @@ render_line (list<StringText> subtitles, dcp::Size target, DCPTime time, int fra
 		context->set_line_width (border_width);
 		context->set_line_join (Cairo::LINE_JOIN_ROUND);
 		context->move_to (x_offset, y_offset);
-		layout->add_to_cairo_context (context);
+		layout.pango->add_to_cairo_context (context);
 		context->stroke ();
 	}
 
@@ -368,16 +396,16 @@ render_line (list<StringText> subtitles, dcp::Size target, DCPTime time, int fra
 	set_source_rgba (context, first.colour(), fade_factor);
 
 	context->move_to (x_offset, y_offset);
-	layout->add_to_cairo_context (context);
+	layout.pango->add_to_cairo_context (context);
 	context->fill ();
 
 	context->set_line_width (0.5);
 	context->move_to (x_offset, y_offset);
-	layout->add_to_cairo_context (context);
+	layout.pango->add_to_cairo_context (context);
 	context->stroke ();
 
-	int const x = x_position (first, target.width, size.width);
-	int const y = y_position (first, target.height, ink.get_y() / Pango::SCALE, size.height);
+	int const x = x_position(first.h_align(), first.h_position(), target.width, layout.size.width);
+	int const y = y_position(first.valign_standard, first.v_align(), first.v_position(), target.height, layout.position.y, layout.size.height);
 	return PositionImage (image, Position<int>(max (0, x), max(0, y)));
 }
 
@@ -408,6 +436,38 @@ render_text (list<StringText> subtitles, dcp::Size target, DCPTime time, int fra
 }
 
 
+list<dcpomatic::Rect<int>>
+bounding_box(list<StringText> subtitles, dcp::Size target, optional<dcp::SubtitleStandard> override_standard)
+{
+	list<StringText> pending;
+	list<dcpomatic::Rect<int>> rects;
+
+	auto use_pending = [&pending, &rects, target, override_standard]() {
+		auto const& subtitle = pending.front();
+		auto standard = override_standard.get_value_or(subtitle.valign_standard);
+		/* We can provide dummy values for time and frame rate here as they are only used to calculate fades */
+		auto layout = setup_layout(pending, target, DCPTime(), 24);
+		int const x = x_position(subtitle.h_align(), subtitle.h_position(), target.width, layout.size.width);
+		int const y = y_position(standard, subtitle.v_align(), subtitle.v_position(), target.height, layout.position.y, layout.size.height);
+		rects.push_back({Position<int>(x, y), layout.size.width, layout.size.height});
+	};
+
+	for (auto const& i: subtitles) {
+		if (!pending.empty() && (i.v_align() != pending.back().v_align() || fabs(i.v_position() - pending.back().v_position()) > 1e-4)) {
+			use_pending();
+			pending.clear();
+		}
+		pending.push_back(i);
+	}
+
+	if (!pending.empty()) {
+		use_pending();
+	}
+
+	return rects;
+}
+
+
 float
 FontMetrics::height(StringText const& subtitle)
 {
@@ -433,10 +493,9 @@ FontMetrics::get(StringText const& subtitle)
 	}
 
 	auto const font_name = setup_font(subtitle.font);
-	auto layout = create_layout();
 	auto copy = subtitle;
 	copy.set_text("Qypjg");
-	setup_layout(layout, font_name, marked_up({copy}, _target_height, 1, font_name));
+	auto layout = create_layout(font_name, marked_up({copy}, _target_height, 1, font_name));
 	auto ink = layout->get_ink_extents();
 	auto const scale = float(_target_height * Pango::SCALE);
 	return _cache.insert({id, { ink.get_y() / scale, ink.get_height() / scale}}).first;
diff --git a/src/lib/render_text.h b/src/lib/render_text.h
index 762d79446..6d20912a2 100644
--- a/src/lib/render_text.h
+++ b/src/lib/render_text.h
@@ -19,8 +19,9 @@
 */
 
 
-#include "position_image.h"
 #include "dcpomatic_time.h"
+#include "position_image.h"
+#include "rect.h"
 #include "string_text.h"
 #include <dcp/util.h>
 #include <memory>
@@ -33,6 +34,7 @@ namespace dcpomatic {
 
 std::string marked_up (std::list<StringText> subtitles, int target_height, float fade_factor, std::string font_name);
 std::list<PositionImage> render_text (std::list<StringText>, dcp::Size, dcpomatic::DCPTime, int);
+std::list<dcpomatic::Rect<int>> bounding_box(std::list<StringText> subtitles, dcp::Size target, boost::optional<dcp::SubtitleStandard> override_standard = boost::none);
 
 
 class FontMetrics
diff --git a/src/lib/string_text.h b/src/lib/string_text.h
index 4eef7da05..787231b8c 100644
--- a/src/lib/string_text.h
+++ b/src/lib/string_text.h
@@ -24,6 +24,7 @@
 
 
 #include "font.h"
+#include <dcp/subtitle_standard.h>
 #include <dcp/subtitle_string.h>
 
 
@@ -40,7 +41,7 @@
 class StringText : public dcp::SubtitleString
 {
 public:
-	StringText(dcp::SubtitleString dcp_, int outline_width_, std::shared_ptr<dcpomatic::Font> font_, dcp::Standard valign_standard_)
+	StringText(dcp::SubtitleString dcp_, int outline_width_, std::shared_ptr<dcpomatic::Font> font_, dcp::SubtitleStandard valign_standard_)
 		: dcp::SubtitleString (dcp_)
 		, outline_width (outline_width_)
 		, font (font_)
@@ -49,18 +50,24 @@ public:
 
 	int outline_width;
 	std::shared_ptr<dcpomatic::Font> font;
+
 	/** Interop and SMPTE use the same VAlign choices (top, center, bottom) but give them different
-	 *  meanings.  This is the standard which should be used to interpret v_align() in this subtitle;
-	 *  valign_standard == SMPTE means:
+	 *  meanings.  To add some extra confusion, it seems that SMPTE changed their minds on this topic
+	 *  between the 2010 and 2014 versions of standard 428-7, so there isn't even one answer for SMPTE.
+	 *
+	 *  This is the standard which should be used to interpret v_align() in this subtitle.
+         *
+	 *  valign_standard == SMPTE_{2007,2010} means:
 	 *     top - top of screen to top of subtitle
 	 *     center - centre of screen to center of subtitle
 	 *     bottom - bottom of screen to bottom of subtitle
-	 *  valign_standard == Interop means:
+	 *
+	 *  valign_standard == {INTEROP,SMPTE_2014} means:
 	 *     top - top of screen to baseline of subtitle
 	 *     center - centre of screen to baseline of subtitle
 	 *     bottom - bottom of screen to baseline of subtitle
 	 */
-	dcp::Standard valign_standard;
+	dcp::SubtitleStandard valign_standard;
 };
 
 
diff --git a/src/lib/text_content.cc b/src/lib/text_content.cc
index a85b271a8..e4cbc601a 100644
--- a/src/lib/text_content.cc
+++ b/src/lib/text_content.cc
@@ -81,9 +81,9 @@ TextContent::TextContent (Content* parent, TextType type, TextType original_type
 }
 
 /** @return TextContents from node or <Text> nodes under node (according to version).
- *  The list could be empty if no TextContents are found.
+ *  The vector could be empty if no TextContents are found.
  */
-list<shared_ptr<TextContent>>
+vector<shared_ptr<TextContent>>
 TextContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version, list<string>& notes)
 {
 	if (version < 34) {
@@ -104,14 +104,15 @@ TextContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version, li
 		return { make_shared<TextContent>(parent, node, version, notes) };
 	}
 
-	list<shared_ptr<TextContent>> c;
+	vector<shared_ptr<TextContent>> content;
 	for (auto i: node->node_children("Text")) {
-		c.push_back (make_shared<TextContent>(parent, i, version, notes));
+		content.push_back(make_shared<TextContent>(parent, i, version, notes));
 	}
 
-	return c;
+	return content;
 }
 
+
 TextContent::TextContent (Content* parent, cxml::ConstNodePtr node, int version, list<string>& notes)
 	: ContentPart (parent)
 	, _use (false)
diff --git a/src/lib/text_content.h b/src/lib/text_content.h
index 7c060cd48..4d4bdc507 100644
--- a/src/lib/text_content.h
+++ b/src/lib/text_content.h
@@ -199,7 +199,7 @@ public:
 		return _language_is_additional;
 	}
 
-	static std::list<std::shared_ptr<TextContent>> from_xml (Content* parent, cxml::ConstNodePtr, int version, std::list<std::string>& notes);
+	static std::vector<std::shared_ptr<TextContent>> from_xml(Content* parent, cxml::ConstNodePtr, int version, std::list<std::string>& notes);
 
 private:
 	friend struct ffmpeg_pts_offset_test;
diff --git a/src/lib/text_decoder.cc b/src/lib/text_decoder.cc
index 6fd036ae1..58f631e59 100644
--- a/src/lib/text_decoder.cc
+++ b/src/lib/text_decoder.cc
@@ -88,7 +88,7 @@ set_forced_appearance(shared_ptr<const TextContent> content, StringText& subtitl
 
 
 void
-TextDecoder::emit_plain_start (ContentTime from, vector<dcp::SubtitleString> subtitles, dcp::Standard valign_standard)
+TextDecoder::emit_plain_start(ContentTime from, vector<dcp::SubtitleString> subtitles, dcp::SubtitleStandard valign_standard)
 {
 	vector<StringText> string_texts;
 
@@ -153,13 +153,20 @@ TextDecoder::emit_plain_start (ContentTime from, sub::Subtitle const & sub_subti
 				switch (line.vertical_position.reference.get_value_or(sub::BOTTOM_OF_SCREEN)) {
 				case sub::BOTTOM_OF_SCREEN:
 				case sub::TOP_OF_SUBTITLE:
-					/* This 0.9 is an arbitrary value to lift the bottom sub off the bottom
+					/* This 0.1 is an arbitrary value to lift the bottom sub off the bottom
 					   of the screen a bit to a pleasing degree.
 					   */
-					v_position = 0.9 -
+					v_position = 0.1 +
 						(1 + bottom_line.get() - line.vertical_position.line.get()) * multiplier;
 
-					v_align = dcp::VAlign::TOP;
+					/* Align our subtitles to the bottom of the screen, because if we are making a SMPTE
+					 * DCP and the projection system uses the wrong standard to interpret vertical position,
+					 * a bottom-aligned subtitle will be less wrong than a top-aligned one.  This is because
+					 * in the top-aligned case the difference will be the distance between bbox top an
+					 * baseline, but in the bottom-aligned case the difference will be between bbox bottom
+					 * and baseline (which is shorter).
+					 */
+					v_align = dcp::VAlign::BOTTOM;
 					break;
 				case sub::TOP_OF_SCREEN:
 					/* This 0.1 is another fudge factor to bring the top line away from the top of the screen a little */
@@ -265,7 +272,7 @@ TextDecoder::emit_plain_start (ContentTime from, sub::Subtitle const & sub_subti
 				dcp_subtitle,
 				content()->outline_width(),
 				content()->get_font(block.font.get_value_or("")),
-				dcp::Standard::SMPTE
+				dcp::SubtitleStandard::SMPTE_2014
 				);
 			set_forced_appearance(content(), string_text);
 			string_texts.push_back(string_text);
@@ -285,7 +292,7 @@ TextDecoder::emit_stop (ContentTime to)
 
 
 void
-TextDecoder::emit_plain (ContentTimePeriod period, vector<dcp::SubtitleString> subtitles, dcp::Standard valign_standard)
+TextDecoder::emit_plain(ContentTimePeriod period, vector<dcp::SubtitleString> subtitles, dcp::SubtitleStandard valign_standard)
 {
 	emit_plain_start (period.from, subtitles, valign_standard);
 	emit_stop (period.to);
diff --git a/src/lib/text_decoder.h b/src/lib/text_decoder.h
index 5362540c2..3b25e54cb 100644
--- a/src/lib/text_decoder.h
+++ b/src/lib/text_decoder.h
@@ -23,10 +23,13 @@
 #define DCPOMATIC_CAPTION_DECODER_H
 
 
+#include "content_text.h"
 #include "decoder.h"
+#include "decoder_part.h"
 #include "rect.h"
 #include "content_text.h"
-#include "decoder_part.h"
+#include "types.h"
+#include <dcp/subtitle_standard.h>
 #include <dcp/subtitle_string.h>
 #include <boost/signals2.hpp>
 
@@ -49,9 +52,9 @@ public:
 
 	void emit_bitmap_start (ContentBitmapText const& bitmap);
 	void emit_bitmap (dcpomatic::ContentTimePeriod period, std::shared_ptr<const Image> image, dcpomatic::Rect<double> rect);
-	void emit_plain_start (dcpomatic::ContentTime from, std::vector<dcp::SubtitleString> s, dcp::Standard valign_standard);
+	void emit_plain_start(dcpomatic::ContentTime from, std::vector<dcp::SubtitleString> s, dcp::SubtitleStandard valign_standard);
 	void emit_plain_start (dcpomatic::ContentTime from, sub::Subtitle const & subtitle);
-	void emit_plain (dcpomatic::ContentTimePeriod period, std::vector<dcp::SubtitleString> s, dcp::Standard valign_standard);
+	void emit_plain(dcpomatic::ContentTimePeriod period, std::vector<dcp::SubtitleString> s, dcp::SubtitleStandard valign_standard);
 	void emit_plain (dcpomatic::ContentTimePeriod period, sub::Subtitle const & subtitle);
 	void emit_stop (dcpomatic::ContentTime to);
 
diff --git a/src/lib/util.cc b/src/lib/util.cc
index 086a99f24..82f31b8f1 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -412,7 +412,7 @@ LIBDCP_ENABLE_WARNINGS
 		optional<string>(), false, false, false, dcp::Colour(), 42, 1, dcp::Time(), dcp::Time(), 0, dcp::HAlign::CENTER, 0, dcp::VAlign::CENTER, 0, dcp::Direction::LTR,
 		"Hello dolly", dcp::Effect::NONE, dcp::Colour(), dcp::Time(), dcp::Time(), 0
 		);
-	subs.push_back (StringText(ss, 0, {}, dcp::Standard::SMPTE));
+	subs.push_back(StringText(ss, 0, {}, dcp::SubtitleStandard::SMPTE_2014));
 	render_text (subs, dcp::Size(640, 480), DCPTime(), 24);
 #endif