Fix font handling for DCP subtitles.
authorCarl Hetherington <cth@carlh.net>
Sat, 9 Jul 2022 18:22:38 +0000 (20:22 +0200)
committerCarl Hetherington <cth@carlh.net>
Mon, 11 Jul 2022 10:22:11 +0000 (12:22 +0200)
15 files changed:
src/lib/check_content_job.cc
src/lib/content.h
src/lib/dcp_content.cc
src/lib/dcp_content.h
src/lib/dcp_decoder.cc
src/lib/dcp_examiner.cc
src/lib/dcp_examiner.h
src/lib/dcp_subtitle_content.cc
src/lib/font.h
src/lib/string_text_file_content.cc
test/data
test/reels_test.cc
test/subtitle_font_id_change_test.cc
test/subtitle_font_id_test.cc [new file with mode: 0644]
test/wscript

index 2b6e25da8b1570fb9be83ae3ce1707071391db25..b74b71cc0c57522090d0d2fe17b63838249b4eb9 100644 (file)
@@ -21,6 +21,7 @@
 
 #include "check_content_job.h"
 #include "content.h"
+#include "dcp_content.h"
 #include "examine_content_job.h"
 #include "film.h"
 #include "job_manager.h"
@@ -69,21 +70,23 @@ CheckContentJob::run ()
        std::vector<shared_ptr<Content>> changed;
        std::copy_if (content.begin(), content.end(), std::back_inserter(changed), [](shared_ptr<Content> c) { return c->changed(); });
 
-       if (!changed.empty()) {
-               for (auto i: changed) {
-                       JobManager::instance()->add(make_shared<ExamineContentJob>(_film, i));
-               }
-               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();
+                       } else if (auto dcp = dynamic_pointer_cast<DCPContent>(c)) {
+                               dcp->check_font_ids();
                        }
                }
        }
 
+       if (!changed.empty()) {
+               for (auto i: changed) {
+                       JobManager::instance()->add(make_shared<ExamineContentJob>(_film, i));
+               }
+               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."));
+       }
+
        set_progress (1);
        set_state (FINISHED_OK);
 }
index 7c02ee0e39effc323216cfc28d29da07ea63d236..d0faeb9d42883f1fcdcd8cb7774b09fec159bc6e 100644 (file)
@@ -226,6 +226,7 @@ private:
        friend struct best_dcp_frame_rate_test_single;
        friend struct best_dcp_frame_rate_test_double;
        friend struct audio_sampling_rate_test;
+       friend struct subtitle_font_id_change_test2;
        template<class, class> friend class ChangeSignaller;
 
        void signal_change (ChangeType, int);
index f8639cef9cc187f7ed2b35bc2821d955329be0e9..2e89adff0421d43228afa5698557fb1327a425d7 100644 (file)
@@ -268,6 +268,7 @@ DCPContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
        for (int i = 0; i < examiner->text_count(TextType::OPEN_SUBTITLE); ++i) {
                auto c = make_shared<TextContent>(this, TextType::OPEN_SUBTITLE, TextType::OPEN_SUBTITLE);
                c->set_language (examiner->open_subtitle_language());
+               add_fonts_from_examiner(c, examiner->fonts());
                new_text.push_back (c);
        }
 
@@ -820,3 +821,41 @@ DCPContent::resolution () const
        return Resolution::TWO_K;
 }
 
+
+void
+add_fonts_from_examiner(shared_ptr<TextContent> text, vector<vector<shared_ptr<Font>>> const & all_fonts)
+{
+       int reel_number = 0;
+       for (auto reel_fonts: all_fonts) {
+               for (auto font: reel_fonts) {
+                       /* Each reel could have its own font with the same ID, so we disambiguate them here
+                        * by prepending the reel number.  We do the same disambiguation when emitting the
+                        * subtitles in the DCP decoder.
+                        */
+                       font->set_id(id_for_font_in_reel(font->id(), reel_number));
+                       text->add_font(font);
+               }
+               ++reel_number;
+       }
+
+}
+
+
+string
+id_for_font_in_reel(string id, int reel)
+{
+       return String::compose("%1_%2", reel, id);
+}
+
+
+void
+DCPContent::check_font_ids()
+{
+       if (text.empty()) {
+               return;
+       }
+
+       DCPExaminer examiner(shared_from_this(), true);
+       add_fonts_from_examiner(text.front(), examiner.fonts());
+}
+
index 55773b94439217540bc9ea5a916527e7a96cd388..1b73e8fc71740c698aac7374b6f264388801636f 100644 (file)
@@ -29,6 +29,7 @@
 
 
 #include "content.h"
+#include "font.h"
 #include <libcxml/cxml.h>
 #include <dcp/encrypted_kdm.h>
 #include <dcp/rating.h>
@@ -170,6 +171,8 @@ public:
                return _content_versions;
        }
 
+       void check_font_ids();
+
 private:
        friend struct reels_test5;
 
@@ -222,4 +225,9 @@ private:
 };
 
 
+extern std::string id_for_font_in_reel(std::string id, int reel);
+extern void add_fonts_from_examiner(std::shared_ptr<TextContent> text, std::vector<std::vector<std::shared_ptr<dcpomatic::Font>>> const& fonts);
+
+
+
 #endif
index ad7d3e11277910fb62eb27611f0472a6ed97a94c..7e2001e0ef9b75d4d145d89403048e351928b241 100644 (file)
@@ -308,7 +308,9 @@ DCPDecoder::pass_texts (
                                        strings.clear ();
                                }
 
-                               strings.push_back (*is);
+                               dcp::SubtitleString is_copy = *is;
+                               is_copy.set_font(id_for_font_in_reel(is_copy.font().get_value_or(""), _reel - _reels.begin()));
+                               strings.push_back(is_copy);
                        }
 
                        /* XXX: perhaps these image subs should also be collected together like the string ones are;
index f2ec68bddd8786a9a73f6258dc8d757c8600b50b..8fa41a8a59d536dfe899d9cf0bd54a75a8dfebaa 100644 (file)
 
 using std::cout;
 using std::dynamic_pointer_cast;
+using std::make_shared;
 using std::shared_ptr;
 using std::string;
+using std::vector;
 using boost::optional;
 
 
@@ -132,6 +134,7 @@ DCPExaminer::DCPExaminer (shared_ptr<const DCPContent> content, bool tolerant)
 
        for (auto i: cpl->reels()) {
                LOG_GENERAL ("Reel %1", i->id());
+               vector<shared_ptr<dcpomatic::Font>> reel_fonts;
 
                if (i->main_picture ()) {
                        if (!i->main_picture()->asset_ref().resolved()) {
@@ -203,6 +206,10 @@ DCPExaminer::DCPExaminer (shared_ptr<const DCPContent> content, bool tolerant)
 
                        _text_count[static_cast<int>(TextType::OPEN_SUBTITLE)] = 1;
                        _open_subtitle_language = try_to_parse_language (i->main_subtitle()->language());
+
+                       for (auto const& font: i->main_subtitle()->asset()->font_data()) {
+                               reel_fonts.push_back(make_shared<dcpomatic::Font>(font.first, font.second));
+                       }
                }
 
                for (auto j: i->closed_captions()) {
@@ -244,6 +251,8 @@ DCPExaminer::DCPExaminer (shared_ptr<const DCPContent> content, bool tolerant)
                } else if (!i->atmos()) {
                        _reel_lengths.push_back (i->atmos()->actual_duration());
                }
+
+               _fonts.push_back(reel_fonts);
        }
 
        _encrypted = cpl->any_encrypted ();
index a5bf2434e8d23ca28a510bc762a4409cb65c105f..ac43805977c98f75d5cc9eea1a0c6e94a8871c64 100644 (file)
@@ -166,6 +166,11 @@ public:
                return _atmos_edit_rate;
        }
 
+       /** @return fonts in each reel */
+       std::vector<std::vector<std::shared_ptr<dcpomatic::Font>>> fonts() const {
+               return _fonts;
+       }
+
 private:
        boost::optional<double> _video_frame_rate;
        boost::optional<dcp::Size> _video_size;
@@ -198,4 +203,5 @@ private:
        bool _has_atmos = false;
        Frame _atmos_length = 0;
        dcp::Fraction _atmos_edit_rate;
+       std::vector<std::vector<std::shared_ptr<dcpomatic::Font>>> _fonts;
 };
index 3bae6e88fb572e686c418995759adcc2d698752f..a6cfd8d93a42c1ebbe4e5f46695ad94bcda139a3 100644 (file)
@@ -74,8 +74,14 @@ DCPSubtitleContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
 
        sc->fix_empty_font_ids ();
 
-       for (auto i: sc->load_font_nodes()) {
-               only_text()->add_font(make_shared<Font>(i->id));
+       auto font_data = sc->font_data();
+       for (auto node: sc->load_font_nodes()) {
+               auto data = font_data.find(node->id);
+               if (data != font_data.end()) {
+                       only_text()->add_font(make_shared<Font>(node->id, data->second));
+               } else {
+                       only_text()->add_font(make_shared<Font>(node->id));
+               }
        }
 }
 
index c1405d0f61c81d1695c95bca5d72009f87f63e43..24d8ed2bb93103027795c838a3b800ad1668b5c0 100644 (file)
@@ -47,6 +47,11 @@ public:
                , _file (file)
        {}
 
+       Font (std::string id, dcp::ArrayData data)
+               : _id (id)
+               , _data (data)
+       {}
+
        void as_xml (xmlpp::Node* node);
 
        std::string id () const {
index dae6811b56120a12f8e576669d5029745872e65e..d8c195be77af1fe53ada9e66830d776ad4810826 100644 (file)
@@ -189,7 +189,10 @@ StringTextFileContent::check_font_ids()
        auto names = font_names(file);
 
        auto content = only_text();
-       auto legacy_font_file = content->get_font("font")->file();
+       optional<boost::filesystem::path> legacy_font_file;
+       if (auto legacy_font = content->get_font("font")) {
+               legacy_font_file = legacy_font->file();
+       }
 
        for (auto name: names) {
                if (!content->get_font(name)) {
index 375fe9911e941ccbb6c5d8c4a96be62af0792142..6d4d01a10bc14f3a4a0db5c3e0a4be72176543bf 160000 (submodule)
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit 375fe9911e941ccbb6c5d8c4a96be62af0792142
+Subproject commit 6d4d01a10bc14f3a4a0db5c3e0a4be72176543bf
index 10fdc6c1ba440d845273dc724f64d4bcff1369af..5bee4a81918462bb45b3c26d260bd4698b04669d 100644 (file)
@@ -240,6 +240,7 @@ BOOST_AUTO_TEST_CASE (reels_test4)
 BOOST_AUTO_TEST_CASE (reels_test5)
 {
        auto dcp = make_shared<DCPContent>("test/data/reels_test4");
+       dcp->check_font_ids();
        auto film = new_test_film2 ("reels_test5", {dcp});
        film->set_sequence (false);
 
index 32cf57400c0a33475418ab8e521379b05979840e..f325419800b67233a5e92eb43a7abf6edda3d1ee 100644 (file)
@@ -84,6 +84,7 @@ BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test1)
 
        CheckContentJob check(film);
        check.run();
+       BOOST_REQUIRE (!wait_for_jobs());
 
        make_and_verify_dcp(film, { dcp::VerificationNote::Code::INVALID_STANDARD });
 }
@@ -105,9 +106,14 @@ BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test2)
        BOOST_REQUIRE_EQUAL(content[0]->text.size(), 1U);
 
        content[0]->set_paths({"test/data/short.srt"});
+       /* Make sure the content doesn't look like it's changed, otherwise it will be re-examined
+        * which obscures the point of this test.
+        */
+       content[0]->_last_write_times[0] = boost::filesystem::last_write_time("test/data/short.srt");
 
        CheckContentJob check(film);
        check.run();
+       BOOST_REQUIRE (!wait_for_jobs());
 
        auto font = content[0]->text.front()->get_font("");
        BOOST_REQUIRE(font->file());
@@ -136,6 +142,7 @@ BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test3)
 
        CheckContentJob check(film);
        check.run();
+       BOOST_REQUIRE (!wait_for_jobs());
 
        auto font = content[0]->text.front()->get_font("Arial Black");
        BOOST_REQUIRE(font->file());
@@ -147,3 +154,29 @@ BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test3)
 
        make_and_verify_dcp(film, { dcp::VerificationNote::Code::INVALID_STANDARD });
 }
+
+
+BOOST_AUTO_TEST_CASE(subtitle_font_id_change_test4)
+{
+       auto film = new_test_film2("subtitle_font_id_change_test4");
+       boost::filesystem::remove(film->file("metadata.xml"));
+       boost::filesystem::copy_file("test/data/subtitle_font_id_change_test4.xml", film->file("metadata.xml"));
+
+       {
+               Editor editor(film->file("metadata.xml"));
+               editor.replace("dcpomatic-test-private", TestPaths::private_data().string());
+       }
+
+       film->read_metadata();
+
+       auto content = film->content();
+       BOOST_REQUIRE_EQUAL(content.size(), 1U);
+       BOOST_REQUIRE_EQUAL(content[0]->text.size(), 1U);
+
+       CheckContentJob check(film);
+       check.run();
+       BOOST_REQUIRE(!wait_for_jobs());
+
+       make_and_verify_dcp(film, { dcp::VerificationNote::Code::INVALID_STANDARD });
+}
+
diff --git a/test/subtitle_font_id_test.cc b/test/subtitle_font_id_test.cc
new file mode 100644 (file)
index 0000000..41bc3a9
--- /dev/null
@@ -0,0 +1,92 @@
+/*
+    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/>.
+
+*/
+
+
+#include "lib/content_factory.h"
+#include "lib/dcp_content.h"
+#include "lib/film.h"
+#include "lib/font.h"
+#include "lib/text_content.h"
+#include "test.h"
+#include <boost/test/unit_test.hpp>
+
+
+using std::make_shared;
+
+
+BOOST_AUTO_TEST_CASE(full_dcp_subtitle_font_id_test)
+{
+       auto dcp = make_shared<DCPContent>(TestPaths::private_data() / "JourneyToJah_TLR-1_F_EN-DE-FR_CH_51_2K_LOK_20140225_DGL_SMPTE_OV");
+       auto film = new_test_film2("full_dcp_subtitle_font_id_test", { dcp });
+
+       auto content = film->content();
+       BOOST_REQUIRE_EQUAL(content.size(), 1U);
+       auto text = content[0]->only_text();
+       BOOST_REQUIRE(text);
+
+       BOOST_REQUIRE_EQUAL(text->fonts().size(), 1U);
+       auto font = text->fonts().front();
+       BOOST_CHECK_EQUAL(font->id(), "0_theFontId");
+       BOOST_REQUIRE(font->data());
+       BOOST_CHECK_EQUAL(font->data()->size(), 367112);
+}
+
+
+BOOST_AUTO_TEST_CASE(dcp_subtitle_font_id_test)
+{
+       auto subs = content_factory(TestPaths::private_data() / "JourneyToJah_TLR-1_F_EN-DE-FR_CH_51_2K_LOK_20140225_DGL_SMPTE_OV" / "8b48f6ae-c74b-4b80-b994-a8236bbbad74_sub.mxf").front();
+       auto film = new_test_film2("dcp_subtitle_font_id_test", { subs });
+
+       auto content = film->content();
+       BOOST_REQUIRE_EQUAL(content.size(), 1U);
+       auto text = content[0]->only_text();
+       BOOST_REQUIRE(text);
+
+       BOOST_REQUIRE_EQUAL(text->fonts().size(), 1U);
+       auto font = text->fonts().front();
+       BOOST_CHECK_EQUAL(font->id(), "theFontId");
+       BOOST_REQUIRE(font->data());
+       BOOST_CHECK_EQUAL(font->data()->size(), 367112);
+}
+
+
+BOOST_AUTO_TEST_CASE(make_dcp_with_subs_from_interop_dcp)
+{
+       auto dcp = make_shared<DCPContent>("test/data/Iopsubs_FTR-1_F_XX-XX_MOS_2K_20220710_IOP_OV");
+       auto film = new_test_film2("make_dcp_with_subs_from_interop_dcp", { dcp });
+       dcp->text.front()->set_use(true);
+       make_and_verify_dcp(
+               film,
+               {
+                       dcp::VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE,
+                       dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME
+               }
+       );
+}
+
+
+BOOST_AUTO_TEST_CASE(make_dcp_with_subs_from_smpte_dcp)
+{
+       auto dcp = make_shared<DCPContent>(TestPaths::private_data() / "JourneyToJah_TLR-1_F_EN-DE-FR_CH_51_2K_LOK_20140225_DGL_SMPTE_OV");
+       auto film = new_test_film2("make_dcp_with_subs_from_smpte_dcp", { dcp });
+       dcp->text.front()->set_use(true);
+       make_and_verify_dcp(film);
+}
+
index b7dc1669fa41c066967ce31d350f562df18e3764..0bcecad0c39dc4fd43af93fd6abc749965dd761f 100644 (file)
@@ -135,6 +135,7 @@ def build(bld):
                  ssa_subtitle_test.cc
                  stream_test.cc
                  subtitle_charset_test.cc
+                 subtitle_font_id_test.cc
                  subtitle_font_id_change_test.cc
                  subtitle_language_test.cc
                  subtitle_metadata_test.cc