Don't write empty <Text> nodes in subtitles/closed captions.

6 files changed, 75 insertions, 30 deletions

diff --git a/cscript b/cscript
index a1684858a..ee4830945 100644
--- 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
diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index 521ba55df..7b9ea09bf 100644
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -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) {
diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h
index 804a93c05..5de00f641 100644
--- a/src/lib/reel_writer.h
+++ b/src/lib/reel_writer.h
@@ -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;
diff --git a/test/data b/test/data
-Subproject 1ba6c3a1d4b3b507ef868aaacf9fa0383a7f22a
+Subproject 1f4ca89620199751370421f59ecd2e5d0331838
diff --git a/test/empty_caption_test.cc b/test/empty_caption_test.cc
new file mode 100644
index 000000000..83f5cce5e
--- /dev/null
+++ b/test/empty_caption_test.cc
@@ -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
+		});
+}
+
diff --git a/test/wscript b/test/wscript
index 797e5eefa..a158ffca1 100644
--- a/test/wscript
+++ b/test/wscript
@@ -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