From 4ba4258d1a3c89aa1ec4bdcfacb3ceec18adb6b7 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 8 Dec 2020 22:22:30 +0100 Subject: [PATCH] In a DCP with any subs/ccaps, make sure every reel has them (#1340). --- src/lib/reel_writer.cc | 62 ++++++++++++++++++++---- src/lib/reel_writer.h | 10 +++- src/lib/writer.cc | 5 +- src/lib/writer.h | 5 ++ test/subtitle_reel_test.cc | 98 +++++++++++++++++++++++++++++++++++++- 5 files changed, 166 insertions(+), 14 deletions(-) diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc index 8ced98a11..2bd732d5b 100644 --- a/src/lib/reel_writer.cc +++ b/src/lib/reel_writer.cc @@ -60,6 +60,7 @@ using std::string; using std::cout; using std::exception; using std::map; +using std::set; using std::vector; using boost::shared_ptr; using boost::optional; @@ -616,25 +617,57 @@ ReelWriter::create_reel_text ( list const & refs, list > const& fonts, int64_t duration, - boost::filesystem::path output_dcp + boost::filesystem::path output_dcp, + bool ensure_subtitles, + set ensure_closed_captions ) const { shared_ptr subtitle = maybe_add_text ( _subtitle_asset, duration, reel, refs, fonts, film(), _period, output_dcp, _text_only ); - if (subtitle && !film()->subtitle_languages().empty()) { - subtitle->set_language (film()->subtitle_languages().front()); + + if (subtitle) { + /* We have a subtitle asset that we either made or are referencing */ + if (!film()->subtitle_languages().empty()) { + subtitle->set_language (film()->subtitle_languages().front()); + } + } else if (ensure_subtitles) { + /* We had no subtitle asset, but we've been asked to make sure there is one */ + subtitle = maybe_add_text( + empty_text_asset(TEXT_OPEN_SUBTITLE, optional()), + duration, + reel, + refs, + fonts, + film(), + _period, + output_dcp, + _text_only + ); } for (map >::const_iterator i = _closed_caption_assets.begin(); i != _closed_caption_assets.end(); ++i) { shared_ptr a = maybe_add_text ( i->second, duration, reel, refs, fonts, film(), _period, output_dcp, _text_only ); - if (a) { - a->set_annotation_text (i->first.name); - if (!i->first.language.empty()) { - a->set_language (dcp::LanguageTag(i->first.language)); - } + DCPOMATIC_ASSERT (a); + a->set_annotation_text (i->first.name); + if (!i->first.language.empty()) { + a->set_language (dcp::LanguageTag(i->first.language)); + } + + ensure_closed_captions.erase (i->first); + } + + /* Make empty tracks for anything we've been asked to ensure but that we haven't added */ + BOOST_FOREACH (DCPTextTrack i, ensure_closed_captions) { + shared_ptr a = maybe_add_text ( + empty_text_asset(TEXT_CLOSED_CAPTION, i), duration, reel, refs, fonts, film(), _period, output_dcp, _text_only + ); + DCPOMATIC_ASSERT (a); + a->set_annotation_text (i.name); + if (!i.language.empty()) { + a->set_language (dcp::LanguageTag(i.language)); } } } @@ -666,8 +699,17 @@ ReelWriter::create_reel_markers (shared_ptr reel) const } +/** @param ensure_subtitles true to make sure the reel has a subtitle asset. + * @param ensure_closed_captions make sure the reel has these closed caption tracks. + */ shared_ptr -ReelWriter::create_reel (list const & refs, list > const & fonts, boost::filesystem::path output_dcp) +ReelWriter::create_reel ( + list const & refs, + list > const & fonts, + boost::filesystem::path output_dcp, + bool ensure_subtitles, + set ensure_closed_captions + ) { LOG_GENERAL ("create_reel for %1-%2; %3 of %4", _period.from.get(), _period.to.get(), _reel_index, _reel_count); @@ -685,7 +727,7 @@ ReelWriter::create_reel (list const & refs, listadd (shared_ptr(new dcp::ReelAtmosAsset(_atmos_asset, 0))); diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h index 08b85a785..388494247 100644 --- a/src/lib/reel_writer.h +++ b/src/lib/reel_writer.h @@ -77,7 +77,11 @@ public: void finish (boost::filesystem::path output_dcp); boost::shared_ptr create_reel ( - std::list const & refs, std::list > const & fonts, boost::filesystem::path output_dcp + std::list const & refs, + std::list > const & fonts, + boost::filesystem::path output_dcp, + bool ensure_subtitles, + std::set ensure_closed_captions ); void calculate_digests (boost::function set_progress); @@ -109,7 +113,9 @@ private: boost::shared_ptr reel, std::list const & refs, std::list > const& fonts, int64_t duration, - boost::filesystem::path output_dcp + boost::filesystem::path output_dcp, + bool ensure_subtitles, + std::set ensure_closed_captions ) const; void create_reel_markers (boost::shared_ptr reel) const; diff --git a/src/lib/writer.cc b/src/lib/writer.cc index 0b85a7f32..fcf084c56 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -95,6 +95,7 @@ Writer::Writer (weak_ptr weak_film, weak_ptr j, bool text_only) , _repeat_written (0) , _pushed_to_disk (0) , _text_only (text_only) + , _have_subtitles (false) { shared_ptr job = _job.lock (); @@ -590,7 +591,7 @@ Writer::finish (boost::filesystem::path output_dcp) /* Add reels */ BOOST_FOREACH (ReelWriter& i, _reels) { - cpl->add (i.create_reel(_reel_assets, _fonts, output_dcp)); + cpl->add (i.create_reel(_reel_assets, _fonts, output_dcp, _have_subtitles, _have_closed_captions)); } /* Add metadata */ @@ -769,11 +770,13 @@ Writer::write (PlayerText text, TextType type, optional track, DCP switch (type) { case TEXT_OPEN_SUBTITLE: reel = &_subtitle_reel; + _have_subtitles = true; break; case TEXT_CLOSED_CAPTION: DCPOMATIC_ASSERT (track); DCPOMATIC_ASSERT (_caption_reels.find(*track) != _caption_reels.end()); reel = &_caption_reels[*track]; + _have_closed_captions.insert (*track); break; default: DCPOMATIC_ASSERT (false); diff --git a/src/lib/writer.h b/src/lib/writer.h index 3b5cc3dc3..52cb222e7 100644 --- a/src/lib/writer.h +++ b/src/lib/writer.h @@ -197,4 +197,9 @@ private: std::list _reel_assets; std::list > _fonts; + + /** true if any reel has any subtitles */ + bool _have_subtitles; + /** all closed caption tracks that we have on any reel */ + std::set _have_closed_captions; }; diff --git a/test/subtitle_reel_test.cc b/test/subtitle_reel_test.cc index 156581932..2938c6d10 100644 --- a/test/subtitle_reel_test.cc +++ b/test/subtitle_reel_test.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2019 Carl Hetherington + Copyright (C) 2019-2020 Carl Hetherington This file is part of DCP-o-matic. @@ -18,21 +18,29 @@ */ +#include "lib/content_factory.h" #include "lib/film.h" #include "lib/image_content.h" #include "lib/dcp_subtitle_content.h" +#include "lib/text_content.h" #include "lib/video_content.h" #include "test.h" #include #include #include #include +#include #include +#include #include + using std::list; +using std::string; +using boost::optional; using boost::shared_ptr; + /* Check that timings are done correctly for multi-reel DCPs with PNG subs */ BOOST_AUTO_TEST_CASE (subtitle_reel_test) { @@ -86,3 +94,91 @@ BOOST_AUTO_TEST_CASE (subtitle_reel_test) /* These times should be the same as they are should be offset from the start of the reel */ BOOST_CHECK (A->subtitles().front()->in() == B->subtitles().front()->in()); } + + + +/** Check that with a SMPTE DCP if we have subtitles in one reel, all reels have a + * SubtitleAsset (even if it's empty); SMPTE Bv2.1 section 8.3.1. + */ +BOOST_AUTO_TEST_CASE (subtitle_in_all_reels_test) +{ + shared_ptr film = new_test_film2 ("subtitle_in_all_reels_test"); + film->set_interop (false); + film->set_sequence (false); + film->set_reel_type (REELTYPE_BY_VIDEO_CONTENT); + for (int i = 0; i < 3; ++i) { + shared_ptr video = content_factory("test/data/flat_red.png").front(); + film->examine_and_add_content (video); + BOOST_REQUIRE (!wait_for_jobs()); + video->video->set_length (15 * 24); + video->set_position (film, dcpomatic::DCPTime::from_seconds(15 * i)); + } + shared_ptr subs = content_factory("test/data/15s.srt").front(); + film->examine_and_add_content (subs); + BOOST_REQUIRE (!wait_for_jobs()); + film->make_dcp (); + BOOST_REQUIRE (!wait_for_jobs()); + + dcp::DCP dcp ("build/test/subtitle_in_all_reels_test/" + film->dcp_name()); + dcp.read (); + BOOST_REQUIRE_EQUAL (dcp.cpls().size(), 1); + shared_ptr cpl = dcp.cpls().front(); + BOOST_REQUIRE_EQUAL (cpl->reels().size(), 3); + + BOOST_FOREACH (shared_ptr i, cpl->reels()) { + BOOST_CHECK (i->main_subtitle()); + } +} + + +/** Check that with a SMPTE DCP if we have closed captions in one reel, all reels have a + * ClosedCaptionAssets for the same set of tracks (even if they are empty); SMPTE Bv2.1 section 8.3.1. + */ +BOOST_AUTO_TEST_CASE (closed_captions_in_all_reels_test) +{ + shared_ptr film = new_test_film2 ("closed_captions_in_all_reels_test"); + film->set_interop (false); + film->set_sequence (false); + film->set_reel_type (REELTYPE_BY_VIDEO_CONTENT); + + for (int i = 0; i < 3; ++i) { + shared_ptr video = content_factory("test/data/flat_red.png").front(); + film->examine_and_add_content (video); + BOOST_REQUIRE (!wait_for_jobs()); + video->video->set_length (15 * 24); + video->set_position (film, dcpomatic::DCPTime::from_seconds(15 * i)); + } + + shared_ptr ccap1 = content_factory("test/data/15s.srt").front(); + film->examine_and_add_content (ccap1); + BOOST_REQUIRE (!wait_for_jobs()); + ccap1->text.front()->set_type (TEXT_CLOSED_CAPTION); + ccap1->text.front()->set_dcp_track (DCPTextTrack("Test", "de-DE")); + + shared_ptr ccap2 = content_factory("test/data/15s.srt").front(); + film->examine_and_add_content (ccap2); + BOOST_REQUIRE (!wait_for_jobs()); + ccap2->text.front()->set_type (TEXT_CLOSED_CAPTION); + ccap2->text.front()->set_dcp_track (DCPTextTrack("Other", "en-GB")); + + film->make_dcp (); + BOOST_REQUIRE (!wait_for_jobs()); + + dcp::DCP dcp ("build/test/closed_captions_in_all_reels_test/" + film->dcp_name()); + dcp.read (); + BOOST_REQUIRE_EQUAL (dcp.cpls().size(), 1); + shared_ptr cpl = dcp.cpls().front(); + BOOST_REQUIRE_EQUAL (cpl->reels().size(), 3); + + BOOST_FOREACH (shared_ptr i, cpl->reels()) { + BOOST_REQUIRE_EQUAL (i->closed_captions().size(), 2); + optional first = i->closed_captions().front()->language(); + optional second = i->closed_captions().back()->language(); + BOOST_REQUIRE (first); + BOOST_REQUIRE (second); + BOOST_CHECK ( + (*first == "en-GB" && *second == "de-DE") || + (*first == "de-DE" && *second == "en-GB") + ); + } +} -- 2.30.2