Merge branch '2694-remove-odd-check'

Allow referring to OVs where not every reel has a subtitle/ccap,
fixing it if required by adding our own reels.

7 files changed, 113 insertions, 23 deletions

diff --git a/cscript b/cscript
index 3903bd074..fde56ad77 100644
--- a/cscript
+++ b/cscript
@@ -507,7 +507,7 @@ def dependencies(target, options):
         # Use distro-provided FFmpeg on Arch
         deps = []
 
-    deps.append(('libdcp', 'v1.8.89'))
+    deps.append(('libdcp', 'v1.8.92'))
     deps.append(('libsub', 'v1.6.45'))
     deps.append(('leqm-nrt', '30dcaea1373ac62fba050e02ce5b0c1085797a23'))
     deps.append(('rtaudio', 'f619b76'))
diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index 2d441353a..b9b64149f 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -720,14 +720,6 @@ DCPContent::can_reference_audio (shared_ptr<const Film> film, string& why_not) c
 		return false;
 	}
 
-        for (auto i: decoder->reels()) {
-                if (!i->main_sound()) {
-			/// TRANSLATORS: this string will follow "Cannot reference this DCP: "
-                        why_not = _("it does not have sound in all its reels.");
-                        return false;
-                }
-        }
-
 	/// TRANSLATORS: this string will follow "Cannot reference this DCP: "
 	return can_reference(
 		film, [](shared_ptr<const Content> c) {
@@ -755,22 +747,13 @@ DCPContent::can_reference_text (shared_ptr<const Film> film, TextType type, stri
 
         for (auto i: decoder->reels()) {
                 if (type == TextType::OPEN_SUBTITLE) {
-			if (!i->main_subtitle()) {
-				/// TRANSLATORS: this string will follow "Cannot reference this DCP: "
-				why_not = _("it does not have open subtitles in all its reels.");
-				return false;
-			} else if (i->main_subtitle()->entry_point().get_value_or(0) != 0) {
+			if (i->main_subtitle() && i->main_subtitle()->entry_point().get_value_or(0) != 0) {
 				/// TRANSLATORS: this string will follow "Cannot reference this DCP: "
 				why_not = _("one of its subtitle reels has a non-zero entry point so it must be re-written.");
 				return false;
 			}
                 }
 		if (type == TextType::CLOSED_CAPTION) {
-			if (i->closed_captions().empty()) {
-				/// TRANSLATORS: this string will follow "Cannot reference this DCP: "
-				why_not = _("it does not have closed captions in all its reels.");
-				return false;
-			}
 			for (auto j: i->closed_captions()) {
 				if (j->entry_point().get_value_or(0) != 0) {
 					/// TRANSLATORS: this string will follow "Cannot reference this DCP: "
diff --git a/src/lib/font_id_allocator.cc b/src/lib/font_id_allocator.cc
index 70eda2b06..c566c3676 100644
--- a/src/lib/font_id_allocator.cc
+++ b/src/lib/font_id_allocator.cc
@@ -44,11 +44,15 @@ FontIDAllocator::add_fonts_from_reels(vector<shared_ptr<dcp::Reel>> const& reels
 	int reel_index = 0;
 	for (auto reel: reels) {
 		if (auto sub = reel->main_subtitle()) {
-			add_fonts_from_asset(reel_index, sub->asset());
+			if (sub->asset_ref().resolved()) {
+				add_fonts_from_asset(reel_index, sub->asset());
+			}
 		}
 
 		for (auto ccap: reel->closed_captions()) {
-			add_fonts_from_asset(reel_index, ccap->asset());
+			if (ccap->asset_ref().resolved()) {
+				add_fonts_from_asset(reel_index, ccap->asset());
+			}
 		}
 
 		++reel_index;
diff --git a/src/lib/referenced_reel_asset.cc b/src/lib/referenced_reel_asset.cc
index bd87b905a..3175d772d 100644
--- a/src/lib/referenced_reel_asset.cc
+++ b/src/lib/referenced_reel_asset.cc
@@ -116,7 +116,7 @@ get_referenced_reel_assets(shared_ptr<const Film> film, shared_ptr<const Playlis
 				maybe_add_asset (reel_assets, reel->main_sound(), reel_trim_start, reel_trim_end, from, frame_rate);
 			}
 
-			if (dcp->reference_text(TextType::OPEN_SUBTITLE)) {
+			if (dcp->reference_text(TextType::OPEN_SUBTITLE) && reel->main_subtitle()) {
 				maybe_add_asset (reel_assets, reel->main_subtitle(), reel_trim_start, reel_trim_end, from, frame_rate);
 			}
 
diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 8863816e8..fbe2d248d 100644
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -41,7 +41,9 @@
 #include <dcp/cpl.h>
 #include <dcp/locale_convert.h>
 #include <dcp/raw_convert.h>
+#include <dcp/reel_closed_caption_asset.h>
 #include <dcp/reel_file_asset.h>
+#include <dcp/reel_subtitle_asset.h>
 #include <cerrno>
 #include <cfloat>
 #include <set>
@@ -930,6 +932,22 @@ void
 Writer::write (ReferencedReelAsset asset)
 {
 	_reel_assets.push_back (asset);
+
+	if (dynamic_pointer_cast<dcp::ReelSubtitleAsset>(asset.asset)) {
+		_have_subtitles = true;
+	} else if (auto ccap = dynamic_pointer_cast<dcp::ReelClosedCaptionAsset>(asset.asset)) {
+		/* This feels quite fragile. We have a referenced reel and want to know if it's
+		 * part of a given closed-caption track so that we can fill if it has any
+		 * missing reels.  I guess for that purpose almost any DCPTextTrack values are
+		 * fine so long as they are consistent.
+		 */
+		DCPTextTrack track;
+		track.name = ccap->annotation_text().get_value_or("");
+		track.language = dcp::LanguageTag(ccap->language().get_value_or("en-US"));
+		if (_have_closed_captions.find(track) == _have_closed_captions.end()) {
+			_have_closed_captions.insert(track);
+		}
+	}
 }
 
 
diff --git a/test/data b/test/data
-Subproject 154eb8c751e43e350dac04943dc48e6ab7f9c98
+Subproject a513e2cd9f48dd0f6f748e4b5a77c559bbf915c
diff --git a/test/vf_test.cc b/test/vf_test.cc
index 249f9c5b0..2311e3537 100644
--- a/test/vf_test.cc
+++ b/test/vf_test.cc
@@ -34,14 +34,20 @@
 #include "lib/job_manager.h"
 #include "lib/make_dcp.h"
 #include "lib/player.h"
+#include "lib/ratio.h"
 #include "lib/text_content.h"
 #include "lib/referenced_reel_asset.h"
 #include "lib/video_content.h"
 #include "test.h"
 #include <dcp/cpl.h>
+#include <dcp/mono_picture_asset.h>
+#include <dcp/picture_asset_writer.h>
 #include <dcp/reel.h>
-#include <dcp/reel_picture_asset.h>
+#include <dcp/reel_mono_picture_asset.h>
 #include <dcp/reel_sound_asset.h>
+#include <dcp/reel_smpte_subtitle_asset.h>
+#include <dcp/smpte_subtitle_asset.h>
+#include <dcp/subtitle_string.h>
 #include <boost/test/unit_test.hpp>
 #include <iostream>
 
@@ -52,6 +58,7 @@ using std::list;
 using std::make_shared;
 using std::shared_ptr;
 using std::string;
+using std::vector;
 using namespace dcpomatic;
 
 
@@ -470,3 +477,81 @@ BOOST_AUTO_TEST_CASE(test_duplicate_font_id_in_vf)
 		});
 }
 
+
+BOOST_AUTO_TEST_CASE(test_referencing_ov_with_missing_subtitle_in_some_reels)
+{
+	auto const path = boost::filesystem::path("build/test/test_referencing_ov_with_missing_subtitle_in_some_reels");
+	boost::filesystem::remove_all(path);
+
+	boost::filesystem::create_directories(path / "ov");
+	dcp::DCP ov(path / "ov");
+
+	auto make_picture = [path](string filename) {
+		auto pic = make_shared<dcp::MonoPictureAsset>(dcp::Fraction(24, 1), dcp::Standard::SMPTE);
+		auto writer = pic->start_write(path / "ov" / filename, dcp::PictureAsset::Behaviour::MAKE_NEW);
+		auto frame = dcp::ArrayData("test/data/picture.j2c");
+		for (int i = 0; i < 240; ++i) {
+			writer->write(frame);
+		}
+		writer->finalize();
+		return pic;
+	};
+
+	auto pic1 = make_picture("pic1.mxf");
+	auto pic2 = make_picture("pic2.mxf");
+
+	auto sub1 = make_shared<dcp::SMPTESubtitleAsset>();
+
+	sub1->add(std::make_shared<dcp::SubtitleString>(
+		boost::optional<string>(), false, false, false, dcp::Colour(255, 255, 255),
+		42, 1, dcp::Time(0, 0, 5, 0, 24), dcp::Time(0, 0, 9, 0, 24),
+		0, dcp::HAlign::CENTER,
+		0, dcp::VAlign::CENTER,
+		0, dcp::Direction::LTR,
+		"Hello",
+		dcp::Effect::NONE, dcp::Colour(0, 0, 0),
+		dcp::Time{}, dcp::Time{},
+		0, vector<dcp::Ruby>{}
+		));
+	sub1->write(path / "ov" / "sub.mxf");
+
+	auto reel1_pic = make_shared<dcp::ReelMonoPictureAsset>(pic1, 0);
+	auto reel1_sub = make_shared<dcp::ReelSMPTESubtitleAsset>(sub1, dcp::Fraction(24, 1), 240, 0);
+
+	auto reel2_pic = make_shared<dcp::ReelMonoPictureAsset>(pic1, 0);
+
+	auto reel1 = make_shared<dcp::Reel>(reel1_pic, shared_ptr<dcp::ReelSoundAsset>(), reel1_sub);
+	auto reel2 = make_shared<dcp::Reel>(reel2_pic);
+
+	auto cpl = make_shared<dcp::CPL>("Test CPL", dcp::ContentKind::FEATURE, dcp::Standard::SMPTE);
+	cpl->add(reel1);
+	cpl->add(reel2);
+
+	ov.add(cpl);
+	ov.write_xml();
+
+	auto dcp_ov = make_shared<DCPContent>(path / "ov");
+	auto vf = make_shared<Film>(path / "vf");
+	vf->set_dcp_content_type(DCPContentType::from_isdcf_name("TST"));
+	vf->set_container(Ratio::from_id("185"));
+	vf->write_metadata();
+	vf->examine_and_add_content(dcp_ov);
+	BOOST_REQUIRE(!wait_for_jobs());
+	vf->set_reel_type(ReelType::BY_VIDEO_CONTENT);
+	dcp_ov->set_reference_video(true);
+	dcp_ov->set_reference_text(TextType::OPEN_SUBTITLE, true);
+
+	vf->write_metadata();
+	make_dcp(vf, TranscodeJob::ChangedBehaviour::IGNORE);
+	BOOST_REQUIRE(!wait_for_jobs());
+
+	vector<dcp::VerificationNote::Code> ignore = {
+		dcp::VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE,
+		dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME,
+		dcp::VerificationNote::Code::INVALID_SUBTITLE_SPACING,
+		dcp::VerificationNote::Code::EXTERNAL_ASSET,
+	};
+
+	verify_dcp(vf->dir(vf->dcp_name()), ignore);
+}
+