Handle vertical alignment of subs correctly wrt the difference between Interop and SMPTE.

9 files changed, 82 insertions, 32 deletions

diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc
index 7e2001e0e..9064627ba 100644
--- a/src/lib/dcp_decoder.cc
+++ b/src/lib/dcp_decoder.cc
@@ -303,7 +303,8 @@ DCPDecoder::pass_texts (
 							ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.in().as_seconds()),
 							ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.out().as_seconds())
 							),
-						strings
+						strings,
+						_dcp_content->standard()
 						);
 					strings.clear ();
 				}
@@ -338,7 +339,8 @@ DCPDecoder::pass_texts (
 					ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.in().as_seconds()),
 					ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.out().as_seconds())
 					),
-				strings
+				strings,
+				_dcp_content->standard()
 				);
 			strings.clear ();
 		}
diff --git a/src/lib/dcp_subtitle_decoder.cc b/src/lib/dcp_subtitle_decoder.cc
index 08c2bf7ec..cbfe6fdbe 100644
--- a/src/lib/dcp_subtitle_decoder.cc
+++ b/src/lib/dcp_subtitle_decoder.cc
@@ -46,6 +46,12 @@ 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;
+	}
+
 	text.push_back (make_shared<TextDecoder>(this, content->only_text()));
 	update_position();
 }
@@ -102,7 +108,7 @@ DCPSubtitleDecoder::pass ()
 		}
 	}
 
-	only_text()->emit_plain (p, s);
+	only_text()->emit_plain(p, s, _standard);
 
 	update_position();
 
diff --git a/src/lib/dcp_subtitle_decoder.h b/src/lib/dcp_subtitle_decoder.h
index 16f89b530..3eed4ad24 100644
--- a/src/lib/dcp_subtitle_decoder.h
+++ b/src/lib/dcp_subtitle_decoder.h
@@ -42,4 +42,6 @@ private:
 
 	std::vector<std::shared_ptr<const dcp::Subtitle>> _subtitles;
 	std::vector<std::shared_ptr<const dcp::Subtitle>>::const_iterator _next;
+
+	dcp::Standard _standard;
 };
diff --git a/src/lib/render_text.cc b/src/lib/render_text.cc
index 574ee21a0..9c1c233c2 100644
--- a/src/lib/render_text.cc
+++ b/src/lib/render_text.cc
@@ -245,27 +245,41 @@ x_position (StringText const& first, int target_width, int layout_width)
 
 
 static int
-y_position (StringText const& first, int target_height, int layout_height)
+y_position (StringText const& first, int target_height, int baseline_to_bottom, int layout_height)
 {
 	int y = 0;
-	switch (first.v_align()) {
-	case dcp::VAlign::TOP:
-		/* SMPTE says that v_position is the distance between top
-		   of frame and top of subtitle, but this doesn't always seem to be
-		   the case in practice; Gunnar Ásgeirsson's Dolby server appears
-		   to put VAlign::TOP subs with v_position as the distance between top
-		   of frame and bottom of subtitle.
-		*/
-		y = first.v_position() * target_height - layout_height;
-		break;
-	case dcp::VAlign::CENTER:
-		/* v_position is distance between centre of frame and centre of subtitle */
-		y = (0.5 + first.v_position()) * target_height - layout_height / 2;
-		break;
-	case dcp::VAlign::BOTTOM:
-		/* v_position is distance between bottom of frame and bottom of subtitle */
-		y = (1.0 - first.v_position()) * target_height - layout_height;
+	switch (first.valign_standard) {
+	case dcp::Standard::INTEROP:
+		switch (first.v_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);
+			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);
+			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);
+			break;
+		}
 		break;
+	case dcp::Standard::SMPTE:
+		switch (first.v_align()) {
+		case dcp::VAlign::TOP:
+			/* v_position is distance from top of frame to top of subtitle */
+			y = first.v_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;
+			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;
+			break;
+		}
 	}
 
 	return y;
@@ -359,7 +373,7 @@ render_line (list<StringText> subtitles, dcp::Size target, DCPTime time, int fra
 	context->stroke ();
 
 	int const x = x_position (first, target.width, size.width);
-	int const y = y_position (first, target.height, size.height);
+	int const y = y_position (first, target.height, ink.get_y() / Pango::SCALE, size.height);
 	return PositionImage (image, Position<int>(max (0, x), max(0, y)));
 }
 
diff --git a/src/lib/string_text.h b/src/lib/string_text.h
index 8c505f36a..4eef7da05 100644
--- a/src/lib/string_text.h
+++ b/src/lib/string_text.h
@@ -32,20 +32,35 @@
  *    - include settings that are not applicable to true DCP subtitles.
  *      For example, we can set outline width for burn-in but this cannot be specified in DCP XML.
  *
+ *    - include details of how v_align should be interpreted
+ *
  *    - specify the font by referring to a Font object from the content we came from, rather than
  *      having to use a DCP ID like in dcp::SubtitleString.
  */
 class StringText : public dcp::SubtitleString
 {
 public:
-	StringText (dcp::SubtitleString dcp_, int outline_width_, std::shared_ptr<dcpomatic::Font> font_)
+	StringText(dcp::SubtitleString dcp_, int outline_width_, std::shared_ptr<dcpomatic::Font> font_, dcp::Standard valign_standard_)
 		: dcp::SubtitleString (dcp_)
 		, outline_width (outline_width_)
 		, font (font_)
+		, valign_standard (valign_standard_)
 	{}
 
 	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:
+	 *     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:
+	 *     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;
 };
 
 
diff --git a/src/lib/text_decoder.cc b/src/lib/text_decoder.cc
index b1b6cbcc4..930262a74 100644
--- a/src/lib/text_decoder.cc
+++ b/src/lib/text_decoder.cc
@@ -102,12 +102,17 @@ set_forced_appearance(shared_ptr<const TextContent> content, StringText& subtitl
 
 
 void
-TextDecoder::emit_plain_start (ContentTime from, vector<dcp::SubtitleString> subtitles)
+TextDecoder::emit_plain_start (ContentTime from, vector<dcp::SubtitleString> subtitles, dcp::Standard valign_standard)
 {
 	vector<StringText> string_texts;
 
 	for (auto& subtitle: subtitles) {
-		auto string_text = StringText(subtitle, content()->outline_width(), subtitle.font() ? content()->get_font(*subtitle.font()) : shared_ptr<Font>());
+		auto string_text = StringText(
+			subtitle,
+			content()->outline_width(),
+			subtitle.font() ? content()->get_font(*subtitle.font()) : shared_ptr<Font>(),
+			valign_standard
+			);
 		string_text.set_text(escape_text(string_text.text()));
 		set_forced_appearance(content(), string_text);
 		string_texts.push_back(string_text);
@@ -265,7 +270,12 @@ TextDecoder::emit_plain_start (ContentTime from, sub::Subtitle const & sub_subti
 				0
 				);
 
-			auto string_text = StringText(dcp_subtitle, content()->outline_width(), content()->get_font(block.font.get_value_or("")));
+			auto string_text = StringText(
+				dcp_subtitle,
+				content()->outline_width(),
+				content()->get_font(block.font.get_value_or("")),
+				dcp::Standard::SMPTE
+				);
 			set_forced_appearance(content(), string_text);
 			string_texts.push_back(string_text);
 		}
@@ -284,9 +294,9 @@ TextDecoder::emit_stop (ContentTime to)
 
 
 void
-TextDecoder::emit_plain (ContentTimePeriod period, vector<dcp::SubtitleString> subtitles)
+TextDecoder::emit_plain (ContentTimePeriod period, vector<dcp::SubtitleString> subtitles, dcp::Standard valign_standard)
 {
-	emit_plain_start (period.from, subtitles);
+	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 5f01c5b1f..9b3050f71 100644
--- a/src/lib/text_decoder.h
+++ b/src/lib/text_decoder.h
@@ -50,9 +50,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);
+	void emit_plain_start (dcpomatic::ContentTime from, std::vector<dcp::SubtitleString> s, dcp::Standard valign_standard);
 	void emit_plain_start (dcpomatic::ContentTime from, sub::Subtitle const & subtitle);
-	void emit_plain (dcpomatic::ContentTimePeriod period, std::vector<dcp::SubtitleString> s);
+	void emit_plain (dcpomatic::ContentTimePeriod period, std::vector<dcp::SubtitleString> s, dcp::Standard 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 dd9bbb7eb..cfeacdb92 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -411,7 +411,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, dcp::Direction::LTR,
 		"Hello dolly", dcp::Effect::NONE, dcp::Colour(), dcp::Time(), dcp::Time(), 0
 		);
-	subs.push_back (StringText(ss, 0, {}));
+	subs.push_back (StringText(ss, 0, {}, dcp::Standard::SMPTE));
 	render_text (subs, dcp::Size(640, 480), DCPTime(), 24);
 #endif
 
diff --git a/test/render_subtitles_test.cc b/test/render_subtitles_test.cc
index cd696c269..282c9311f 100644
--- a/test/render_subtitles_test.cc
+++ b/test/render_subtitles_test.cc
@@ -64,7 +64,8 @@ add (std::list<StringText>& s, std::string text, bool italic, bool bold, bool un
 				0
 				),
 			2,
-			std::shared_ptr<dcpomatic::Font>()
+			std::shared_ptr<dcpomatic::Font>(),
+			dcp::Standard::SMPTE
 			)
 		);
 }