From: Carl Hetherington Date: Sat, 9 Jul 2022 18:22:38 +0000 (+0200) Subject: Fix font handling for DCP subtitles. X-Git-Tag: v2.16.17~6 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=8b9888ed8247109dc3c09492302e865fa4731460 Fix font handling for DCP subtitles. --- diff --git a/src/lib/check_content_job.cc b/src/lib/check_content_job.cc index 2b6e25da8..b74b71cc0 100644 --- a/src/lib/check_content_job.cc +++ b/src/lib/check_content_job.cc @@ -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> changed; std::copy_if (content.begin(), content.end(), std::back_inserter(changed), [](shared_ptr c) { return c->changed(); }); - if (!changed.empty()) { - for (auto i: changed) { - JobManager::instance()->add(make_shared(_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(c)) { stf->check_font_ids(); + } else if (auto dcp = dynamic_pointer_cast(c)) { + dcp->check_font_ids(); } } } + if (!changed.empty()) { + for (auto i: changed) { + JobManager::instance()->add(make_shared(_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); } diff --git a/src/lib/content.h b/src/lib/content.h index 7c02ee0e3..d0faeb9d4 100644 --- a/src/lib/content.h +++ b/src/lib/content.h @@ -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 friend class ChangeSignaller; void signal_change (ChangeType, int); diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc index f8639cef9..2e89adff0 100644 --- a/src/lib/dcp_content.cc +++ b/src/lib/dcp_content.cc @@ -268,6 +268,7 @@ DCPContent::examine (shared_ptr film, shared_ptr job) for (int i = 0; i < examiner->text_count(TextType::OPEN_SUBTITLE); ++i) { auto c = make_shared(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 text, vector>> 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()); +} + diff --git a/src/lib/dcp_content.h b/src/lib/dcp_content.h index 55773b944..1b73e8fc7 100644 --- a/src/lib/dcp_content.h +++ b/src/lib/dcp_content.h @@ -29,6 +29,7 @@ #include "content.h" +#include "font.h" #include #include #include @@ -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 text, std::vector>> const& fonts); + + + #endif diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc index ad7d3e112..7e2001e0e 100644 --- a/src/lib/dcp_decoder.cc +++ b/src/lib/dcp_decoder.cc @@ -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; diff --git a/src/lib/dcp_examiner.cc b/src/lib/dcp_examiner.cc index f2ec68bdd..8fa41a8a5 100644 --- a/src/lib/dcp_examiner.cc +++ b/src/lib/dcp_examiner.cc @@ -54,8 +54,10 @@ 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 content, bool tolerant) for (auto i: cpl->reels()) { LOG_GENERAL ("Reel %1", i->id()); + vector> reel_fonts; if (i->main_picture ()) { if (!i->main_picture()->asset_ref().resolved()) { @@ -203,6 +206,10 @@ DCPExaminer::DCPExaminer (shared_ptr content, bool tolerant) _text_count[static_cast(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(font.first, font.second)); + } } for (auto j: i->closed_captions()) { @@ -244,6 +251,8 @@ DCPExaminer::DCPExaminer (shared_ptr content, bool tolerant) } else if (!i->atmos()) { _reel_lengths.push_back (i->atmos()->actual_duration()); } + + _fonts.push_back(reel_fonts); } _encrypted = cpl->any_encrypted (); diff --git a/src/lib/dcp_examiner.h b/src/lib/dcp_examiner.h index a5bf2434e..ac4380597 100644 --- a/src/lib/dcp_examiner.h +++ b/src/lib/dcp_examiner.h @@ -166,6 +166,11 @@ public: return _atmos_edit_rate; } + /** @return fonts in each reel */ + std::vector>> fonts() const { + return _fonts; + } + private: boost::optional _video_frame_rate; boost::optional _video_size; @@ -198,4 +203,5 @@ private: bool _has_atmos = false; Frame _atmos_length = 0; dcp::Fraction _atmos_edit_rate; + std::vector>> _fonts; }; diff --git a/src/lib/dcp_subtitle_content.cc b/src/lib/dcp_subtitle_content.cc index 3bae6e88f..a6cfd8d93 100644 --- a/src/lib/dcp_subtitle_content.cc +++ b/src/lib/dcp_subtitle_content.cc @@ -74,8 +74,14 @@ DCPSubtitleContent::examine (shared_ptr film, shared_ptr job) sc->fix_empty_font_ids (); - for (auto i: sc->load_font_nodes()) { - only_text()->add_font(make_shared(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(node->id, data->second)); + } else { + only_text()->add_font(make_shared(node->id)); + } } } diff --git a/src/lib/font.h b/src/lib/font.h index c1405d0f6..24d8ed2bb 100644 --- a/src/lib/font.h +++ b/src/lib/font.h @@ -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 { diff --git a/src/lib/string_text_file_content.cc b/src/lib/string_text_file_content.cc index dae6811b5..d8c195be7 100644 --- a/src/lib/string_text_file_content.cc +++ b/src/lib/string_text_file_content.cc @@ -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 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)) { diff --git a/test/data b/test/data index 375fe9911..6d4d01a10 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 375fe9911e941ccbb6c5d8c4a96be62af0792142 +Subproject commit 6d4d01a10bc14f3a4a0db5c3e0a4be72176543bf diff --git a/test/reels_test.cc b/test/reels_test.cc index 10fdc6c1b..5bee4a819 100644 --- a/test/reels_test.cc +++ b/test/reels_test.cc @@ -240,6 +240,7 @@ BOOST_AUTO_TEST_CASE (reels_test4) BOOST_AUTO_TEST_CASE (reels_test5) { auto dcp = make_shared("test/data/reels_test4"); + dcp->check_font_ids(); auto film = new_test_film2 ("reels_test5", {dcp}); film->set_sequence (false); diff --git a/test/subtitle_font_id_change_test.cc b/test/subtitle_font_id_change_test.cc index 32cf57400..f32541980 100644 --- a/test/subtitle_font_id_change_test.cc +++ b/test/subtitle_font_id_change_test.cc @@ -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 index 000000000..41bc3a923 --- /dev/null +++ b/test/subtitle_font_id_test.cc @@ -0,0 +1,92 @@ +/* + 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 . + +*/ + + +#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 + + +using std::make_shared; + + +BOOST_AUTO_TEST_CASE(full_dcp_subtitle_font_id_test) +{ + auto dcp = make_shared(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("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(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); +} + diff --git a/test/wscript b/test/wscript index b7dc1669f..0bcecad0c 100644 --- a/test/wscript +++ b/test/wscript @@ -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