From fa2c49210c9fcf0f26205927aec0aceb13ca69ce Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 9 Jul 2018 20:10:09 +0100 Subject: [PATCH] Don't write multiple tags to Interop subtitles (#1273). --- src/lib/dcp_encoder.cc | 13 ++++++++++++- test/dcp_subtitle_test.cc | 38 +++++++++++++++++++++++++++++++------- test/srt_subtitle_test.cc | 22 +++------------------- test/ssa_subtitle_test.cc | 22 +++------------------- test/test.cc | 24 ++++++++++++++++++++++++ test/test.h | 3 ++- 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 > fonts = _player->get_subtitle_fonts (); + + if (fonts.size() > 1 && _film->interop()) { + /* Interop will ignore second and subsequent s so don't even + write them as they upset some validators. + */ + shared_ptr 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 + Copyright (C) 2014-2018 Carl Hetherington 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 @@ -66,14 +67,14 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_test) film->set_interop (false); shared_ptr 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 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 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 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 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 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 decoder (new DCPSubtitleDecoder (content, film->log())); stored = optional (); @@ -157,3 +158,26 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_test3) } } } + +/** Check that Interop DCPs aren't made with more than one (#1273) */ +BOOST_AUTO_TEST_CASE (dcp_subtitle_test4) +{ + shared_ptr film = new_test_film2 ("dcp_subtitle_test4"); + film->set_interop (true); + + shared_ptr content (new DCPSubtitleContent (film, "test/data/dcp_sub3.xml")); + film->examine_and_add_content (content); + shared_ptr 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 (new Font ("font1"))); + content2->subtitle->add_font (shared_ptr (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, 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 ignore; - ignore.push_back ("SubtitleID"); - check_xml (*j, ref, ignore); - } - } - } - } + list 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 ignore; - ignore.push_back ("SubtitleID"); - check_xml (*j, private_data / "DKH_UT_EN20160601def.xml", ignore); - } - } - } - } + list 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 film, string prefix) BOOST_REQUIRE (i != boost::filesystem::directory_iterator()); return i->path(); } + +boost::filesystem::path +subtitle_file (shared_ptr 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 + Copyright (C) 2013-2018 Carl Hetherington 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 image, boost::filesystem::path file, std::string format); boost::filesystem::path dcp_file (boost::shared_ptr 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); -- 2.30.2