From: Carl Hetherington Date: Thu, 9 Jun 2022 20:40:02 +0000 (+0200) Subject: Fix problems when loading old projects with the new subtitle font code (#2271). X-Git-Tag: v2.16.14~8 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=14e02ad2f79bdc6fbc320ec7b9282b5faabdb825 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 a789ed9e0..2b6e25da8 100644 --- 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 #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(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 95a282174..7ee870e18 100644 --- 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 +font_names(StringTextFile const& string_text_file) +{ + std::set 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 film, shared_ptr job) { @@ -71,14 +89,7 @@ StringTextFileContent::examine (shared_ptr film, shared_ptr job /* Default to turning these subtitles on */ only_text()->set_use (true); - std::set 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 names = font_names(file); for (auto name: names) { optional 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 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 tag for every different font "id" (i.e. name) found in the source + * file (including a 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 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(name, *legacy_font_file)); + } else { + content->add_font(make_shared(name)); + } + } + } +} + diff --git a/src/lib/string_text_file_content.h b/src/lib/string_text_file_content.h index 9c7d4cea0..30f543381 100644 --- 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 a89a381e3..14172d205 160000 --- 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 index 000000000..32cf57400 --- /dev/null +++ b/test/subtitle_font_id_change_test.cc @@ -0,0 +1,149 @@ +/* + Copyright (C) 2022 Carl Hetherington + + 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 . + +*/ + + +/** @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 +#include +#include + + +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 }); +}