Fix failure to open v2.14.x documents with invalid or empty subtitle languages (#2085).

10 files changed, 60 insertions, 17 deletions

diff --git a/src/lib/content_factory.cc b/src/lib/content_factory.cc
index 9f88caf46..b64c1b9a3 100644
--- a/src/lib/content_factory.cc
+++ b/src/lib/content_factory.cc
@@ -88,7 +88,7 @@ content_factory (cxml::ConstNodePtr node, int version, list<string>& notes)
 			);
 
 	} else if (type == "SubRip" || type == "TextSubtitle") {
-		content.reset (new StringTextFileContent(node, version));
+		content = make_shared<StringTextFileContent>(node, version, notes);
 	} else if (type == "DCP") {
 		content = make_shared<DCPContent>(node, version);
 	} else if (type == "DCPSubtitle") {
diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index fa6ec383f..e454853a1 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -96,7 +96,8 @@ DCPContent::DCPContent (cxml::ConstNodePtr node, int version)
 {
 	video = VideoContent::from_xml (this, node, version);
 	audio = AudioContent::from_xml (this, node, version);
-	text = TextContent::from_xml (this, node, version);
+	list<string> notes;
+	text = TextContent::from_xml (this, node, version, notes);
 	atmos = AtmosContent::from_xml (this, node);
 
 	for (int i = 0; i < static_cast<int>(TextType::COUNT); ++i) {
diff --git a/src/lib/dcp_subtitle_content.cc b/src/lib/dcp_subtitle_content.cc
index 858849ca3..3bae6e88f 100644
--- a/src/lib/dcp_subtitle_content.cc
+++ b/src/lib/dcp_subtitle_content.cc
@@ -48,7 +48,8 @@ DCPSubtitleContent::DCPSubtitleContent (cxml::ConstNodePtr node, int version)
 	: Content (node)
 	, _length (node->number_child<ContentTime::Type> ("Length"))
 {
-	text = TextContent::from_xml (this, node, version);
+	list<string> notes;
+	text = TextContent::from_xml (this, node, version, notes);
 }
 
 void
diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index 516962936..9017ad605 100644
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -92,7 +92,7 @@ FFmpegContent::FFmpegContent (cxml::ConstNodePtr node, int version, list<string>
 {
 	video = VideoContent::from_xml (this, node, version);
 	audio = AudioContent::from_xml (this, node, version);
-	text = TextContent::from_xml (this, node, version);
+	text = TextContent::from_xml (this, node, version, notes);
 
 	for (auto i: node->node_children("SubtitleStream")) {
 		_subtitle_streams.push_back (make_shared<FFmpegSubtitleStream>(i, version));
diff --git a/src/lib/string_text_file_content.cc b/src/lib/string_text_file_content.cc
index 25d59ec61..2ce343f2e 100644
--- a/src/lib/string_text_file_content.cc
+++ b/src/lib/string_text_file_content.cc
@@ -34,6 +34,7 @@
 
 
 using std::cout;
+using std::list;
 using std::make_shared;
 using std::shared_ptr;
 using std::string;
@@ -48,11 +49,11 @@ StringTextFileContent::StringTextFileContent (boost::filesystem::path path)
 }
 
 
-StringTextFileContent::StringTextFileContent (cxml::ConstNodePtr node, int version)
+StringTextFileContent::StringTextFileContent (cxml::ConstNodePtr node, int version, list<string>& notes)
 	: Content (node)
 	, _length (node->number_child<ContentTime::Type>("Length"))
 {
-	text = TextContent::from_xml (this, node, version);
+	text = TextContent::from_xml (this, node, version, notes);
 }
 
 
diff --git a/src/lib/string_text_file_content.h b/src/lib/string_text_file_content.h
index aec86181c..ef908051c 100644
--- a/src/lib/string_text_file_content.h
+++ b/src/lib/string_text_file_content.h
@@ -32,7 +32,7 @@ class StringTextFileContent : public Content
 {
 public:
 	StringTextFileContent (boost::filesystem::path);
-	StringTextFileContent (cxml::ConstNodePtr, int);
+	StringTextFileContent (cxml::ConstNodePtr, int, std::list<std::string>&);
 
 	std::shared_ptr<StringTextFileContent> shared_from_this () {
 		return std::dynamic_pointer_cast<StringTextFileContent> (Content::shared_from_this ());
diff --git a/src/lib/text_content.cc b/src/lib/text_content.cc
index 5ae8dd45e..1e9c609c9 100644
--- a/src/lib/text_content.cc
+++ b/src/lib/text_content.cc
@@ -84,7 +84,7 @@ TextContent::TextContent (Content* parent, TextType type, TextType original_type
  *  The list could be empty if no TextContents are found.
  */
 list<shared_ptr<TextContent>>
-TextContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version)
+TextContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version, list<string>& notes)
 {
 	if (version < 34) {
 		/* With old metadata FFmpeg content has the subtitle-related tags even with no
@@ -101,18 +101,18 @@ TextContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version)
 		if (!node->optional_number_child<double>("SubtitleXOffset") && !node->optional_number_child<double>("SubtitleOffset")) {
 			return {};
 		}
-		return { make_shared<TextContent>(parent, node, version) };
+		return { make_shared<TextContent>(parent, node, version, notes) };
 	}
 
 	list<shared_ptr<TextContent>> c;
 	for (auto i: node->node_children("Text")) {
-		c.push_back (make_shared<TextContent>(parent, i, version));
+		c.push_back (make_shared<TextContent>(parent, i, version, notes));
 	}
 
 	return c;
 }
 
-TextContent::TextContent (Content* parent, cxml::ConstNodePtr node, int version)
+TextContent::TextContent (Content* parent, cxml::ConstNodePtr node, int version, list<string>& notes)
 	: ContentPart (parent)
 	, _use (false)
 	, _burn (false)
@@ -234,9 +234,24 @@ TextContent::TextContent (Content* parent, cxml::ConstNodePtr node, int version)
 
 	auto lang = node->optional_node_child("Language");
 	if (lang) {
-		_language = dcp::LanguageTag(lang->content());
-		auto add = lang->optional_bool_attribute("Additional");
-		_language_is_additional = add && *add;
+		try {
+			_language = dcp::LanguageTag(lang->content());
+			auto add = lang->optional_bool_attribute("Additional");
+			_language_is_additional = add && *add;
+		} catch (dcp::LanguageTagError&) {
+			/* The language tag can be empty or invalid if it was loaded from a
+			 * 2.14.x metadata file; we'll just ignore it in that case.
+			 */
+			if (version <= 37) {
+				if (!lang->content().empty()) {
+					notes.push_back (String::compose(
+						_("A subtitle or closed caption file in this project is marked with the language '%1', "
+						  "which DCP-o-matic does not recognise.  The file's language has been cleared."), lang->content()));
+				}
+			} else {
+				throw;
+			}
+		}
 	}
 }
 
diff --git a/src/lib/text_content.h b/src/lib/text_content.h
index 4c6918a42..d3e9b564b 100644
--- a/src/lib/text_content.h
+++ b/src/lib/text_content.h
@@ -69,8 +69,8 @@ class TextContent : public ContentPart
 {
 public:
 	TextContent (Content* parent, TextType type, TextType original_type);
-	TextContent (Content* parent, std::vector<std::shared_ptr<Content> >);
-	TextContent (Content* parent, cxml::ConstNodePtr, int version);
+	TextContent (Content* parent, std::vector<std::shared_ptr<Content>>);
+	TextContent (Content* parent, cxml::ConstNodePtr, int version, std::list<std::string>& notes);
 
 	void as_xml (xmlpp::Node *) const;
 	std::string identifier () const;
@@ -197,7 +197,7 @@ public:
 		return _language_is_additional;
 	}
 
-	static std::list<std::shared_ptr<TextContent>> from_xml (Content* parent, cxml::ConstNodePtr, int version);
+	static std::list<std::shared_ptr<TextContent>> from_xml (Content* parent, cxml::ConstNodePtr, int version, std::list<std::string>& notes);
 
 private:
 	friend struct ffmpeg_pts_offset_test;
diff --git a/test/data b/test/data
-Subproject 9164d0cb488c83d6cb4ab379ede53aaa5769e8e
+Subproject ea55876b67767a6a4a9474a4af860c51dc9ee9e
diff --git a/test/film_metadata_test.cc b/test/film_metadata_test.cc
index 92c06210a..6d4c606e2 100644
--- a/test/film_metadata_test.cc
+++ b/test/film_metadata_test.cc
@@ -101,3 +101,28 @@ BOOST_AUTO_TEST_CASE (multiple_text_nodes_are_allowed)
 	auto test = make_shared<Film>(boost::filesystem::path("build/test/multiple_text_nodes_are_allowed2"));
 	test->read_metadata();
 }
+
+
+/** Read some metadata from v2.14.x that fails to open on 2.15.x */
+BOOST_AUTO_TEST_CASE (metadata_loads_from_2_14_x_1)
+{
+	namespace fs = boost::filesystem;
+	auto film = make_shared<Film>(fs::path("build/test/metadata_loads_from_2_14_x_1"));
+	auto notes = film->read_metadata(fs::path("test/data/2.14.x.metadata.1.xml"));
+	BOOST_REQUIRE_EQUAL (notes.size(), 0U);
+}
+
+
+/** Read some more metadata from v2.14.x that fails to open on 2.15.x */
+BOOST_AUTO_TEST_CASE (metadata_loads_from_2_14_x_2)
+{
+	namespace fs = boost::filesystem;
+	auto film = make_shared<Film>(fs::path("build/test/metadata_loads_from_2_14_x_2"));
+	auto notes = film->read_metadata(fs::path("test/data/2.14.x.metadata.2.xml"));
+	BOOST_REQUIRE_EQUAL (notes.size(), 1U);
+	BOOST_REQUIRE_EQUAL (notes.front(),
+		       "A subtitle or closed caption file in this project is marked with the language 'eng', "
+		       "which DCP-o-matic does not recognise.  The file's language has been cleared."
+		       );
+}
+