From c7916079e06d985121842962b9736a6673e22dfe Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 19 Sep 2021 21:45:22 +0200 Subject: [PATCH] Fix failure to open v2.14.x documents with invalid or empty subtitle languages (#2085). --- src/lib/content_factory.cc | 2 +- src/lib/dcp_content.cc | 3 ++- src/lib/dcp_subtitle_content.cc | 3 ++- src/lib/ffmpeg_content.cc | 2 +- src/lib/string_text_file_content.cc | 5 +++-- src/lib/string_text_file_content.h | 2 +- src/lib/text_content.cc | 29 ++++++++++++++++++++++------- src/lib/text_content.h | 6 +++--- test/data | 2 +- test/film_metadata_test.cc | 25 +++++++++++++++++++++++++ 10 files changed, 61 insertions(+), 18 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& notes) ); } else if (type == "SubRip" || type == "TextSubtitle") { - content.reset (new StringTextFileContent(node, version)); + content = make_shared(node, version, notes); } else if (type == "DCP") { content = make_shared(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 notes; + text = TextContent::from_xml (this, node, version, notes); atmos = AtmosContent::from_xml (this, node); for (int i = 0; i < static_cast(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 ("Length")) { - text = TextContent::from_xml (this, node, version); + list 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 { 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(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& notes) : Content (node) , _length (node->number_child("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::shared_ptr shared_from_this () { return std::dynamic_pointer_cast (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> -TextContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version) +TextContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version, list& 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("SubtitleXOffset") && !node->optional_number_child("SubtitleOffset")) { return {}; } - return { make_shared(parent, node, version) }; + return { make_shared(parent, node, version, notes) }; } list> c; for (auto i: node->node_children("Text")) { - c.push_back (make_shared(parent, i, version)); + c.push_back (make_shared(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& 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 >); - TextContent (Content* parent, cxml::ConstNodePtr, int version); + TextContent (Content* parent, std::vector>); + TextContent (Content* parent, cxml::ConstNodePtr, int version, std::list& notes); void as_xml (xmlpp::Node *) const; std::string identifier () const; @@ -197,7 +197,7 @@ public: return _language_is_additional; } - static std::list> from_xml (Content* parent, cxml::ConstNodePtr, int version); + static std::list> from_xml (Content* parent, cxml::ConstNodePtr, int version, std::list& notes); private: friend struct ffmpeg_pts_offset_test; diff --git a/test/data b/test/data index 9164d0cb4..ea55876b6 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 9164d0cb488c83d6cb4ab379ede53aaa5769e8e6 +Subproject commit ea55876b67767a6a4a9474a4af860c51dc9ee9ec 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(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(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(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." + ); +} + -- 2.30.2