Order subtitles in the XML according to their vertical position (DoM bug #2106).v1.8.4

4 files changed, 351 insertions, 0 deletions

diff --git a/test/data/verify_incorrect_closed_caption_ordering3.xml b/test/data/verify_incorrect_closed_caption_ordering3.xml
new file mode 100644
index 00000000..c6dffa29
--- /dev/null
+++ b/test/data/verify_incorrect_closed_caption_ordering3.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<SubtitleReel xmlns="http://www.smpte-ra.org/schemas/428-7/2010/DCST" xmlns:xs="http://www.w3.org/2001/XMLSchema">
+  <Id>urn:uuid:fc754ec1-eb8d-4029-8023-80d7dadc6313</Id>
+  <ContentTitleText></ContentTitleText>
+  <IssueDate>2021-10-24T00:23:49.000+02:00</IssueDate>
+  <Language>de-DE</Language>
+  <EditRate>24 1</EditRate>
+  <TimeCodeRate>24</TimeCodeRate>
+  <StartTime>00:00:00:00</StartTime>
+  <SubtitleList>
+    <Font AspectAdjust="1.0" Color="FF000000" Effect="none" EffectColor="FF000000" Italic="no" Script="normal" Size="42" Underline="no" Weight="normal">
+      <Subtitle FadeDownTime="00:00:00:00" FadeUpTime="00:00:00:00" SpotNumber="1" TimeIn="00:00:04:00" TimeOut="00:00:12:12">
+        <Text Valign="center" Vposition="20">This</Text>
+        <Text Valign="center" Vposition="10">is</Text>
+        <Text Valign="center" Vposition="0">also not fine</Text>
+      </Subtitle>
+    </Font>
+  </SubtitleList>
+</SubtitleReel>
+
diff --git a/test/data/verify_incorrect_closed_caption_ordering4.xml b/test/data/verify_incorrect_closed_caption_ordering4.xml
new file mode 100644
index 00000000..5fc2b49b
--- /dev/null
+++ b/test/data/verify_incorrect_closed_caption_ordering4.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<SubtitleReel xmlns="http://www.smpte-ra.org/schemas/428-7/2010/DCST" xmlns:xs="http://www.w3.org/2001/XMLSchema">
+  <Id>urn:uuid:fc754ec1-eb8d-4029-8023-80d7dadc6313</Id>
+  <ContentTitleText></ContentTitleText>
+  <IssueDate>2021-10-24T00:23:49.000+02:00</IssueDate>
+  <Language>de-DE</Language>
+  <EditRate>24 1</EditRate>
+  <TimeCodeRate>24</TimeCodeRate>
+  <StartTime>00:00:00:00</StartTime>
+  <SubtitleList>
+    <Font AspectAdjust="1.0" Color="FF000000" Effect="none" EffectColor="FF000000" Italic="no" Script="normal" Size="42" Underline="no" Weight="normal">
+      <Subtitle FadeDownTime="00:00:00:00" FadeUpTime="00:00:00:00" SpotNumber="1" TimeIn="00:00:04:00" TimeOut="00:00:12:12">
+        <Text Valign="bottom" Vposition="20">This</Text>
+        <Text Valign="bottom" Vposition="10">is</Text>
+        <Text Valign="bottom" Vposition="0">fine, though.</Text>
+      </Subtitle>
+    </Font>
+  </SubtitleList>
+</SubtitleReel>
+
diff --git a/test/smpte_subtitle_test.cc b/test/smpte_subtitle_test.cc
index b1ad4058..2b8491a3 100644
--- a/test/smpte_subtitle_test.cc
+++ b/test/smpte_subtitle_test.cc
@@ -526,3 +526,178 @@ BOOST_AUTO_TEST_CASE (write_smpte_subtitle_test3)
 	BOOST_CHECK (image->fade_up_time() == dcp::Time(0, 0, 0, 0, 24));
 	BOOST_CHECK (image->fade_down_time() == dcp::Time(0, 0, 0, 0, 24));
 }
+
+
+/* Some closed caption systems require the <Text> elements to be written in order of their
+ * vertical position (see DoM bug #2106).
+ */
+BOOST_AUTO_TEST_CASE (write_subtitles_in_vertical_order_with_top_alignment)
+{
+	dcp::SMPTESubtitleAsset c;
+	c.set_reel_number (1);
+	c.set_language (dcp::LanguageTag("en"));
+	c.set_content_title_text ("Test");
+	c.set_issue_date (dcp::LocalTime ("2016-04-01T03:52:00+00:00"));
+
+	c.add (
+		make_shared<dcp::SubtitleString>(
+			string ("Arial"),
+			false,
+			false,
+			false,
+			dcp::Colour (255, 255, 255),
+			48,
+			1.0,
+			dcp::Time (0, 0, 1, 0, 24),
+			dcp::Time (0, 0, 9, 0, 24),
+			0,
+			dcp::HAlign::CENTER,
+			0.8,
+			dcp::VAlign::TOP,
+			dcp::Direction::LTR,
+			"Top line",
+			dcp::Effect::NONE,
+			dcp::Colour (0, 0, 0),
+			dcp::Time (0, 0, 0, 0, 24),
+			dcp::Time (0, 0, 0, 0, 24),
+			0
+			)
+		);
+
+	c.add (
+		make_shared<dcp::SubtitleString>(
+			string ("Arial"),
+			false,
+			false,
+			false,
+			dcp::Colour (255, 255, 255),
+			48,
+			1.0,
+			dcp::Time (0, 0, 1, 0, 24),
+			dcp::Time (0, 0, 9, 0, 24),
+			0,
+			dcp::HAlign::CENTER,
+			0.9,
+			dcp::VAlign::TOP,
+			dcp::Direction::LTR,
+			"Bottom line",
+			dcp::Effect::NONE,
+			dcp::Colour (0, 0, 0),
+			dcp::Time (0, 0, 0, 0, 24),
+			dcp::Time (0, 0, 0, 0, 24),
+			0
+			)
+		);
+
+	c._xml_id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
+
+	check_xml (
+		c.xml_as_string(),
+		"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+		"<SubtitleReel xmlns=\"http://www.smpte-ra.org/schemas/428-7/2010/DCST\" xmlns:xs=\"http://www.w3.org/2001/XMLSchema\">"
+		  "<Id>urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6</Id>"
+		  "<ContentTitleText>Test</ContentTitleText>"
+		  "<IssueDate>2016-04-01T03:52:00.000+00:00</IssueDate>"
+		  "<ReelNumber>1</ReelNumber>"
+		  "<Language>en</Language>"
+		  "<EditRate>24 1</EditRate>"
+		  "<TimeCodeRate>24</TimeCodeRate>"
+		  "<SubtitleList>"
+		    "<Font AspectAdjust=\"1.0\" Color=\"FFFFFFFF\" Effect=\"none\" EffectColor=\"FF000000\" ID=\"Arial\" Italic=\"no\" Script=\"normal\" Size=\"48\" Underline=\"no\" Weight=\"normal\">"
+		      "<Subtitle SpotNumber=\"1\" TimeIn=\"00:00:01:00\" TimeOut=\"00:00:09:00\" FadeUpTime=\"00:00:00:00\" FadeDownTime=\"00:00:00:00\">"
+		        "<Text Valign=\"top\" Vposition=\"80\">Top line</Text>"
+		        "<Text Valign=\"top\" Vposition=\"90\">Bottom line</Text>"
+		      "</Subtitle>"
+		    "</Font>"
+		  "</SubtitleList>"
+		"</SubtitleReel>",
+		{}
+		);
+}
+
+
+/* See the test above */
+BOOST_AUTO_TEST_CASE (write_subtitles_in_vertical_order_with_bottom_alignment)
+{
+	dcp::SMPTESubtitleAsset c;
+	c.set_reel_number (1);
+	c.set_language (dcp::LanguageTag("en"));
+	c.set_content_title_text ("Test");
+	c.set_issue_date (dcp::LocalTime ("2016-04-01T03:52:00+00:00"));
+
+	c.add (
+		make_shared<dcp::SubtitleString>(
+			string ("Arial"),
+			false,
+			false,
+			false,
+			dcp::Colour (255, 255, 255),
+			48,
+			1.0,
+			dcp::Time (0, 0, 1, 0, 24),
+			dcp::Time (0, 0, 9, 0, 24),
+			0,
+			dcp::HAlign::CENTER,
+			0.8,
+			dcp::VAlign::BOTTOM,
+			dcp::Direction::LTR,
+			"Top line",
+			dcp::Effect::NONE,
+			dcp::Colour (0, 0, 0),
+			dcp::Time (0, 0, 0, 0, 24),
+			dcp::Time (0, 0, 0, 0, 24),
+			0
+			)
+		);
+
+	c.add (
+		make_shared<dcp::SubtitleString>(
+			string ("Arial"),
+			false,
+			false,
+			false,
+			dcp::Colour (255, 255, 255),
+			48,
+			1.0,
+			dcp::Time (0, 0, 1, 0, 24),
+			dcp::Time (0, 0, 9, 0, 24),
+			0,
+			dcp::HAlign::CENTER,
+			0.7,
+			dcp::VAlign::BOTTOM,
+			dcp::Direction::LTR,
+			"Bottom line",
+			dcp::Effect::NONE,
+			dcp::Colour (0, 0, 0),
+			dcp::Time (0, 0, 0, 0, 24),
+			dcp::Time (0, 0, 0, 0, 24),
+			0
+			)
+		);
+
+	c._xml_id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
+
+	check_xml (
+		c.xml_as_string(),
+		"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+		"<SubtitleReel xmlns=\"http://www.smpte-ra.org/schemas/428-7/2010/DCST\" xmlns:xs=\"http://www.w3.org/2001/XMLSchema\">"
+		  "<Id>urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6</Id>"
+		  "<ContentTitleText>Test</ContentTitleText>"
+		  "<IssueDate>2016-04-01T03:52:00.000+00:00</IssueDate>"
+		  "<ReelNumber>1</ReelNumber>"
+		  "<Language>en</Language>"
+		  "<EditRate>24 1</EditRate>"
+		  "<TimeCodeRate>24</TimeCodeRate>"
+		  "<SubtitleList>"
+		    "<Font AspectAdjust=\"1.0\" Color=\"FFFFFFFF\" Effect=\"none\" EffectColor=\"FF000000\" ID=\"Arial\" Italic=\"no\" Script=\"normal\" Size=\"48\" Underline=\"no\" Weight=\"normal\">"
+		      "<Subtitle SpotNumber=\"1\" TimeIn=\"00:00:01:00\" TimeOut=\"00:00:09:00\" FadeUpTime=\"00:00:00:00\" FadeDownTime=\"00:00:00:00\">"
+		        "<Text Valign=\"bottom\" Vposition=\"80\">Top line</Text>"
+		        "<Text Valign=\"bottom\" Vposition=\"70\">Bottom line</Text>"
+		      "</Subtitle>"
+		    "</Font>"
+		  "</SubtitleList>"
+		"</SubtitleReel>",
+		{}
+		);
+}
+
diff --git a/test/verify_test.cc b/test/verify_test.cc
index ab0a2f26..c8d0e579 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -1618,6 +1618,43 @@ dcp_with_text (path dir, vector<TestText> subs)
 }
 
 
+template <class T>
+shared_ptr<dcp::CPL>
+dcp_with_text_from_file (path dir, boost::filesystem::path subs_xml)
+{
+	prepare_directory (dir);
+	auto asset = make_shared<dcp::SMPTESubtitleAsset>(subs_xml);
+	asset->set_start_time (dcp::Time());
+	asset->set_language (dcp::LanguageTag("de-DE"));
+
+	auto subs_mxf = dir / "subs.mxf";
+	asset->write (subs_mxf);
+
+	/* The call to write() puts the asset into the DCP correctly but it will have
+	 * XML re-written by our parser.  Overwrite the MXF using the given file's verbatim
+	 * contents.
+	 */
+	ASDCP::TimedText::MXFWriter writer;
+	ASDCP::WriterInfo writer_info;
+	writer_info.LabelSetType = ASDCP::LS_MXF_SMPTE;
+	unsigned int c;
+	Kumu::hex2bin (asset->id().c_str(), writer_info.AssetUUID, Kumu::UUID_Length, &c);
+	DCP_ASSERT (c == Kumu::UUID_Length);
+	ASDCP::TimedText::TimedTextDescriptor descriptor;
+	descriptor.ContainerDuration = asset->intrinsic_duration();
+	Kumu::hex2bin (asset->xml_id()->c_str(), descriptor.AssetID, ASDCP::UUIDlen, &c);
+	DCP_ASSERT (c == Kumu::UUID_Length);
+	ASDCP::Result_t r = writer.OpenWrite (subs_mxf.string().c_str(), writer_info, descriptor, 16384);
+	BOOST_REQUIRE (!ASDCP_FAILURE(r));
+	r = writer.WriteTimedTextResource (dcp::file_to_string(subs_xml));
+	BOOST_REQUIRE (!ASDCP_FAILURE(r));
+	writer.Finalize ();
+
+	auto reel_asset = make_shared<T>(asset, dcp::Fraction(24, 1), asset->intrinsic_duration(), 0);
+	return write_dcp_with_single_asset (dir, reel_asset);
+}
+
+
 BOOST_AUTO_TEST_CASE (verify_invalid_subtitle_first_text_time)
 {
 	auto const dir = path("build/test/verify_invalid_subtitle_first_text_time");
@@ -1955,6 +1992,105 @@ BOOST_AUTO_TEST_CASE (verify_invalid_closed_caption_line_length)
 }
 
 
+BOOST_AUTO_TEST_CASE (verify_mismatched_closed_caption_valign1)
+{
+	auto const dir = path ("build/test/verify_mismatched_closed_caption_valign1");
+	auto cpl = dcp_with_text<dcp::ReelSMPTEClosedCaptionAsset> (
+		dir,
+		{
+			{ 96, 300, 0.0, dcp::VAlign::TOP, "This" },
+			{ 96, 300, 0.1, dcp::VAlign::TOP, "is" },
+			{ 96, 300, 0.2, dcp::VAlign::TOP, "fine" },
+		});
+	check_verify_result (
+		{dir},
+		{
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+		});
+}
+
+
+BOOST_AUTO_TEST_CASE (verify_mismatched_closed_caption_valign2)
+{
+	auto const dir = path ("build/test/verify_mismatched_closed_caption_valign2");
+	auto cpl = dcp_with_text<dcp::ReelSMPTEClosedCaptionAsset> (
+		dir,
+		{
+			{ 96, 300, 0.0, dcp::VAlign::TOP, "This" },
+			{ 96, 300, 0.1, dcp::VAlign::TOP, "is" },
+			{ 96, 300, 0.2, dcp::VAlign::CENTER, "not fine" },
+		});
+	check_verify_result (
+		{dir},
+		{
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::MISMATCHED_CLOSED_CAPTION_VALIGN },
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+		});
+}
+
+
+BOOST_AUTO_TEST_CASE (verify_incorrect_closed_caption_ordering1)
+{
+	auto const dir = path ("build/test/verify_invalid_incorrect_closed_caption_ordering1");
+	auto cpl = dcp_with_text<dcp::ReelSMPTEClosedCaptionAsset> (
+		dir,
+		{
+			{ 96, 300, 0.0, dcp::VAlign::TOP, "This" },
+			{ 96, 300, 0.1, dcp::VAlign::TOP, "is" },
+			{ 96, 300, 0.2, dcp::VAlign::TOP, "fine" },
+		});
+	check_verify_result (
+		{dir},
+		{
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+		});
+}
+
+
+BOOST_AUTO_TEST_CASE (verify_incorrect_closed_caption_ordering2)
+{
+	auto const dir = path ("build/test/verify_invalid_incorrect_closed_caption_ordering2");
+	auto cpl = dcp_with_text<dcp::ReelSMPTEClosedCaptionAsset> (
+		dir,
+		{
+			{ 96, 300, 0.2, dcp::VAlign::BOTTOM, "This" },
+			{ 96, 300, 0.1, dcp::VAlign::BOTTOM, "is" },
+			{ 96, 300, 0.0, dcp::VAlign::BOTTOM, "also fine" },
+		});
+	check_verify_result (
+		{dir},
+		{
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+		});
+}
+
+
+BOOST_AUTO_TEST_CASE (verify_incorrect_closed_caption_ordering3)
+{
+	auto const dir = path ("build/test/verify_incorrect_closed_caption_ordering3");
+	auto cpl = dcp_with_text_from_file<dcp::ReelSMPTEClosedCaptionAsset> (dir, "test/data/verify_incorrect_closed_caption_ordering3.xml");
+	check_verify_result (
+		{dir},
+		{
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INCORRECT_CLOSED_CAPTION_ORDERING },
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+		});
+}
+
+
+BOOST_AUTO_TEST_CASE (verify_incorrect_closed_caption_ordering4)
+{
+	auto const dir = path ("build/test/verify_incorrect_closed_caption_ordering4");
+	auto cpl = dcp_with_text_from_file<dcp::ReelSMPTEClosedCaptionAsset> (dir, "test/data/verify_incorrect_closed_caption_ordering4.xml");
+	check_verify_result (
+		{dir},
+		{
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+		});
+}
+
+
+
 BOOST_AUTO_TEST_CASE (verify_invalid_sound_frame_rate)
 {
 	path const dir("build/test/verify_invalid_sound_frame_rate");