Verify error if SMPTE subtitle asset has <Text> but no <LoadFont>.v1.8.73

6 files changed, 108 insertions, 8 deletions

diff --git a/src/verify.cc b/src/verify.cc
index 1fc9e851..b3fe83a2 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -859,7 +859,8 @@ verify_text_details (
 	vector<VerificationNote>& notes,
 	std::function<bool (shared_ptr<Reel>)> check,
 	std::function<optional<string> (shared_ptr<Reel>)> xml,
-	std::function<int64_t (shared_ptr<Reel>)> duration
+	std::function<int64_t (shared_ptr<Reel>)> duration,
+	std::function<std::string (shared_ptr<Reel>)> id
 	)
 {
 	/* end of last subtitle (in editable units) */
@@ -871,16 +872,18 @@ verify_text_details (
 	auto empty_text = false;
 	/* current reel start time (in editable units) */
 	int64_t reel_offset = 0;
-	vector<string> font_ids;
 	optional<string> missing_load_font_id;
 
-	std::function<void (cxml::ConstNodePtr, optional<int>, optional<Time>, int, bool)> parse;
-	parse = [&parse, &last_out, &too_short, &too_close, &too_early, &empty_text, &reel_offset, &font_ids, &missing_load_font_id](
+	std::function<void (cxml::ConstNodePtr, optional<int>, optional<Time>, int, bool, bool&, vector<string>&)> parse;
+
+	parse = [&parse, &last_out, &too_short, &too_close, &too_early, &empty_text, &reel_offset, &missing_load_font_id](
 		cxml::ConstNodePtr node,
 		optional<int> tcr,
 		optional<Time> start_time,
 		int er,
-		bool first_reel
+		bool first_reel,
+		bool& has_text,
+		vector<string>& font_ids
 		) {
 		if (node->name() == "Subtitle") {
 			Time in (node->string_attribute("TimeIn"), tcr);
@@ -921,9 +924,12 @@ verify_text_details (
 			if (!node_has_content(node)) {
 				empty_text = true;
 			}
+			has_text = true;
 		} else if (node->name() == "LoadFont") {
 			if (auto const id = node->optional_string_attribute("Id")) {
 				font_ids.push_back(*id);
+			} else if (auto const id = node->optional_string_attribute("ID")) {
+				font_ids.push_back(*id);
 			}
 		} else if (node->name() == "Font") {
 			if (auto const font_id = node->optional_string_attribute("Id")) {
@@ -933,7 +939,7 @@ verify_text_details (
 			}
 		}
 		for (auto i: node->node_children()) {
-			parse(i, tcr, start_time, er, first_reel);
+			parse(i, tcr, start_time, er, first_reel, has_text, font_ids);
 		}
 	};
 
@@ -969,12 +975,18 @@ verify_text_details (
 			}
 			break;
 		}
-		parse (doc, tcr, start_time, edit_rate, i == 0);
+		bool has_text = false;
+		vector<string> font_ids;
+		parse(doc, tcr, start_time, edit_rate, i == 0, has_text, font_ids);
 		auto end = reel_offset + duration(reels[i]);
 		if (last_out && *last_out > end) {
 			reel_overlap = true;
 		}
 		reel_offset = end;
+
+		if (standard == dcp::Standard::SMPTE && has_text && font_ids.empty()) {
+			notes.push_back(dcp::VerificationNote(dcp::VerificationNote::Type::ERROR, VerificationNote::Code::MISSING_LOAD_FONT).set_id(id(reels[i])));
+		}
 	}
 
 	if (last_out && *last_out > reel_offset) {
@@ -1241,6 +1253,9 @@ verify_text_details(dcp::Standard standard, vector<shared_ptr<Reel>> reels, vect
 			},
 			[](shared_ptr<Reel> reel) {
 				return reel->main_subtitle()->actual_duration();
+			},
+			[](shared_ptr<Reel> reel) {
+				return reel->main_subtitle()->id();
 			}
 		);
 	}
@@ -1255,6 +1270,9 @@ verify_text_details(dcp::Standard standard, vector<shared_ptr<Reel>> reels, vect
 			},
 			[i](shared_ptr<Reel> reel) {
 				return reel->closed_captions()[i]->actual_duration();
+			},
+			[i](shared_ptr<Reel> reel) {
+				return reel->closed_captions()[i]->id();
 			}
 		);
 	}
@@ -2062,6 +2080,8 @@ dcp::note_to_string (VerificationNote note)
 		return String::compose("The XML in the subtitle asset %1 has more than one namespace declaration.", note.note().get());
 	case VerificationNote::Code::MISSING_LOAD_FONT_FOR_FONT:
 		return String::compose("A subtitle or closed caption refers to a font with ID %1 that does not have a corresponding <LoadFont> node", note.id().get());
+	case VerificationNote::Code::MISSING_LOAD_FONT:
+		return String::compose("The SMPTE subtitle asset %1 has <Text> nodes but no <LoadFont> node", note.id().get());
 	}
 
 	return "";
diff --git a/src/verify.h b/src/verify.h
index a282d328..0474dca2 100644
--- a/src/verify.h
+++ b/src/verify.h
@@ -453,6 +453,10 @@ public:
 		 *  id contains the ID of the <Font> tag.
 		 */
 		MISSING_LOAD_FONT_FOR_FONT,
+		/** A SMPTE subtitle asset has at least one <Text> element but no <LoadFont>
+		 *  id contains the ID of the subtitle asset.
+		 */
+		MISSING_LOAD_FONT
 	};
 
 	VerificationNote (Type type, Code code)
diff --git a/test/data/verify_incorrect_closed_caption_ordering3.xml b/test/data/verify_incorrect_closed_caption_ordering3.xml
index c6dffa29..3895b179 100644
--- a/test/data/verify_incorrect_closed_caption_ordering3.xml
+++ b/test/data/verify_incorrect_closed_caption_ordering3.xml
@@ -7,6 +7,7 @@
   <EditRate>24 1</EditRate>
   <TimeCodeRate>24</TimeCodeRate>
   <StartTime>00:00:00:00</StartTime>
+  <LoadFont ID="font">urn:uuid:e4f0ff0a-9eba-49e0-92ee-d89a88a575f6</LoadFont>
   <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">
diff --git a/test/data/verify_incorrect_closed_caption_ordering4.xml b/test/data/verify_incorrect_closed_caption_ordering4.xml
index 5fc2b49b..38d68eaf 100644
--- a/test/data/verify_incorrect_closed_caption_ordering4.xml
+++ b/test/data/verify_incorrect_closed_caption_ordering4.xml
@@ -7,6 +7,7 @@
   <EditRate>24 1</EditRate>
   <TimeCodeRate>24</TimeCodeRate>
   <StartTime>00:00:00:00</StartTime>
+  <LoadFont ID="font">urn:uuid:e4f0ff0a-9eba-49e0-92ee-d89a88a575f6</LoadFont>
   <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">
diff --git a/test/test.cc b/test/test.cc
index 2dac199f..ffb85386 100644
--- a/test/test.cc
+++ b/test/test.cc
@@ -450,6 +450,8 @@ make_simple_with_smpte_subs (boost::filesystem::path path)
 	subs->set_language (dcp::LanguageTag("de-DE"));
 	subs->set_start_time (dcp::Time());
 	subs->add (simple_subtitle());
+	dcp::ArrayData fake_font(1024);
+	subs->add_font("font", fake_font);
 
 	subs->write (path / "subs.mxf");
 
@@ -485,6 +487,8 @@ make_simple_with_smpte_ccaps (boost::filesystem::path path)
 	subs->set_language (dcp::LanguageTag("de-DE"));
 	subs->set_start_time (dcp::Time());
 	subs->add (simple_subtitle());
+	dcp::ArrayData fake_font(1024);
+	subs->add_font("font", fake_font);
 	subs->write (path / "ccap.mxf");
 
 	auto reel_caps = make_shared<dcp::ReelSMPTEClosedCaptionAsset>(subs, dcp::Fraction(24, 1), 192, 0);
diff --git a/test/verify_test.cc b/test/verify_test.cc
index 05e6ccd9..c7579e2e 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -359,6 +359,15 @@ check_verify_result_after_replace (string suffix, boost::function<path (string)>
 }
 
 
+static
+void
+add_font(shared_ptr<dcp::SubtitleAsset> asset)
+{
+	dcp::ArrayData fake_font(1024);
+	asset->add_font("font", fake_font);
+}
+
+
 BOOST_AUTO_TEST_CASE (verify_no_error)
 {
 	stages.clear ();
@@ -1412,6 +1421,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_closed_caption_xml_size_in_bytes)
 	for (int i = 0; i < 2048; ++i) {
 		add_test_subtitle (asset, i * 24, i * 24 + 20);
 	}
+	add_font(asset);
 	asset->set_language (dcp::LanguageTag("de-DE"));
 	asset->write (dir / "subs.mxf");
 	auto reel_asset = make_shared<dcp::ReelSMPTEClosedCaptionAsset>(asset, dcp::Fraction(24, 1), 49148, 0);
@@ -1424,7 +1434,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_closed_caption_xml_size_in_bytes)
 			{
 				dcp::VerificationNote::Type::BV21_ERROR,
 				dcp::VerificationNote::Code::INVALID_CLOSED_CAPTION_XML_SIZE_IN_BYTES,
-				string("419292"),
+				string("419371"),
 				canonical(dir / "subs.mxf")
 			},
 			{ dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME },
@@ -1539,6 +1549,7 @@ BOOST_AUTO_TEST_CASE (verify_mismatched_subtitle_languages)
 		auto subs = make_shared<dcp::SMPTESubtitleAsset>();
 		subs->set_language (dcp::LanguageTag("de-DE"));
 		subs->add (simple_subtitle());
+		add_font(subs);
 		subs->write (path / "subs1.mxf");
 		auto reel_subs = make_shared<dcp::ReelSMPTESubtitleAsset>(subs, dcp::Fraction(24, 1), reel_length, 0);
 		cpl->reels()[0]->add(reel_subs);
@@ -1548,6 +1559,7 @@ BOOST_AUTO_TEST_CASE (verify_mismatched_subtitle_languages)
 		auto subs = make_shared<dcp::SMPTESubtitleAsset>();
 		subs->set_language (dcp::LanguageTag("en-US"));
 		subs->add (simple_subtitle());
+		add_font(subs);
 		subs->write (path / "subs2.mxf");
 		auto reel_subs = make_shared<dcp::ReelSMPTESubtitleAsset>(subs, dcp::Fraction(24, 1), reel_length, 0);
 		cpl->reels()[1]->add(reel_subs);
@@ -1577,6 +1589,7 @@ BOOST_AUTO_TEST_CASE (verify_multiple_closed_caption_languages_allowed)
 		auto ccaps = make_shared<dcp::SMPTESubtitleAsset>();
 		ccaps->set_language (dcp::LanguageTag("de-DE"));
 		ccaps->add (simple_subtitle());
+		add_font(ccaps);
 		ccaps->write (path / "subs1.mxf");
 		auto reel_ccaps = make_shared<dcp::ReelSMPTEClosedCaptionAsset>(ccaps, dcp::Fraction(24, 1), reel_length, 0);
 		cpl->reels()[0]->add(reel_ccaps);
@@ -1586,6 +1599,7 @@ BOOST_AUTO_TEST_CASE (verify_multiple_closed_caption_languages_allowed)
 		auto ccaps = make_shared<dcp::SMPTESubtitleAsset>();
 		ccaps->set_language (dcp::LanguageTag("en-US"));
 		ccaps->add (simple_subtitle());
+		add_font(ccaps);
 		ccaps->write (path / "subs2.mxf");
 		auto reel_ccaps = make_shared<dcp::ReelSMPTEClosedCaptionAsset>(ccaps, dcp::Fraction(24, 1), reel_length, 0);
 		cpl->reels()[1]->add(reel_ccaps);
@@ -1730,6 +1744,7 @@ dcp_with_text (path dir, vector<TestText> subs)
 		add_test_subtitle (asset, i.in, i.out, i.v_position, i.v_align, i.text);
 	}
 	asset->set_language (dcp::LanguageTag("de-DE"));
+	add_font(asset);
 	asset->write (dir / "subs.mxf");
 
 	auto reel_asset = make_shared<T>(asset, dcp::Fraction(24, 1), asset->intrinsic_duration(), 0);
@@ -1808,6 +1823,7 @@ BOOST_AUTO_TEST_CASE (verify_valid_subtitle_first_text_time_on_second_reel)
 	/* Just late enough */
 	add_test_subtitle (asset1, 4 * 24, 5 * 24);
 	asset1->set_language (dcp::LanguageTag("de-DE"));
+	add_font(asset1);
 	asset1->write (dir / "subs1.mxf");
 	auto reel_asset1 = make_shared<dcp::ReelSMPTESubtitleAsset>(asset1, dcp::Fraction(24, 1), 5 * 24, 0);
 	auto reel1 = make_shared<dcp::Reel>();
@@ -1818,6 +1834,7 @@ BOOST_AUTO_TEST_CASE (verify_valid_subtitle_first_text_time_on_second_reel)
 
 	auto asset2 = make_shared<dcp::SMPTESubtitleAsset>();
 	asset2->set_start_time (dcp::Time());
+	add_font(asset2);
 	/* This would be too early on first reel but should be OK on the second */
 	add_test_subtitle (asset2, 3, 4 * 24);
 	asset2->set_language (dcp::LanguageTag("de-DE"));
@@ -1900,6 +1917,7 @@ BOOST_AUTO_TEST_CASE (verify_subtitle_overlapping_reel_boundary)
 	auto asset = make_shared<dcp::SMPTESubtitleAsset>();
 	asset->set_start_time (dcp::Time());
 	add_test_subtitle (asset, 0, 4 * 24);
+	add_font(asset);
 	asset->set_language (dcp::LanguageTag("de-DE"));
 	asset->write (dir / "subs.mxf");
 
@@ -2347,6 +2365,7 @@ verify_subtitles_must_be_in_all_reels_check (path dir, bool add_to_reel1, bool a
 	subs->set_language (dcp::LanguageTag("de-DE"));
 	subs->set_start_time (dcp::Time());
 	subs->add (simple_subtitle());
+	add_font(subs);
 	subs->write (dir / "subs.mxf");
 	auto reel_subs = make_shared<dcp::ReelSMPTESubtitleAsset>(subs, dcp::Fraction(24, 1), reel_length, 0);
 
@@ -2430,6 +2449,7 @@ verify_closed_captions_must_be_in_all_reels_check (path dir, int caps_in_reel1,
 	subs->set_language (dcp::LanguageTag("de-DE"));
 	subs->set_start_time (dcp::Time());
 	subs->add (simple_subtitle());
+	add_font(subs);
 	subs->write (dir / "subs.mxf");
 
 	auto reel1 = make_shared<dcp::Reel>(
@@ -2511,6 +2531,7 @@ verify_text_entry_point_check (path dir, dcp::VerificationNote::Code code, boost
 	subs->set_language (dcp::LanguageTag("de-DE"));
 	subs->set_start_time (dcp::Time());
 	subs->add (simple_subtitle());
+	add_font(subs);
 	subs->write (dir / "subs.mxf");
 	auto reel_text = make_shared<T>(subs, dcp::Fraction(24, 1), reel_length, 0);
 	adjust (reel_text);
@@ -3158,6 +3179,7 @@ BOOST_AUTO_TEST_CASE (verify_mismatched_subtitle_resource_id)
 		"<EditRate>25 1</EditRate>"
 		"<TimeCodeRate>25</TimeCodeRate>"
 		"<StartTime>00:00:00:00</StartTime>"
+		"<LoadFont ID=\"arial\">urn:uuid:e4f0ff0a-9eba-49e0-92ee-d89a88a575f6</LoadFont>"
 		"<SubtitleList>"
 		"<Font ID=\"arial\" Color=\"FFFEFEFE\" Weight=\"normal\" Size=\"42\" Effect=\"border\" EffectColor=\"FF181818\" AspectAdjust=\"1.00\">"
 		"<Subtitle SpotNumber=\"1\" TimeIn=\"00:00:03:00\" TimeOut=\"00:00:04:10\" FadeUpTime=\"00:00:00:00\" FadeDownTime=\"00:00:00:00\">"
@@ -3222,6 +3244,7 @@ BOOST_AUTO_TEST_CASE (verify_incorrect_timed_text_id)
 		"<EditRate>25 1</EditRate>"
 		"<TimeCodeRate>25</TimeCodeRate>"
 		"<StartTime>00:00:00:00</StartTime>"
+		"<LoadFont ID=\"font\">urn:uuid:0ce6e0ba-58b9-4344-8929-4d9c959c2d55</LoadFont>"
 		"<SubtitleList>"
 		"<Font ID=\"arial\" Color=\"FFFEFEFE\" Weight=\"normal\" Size=\"42\" Effect=\"border\" EffectColor=\"FF181818\" AspectAdjust=\"1.00\">"
 		"<Subtitle SpotNumber=\"1\" TimeIn=\"00:00:03:00\" TimeOut=\"00:00:04:10\" FadeUpTime=\"00:00:00:00\" FadeDownTime=\"00:00:00:00\">"
@@ -3679,3 +3702,50 @@ BOOST_AUTO_TEST_CASE(verify_missing_load_font_for_font)
 
 }
 
+
+BOOST_AUTO_TEST_CASE(verify_missing_load_font)
+{
+	boost::filesystem::path const dir = dcp::String::compose("build/test/%1", boost::unit_test::framework::current_test_case().full_name());
+	prepare_directory(dir);
+	auto dcp = make_simple (dir, 1, 202);
+
+	string const xml =
+		"<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+		"<SubtitleReel xmlns=\"http://www.smpte-ra.org/schemas/428-7/2010/DCST\">"
+		"<Id>urn:uuid:e6a8ae03-ebbf-41ed-9def-913a87d1493a</Id>"
+		"<ContentTitleText>Content</ContentTitleText>"
+		"<AnnotationText>Annotation</AnnotationText>"
+		"<IssueDate>2018-10-02T12:25:14+02:00</IssueDate>"
+		"<ReelNumber>1</ReelNumber>"
+		"<EditRate>24 1</EditRate>"
+		"<TimeCodeRate>24</TimeCodeRate>"
+		"<StartTime>00:00:00:00</StartTime>"
+		"<Language>de-DE</Language>"
+		"<SubtitleList>"
+		"<Font ID=\"arial\" Color=\"FFFEFEFE\" Weight=\"normal\" Size=\"42\" Effect=\"border\" EffectColor=\"FF181818\" AspectAdjust=\"1.00\">"
+		"<Subtitle SpotNumber=\"1\" TimeIn=\"00:00:06:00\" TimeOut=\"00:00:08:10\" FadeUpTime=\"00:00:00:00\" FadeDownTime=\"00:00:00:00\">"
+		"<Text Hposition=\"0.0\" Halign=\"center\" Valign=\"bottom\" Vposition=\"13.5\" Direction=\"ltr\">Hello world</Text>"
+		"</Subtitle>"
+		"</Font>"
+		"</SubtitleList>"
+		"</SubtitleReel>";
+
+	dcp::File xml_file(dir / "subs.xml", "w");
+	BOOST_REQUIRE(xml_file);
+	xml_file.write(xml.c_str(), xml.size(), 1);
+	xml_file.close();
+	auto subs = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.xml");
+	subs->write(dir / "subs.mxf");
+
+	auto reel_subs = make_shared<dcp::ReelSMPTESubtitleAsset>(subs, dcp::Fraction(24, 1), 202, 0);
+	dcp->cpls()[0]->reels()[0]->add(reel_subs);
+	dcp->set_annotation_text("A Test DCP");
+	dcp->write_xml();
+
+	check_verify_result (
+		{ dir },
+		{
+			dcp::VerificationNote(dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::MISSING_LOAD_FONT).set_id(reel_subs->id())
+		});
+}
+