Don't write multiple <LoadFont> tags to Interop subtitles (#1273).

6 files changed, 75 insertions, 47 deletions

diff --git a/src/lib/dcp_encoder.cc b/src/lib/dcp_encoder.cc
index 3a4a793a8..f1e819083 100644
--- a/src/lib/dcp_encoder.cc
+++ b/src/lib/dcp_encoder.cc
@@ -94,7 +94,18 @@ DCPEncoder::go ()
 	}
 
 	if (_non_burnt_subtitles) {
-		_writer->write (_player->get_subtitle_fonts ());
+		list<shared_ptr<Font> > fonts = _player->get_subtitle_fonts ();
+
+		if (fonts.size() > 1 && _film->interop()) {
+			/* Interop will ignore second and subsequent <LoadFont>s so don't even
+			   write them as they upset some validators.
+			*/
+			shared_ptr<Font> first = fonts.front ();
+			fonts.clear ();
+			fonts.push_back (first);
+		}
+
+		_writer->write (fonts);
 	}
 
 	while (!_player->pass ()) {}
diff --git a/test/dcp_subtitle_test.cc b/test/dcp_subtitle_test.cc
index 407ac2616..283fe8fcd 100644
--- a/test/dcp_subtitle_test.cc
+++ b/test/dcp_subtitle_test.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2014-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2014-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -34,6 +34,7 @@
 #include "lib/subtitle_content.h"
 #include "lib/content_subtitle.h"
 #include "lib/subtitle_decoder.h"
+#include "lib/font.h"
 #include "test.h"
 #include <iostream>
 
@@ -66,14 +67,14 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_test)
 	film->set_interop (false);
 	shared_ptr<DCPSubtitleContent> content (new DCPSubtitleContent (film, "test/data/dcp_sub.xml"));
 	film->examine_and_add_content (content);
-	wait_for_jobs ();
+	BOOST_REQUIRE (!wait_for_jobs ());
 
 	BOOST_CHECK_EQUAL (content->full_length().get(), DCPTime::from_seconds(2).get());
 
 	content->subtitle->set_use (true);
 	content->subtitle->set_burn (false);
 	film->make_dcp ();
-	wait_for_jobs ();
+	BOOST_REQUIRE (!wait_for_jobs ());
 
 	check_dcp ("test/data/dcp_subtitle_test", film->dir (film->dcp_name ()));
 }
@@ -87,7 +88,7 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_within_dcp_test)
 	film->set_name ("frobozz");
 	shared_ptr<DCPContent> content (new DCPContent (film, private_data / "JourneyToJah_TLR-1_F_EN-DE-FR_CH_51_2K_LOK_20140225_DGL_SMPTE_OV"));
 	film->examine_and_add_content (content);
-	wait_for_jobs ();
+	BOOST_REQUIRE (!wait_for_jobs ());
 
 	shared_ptr<DCPDecoder> decoder (new DCPDecoder (content, film->log(), false));
 	decoder->subtitle->TextStart.connect (bind (store, _1));
@@ -110,7 +111,7 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_test2)
 	film->set_name ("frobozz");
 	shared_ptr<DCPSubtitleContent> content (new DCPSubtitleContent (film, "test/data/dcp_sub2.xml"));
 	film->examine_and_add_content (content);
-	wait_for_jobs ();
+	BOOST_REQUIRE (!wait_for_jobs ());
 
 	shared_ptr<DCPSubtitleDecoder> decoder (new DCPSubtitleDecoder (content, film->log()));
 	decoder->subtitle->TextStart.connect (bind (store, _1));
@@ -133,10 +134,10 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_test3)
 	film->set_interop (true);
 	shared_ptr<DCPSubtitleContent> content (new DCPSubtitleContent (film, "test/data/dcp_sub3.xml"));
 	film->examine_and_add_content (content);
-	wait_for_jobs ();
+	BOOST_REQUIRE (!wait_for_jobs ());
 
 	film->make_dcp ();
-	wait_for_jobs ();
+	BOOST_REQUIRE (!wait_for_jobs ());
 
 	shared_ptr<DCPSubtitleDecoder> decoder (new DCPSubtitleDecoder (content, film->log()));
 	stored = optional<ContentTextSubtitle> ();
@@ -157,3 +158,26 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_test3)
 		}
 	}
 }
+
+/** Check that Interop DCPs aren't made with more than one <LoadFont> (#1273) */
+BOOST_AUTO_TEST_CASE (dcp_subtitle_test4)
+{
+	shared_ptr<Film> film = new_test_film2 ("dcp_subtitle_test4");
+	film->set_interop (true);
+
+	shared_ptr<DCPSubtitleContent> content (new DCPSubtitleContent (film, "test/data/dcp_sub3.xml"));
+	film->examine_and_add_content (content);
+	shared_ptr<DCPSubtitleContent> content2 (new DCPSubtitleContent (film, "test/data/dcp_sub3.xml"));
+	film->examine_and_add_content (content2);
+	BOOST_REQUIRE (!wait_for_jobs ());
+
+	content->subtitle->add_font (shared_ptr<Font> (new Font ("font1")));
+	content2->subtitle->add_font (shared_ptr<Font> (new Font ("font2")));
+
+	film->make_dcp ();
+	BOOST_REQUIRE (!wait_for_jobs ());
+
+	cxml::Document doc ("DCSubtitle");
+	doc.read_file (subtitle_file (film));
+	BOOST_REQUIRE_EQUAL (doc.node_children("LoadFont").size(), 1);
+}
diff --git a/test/srt_subtitle_test.cc b/test/srt_subtitle_test.cc
index a601ea58a..6c2928fa8 100644
--- a/test/srt_subtitle_test.cc
+++ b/test/srt_subtitle_test.cc
@@ -89,25 +89,9 @@ static void
 check_subtitle_file (shared_ptr<Film> film, boost::filesystem::path ref)
 {
 	/* Find the subtitle file and check it */
-	for (
-		boost::filesystem::directory_iterator i = boost::filesystem::directory_iterator (film->directory().get() / film->dcp_name (false));
-		i != boost::filesystem::directory_iterator ();
-		++i) {
-
-		if (boost::filesystem::is_directory (i->path ())) {
-			for (
-				boost::filesystem::directory_iterator j = boost::filesystem::directory_iterator (i->path ());
-				j != boost::filesystem::directory_iterator ();
-				++j) {
-
-				if (boost::algorithm::starts_with (j->path().leaf().string(), "sub_")) {
-					list<string> ignore;
-					ignore.push_back ("SubtitleID");
-					check_xml (*j, ref, ignore);
-				}
-			}
-		}
-	}
+	list<string> ignore;
+	ignore.push_back ("SubtitleID");
+	check_xml (subtitle_file(film), ref, ignore);
 }
 
 /** Make another DCP with a longer .srt file */
diff --git a/test/ssa_subtitle_test.cc b/test/ssa_subtitle_test.cc
index 63a90b448..bf460247c 100644
--- a/test/ssa_subtitle_test.cc
+++ b/test/ssa_subtitle_test.cc
@@ -58,23 +58,7 @@ BOOST_AUTO_TEST_CASE (ssa_subtitle_test1)
 	wait_for_jobs ();
 
 	/* Find the subtitle file and check it */
-	for (
-		boost::filesystem::directory_iterator i = boost::filesystem::directory_iterator (film->directory().get() / film->dcp_name (false));
-		i != boost::filesystem::directory_iterator ();
-		++i) {
-
-		if (boost::filesystem::is_directory (i->path ())) {
-			for (
-				boost::filesystem::directory_iterator j = boost::filesystem::directory_iterator (i->path ());
-				j != boost::filesystem::directory_iterator ();
-				++j) {
-
-				if (boost::algorithm::starts_with (j->path().leaf().string(), "sub_")) {
-					list<string> ignore;
-					ignore.push_back ("SubtitleID");
-					check_xml (*j, private_data / "DKH_UT_EN20160601def.xml", ignore);
-				}
-			}
-		}
-	}
+	list<string> ignore;
+	ignore.push_back ("SubtitleID");
+	check_xml (subtitle_file(film), private_data / "DKH_UT_EN20160601def.xml", ignore);
 }
diff --git a/test/test.cc b/test/test.cc
index c7a8bbed6..93cd5ee14 100644
--- a/test/test.cc
+++ b/test/test.cc
@@ -481,3 +481,27 @@ dcp_file (shared_ptr<const Film> film, string prefix)
 	BOOST_REQUIRE (i != boost::filesystem::directory_iterator());
 	return i->path();
 }
+
+boost::filesystem::path
+subtitle_file (shared_ptr<Film> film)
+{
+	for (
+		boost::filesystem::directory_iterator i = boost::filesystem::directory_iterator (film->directory().get() / film->dcp_name (false));
+		i != boost::filesystem::directory_iterator ();
+		++i) {
+
+		if (boost::filesystem::is_directory (i->path ())) {
+			for (
+				boost::filesystem::directory_iterator j = boost::filesystem::directory_iterator (i->path ());
+				j != boost::filesystem::directory_iterator ();
+				++j) {
+
+				if (boost::algorithm::starts_with (j->path().leaf().string(), "sub_")) {
+					return j->path();
+				}
+			}
+		}
+	}
+
+	BOOST_REQUIRE (false);
+}
diff --git a/test/test.h b/test/test.h
index 5e030f0cc..4020dc772 100644
--- a/test/test.h
+++ b/test/test.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -41,3 +41,4 @@ extern boost::filesystem::path test_film_dir (std::string);
 extern void write_image (boost::shared_ptr<const Image> image, boost::filesystem::path file, std::string format);
 boost::filesystem::path dcp_file (boost::shared_ptr<const Film> film, std::string prefix);
 void check_one_frame (boost::filesystem::path dcp, int64_t index, boost::filesystem::path ref);
+extern boost::filesystem::path subtitle_file (boost::shared_ptr<Film> film);