From 3c802dd6d1451c2c8a7e188f8379738d72e907eb Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 14 Oct 2023 21:48:25 +0200 Subject: [PATCH] Fix DCP content font ID allocation to cope with DCPs that have multiple fonts with the same name in the same reel (#2600). Previously we had this id_for_font_in_reel() which would give an ID of N_font-ID. This means we got duplicate font IDs. Here we replace that method with FontAllocator, which gives an ID of N_font-ID for the first font and M_font-ID, where M is a number higher than the highest reel index. The idea is to support the required new IDs without breaking exisiting projects. There is some documentation of how it works in doc/design/fonts --- doc/design/fonts | 59 ++++++++++++++++ src/lib/dcp_content.cc | 33 +-------- src/lib/dcp_content.h | 5 -- src/lib/dcp_decoder.cc | 6 +- src/lib/dcp_decoder.h | 3 + src/lib/dcp_examiner.cc | 44 +++++++++--- src/lib/dcp_examiner.h | 23 +++++-- src/lib/font_id_allocator.cc | 119 +++++++++++++++++++++++++++++++++ src/lib/font_id_allocator.h | 102 ++++++++++++++++++++++++++++ src/lib/writer.cc | 46 +------------ src/lib/wscript | 1 + test/data | 2 +- test/font_id_allocator_test.cc | 72 ++++++++++++++++++++ test/hints_test.cc | 5 +- test/vf_test.cc | 47 +++++++++++++ test/writer_test.cc | 60 ----------------- test/wscript | 1 + 17 files changed, 469 insertions(+), 159 deletions(-) create mode 100644 doc/design/fonts create mode 100644 src/lib/font_id_allocator.cc create mode 100644 src/lib/font_id_allocator.h create mode 100644 test/font_id_allocator_test.cc diff --git a/doc/design/fonts b/doc/design/fonts new file mode 100644 index 000000000..c431d52e9 --- /dev/null +++ b/doc/design/fonts @@ -0,0 +1,59 @@ +How a font makes its way through the encoding process + + +Import a DCP containing some subtitles with fonts. + +* Examiner + +Builds _fonts containing (font-ID, font-TTF-data) +Add to allocator (asset-ID, font-ID) + +font-ID will be unique in its own asset, but not more widely. + +Use the allocator to set the font ID to N_font-ID where N is an integer unique for all fonts in the DCP. + +If there's no fonts in the DCP, add one with an empty ID - we want something in the content for users +to edit. + + +Now the text content contains fonts with IDs unique within the content. + + +* DCP Decoder + +Some subtitle arrives with an "original" font ID. +Use an allocator (built the same way as in the examiner) to replace the ID with a new one N_font-ID. + + +Q: Why do we need the allocator? +A: Because we need an ID to refer to each font in the content (to be stored in metadata.xml) + and we need to turn this ID back into an actual Font C++ object so it must be unique within + the content. Also we allow these fonts to have their settings altered so they must have unique + IDs for that. + + +* Text Decoder + +Calls content->get_font() to get the Font C++ object by the (newly-allocated) ID. This works because +the allocated font-ID is unique within the content. + +The Font C++ object pointer is written to the subtitle. + + +* Player + +Passes subtitles through. + + +* Writer + +Gets all fonts, puts them in the font ID map using the font's original ID. This is OK because we +don't need uniqueness in the DCP any more. + + +* Reel Writer + +Gets subtitles, uses font ID map to find the ID from the Font C++ object pointer. Puts this ID in +the font and writes it to the asset. Ensures the required LoadFont is added. + + diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc index 770e5bfad..249eb47b5 100644 --- a/src/lib/dcp_content.cc +++ b/src/lib/dcp_content.cc @@ -280,14 +280,14 @@ 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()); + examiner->add_fonts(c); new_text.push_back (c); } for (int i = 0; i < examiner->text_count(TextType::CLOSED_CAPTION); ++i) { auto c = make_shared(this, TextType::CLOSED_CAPTION, TextType::CLOSED_CAPTION); c->set_dcp_track (examiner->dcp_text_track(i)); - add_fonts_from_examiner(c, examiner->fonts()); + examiner->add_fonts(c); new_text.push_back (c); } @@ -842,33 +842,6 @@ DCPContent::resolution () const } -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. - */ - auto font_copy = make_shared(*font); - font_copy->set_id(id_for_font_in_reel(font->id(), reel_number)); - text->add_font(font_copy); - } - ++reel_number; - } - -} - - -string -id_for_font_in_reel(string id, int reel) -{ - return String::compose("%1_%2", reel, id); -} - - void DCPContent::check_font_ids() { @@ -877,7 +850,7 @@ DCPContent::check_font_ids() } DCPExaminer examiner(shared_from_this(), true); - add_fonts_from_examiner(text.front(), examiner.fonts()); + examiner.add_fonts(text.front()); } diff --git a/src/lib/dcp_content.h b/src/lib/dcp_content.h index fd78cd0ac..3753740a2 100644 --- a/src/lib/dcp_content.h +++ b/src/lib/dcp_content.h @@ -232,9 +232,4 @@ 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 e72573ebc..0a57ce7f5 100644 --- a/src/lib/dcp_decoder.cc +++ b/src/lib/dcp_decoder.cc @@ -23,6 +23,7 @@ #include "audio_content.h" #include "audio_decoder.h" #include "config.h" +#include "constants.h" #include "dcp_content.h" #include "dcp_decoder.h" #include "digester.h" @@ -136,6 +137,9 @@ DCPDecoder::DCPDecoder (shared_ptr film, shared_ptrid(), is_copy.font().get_value_or(""))); strings.push_back(is_copy); } diff --git a/src/lib/dcp_decoder.h b/src/lib/dcp_decoder.h index 803c93a86..2c0cd8f41 100644 --- a/src/lib/dcp_decoder.h +++ b/src/lib/dcp_decoder.h @@ -26,6 +26,7 @@ #include "atmos_metadata.h" #include "decoder.h" +#include "font_id_allocator.h" #include #include #include @@ -106,4 +107,6 @@ private: boost::optional _forced_reduction; std::string _lazy_digest; + + FontIDAllocator _font_id_allocator; }; diff --git a/src/lib/dcp_examiner.cc b/src/lib/dcp_examiner.cc index 3163f59c4..a9a6dee5e 100644 --- a/src/lib/dcp_examiner.cc +++ b/src/lib/dcp_examiner.cc @@ -20,11 +20,13 @@ #include "config.h" +#include "constants.h" #include "dcp_content.h" #include "dcp_examiner.h" #include "dcpomatic_log.h" #include "exceptions.h" #include "image.h" +#include "text_content.h" #include "util.h" #include #include @@ -128,9 +130,9 @@ DCPExaminer::DCPExaminer (shared_ptr content, bool tolerant) LOG_GENERAL("Looking at %1 reels", selected_cpl->reels().size()); + int reel_index = 0; for (auto reel: selected_cpl->reels()) { LOG_GENERAL("Reel %1", reel->id()); - vector> reel_fonts; if (reel->main_picture()) { if (!reel->main_picture()->asset_ref().resolved()) { @@ -205,8 +207,12 @@ DCPExaminer::DCPExaminer (shared_ptr content, bool tolerant) _text_count[TextType::OPEN_SUBTITLE] = 1; _open_subtitle_language = try_to_parse_language(reel->main_subtitle()->language()); - for (auto const& font: reel->main_subtitle()->asset()->font_data()) { - reel_fonts.push_back(make_shared(font.first, font.second)); + auto asset = reel->main_subtitle()->asset(); + for (auto const& font: asset->font_data()) { + _fonts.push_back({reel_index, asset->id(), make_shared(font.first, font.second)}); + } + if (asset->font_data().empty()) { + _fonts.push_back({reel_index, asset->id(), make_shared("")}); } } @@ -232,8 +238,12 @@ DCPExaminer::DCPExaminer (shared_ptr content, bool tolerant) LOG_GENERAL("Closed caption %1 of reel %2 found", ccap->id(), reel->id()); - for (auto const& font: ccap->asset()->font_data()) { - reel_fonts.push_back(make_shared(font.first, font.second)); + auto asset = ccap->asset(); + for (auto const& font: asset->font_data()) { + _fonts.push_back({reel_index, asset->id(), make_shared(font.first, font.second)}); + } + if (asset->font_data().empty()) { + _fonts.push_back({reel_index, asset->id(), make_shared("")}); } } @@ -263,11 +273,7 @@ DCPExaminer::DCPExaminer (shared_ptr content, bool tolerant) _reel_lengths.push_back(reel->atmos()->actual_duration()); } - if (reel_fonts.empty()) { - reel_fonts.push_back(make_shared("")); - } - - _fonts.push_back(reel_fonts); + ++reel_index; } _encrypted = selected_cpl->any_encrypted(); @@ -355,3 +361,21 @@ DCPExaminer::DCPExaminer (shared_ptr content, bool tolerant) _cpl = selected_cpl->id(); } + + +void +DCPExaminer::add_fonts(shared_ptr content) +{ + for (auto const& font: _fonts) { + _font_id_allocator.add_font(font.reel_index, font.asset_id, font.font->id()); + } + + _font_id_allocator.allocate(); + + for (auto const& font: _fonts) { + auto font_copy = make_shared(*font.font); + font_copy->set_id(_font_id_allocator.font_id(font.reel_index, font.asset_id, font.font->id())); + content->add_font(font_copy); + } +} + diff --git a/src/lib/dcp_examiner.h b/src/lib/dcp_examiner.h index 1a3615867..54e283548 100644 --- a/src/lib/dcp_examiner.h +++ b/src/lib/dcp_examiner.h @@ -27,6 +27,7 @@ #include "audio_examiner.h" #include "dcp_text_track.h" #include "dcpomatic_assert.h" +#include "font_id_allocator.h" #include "video_examiner.h" #include #include @@ -173,10 +174,7 @@ public: return _atmos_edit_rate; } - /** @return fonts in each reel */ - std::vector>> fonts() const { - return _fonts; - } + void add_fonts(std::shared_ptr content); private: boost::optional _video_frame_rate; @@ -211,5 +209,20 @@ private: bool _has_atmos = false; Frame _atmos_length = 0; dcp::Fraction _atmos_edit_rate; - std::vector>> _fonts; + + struct Font + { + Font(int reel_index_, std::string asset_id_, std::shared_ptr font_) + : reel_index(reel_index_) + , asset_id(asset_id_) + , font(font_) + {} + + int reel_index; + std::string asset_id; + std::shared_ptr font; + }; + + std::vector _fonts; + FontIDAllocator _font_id_allocator; }; diff --git a/src/lib/font_id_allocator.cc b/src/lib/font_id_allocator.cc new file mode 100644 index 000000000..ef25dc642 --- /dev/null +++ b/src/lib/font_id_allocator.cc @@ -0,0 +1,119 @@ +/* + Copyright (C) 2023 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 "compose.hpp" +#include "constants.h" +#include "dcpomatic_assert.h" +#include "font_id_allocator.h" +#include +#include +#include +#include +#include +#include +#include + + +using std::shared_ptr; +using std::set; +using std::string; +using std::vector; + + +void +FontIDAllocator::add_fonts_from_reels(vector> const& reels) +{ + int reel_index = 0; + for (auto reel: reels) { + if (auto sub = reel->main_subtitle()) { + add_fonts_from_asset(reel_index, sub->asset()); + } + + for (auto ccap: reel->closed_captions()) { + add_fonts_from_asset(reel_index, ccap->asset()); + } + + ++reel_index; + } +} + + +void +FontIDAllocator::add_fonts_from_asset(int reel_index, shared_ptr asset) +{ + for (auto const& font: asset->font_data()) { + _map[Font(reel_index, asset->id(), font.first)] = 0; + } + + if (asset->font_data().empty()) { + _map[Font(reel_index, asset->id(), "")] = 0; + } +} + + +void +FontIDAllocator::add_font(int reel_index, string asset_id, string font_id) +{ + _map[Font(reel_index, asset_id, font_id)] = 0; +} + + +void +FontIDAllocator::allocate() +{ + /* Last reel index that we have; i.e. the last prefix number that would be used by "old" + * font IDs (i.e. ones before this FontIDAllocator was added and used) + */ + auto const last_reel = std::max_element( + _map.begin(), + _map.end(), + [] (std::pair const& a, std::pair const& b) { + return a.first.reel_index < b.first.reel_index; + })->first.reel_index; + + /* Number of times each ID has been used */ + std::map used_count; + + for (auto& font: _map) { + auto const proposed = String::compose("%1_%2", font.first.reel_index, font.first.font_id); + if (used_count.find(proposed) != used_count.end()) { + /* This ID was already used; we need to disambiguate it. Do so by using + * an ID above last_reel + */ + font.second = last_reel + used_count[proposed]; + ++used_count[proposed]; + } else { + /* This ID was not yet used */ + used_count[proposed] = 1; + font.second = font.first.reel_index; + } + } +} + + +string +FontIDAllocator::font_id(int reel_index, string asset_id, string font_id) const +{ + auto iter = _map.find(Font(reel_index, asset_id, font_id)); + DCPOMATIC_ASSERT(iter != _map.end()); + return String::compose("%1_%2", iter->second, font_id); +} + diff --git a/src/lib/font_id_allocator.h b/src/lib/font_id_allocator.h new file mode 100644 index 000000000..bd99cad63 --- /dev/null +++ b/src/lib/font_id_allocator.h @@ -0,0 +1,102 @@ +/* + Copyright (C) 2023 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 . + +*/ + + +#ifndef DCPOMATIC_FONT_ID_ALLOCATOR_H +#define DCPOMATIC_FONT_ID_ALLOCATOR_H + + +#include +#include +#include +#include + + +namespace dcp { + class Reel; + class SubtitleAsset; +} + + +/** A class which, given some pairs of (asset-id, font-id) can return a font ID + * which is unique in a piece of content. + * + * When we examine a 2-reel DCP we may have a pair of subtitle assets that + * each have a font with ID "foo". We want to store these in + * TextContent::_fonts in such a way that they are distinguishable. + * + * If TextContent is to carry on storing a list of dcpomatic::Font, we can + * only do this by making each dcpomatic::Font have a different ID (i.e. not + * both "foo"). + * + * Hence when we add the fonts to the TextContent we re-write them to have + * unique IDs. + * + * When we do this, we must do it in a repeatable way so that when the + * DCPDecoder receives the "foo" font IDs it can obtain the same "new" ID given + * "foo" and the asset ID that it came from. + * + * FontIDAllocator can help with that: call add_fonts_from_reels() or add_font(), + * then allocate(), then it will return repeatable unique "new" font IDs from + * an asset map and "old" ID. + */ +class FontIDAllocator +{ +public: + void add_fonts_from_reels(std::vector> const& reels); + void add_font(int reel_index, std::string asset_id, std::string font_id); + + void allocate(); + + std::string font_id(int reel_index, std::string asset_id, std::string font_id) const; + +private: + void add_fonts_from_asset(int reel_index, std::shared_ptr asset); + + struct Font + { + Font(int reel_index_, std::string asset_id_, std::string font_id_) + : reel_index(reel_index_) + , asset_id(asset_id_) + , font_id(font_id_) + {} + + bool operator<(Font const& other) const { + if (reel_index != other.reel_index) { + return reel_index < other.reel_index; + } + + if (asset_id != other.asset_id) { + return asset_id < other.asset_id; + } + + return font_id < other.font_id; + } + + int reel_index; + std::string asset_id; + std::string font_id; + }; + + std::map _map; +}; + + +#endif diff --git a/src/lib/writer.cc b/src/lib/writer.cc index 6bc3da504..9ab3d4e1e 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -875,53 +875,9 @@ Writer::write (vector> fonts) } _chosen_interop_font = fonts[0]; } else { - set used_ids; - - /* Return the index of a _N at the end of a string, or string::npos */ - auto underscore_number_position = [](string s) { - auto last_underscore = s.find_last_of("_"); - if (last_underscore == string::npos) { - return string::npos; - } - - for (auto i = last_underscore + 1; i < s.size(); ++i) { - if (!isdigit(s[i])) { - return string::npos; - } - } - - return last_underscore; - }; - - /* Write fonts to _fonts, changing any duplicate IDs so that they are unique */ for (auto font: fonts) { - auto id = fix_id(font->id()); - if (used_ids.find(id) == used_ids.end()) { - /* This ID is unique so we can just use it as-is */ - _fonts.put(font, id); - used_ids.insert(id); - } else { - auto end = underscore_number_position(id); - if (end == string::npos) { - /* This string has no _N suffix, so add one */ - id += "_0"; - end = underscore_number_position(id); - } - - ++end; - - /* Increment the suffix until we find a unique one */ - auto number = dcp::raw_convert(id.substr(end)); - while (used_ids.find(id) != used_ids.end()) { - ++number; - id = String::compose("%1_%2", id.substr(0, end - 1), number); - } - used_ids.insert(id); - } - _fonts.put(font, id); + _fonts.put(font, fix_id(font->id())); } - - DCPOMATIC_ASSERT(_fonts.map().size() == used_ids.size()); } } diff --git a/src/lib/wscript b/src/lib/wscript index 251f09cf7..56ffc39fe 100644 --- a/src/lib/wscript +++ b/src/lib/wscript @@ -119,6 +119,7 @@ sources = """ filter.cc font.cc font_config.cc + font_id_allocator.cc font_id_map.cc frame_interval_checker.cc frame_rate_change.cc diff --git a/test/data b/test/data index c0f7e2c3f..c40dcfabf 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit c0f7e2c3f702b469db8e146eb9e650f9998d18a7 +Subproject commit c40dcfabfccce346822a662012fa86814206d6a8 diff --git a/test/font_id_allocator_test.cc b/test/font_id_allocator_test.cc new file mode 100644 index 000000000..7c93c6b78 --- /dev/null +++ b/test/font_id_allocator_test.cc @@ -0,0 +1,72 @@ +/* + Copyright (C) 2023 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/font_id_allocator.h" +#include + + +BOOST_AUTO_TEST_CASE(font_id_allocator_test_without_disambiguation) +{ + FontIDAllocator allocator; + + /* Reel 0 has just one asset with two fonts */ + allocator.add_font(0, "asset1", "font"); + allocator.add_font(0, "asset1", "font2"); + + /* Reel 1 has two assets each with two more fonts */ + allocator.add_font(1, "asset2", "font"); + allocator.add_font(1, "asset2", "font2"); + allocator.add_font(1, "asset3", "font3"); + allocator.add_font(1, "asset3", "font4"); + + allocator.allocate(); + + BOOST_CHECK(allocator.font_id(0, "asset1", "font") == "0_font"); + BOOST_CHECK(allocator.font_id(0, "asset1", "font2") == "0_font2"); + BOOST_CHECK(allocator.font_id(1, "asset2", "font") == "1_font"); + BOOST_CHECK(allocator.font_id(1, "asset2", "font2") == "1_font2"); + BOOST_CHECK(allocator.font_id(1, "asset3", "font3") == "1_font3"); + BOOST_CHECK(allocator.font_id(1, "asset3", "font4") == "1_font4"); +} + + +BOOST_AUTO_TEST_CASE(font_id_allocator_test_with_disambiguation) +{ + FontIDAllocator allocator; + + /* Reel 0 has two assets each with a font with the same ID (perhaps a subtitle and a ccap). + * This would have crashed DCP-o-matic before the FontIDAllocator change (bug #2600) + * so it's OK if the second font gets a new index that we didn't see before. + */ + allocator.add_font(0, "asset1", "font"); + allocator.add_font(0, "asset2", "font"); + + /* Reel 1 has one asset with another font */ + allocator.add_font(1, "asset3", "font1"); + + allocator.allocate(); + + BOOST_CHECK(allocator.font_id(0, "asset1", "font") == "0_font"); + /* This should get a prefix that is higher than any reel index */ + BOOST_CHECK(allocator.font_id(0, "asset2", "font") == "2_font"); + BOOST_CHECK(allocator.font_id(1, "asset3", "font1") == "1_font1"); +} + diff --git a/test/hints_test.cc b/test/hints_test.cc index 0e856c4f9..a989d3aeb 100644 --- a/test/hints_test.cc +++ b/test/hints_test.cc @@ -193,12 +193,13 @@ BOOST_AUTO_TEST_CASE (hint_subtitle_mxf_too_big) } fake_font.close(); - auto content = content_factory("test/data/" + name + ".srt")[0]; + auto content = content_factory(String::compose("test/data/%1%2.xml", name, i))[0]; content->text[0]->set_type(TextType::OPEN_SUBTITLE); content->text[0]->set_language(dcp::LanguageTag("en-US")); film->examine_and_add_content(content); BOOST_REQUIRE (!wait_for_jobs()); - content->text[0]->get_font("")->set_file("build/test/hint_subtitle_mxf_too_big.ttf"); + auto const font = content->text[0]->get_font(String::compose("font_%1", i)); + font->set_file("build/test/hint_subtitle_mxf_too_big.ttf"); } auto hints = get_hints (film); diff --git a/test/vf_test.cc b/test/vf_test.cc index ecd615d98..249f9c5b0 100644 --- a/test/vf_test.cc +++ b/test/vf_test.cc @@ -28,8 +28,11 @@ #include "lib/content_factory.h" #include "lib/dcp_content.h" #include "lib/dcp_content_type.h" +#include "lib/examine_content_job.h" #include "lib/ffmpeg_content.h" #include "lib/film.h" +#include "lib/job_manager.h" +#include "lib/make_dcp.h" #include "lib/player.h" #include "lib/text_content.h" #include "lib/referenced_reel_asset.h" @@ -423,3 +426,47 @@ BOOST_AUTO_TEST_CASE(test_referencing_ov_with_subs_when_adding_ccaps) std::cout << why_not << "\n"; } + +BOOST_AUTO_TEST_CASE(test_duplicate_font_id_in_vf) +{ + string const name("test_duplicate_font_id_in_vf"); + auto subs = content_factory("test/data/15s.srt"); + auto ov = new_test_film2(name + "_ov", subs); + make_and_verify_dcp( + ov, + { + dcp::VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE, + dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME, + dcp::VerificationNote::Code::MISSING_CPL_METADATA + }); + + auto ccaps = content_factory("test/data/15s.srt")[0]; + auto ov_dcp = make_shared(ov->dir(ov->dcp_name(false))); + auto vf = new_test_film2(name + "_vf", { ov_dcp, ccaps }); + ov_dcp->set_reference_audio(true); + ov_dcp->set_reference_video(true); + ov_dcp->text[0]->set_use(true); + ccaps->text[0]->set_type(TextType::CLOSED_CAPTION); + string why_not; + BOOST_CHECK_MESSAGE(ov_dcp->can_reference_text(vf, TextType::OPEN_SUBTITLE, why_not), why_not); + ov_dcp->set_reference_text(TextType::OPEN_SUBTITLE, true); + vf->write_metadata(); + make_dcp(vf, TranscodeJob::ChangedBehaviour::IGNORE); + BOOST_REQUIRE(!wait_for_jobs()); + + auto vf_dcp = make_shared(vf->dir(vf->dcp_name(false))); + + auto test = new_test_film2(name + "_test", { vf_dcp }); + vf_dcp->add_ov(ov->dir(ov->dcp_name(false))); + JobManager::instance()->add(make_shared(test, vf_dcp)); + BOOST_CHECK(!wait_for_jobs()); + + make_and_verify_dcp( + test, + { + dcp::VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE, + dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME, + dcp::VerificationNote::Code::MISSING_CPL_METADATA, + }); +} + diff --git a/test/writer_test.cc b/test/writer_test.cc index d5cafe1fb..7b2a2db00 100644 --- a/test/writer_test.cc +++ b/test/writer_test.cc @@ -101,63 +101,3 @@ BOOST_AUTO_TEST_CASE (interrupt_writer) dcpomatic_sleep_seconds (1); cl.run (); } - - -BOOST_AUTO_TEST_CASE (writer_disambiguate_font_ids1) -{ - auto film = new_test_film2("writer_disambiguate_font_ids1", {}); - Writer writer(film, {}); - - auto fonts = vector> { - make_shared("a"), - make_shared("b"), - make_shared("c") - }; - - writer.write(fonts); - - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[0]), "a"); - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[1]), "b"); - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[2]), "c"); -} - - -BOOST_AUTO_TEST_CASE (writer_disambiguate_font_ids2) -{ - auto film = new_test_film2("writer_disambiguate_font_ids2", {}); - Writer writer(film, {}); - - auto fonts = vector> { - make_shared("a"), - make_shared("a"), - make_shared("a") - }; - - writer.write(fonts); - - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[0]), "a"); - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[1]), "a_0"); - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[2]), "a_1"); -} - - -BOOST_AUTO_TEST_CASE (writer_disambiguate_font_ids3) -{ - auto film = new_test_film2("writer_disambiguate_font_ids3", {}); - Writer writer(film, {}); - - auto fonts = vector> { - make_shared("a_2"), - make_shared("a_1"), - make_shared("a_1"), - make_shared("b") - }; - - writer.write(fonts); - - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[1]), "a_1"); - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[0]), "a_2"); - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[2]), "a_3"); - BOOST_CHECK_EQUAL(writer._fonts.get(fonts[3]), "b"); -} - diff --git a/test/wscript b/test/wscript index f40568e3c..0c9db3889 100644 --- a/test/wscript +++ b/test/wscript @@ -99,6 +99,7 @@ def build(bld): film_metadata_test.cc find_missing_test.cc font_comparator_test.cc + font_id_allocator_test.cc frame_interval_checker_test.cc frame_rate_test.cc guess_crop_test.cc -- 2.30.2