Don't write empty <Text> nodes in subtitles/closed captions.
authorCarl Hetherington <cth@carlh.net>
Wed, 11 Aug 2021 23:29:24 +0000 (01:29 +0200)
committerCarl Hetherington <cth@carlh.net>
Wed, 11 Aug 2021 23:30:31 +0000 (01:30 +0200)
cscript
src/lib/reel_writer.cc
src/lib/reel_writer.h
test/data
test/empty_caption_test.cc [new file with mode: 0644]
test/wscript

diff --git a/cscript b/cscript
index a1684858a6a510a26976bb65fba0121b4e422172..ee483094577e4fd6497df6732ac4b882a3ee4a0c 100644 (file)
--- a/cscript
+++ b/cscript
@@ -388,8 +388,8 @@ def dependencies(target, options):
         # Use distro-provided FFmpeg on Arch
         deps = []
 
-    deps.append(('libdcp', '9bc5ced'))
-    deps.append(('libsub', '749c204'))
+    deps.append(('libdcp', 'b3cb9c0'))
+    deps.append(('libsub', '587e475'))
     deps.append(('leqm-nrt', '93ae9e6'))
     deps.append(('rtaudio', 'f619b76'))
     # We get our OpenSSL libraries from the environment, but we
index 521ba55df891f226393fdb7a6d12e5b139ebe6de..7b9ea09bf469cc03e003099c00346bc17047752b 100644 (file)
@@ -641,7 +641,7 @@ ReelWriter::create_reel_text (
        } else if (ensure_subtitles) {
                /* We had no subtitle asset, but we've been asked to make sure there is one */
                subtitle = maybe_add_text<dcp::ReelInteropSubtitleAsset, dcp::ReelSMPTESubtitleAsset, dcp::ReelSubtitleAsset> (
-                       empty_text_asset(TextType::OPEN_SUBTITLE, optional<DCPTextTrack>()),
+                       empty_text_asset(TextType::OPEN_SUBTITLE, optional<DCPTextTrack>(), true),
                        duration,
                        reel,
                        refs,
@@ -670,7 +670,7 @@ ReelWriter::create_reel_text (
        /* Make empty tracks for anything we've been asked to ensure but that we haven't added */
        for (auto i: ensure_closed_captions) {
                auto a = maybe_add_text<dcp::ReelInteropClosedCaptionAsset, dcp::ReelSMPTEClosedCaptionAsset, dcp::ReelClosedCaptionAsset> (
-                       empty_text_asset(TextType::CLOSED_CAPTION, i), duration, reel, refs, fonts, _default_font, film(), _period, output_dcp, _text_only
+                       empty_text_asset(TextType::CLOSED_CAPTION, i, true), duration, reel, refs, fonts, _default_font, film(), _period, output_dcp, _text_only
                        );
                DCPOMATIC_ASSERT (a);
                a->set_annotation_text (i.name);
@@ -784,7 +784,7 @@ ReelWriter::write (shared_ptr<const AudioBuffers> audio)
 
 
 shared_ptr<dcp::SubtitleAsset>
-ReelWriter::empty_text_asset (TextType type, optional<DCPTextTrack> track) const
+ReelWriter::empty_text_asset (TextType type, optional<DCPTextTrack> track, bool with_dummy) const
 {
        shared_ptr<dcp::SubtitleAsset> asset;
 
@@ -815,29 +815,31 @@ ReelWriter::empty_text_asset (TextType type, optional<DCPTextTrack> track) const
                if (film()->encrypted()) {
                        s->set_key (film()->key());
                }
-               s->add (
-                       std::make_shared<dcp::SubtitleString>(
-                               optional<std::string>(),
-                               false,
-                               false,
-                               false,
-                               dcp::Colour(),
-                               42,
-                               1.0,
-                               dcp::Time(0, 0, 0, 0, 24),
-                               dcp::Time(0, 0, 1, 0, 24),
-                               0.5,
-                               dcp::HAlign::CENTER,
-                               0.5,
-                               dcp::VAlign::CENTER,
-                               dcp::Direction::LTR,
-                               "",
-                               dcp::Effect::NONE,
-                               dcp::Colour(),
-                               dcp::Time(),
-                               dcp::Time()
-                               )
-                      );
+               if (with_dummy) {
+                       s->add (
+                               std::make_shared<dcp::SubtitleString>(
+                                       optional<std::string>(),
+                                       false,
+                                       false,
+                                       false,
+                                       dcp::Colour(),
+                                       42,
+                                       1.0,
+                                       dcp::Time(0, 0, 0, 0, 24),
+                                       dcp::Time(0, 0, 1, 0, 24),
+                                       0.5,
+                                       dcp::HAlign::CENTER,
+                                       0.5,
+                                       dcp::VAlign::CENTER,
+                                       dcp::Direction::LTR,
+                                       " ",
+                                       dcp::Effect::NONE,
+                                       dcp::Colour(),
+                                       dcp::Time(),
+                                       dcp::Time()
+                                       )
+                              );
+               }
                asset = s;
        }
 
@@ -863,7 +865,7 @@ ReelWriter::write (PlayerText subs, TextType type, optional<DCPTextTrack> track,
        }
 
        if (!asset) {
-               asset = empty_text_asset (type, track);
+               asset = empty_text_asset (type, track, false);
        }
 
        switch (type) {
index 804a93c05d3c6103b5e4c0e4fc77364f6a02906d..5de00f641be5fe577da14ee158fa507f39d2d510 100644 (file)
@@ -103,7 +103,7 @@ private:
        long frame_info_position (Frame frame, Eyes eyes) const;
        Frame check_existing_picture_asset (boost::filesystem::path asset);
        bool existing_picture_frame_ok (FILE* asset_file, std::shared_ptr<InfoFileHandle> info_file, Frame frame) const;
-       std::shared_ptr<dcp::SubtitleAsset> empty_text_asset (TextType type, boost::optional<DCPTextTrack> track) const;
+       std::shared_ptr<dcp::SubtitleAsset> empty_text_asset (TextType type, boost::optional<DCPTextTrack> track, bool with_dummy) const;
 
        std::shared_ptr<dcp::ReelPictureAsset> create_reel_picture (std::shared_ptr<dcp::Reel> reel, std::list<ReferencedReelAsset> const & refs) const;
        void create_reel_sound (std::shared_ptr<dcp::Reel> reel, std::list<ReferencedReelAsset> const & refs) const;
index 1ba6c3a1d4b3b507ef868aaacf9fa0383a7f22a9..1f4ca89620199751370421f59ecd2e5d03318387 160000 (submodule)
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit 1ba6c3a1d4b3b507ef868aaacf9fa0383a7f22a9
+Subproject commit 1f4ca89620199751370421f59ecd2e5d03318387
diff --git a/test/empty_caption_test.cc b/test/empty_caption_test.cc
new file mode 100644 (file)
index 0000000..83f5cce
--- /dev/null
@@ -0,0 +1,42 @@
+/*
+    Copyright (C) 2021 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.h"
+#include "lib/content_factory.h"
+#include "lib/film.h"
+#include "lib/text_content.h"
+#include "test.h"
+#include <boost/test/unit_test.hpp>
+
+
+BOOST_AUTO_TEST_CASE (check_for_no_empty_text_nodes_in_failure_case)
+{
+       auto content = content_factory("test/data/empty.srt").front();
+       auto film = new_test_film2 ("check_for_no_empty_text_nodes_in_failure_case", {content});
+       auto text = content->text.front();
+       text->set_type (TextType::CLOSED_CAPTION);
+       text->set_dcp_track({"English", dcp::LanguageTag("en-GB")});
+
+       make_and_verify_dcp (film, {
+                       dcp::VerificationNote::Code::MISSING_CPL_METADATA
+               });
+}
+
index 797e5eefa7e7e1f3a276178838170d4965c2b9d7..a158ffca1049475915be70c4e906ea65f613f535 100644 (file)
@@ -69,6 +69,7 @@ def build(bld):
                  dcp_playback_test.cc
                  dcp_subtitle_test.cc
                  digest_test.cc
+                 empty_caption_test.cc
                  empty_test.cc
                  encryption_test.cc
                  ffmpeg_audio_only_test.cc