From: Carl Hetherington <cth@carlh.net>
Date: Sun, 2 Jul 2023 20:03:28 +0000 (+0200)
Subject: Don't insert incorrect <Effect> nodes into metadata (#2581).
X-Git-Tag: v2.16.60~5
X-Git-Url: https://git.carlh.net/gitweb/?a=commitdiff_plain;h=fc10e39ac88ee16a711d1d643f863e84dcd8157d;hp=5f3ba49300f3e3d5ac36a2ce33dadc6d1ffc049c;p=dcpomatic.git

Don't insert incorrect <Effect> nodes into metadata (#2581).

Previously we would add assume Effect=none (i.e. force all subtitles
to have no effect) if neither of the legacy tags Border or Shadow
were present in the metadata.  In this case we should just leave
Effect as unset.
---

diff --git a/src/lib/text_content.cc b/src/lib/text_content.cc
index e4cbc601a..92a35b822 100644
--- a/src/lib/text_content.cc
+++ b/src/lib/text_content.cc
@@ -148,8 +148,6 @@ TextContent::TextContent (Content* parent, cxml::ConstNodePtr node, int version,
 		_effect = dcp::Effect::BORDER;
 	} else if (node->optional_bool_child("Shadow").get_value_or(false)) {
 		_effect = dcp::Effect::SHADOW;
-	} else {
-		_effect = dcp::Effect::NONE;
 	}
 
 	auto effect = node->optional_string_child("Effect");
diff --git a/test/film_metadata_test.cc b/test/film_metadata_test.cc
index 9b855de5b..ada943c80 100644
--- a/test/film_metadata_test.cc
+++ b/test/film_metadata_test.cc
@@ -210,3 +210,24 @@ BOOST_AUTO_TEST_CASE (metadata_video_range_guessed_for_png)
 	BOOST_REQUIRE(film->content()[0]->video);
 	BOOST_CHECK(film->content()[0]->video->range() == VideoRange::FULL);
 }
+
+
+/* Bug #2581 */
+BOOST_AUTO_TEST_CASE(effect_node_not_inserted_incorrectly)
+{
+	auto sub = content_factory("test/data/15s.srt");
+	auto film = new_test_film2("effect_node_not_inserted_incorrectly", sub);
+	film->write_metadata();
+
+	namespace fs = boost::filesystem;
+	auto film2 = make_shared<Film>(fs::path("build/test/effect_node_not_inserted_incorrectly"));
+	film2->read_metadata();
+	film2->write_metadata();
+
+	cxml::Document doc("Metadata");
+	doc.read_file("build/test/effect_node_not_inserted_incorrectly/metadata.xml");
+
+	/* There should be no <Effect> node in the text, since we don't want to force the effect to "none" */
+	BOOST_CHECK(!doc.node_child("Playlist")->node_child("Content")->node_child("Text")->optional_node_child("Effect"));
+}
+