Always add a font with an empty ID (#2649).

It's not just subtitle files with no <LoadFont> that can have
subtitles without a specified font.  In particular, DoM makes a
single space subtitle with no font spec when it creates filler
subtitles in multi-reel DCPs.

diff --git a/src/lib/dcp_examiner.cc b/src/lib/dcp_examiner.cc
index 0f9ae9544812f414bbd7053369c999214a2de5df..ca77f2db3bdaedfc560206e8937a9df9889ef4a0 100644 (file)
--- a/src/lib/dcp_examiner.cc
+++ b/src/lib/dcp_examiner.cc
@@ -211,9 +211,7 @@ DCPExaminer::DCPExaminer (shared_ptr<const DCPContent> content, bool tolerant)
                        for (auto const& font: asset->font_data()) {
                                _fonts.push_back({reel_index, asset->id(), make_shared<dcpomatic::Font>(font.first, font.second)});
                        }
-                       if (asset->font_data().empty()) {
-                               _fonts.push_back({reel_index, asset->id(), make_shared<dcpomatic::Font>("")});
-                       }
+                       _fonts.push_back({reel_index, asset->id(), make_shared<dcpomatic::Font>("")});
                }
 
                _text_count[TextType::CLOSED_CAPTION] = std::max(_text_count[TextType::CLOSED_CAPTION], static_cast<int>(reel->closed_captions().size()));

diff --git a/src/lib/dcp_subtitle_content.cc b/src/lib/dcp_subtitle_content.cc
index e9db5546c1c8b6dc12873dd64007ddd8970a15ba..b3e24d5e2dceef959cfb74d67fecbb64f62b9928 100644 (file)
--- a/src/lib/dcp_subtitle_content.cc
+++ b/src/lib/dcp_subtitle_content.cc
@@ -84,9 +84,7 @@ DCPSubtitleContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
                }
        }
 
-       if (only_text()->fonts().empty()) {
-               only_text()->add_font(make_shared<Font>(""));
-       }
+       only_text()->add_font(make_shared<Font>(""));
 }
 
 DCPTime

diff --git a/src/lib/font_id_allocator.cc b/src/lib/font_id_allocator.cc
index ef25dc6424588450690242109a8854b61f0554b8..70eda2b0613caa2402a24502d9e89d7a769073ed 100644 (file)
--- a/src/lib/font_id_allocator.cc
+++ b/src/lib/font_id_allocator.cc
@@ -63,9 +63,7 @@ FontIDAllocator::add_fonts_from_asset(int reel_index, shared_ptr<const dcp::Subt
                _map[Font(reel_index, asset->id(), font.first)] = 0;
        }
 
-       if (asset->font_data().empty()) {
-               _map[Font(reel_index, asset->id(), "")] = 0;
-       }
+       _map[Font(reel_index, asset->id(), "")] = 0;
 }
 
 

diff --git a/test/subtitle_font_id_test.cc b/test/subtitle_font_id_test.cc
index b4c28e53ef685be0127567f7a6a110df772dd25d..f6bd48c51d0859260b5e36e42c63fd57f3db9f47 100644 (file)
--- a/test/subtitle_font_id_test.cc
+++ b/test/subtitle_font_id_test.cc
@@ -47,7 +47,8 @@ BOOST_AUTO_TEST_CASE(full_dcp_subtitle_font_id_test)
        auto text = content[0]->only_text();
        BOOST_REQUIRE(text);
 
-       BOOST_REQUIRE_EQUAL(text->fonts().size(), 1U);
+       /* There's the font from the DCP and also a dummy one with an empty ID */
+       BOOST_REQUIRE_EQUAL(text->fonts().size(), 2U);
        auto font = text->fonts().front();
        BOOST_CHECK_EQUAL(font->id(), "0_theFontId");
        BOOST_REQUIRE(font->data());
@@ -65,7 +66,8 @@ BOOST_AUTO_TEST_CASE(dcp_subtitle_font_id_test)
        auto text = content[0]->only_text();
        BOOST_REQUIRE(text);
 
-       BOOST_REQUIRE_EQUAL(text->fonts().size(), 1U);
+       /* There's the font from the DCP and also a dummy one with an empty ID */
+       BOOST_REQUIRE_EQUAL(text->fonts().size(), 2U);
        auto font = text->fonts().front();
        BOOST_CHECK_EQUAL(font->id(), "theFontId");
        BOOST_REQUIRE(font->data());
@@ -213,3 +215,49 @@ BOOST_AUTO_TEST_CASE(filler_subtitle_reels_have_load_font_tags)
                });
 }
 
+
+BOOST_AUTO_TEST_CASE(subtitle_with_no_font_test)
+{
+       auto const name_base = boost::unit_test::framework::current_test_case().full_name();
+
+       auto video1 = content_factory("test/data/flat_red.png")[0];
+       auto video2 = content_factory("test/data/flat_red.png")[0];
+       auto subs = content_factory("test/data/short.srt")[0];
+
+       auto bad_film = new_test_film2(name_base + "_bad", { video1, video2, subs });
+       bad_film->set_reel_type(ReelType::BY_VIDEO_CONTENT);
+       video2->set_position(bad_film, video1->end(bad_film));
+       subs->set_position(bad_film, video1->end(bad_film));
+       subs->text[0]->add_font(make_shared<dcpomatic::Font>("foo", "test/data/LiberationSans-Regular.ttf"));
+
+       make_and_verify_dcp(
+               bad_film,
+               {
+                       dcp::VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE,
+                       dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME
+               });
+
+       /* When this test was written, this DCP would have one reel whose subtitles had <LoadFont>s
+        * but the subtitles specified no particular font.  This triggers bug #2649, which this test
+        * is intended to trigger.  First, make sure that the DCP has the required characteristics,
+        * to guard against a case where for some reason the DCP here is different enough that it
+        * doesn't trigger the bug.
+        */
+       dcp::DCP check(bad_film->dir(bad_film->dcp_name()));
+       check.read();
+       BOOST_REQUIRE_EQUAL(check.cpls().size(), 1U);
+       auto cpl = check.cpls()[0];
+       BOOST_REQUIRE_EQUAL(cpl->reels().size(), 2U);
+       auto check_subs_reel = cpl->reels()[0]->main_subtitle();
+       BOOST_REQUIRE(check_subs_reel);
+       auto check_subs = check_subs_reel->asset();
+       BOOST_REQUIRE(check_subs);
+
+       BOOST_CHECK_EQUAL(check_subs->font_data().size(), 1U);
+       BOOST_REQUIRE_EQUAL(check_subs->subtitles().size(), 1U);
+       BOOST_CHECK(!std::dynamic_pointer_cast<const dcp::SubtitleString>(check_subs->subtitles()[0])->font().has_value());
+
+       auto check_film = new_test_film2(name_base + "_check", { make_shared<DCPContent>(bad_film->dir(bad_film->dcp_name())) });
+       make_and_verify_dcp(check_film);
+}
+