Don't write fonts to closed caption assets, even if we have some (re-fixing #2505).2525-ccap-fonts

4 files changed, 41 insertions, 18 deletions

diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index 87e59de50..8f286c5c0 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -278,7 +278,6 @@ DCPContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
 	for (int i = 0; i < examiner->text_count(TextType::CLOSED_CAPTION); ++i) {
 		auto c = make_shared<TextContent>(this, TextType::CLOSED_CAPTION, TextType::CLOSED_CAPTION);
 		c->set_dcp_track (examiner->dcp_text_track(i));
-		add_fonts_from_examiner(c, examiner->fonts());
 		new_text.push_back (c);
 	}
 
diff --git a/src/lib/dcp_examiner.cc b/src/lib/dcp_examiner.cc
index 9c4f899c6..91dae50b2 100644
--- a/src/lib/dcp_examiner.cc
+++ b/src/lib/dcp_examiner.cc
@@ -229,10 +229,6 @@ DCPExaminer::DCPExaminer (shared_ptr<const DCPContent> content, bool tolerant)
 			}
 
 			LOG_GENERAL("Closed caption %1 of reel %2 found", ccap->id(), reel->id());
-
-			for (auto const& font: ccap->asset()->font_data()) {
-				reel_fonts.push_back(make_shared<dcpomatic::Font>(font.first, font.second));
-			}
 		}
 
 		if (reel->main_markers ()) {
diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index 062d3ca81..6f668a72e 100644
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -486,12 +486,16 @@ maybe_add_text (
 	Frame const period_duration = period.duration().frames_round(film->video_frame_rate());
 
 	shared_ptr<Result> reel_asset;
+	auto const font_tags = std::is_same<Result, dcp::ReelClosedCaptionAsset>::value ? dcp::SubtitleAsset::FontTags::OMIT : dcp::SubtitleAsset::FontTags::INCLUDE;
 
 	if (asset) {
 		if (auto interop = dynamic_pointer_cast<dcp::InteropSubtitleAsset>(asset)) {
 			auto directory = output_dcp / interop->id ();
 			boost::filesystem::create_directories (directory);
-			interop->write (directory / subtitle_asset_filename(asset, reel_index, reel_count, content_summary, ".xml"));
+			interop->write(
+				directory / subtitle_asset_filename(asset, reel_index, reel_count, content_summary, ".xml"),
+				font_tags
+				);
 			reel_asset = make_shared<Interop> (
 				interop,
 				dcp::Fraction(film->video_frame_rate(), 1),
@@ -506,7 +510,8 @@ maybe_add_text (
 			*/
 			smpte->set_intrinsic_duration(picture_duration);
 			smpte->write (
-				output_dcp / subtitle_asset_filename(asset, reel_index, reel_count, content_summary, ".mxf")
+				output_dcp / subtitle_asset_filename(asset, reel_index, reel_count, content_summary, ".mxf"),
+				font_tags
 				);
 			reel_asset = make_shared<SMPTE> (
 				smpte,
@@ -951,18 +956,22 @@ ReelWriter::write(PlayerText subs, TextType type, optional<DCPTextTrack> track,
 		i.set_out (dcp::Time(period.to.seconds() - _period.from.seconds(), tcr));
 		i.set_v_position(convert_vertical_position(i, film()->interop() ? dcp::SubtitleStandard::INTEROP : dcp::SubtitleStandard::SMPTE_2014));
 		auto sub = make_shared<dcp::SubtitleString>(i);
-		/* i.font is a shared_ptr<Font> which uniquely identifies the font we want,
-		 * though if we are Interop we can only have one font, so we'll use the chosen
-		 * one instead.
-		 */
-		auto font = film()->interop() ? chosen_interop_font : i.font;
-		/* We can get the corresponding ID from fonts */
-		auto const font_id_to_use = fonts.get(font);
-		/* Give this subtitle the correct font ID */
-		sub->set_font(font_id_to_use);
+		if (type == TextType::OPEN_SUBTITLE) {
+			/* i.font is a shared_ptr<Font> which uniquely identifies the font we want,
+			 * though if we are Interop we can only have one font, so we'll use the chosen
+			 * one instead.
+			 */
+			auto font = film()->interop() ? chosen_interop_font : i.font;
+			/* We can get the corresponding ID from fonts */
+			auto const font_id_to_use = fonts.get(font);
+			/* Give this subtitle the correct font ID */
+			sub->set_font(font_id_to_use);
+			/* Make sure the asset LoadFonts the font we are about to ask for */
+			asset->ensure_font(font_id_to_use, font->data().get_value_or(_default_font));
+		} else {
+			sub->unset_font();
+		}
 		asset->add(sub);
-		/* Make sure the asset LoadFonts the font we just asked for */
-		asset->ensure_font(font_id_to_use, font->data().get_value_or(_default_font));
 	}
 
 	for (auto i: subs.bitmap) {
diff --git a/test/closed_caption_test.cc b/test/closed_caption_test.cc
index 80effc9a0..71673b4e2 100644
--- a/test/closed_caption_test.cc
+++ b/test/closed_caption_test.cc
@@ -19,6 +19,7 @@
 */
 
 
+#include "lib/dcp_content.h"
 #include "lib/film.h"
 #include "lib/string_text_file_content.h"
 #include "lib/text_content.h"
@@ -116,3 +117,21 @@ BOOST_AUTO_TEST_CASE (closed_caption_test2)
 
 	cl.run ();
 }
+
+
+
+BOOST_AUTO_TEST_CASE(fonts_in_inputs_removed_for_ccaps)
+{
+	auto dcp_with_ccaps = make_shared<DCPContent>("test/data/xml_subtitle_test2");
+	auto film = new_test_film2("fonts_in_inputs_removed_for_ccaps", { dcp_with_ccaps });
+	for (auto i: dcp_with_ccaps->text) {
+		i->set_use(true);
+		i->set_type(TextType::CLOSED_CAPTION);
+	}
+	film->set_interop(true);
+	make_and_verify_dcp(film, { dcp::VerificationNote::Code::INVALID_STANDARD, dcp::VerificationNote::Code::INVALID_SUBTITLE_SPACING });
+
+	auto sub = dcp::file_to_string(find_file(film->dir(film->dcp_name()), "sub_"));
+	BOOST_CHECK(sub.find("Font") == std::string::npos);
+}
+