Fix problems when loading old projects with the new subtitle font code (#2271).

diff --git a/src/lib/check_content_job.cc b/src/lib/check_content_job.cc
index a789ed9e0118bacbbfe9011b714105e16fbfd2e3..2b6e25da8b1570fb9be83ae3ce1707071391db25 100644 (file)
--- a/src/lib/check_content_job.cc
+++ b/src/lib/check_content_job.cc
@@ -24,12 +24,14 @@
 #include "examine_content_job.h"
 #include "film.h"
 #include "job_manager.h"
+#include "string_text_file_content.h"
 #include <iostream>
 
 #include "i18n.h"
 
 
 using std::cout;
+using std::dynamic_pointer_cast;
 using std::make_shared;
 using std::shared_ptr;
 using std::string;
@@ -74,6 +76,14 @@ CheckContentJob::run ()
                set_message (_("Some files have been changed since they were added to the project.\n\nThese files will now be re-examined, so you may need to check their settings."));
        }
 
+       if (_film->last_written_by_earlier_than(2, 16, 14)) {
+               for (auto c: content) {
+                       if (auto stf = dynamic_pointer_cast<StringTextFileContent>(c)) {
+                               stf->check_font_ids();
+                       }
+               }
+       }
+
        set_progress (1);
        set_state (FINISHED_OK);
 }

diff --git a/src/lib/string_text_file_content.cc b/src/lib/string_text_file_content.cc
index 95a2821748a344e1a6ea986658f1d409df3f1446..7ee870e1815be9c09d6430ae7427bdcd4f9f509f 100644 (file)
--- a/src/lib/string_text_file_content.cc
+++ b/src/lib/string_text_file_content.cc
@@ -60,6 +60,24 @@ StringTextFileContent::StringTextFileContent (cxml::ConstNodePtr node, int versi
 }
 
 
+static
+std::set<string>
+font_names(StringTextFile const& string_text_file)
+{
+       std::set<string> names;
+
+       for (auto const& subtitle: string_text_file.subtitles()) {
+               for (auto const& line: subtitle.lines) {
+                       for (auto const& block: line.blocks) {
+                               names.insert(block.font.get_value_or(""));
+                       }
+               }
+       }
+
+       return names;
+}
+
+
 void
 StringTextFileContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
 {
@@ -71,14 +89,7 @@ StringTextFileContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job
        /* Default to turning these subtitles on */
        only_text()->set_use (true);
 
-       std::set<string> names;
-       for (auto const& subtitle: file.subtitles()) {
-               for (auto const& line: subtitle.lines) {
-                       for (auto const& block: line.blocks) {
-                               names.insert(block.font.get_value_or(""));
-                       }
-               }
-       }
+       std::set<string> names = font_names(file);
 
        for (auto name: names) {
                optional<boost::filesystem::path> path;
@@ -148,3 +159,48 @@ StringTextFileContent::identifier () const
        s += "_" + only_text()->identifier();
        return s;
 }
+
+
+/** In 5a820bb8fae34591be5ac6d19a73461b9dab532a there were some changes to subtitle font management.
+ *
+ *  With StringTextFileContent we used to write a <Font> tag to the metadata with the id "font".  Users
+ *  could then set a font file that content should use, and (with some luck) it would end up in the DCP
+ *  that way.
+ *
+ *  After the changes we write a <Font> tag for every different font "id" (i.e. name) found in the source
+ *  file (including a <Font> with id "" in the .srt case where there are no font names).
+ *
+ *  However, this meant that making DCPs from old projects would fail, as the new code would see a font name
+ *  in the source, then lookup a Font object for it from the Content, and fail in doing so (since the content
+ *  only contains a font called "font").
+ *
+ *  To put it another way: after the changes, the code expects that any font ID (i.e. name) used in some content
+ *  will have a <Font> in the metadata and so a Font object in the TextContent.  Without that, making DCPs fails.
+ *
+ *  To work around this problem, this check_font_ids() is called for all subtitle content written by DoM versions
+ *  before 2.16.14.  We find all the font IDs in the content and map them all to the "legacy" font name (if there
+ *  is one).  This is more-or-less a re-examine()-ation, except that we try to preserve any settings that
+ *  the user had previously set up.
+ *
+ *  See #2271.
+ */
+void
+StringTextFileContent::check_font_ids()
+{
+       StringTextFile file (shared_from_this());
+       auto names = font_names(file);
+
+       auto content = only_text();
+       auto legacy_font_file = content->get_font("font")->file();
+
+       for (auto name: names) {
+               if (!content->get_font(name)) {
+                       if (legacy_font_file) {
+                               content->add_font(make_shared<Font>(name, *legacy_font_file));
+                       } else {
+                               content->add_font(make_shared<Font>(name));
+                       }
+               }
+       }
+}
+

diff --git a/src/lib/string_text_file_content.h b/src/lib/string_text_file_content.h
index 9c7d4cea0b041b8f94d296518e347a54608b8ce5..30f543381bf68fd0390b025fbac30a888210b881 100644 (file)
--- a/src/lib/string_text_file_content.h
+++ b/src/lib/string_text_file_content.h
@@ -50,6 +50,8 @@ public:
        dcpomatic::DCPTime approximate_length () const override;
        std::string identifier () const override;
 
+       void check_font_ids();
+
 private:
        dcpomatic::ContentTime _length;
 };

diff --git a/test/data b/test/data
index a89a381e341c161ea636c1018a8f5768b629d87e..14172d205b7ecac1cd4961d6e86efe20dee81ab6 160000 (submodule)
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit a89a381e341c161ea636c1018a8f5768b629d87e
+Subproject commit 14172d205b7ecac1cd4961d6e86efe20dee81ab6

diff --git a/test/subtitle_font_id_change_test.cc b/test/subtitle_font_id_change_test.cc
new file mode 100644 (file)
index 0000000..32cf574
--- /dev/null
+++ b/test/subtitle_font_id_change_test.cc
@@ -0,0 +1,149 @@
+/*
+    Copyright (C) 2022 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+
+/** @file  test/subtitle_font_id_change_test.cc
+ *  @brief Check that old projects can still be used after the changes in 5a820bb8fae34591be5ac6d19a73461b9dab532a
+ */
+
+
+#include "lib/check_content_job.h"
+#include "lib/content.h"
+#include "lib/film.h"
+#include "lib/font.h"
+#include "lib/text_content.h"
+#include "test.h"
+#include <dcp/verify.h>
+#include <boost/filesystem.hpp>
+#include <boost/test/unit_test.hpp>
+
+
+using std::string;
+
+
+class Editor
+{
+public:
+       Editor (boost::filesystem::path path)
+               : _path(path)
+               , _content(dcp::file_to_string(path))
+       {
+
+       }
+
+       ~Editor ()
+       {
+               auto f = fopen(_path.string().c_str(), "w");
+               BOOST_REQUIRE(f);
+               fwrite(_content.c_str(), _content.length(), 1, f);
+               fclose(f);
+       }
+
+       void replace (string a, string b)
+       {
+               auto old_content = _content;
+               boost::algorithm::replace_all (_content, a, b);
+               BOOST_REQUIRE (_content != old_content);
+       }
+
+private:
+       boost::filesystem::path _path;
+       std::string _content;
+};
+
+
+BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test1)
+{
+       auto film = new_test_film2("subtitle_font_id_change_test1");
+       boost::filesystem::remove(film->file("metadata.xml"));
+       boost::filesystem::copy_file("test/data/subtitle_font_id_change_test1.xml", film->file("metadata.xml"));
+       film->read_metadata();
+
+       auto content = film->content();
+       BOOST_REQUIRE_EQUAL(content.size(), 1U);
+       BOOST_REQUIRE_EQUAL(content[0]->text.size(), 1U);
+
+       content[0]->set_paths({"test/data/short.srt"});
+
+       CheckContentJob check(film);
+       check.run();
+
+       make_and_verify_dcp(film, { dcp::VerificationNote::Code::INVALID_STANDARD });
+}
+
+
+BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test2)
+{
+       auto film = new_test_film2("subtitle_font_id_change_test2");
+       boost::filesystem::remove(film->file("metadata.xml"));
+       boost::filesystem::copy_file("test/data/subtitle_font_id_change_test2.xml", film->file("metadata.xml"));
+       {
+               Editor editor(film->file("metadata.xml"));
+               editor.replace("/usr/share/fonts/truetype/inconsolata/Inconsolata.otf", "test/data/Inconsolata-VF.ttf");
+       }
+       film->read_metadata();
+
+       auto content = film->content();
+       BOOST_REQUIRE_EQUAL(content.size(), 1U);
+       BOOST_REQUIRE_EQUAL(content[0]->text.size(), 1U);
+
+       content[0]->set_paths({"test/data/short.srt"});
+
+       CheckContentJob check(film);
+       check.run();
+
+       auto font = content[0]->text.front()->get_font("");
+       BOOST_REQUIRE(font->file());
+       BOOST_CHECK_EQUAL(*font->file(), "test/data/Inconsolata-VF.ttf");
+
+       make_and_verify_dcp(film, { dcp::VerificationNote::Code::INVALID_STANDARD });
+}
+
+
+BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test3)
+{
+       auto film = new_test_film2("subtitle_font_id_change_test3");
+       boost::filesystem::remove(film->file("metadata.xml"));
+       boost::filesystem::copy_file("test/data/subtitle_font_id_change_test3.xml", film->file("metadata.xml"));
+       {
+               Editor editor(film->file("metadata.xml"));
+               editor.replace("/usr/share/fonts/truetype/inconsolata/Inconsolata.otf", "test/data/Inconsolata-VF.ttf");
+       }
+       film->read_metadata();
+
+       auto content = film->content();
+       BOOST_REQUIRE_EQUAL(content.size(), 1U);
+       BOOST_REQUIRE_EQUAL(content[0]->text.size(), 1U);
+
+       content[0]->set_paths({"test/data/fonts.ass"});
+
+       CheckContentJob check(film);
+       check.run();
+
+       auto font = content[0]->text.front()->get_font("Arial Black");
+       BOOST_REQUIRE(font->file());
+       BOOST_CHECK_EQUAL(*font->file(), "test/data/Inconsolata-VF.ttf");
+
+       font = content[0]->text.front()->get_font("Helvetica Neue");
+       BOOST_REQUIRE(font->file());
+       BOOST_CHECK_EQUAL(*font->file(), "test/data/Inconsolata-VF.ttf");
+
+       make_and_verify_dcp(film, { dcp::VerificationNote::Code::INVALID_STANDARD });
+}