Don't write subtitles to combined Interop DCPs twice (#3079).

2 files changed, 62 insertions, 7 deletions

diff --git a/src/combine.cc b/src/combine.cc
index 7d5c51fb..275b1d9b 100644
--- a/src/combine.cc
+++ b/src/combine.cc
@@ -126,6 +126,7 @@ dcp::combine(
 
 	vector<boost::filesystem::path> paths;
 	vector<shared_ptr<dcp::Asset>> assets;
+	set<string> already_written;
 
 	for (auto i: inputs) {
 		DCP dcp(i);
@@ -150,10 +151,11 @@ dcp::combine(
 				for (auto const& k: fonts) {
 					sub->set_font_file(k.first, make_unique(output / k.second.filename()));
 				}
-				auto file = sub->file();
+				auto const file = sub->file();
 				DCP_ASSERT(file);
-				auto new_path = make_unique(output / file->filename());
+				auto const new_path = make_unique(output / file->filename());
 				sub->write(new_path);
+				already_written.insert(sub->id());
 				add_to_container(assets, sub->font_assets());
 			}
 
@@ -165,11 +167,13 @@ dcp::combine(
 
 	for (auto i: output_dcp.assets()) {
 		if (!dynamic_pointer_cast<dcp::FontAsset>(i) && !dynamic_pointer_cast<dcp::CPL>(i)) {
-			auto file = i->file();
-			DCP_ASSERT(file);
-			auto new_path = make_unique(output / file->filename());
-			create_hard_link_or_copy(*file, new_path);
-			i->set_file(new_path);
+			if (already_written.find(i->id()) == already_written.end()) {
+				auto file = i->file();
+				DCP_ASSERT(file);
+				auto new_path = make_unique(output / file->filename());
+				create_hard_link_or_copy(*file, new_path);
+				i->set_file(new_path);
+			}
 		}
 	}
 
diff --git a/test/combine_test.cc b/test/combine_test.cc
index b26520f3..a6df6f06 100644
--- a/test/combine_test.cc
+++ b/test/combine_test.cc
@@ -489,5 +489,56 @@ BOOST_AUTO_TEST_CASE(combine_multi_reel_subtitles)
 }
 
 
+BOOST_AUTO_TEST_CASE(combine_ov_with_vf)
+{
+	boost::filesystem::path const ov_path = "build/test/combine_ov_with_vf/ov";
+	auto ov = make_simple(ov_path, 1, 24, dcp::Standard::INTEROP);
+	ov->write_xml();
+
+	boost::filesystem::path const vf_path = "build/test/combine_ov_with_vf/vf";
+	boost::filesystem::remove_all(vf_path);
+
+	auto vf = make_shared<dcp::DCP>(vf_path);
+	auto cpl = make_shared<dcp::CPL>("A Test DCP", dcp::ContentKind::TRAILER, dcp::Standard::INTEROP);
+
+	auto subs = make_shared<dcp::InteropTextAsset>();
+	subs->add(simple_text());
+	subs->write(vf_path / "subs.xml");
+
+	auto reel = make_shared<dcp::Reel>(
+		ov->cpls()[0]->reels()[0]->main_picture(),
+		ov->cpls()[0]->reels()[0]->main_sound(),
+		std::make_shared<dcp::ReelInteropTextAsset>(dcp::TextType::OPEN_SUBTITLE, subs, dcp::Fraction{ 24, 1 }, 256, 0)
+		);
+
+	cpl->add(reel);
+	vf->add(cpl);
+	vf->write_xml();
+
+	boost::filesystem::path const out_path = "build/test/combine_ov_with_vf/combine";
+	boost::filesystem::remove_all(out_path);
+
+	dcp::combine(
+		{ ov_path, vf_path },
+		out_path,
+		dcp::String::compose("libdcp %1", dcp::version),
+		dcp::String::compose("libdcp %1", dcp::version),
+		dcp::LocalTime().as_string(),
+		"A Test DCP"
+		);
+
+
+	int sub_files = 0;
+	for (auto i: boost::filesystem::recursive_directory_iterator(out_path)) {
+		if (boost::filesystem::extension(i.path()) == ".xml" && i.path().filename().string().substr(0, 3) == "sub") {
+			++sub_files;
+		}
+	}
+
+	BOOST_CHECK_EQUAL(sub_files, 1U);
+}
+
+
+
 /* XXX: same CPL names */
 /* XXX: Interop PNG subs */