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

diff --git a/src/lib/dcp_encoder.cc b/src/lib/dcp_encoder.cc
index 3a4a793a87c5fdcfa35c5a1db3b150ce88440a07..f1e819083a1728ffbf603386e52f0b5b5253dfd4 100644 (file)
--- 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 407ac26164e405cc674c0b349f4c4dcd7869f5ee..283fe8fcd20a543aa122d3f6f41e9b186d544d3b 100644 (file)
--- 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 a601ea58a2de12a96b78015333ee4c00c3caa16a..6c2928fa8f0599753932647aca662b345b6a048d 100644 (file)
--- 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 63a90b448c3852726437a92230fad95d957c7ac2..bf460247c7ccf174ef9023fd73e56efe518db6b4 100644 (file)
--- 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 c7a8bbed6cc52fd4a0f0c8d17d1cabfc881dcab7..93cd5ee14de923030e22a9c32c2758133fc26e24 100644 (file)
--- 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 5e030f0ccb3ad2af8dc5fa180e6f06c3af722439..4020dc772204d4355038676c1deab86d51d5d571 100644 (file)
--- 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);