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

diff --git a/src/lib/content_factory.cc b/src/lib/content_factory.cc
index 9f88caf468a1c0b475803ce1032a557a2cb7d2f3..b64c1b9a3d6f383fed236e6e79cf4f8622d86f29 100644 (file)
--- 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 fa6ec383f8ccae10bde9e6990dda77345948fc5c..e454853a1c2bc592db72a8d16686b32a5c28c625 100644 (file)
--- 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 858849ca3134f003803abbfcad881dcb890c1d8e..3bae6e88fb572e686c418995759adcc2d698752f 100644 (file)
--- 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 516962936238824775970f5796dc97c10ca6f62e..9017ad605dd9cb021e10e46bfcff307cc34c3f46 100644 (file)
--- 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 25d59ec61a6744d6538160fc3c4d84a564620501..2ce343f2edb4d700eabff7c8c8916aa52bf1a4af 100644 (file)
--- 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 aec86181ccc8ab58adb902c8a6c4e6bc25345a16..ef908051c623c4cbdeae3aa3be7e6811ad96e2ca 100644 (file)
--- 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 5ae8dd45eb47724454b6ebd18397e4755ec3dd45..1e9c609c9cafc92686f36cadad576d7aa6bf4f55 100644 (file)
--- 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 4c6918a4220d0d5cc39610f7d20a1e89245199cf..d3e9b564b389fcb8787397ff43cefd6660ae1382 100644 (file)
--- 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
index 9164d0cb488c83d6cb4ab379ede53aaa5769e8e6..ea55876b67767a6a4a9474a4af860c51dc9ee9ec 160000 (submodule)
--- 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 92c06210a7f3a4b46440464b7f5628da7a197762..6d4c606e29936c964e1bf374c393da84117efdc5 100644 (file)
--- 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."
+                      );
+}
+