Merge remote-tracking branch 'origin/main' into v1.9.xv1.9.10

3 files changed, 74 insertions, 22 deletions

diff --git a/src/verify.cc b/src/verify.cc
index 1f0d7790..1e27514a 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -81,6 +81,7 @@
 #include <boost/algorithm/string.hpp>
 #include <iostream>
 #include <map>
+#include <numeric>
 #include <regex>
 #include <set>
 #include <vector>
@@ -1200,18 +1201,9 @@ verify_closed_caption_details(Context& context, vector<shared_ptr<Reel>> reels)
 }
 
 
-struct LinesCharactersResult
-{
-	bool warning_length_exceeded = false;
-	bool error_length_exceeded = false;
-	bool line_count_exceeded = false;
-};
-
-
-static
 void
-verify_text_lines_and_characters (
-	shared_ptr<SubtitleAsset> asset,
+dcp::verify_text_lines_and_characters(
+	shared_ptr<const SubtitleAsset> asset,
 	int warning_length,
 	int error_length,
 	LinesCharactersResult* result
@@ -1232,8 +1224,8 @@ verify_text_lines_and_characters (
 		{}
 
 		Time time;
-		int position; //< position from 0 at top of screen to 100 at bottom
-		int characters;
+		int position = 0;   ///< vertical position from 0 at top of screen to 100 at bottom
+		int characters = 0; ///< number of characters in the text of this event
 		shared_ptr<Event> start;
 	};
 
@@ -1252,6 +1244,7 @@ verify_text_lines_and_characters (
 		return 0L;
 	};
 
+	/* Make a list of "subtitle starts" and "subtitle ends" events */
 	for (auto j: asset->subtitles()) {
 		auto text = dynamic_pointer_cast<const SubtitleString>(j);
 		if (text) {
@@ -1261,20 +1254,25 @@ verify_text_lines_and_characters (
 		}
 	}
 
-	std::sort(events.begin(), events.end(), [](shared_ptr<Event> const& a, shared_ptr<Event>const& b) {
+	std::sort(events.begin(), events.end(), [](shared_ptr<Event> const& a, shared_ptr<Event> const& b) {
 		return a->time < b->time;
 	});
 
-	map<int, int> current;
+	/* The number of characters currently displayed at different vertical positions, i.e. on
+	 * what we consider different lines.  Key is the vertical position (0 to 100) and the value
+	 * is a list of the active subtitles in that position.
+	 */
+	map<int, vector<shared_ptr<Event>>> current;
 	for (auto i: events) {
 		if (current.size() > 3) {
 			result->line_count_exceeded = true;
 		}
 		for (auto j: current) {
-			if (j.second > warning_length) {
+			int length = std::accumulate(j.second.begin(), j.second.end(), 0, [](int total, shared_ptr<const Event> event) { return total + event->characters; });
+			if (length > warning_length) {
 				result->warning_length_exceeded = true;
 			}
-			if (j.second > error_length) {
+			if (length > error_length) {
 				result->error_length_exceeded = true;
 			}
 		}
@@ -1282,17 +1280,20 @@ verify_text_lines_and_characters (
 		if (i->start) {
 			/* end of a subtitle */
 			DCP_ASSERT (current.find(i->start->position) != current.end());
-			if (current[i->start->position] == i->start->characters) {
+			auto current_position = current[i->start->position];
+			auto iter = std::find(current_position.begin(), current_position.end(), i->start);
+			if (iter != current_position.end()) {
+				current_position.erase(iter);
+			}
+			if (current_position.empty()) {
 				current.erase(i->start->position);
-			} else {
-				current[i->start->position] -= i->start->characters;
 			}
 		} else {
 			/* start of a subtitle */
 			if (current.find(i->position) == current.end()) {
-				current[i->position] = i->characters;
+				current[i->position] = vector<shared_ptr<Event>>{i};
 			} else {
-				current[i->position] += i->characters;
+				current[i->position].push_back(i);
 			}
 		}
 	}
diff --git a/src/verify.h b/src/verify.h
index d8681b73..e9a67f83 100644
--- a/src/verify.h
+++ b/src/verify.h
@@ -59,6 +59,7 @@ namespace dcp {
 
 
 class DCP;
+class SubtitleAsset;
 
 
 class VerificationNote
@@ -718,6 +719,21 @@ bool operator< (dcp::VerificationNote const& a, dcp::VerificationNote const& b);
 std::ostream& operator<<(std::ostream& s, dcp::VerificationNote const& note);
 
 
+struct LinesCharactersResult
+{
+	bool warning_length_exceeded = false;
+	bool error_length_exceeded = false;
+	bool line_count_exceeded = false;
+};
+
+
+extern void verify_text_lines_and_characters(
+	std::shared_ptr<const dcp::SubtitleAsset> asset,
+	int warning_length,
+	int error_length,
+	dcp::LinesCharactersResult* result
+	);
+
 }
 
 
diff --git a/test/verify_test.cc b/test/verify_test.cc
index a465bac4..3f551790 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -5819,3 +5819,38 @@ BOOST_AUTO_TEST_CASE(verify_invalid_sound_bit_depth)
 	check_verify_result({ dir }, {}, notes);
 }
 
+
+BOOST_AUTO_TEST_CASE(overlapping_subtitles)
+{
+	auto asset = make_shared<dcp::InteropSubtitleAsset>();
+
+	asset->add(std::make_shared<dcp::SubtitleString>(
+			optional<string>{}, false, false, false,
+			dcp::Colour{}, 42, 0,
+			dcp::Time(0, 0, 0, 0, 24),
+			dcp::Time(0, 0, 8, 0, 24),
+			0, dcp::HAlign::CENTER,
+			0, dcp::VAlign::CENTER,
+			0,
+			dcp::Direction::LTR,
+			"",
+			dcp::Effect::NONE, dcp::Colour{}, dcp::Time{}, dcp::Time{}, 0, vector<dcp::Ruby>{}
+			));
+
+	asset->add(std::make_shared<dcp::SubtitleString>(
+			optional<string>{}, false, false, false,
+			dcp::Colour{}, 42, 0,
+			dcp::Time(0, 0, 2, 0, 24),
+			dcp::Time(0, 0, 4, 0, 24),
+			0, dcp::HAlign::CENTER,
+			0, dcp::VAlign::CENTER,
+			0,
+			dcp::Direction::LTR,
+			"Hello",
+			dcp::Effect::NONE, dcp::Colour{}, dcp::Time{}, dcp::Time{}, 0, vector<dcp::Ruby>{}
+			));
+
+	dcp::LinesCharactersResult result;
+	dcp::verify_text_lines_and_characters(asset, 64, 80, &result);
+}
+