Fix erroneous hints about ccaps having more than 3 lines if they contain formatting.

3 files changed, 65 insertions, 15 deletions

diff --git a/run/tests b/run/tests
index 8ae093aaf..40bbdcdf0 100755
--- a/run/tests
+++ b/run/tests
@@ -3,7 +3,7 @@
 # e.g. --run_tests=foo
 set -e
 
-PRIVATE_GIT="805027eb2f6d4b1cbacedd77272cf66b1c06ac98"
+PRIVATE_GIT="d64f166137a3f2ee5c950f112e48e52fd982f781"
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 source $DIR/environment
diff --git a/src/lib/hints.cc b/src/lib/hints.cc
index 35eb640d4..ac41b61b8 100644
--- a/src/lib/hints.cc
+++ b/src/lib/hints.cc
@@ -44,16 +44,19 @@
 #include <fmt/format.h>
 #include <boost/algorithm/string.hpp>
 #include <iostream>
+#include <numeric>
 
 #include "i18n.h"
 
 
 using std::cout;
 using std::make_shared;
+using std::map;
 using std::max;
 using std::shared_ptr;
 using std::string;
 using std::weak_ptr;
+using std::vector;
 using boost::optional;
 using namespace dcpomatic;
 #if BOOST_VERSION >= 106100
@@ -584,24 +587,42 @@ Hints::text(PlayerText text, TextType type, optional<DCPTextTrack> track, DCPTim
 void
 Hints::closed_caption(PlayerText text, DCPTimePeriod period)
 {
-	int lines = text.string.size();
-	for (auto i: text.string) {
-		if (utf8_strlen(i.text()) > MAX_CLOSED_CAPTION_LENGTH) {
-			++lines;
-			if (!_long_ccap) {
-				_long_ccap = true;
-				hint(
-					fmt::format(
-						"At least one of your closed caption lines has more than {} characters.  "
-						"It is advisable to make each line {} characters at most in length.",
-						MAX_CLOSED_CAPTION_LENGTH,
-						MAX_CLOSED_CAPTION_LENGTH)
-				     );
+	map<float, vector<StringText>> lines;
+	for (auto const& line: text.string) {
+		bool added = false;
+		for (auto& existing: lines) {
+			if (std::abs(existing.first - line.v_position()) < dcp::ALIGN_EPSILON) {
+				existing.second.push_back(line);
+				added = true;
 			}
 		}
+		if (!added) {
+			lines[line.v_position()] = { line };
+		}
+	}
+
+	for (auto const& line: lines) {
+		int const length = std::accumulate(
+			line.second.begin(),
+			line.second.end(),
+			0,
+			[](int acc, StringText const& text) {
+				return acc + utf8_strlen(text.text());
+			});
+
+		if (length > MAX_CLOSED_CAPTION_LENGTH && !_long_ccap) {
+			_long_ccap = true;
+			hint(
+				fmt::format(
+					"At least one of your closed caption lines has more than {} characters.  "
+					"It is advisable to make each line {} characters at most in length.",
+					MAX_CLOSED_CAPTION_LENGTH,
+					MAX_CLOSED_CAPTION_LENGTH)
+			     );
+		}
 	}
 
-	if (!_too_many_ccap_lines && lines > MAX_CLOSED_CAPTION_LINES) {
+	if (!_too_many_ccap_lines && lines.size() > MAX_CLOSED_CAPTION_LINES) {
 		hint(fmt::format(_("Some of your closed captions span more than {} lines, so they will be truncated."), MAX_CLOSED_CAPTION_LINES));
 		_too_many_ccap_lines = true;
 	}
diff --git a/test/hints_test.cc b/test/hints_test.cc
index 073415018..8abfc6e62 100644
--- a/test/hints_test.cc
+++ b/test/hints_test.cc
@@ -326,3 +326,32 @@ BOOST_AUTO_TEST_CASE(hints_120fps)
 		BOOST_CHECK(hint.find("There is a large difference between the frame rate of your DCP and that of some of your content.") == std::string::npos);
 	}
 }
+
+
+BOOST_AUTO_TEST_CASE(hints_ccap_not_too_many_lines_xml)
+{
+	auto content = content_factory(TestPaths::private_data() / "ccap_not_too_many_lines.xml")[0];
+	auto film = new_test_film("hints_ccap_not_too_many_lines", { content });
+	content->text[0]->set_type(TextType::CLOSED_CAPTION);
+	auto hints = get_hints(film);
+
+	BOOST_CHECK(
+		std::none_of(hints.begin(), hints.end(), [](string const& hint) {
+			return hint.find("Some of your closed captions span more than 3 lines") != std::string::npos;
+		})
+	);
+}
+
+
+BOOST_AUTO_TEST_CASE(hints_ccap_too_many_lines_xml)
+{
+	auto content = content_factory(TestPaths::private_data() / "ccap_too_many_lines.xml")[0];
+	auto film = new_test_film("hints_ccap_too_many_lines", { content });
+	content->text[0]->set_type(TextType::CLOSED_CAPTION);
+	auto hints = get_hints(film);
+	BOOST_CHECK(
+		std::any_of(hints.begin(), hints.end(), [](string const& hint) {
+			return hint.find("Some of your closed captions span more than 3 lines") != std::string::npos;
+		})
+	);
+}