Better subtitle comparisons in tests.v1.8.30

7 files changed, 150 insertions, 44 deletions

diff --git a/src/subtitle.cc b/src/subtitle.cc
index 9d82a128..15481528 100644
--- a/src/subtitle.cc
+++ b/src/subtitle.cc
@@ -41,6 +41,7 @@
 #include "dcp_time.h"
 
 
+using std::shared_ptr;
 using namespace dcp;
 
 
@@ -66,3 +67,52 @@ Subtitle::Subtitle (
 {
 
 }
+
+
+bool
+Subtitle::equals(shared_ptr<const Subtitle> other, EqualityOptions, NoteHandler note) const
+{
+	bool same = true;
+
+	if (in() != other->in()) {
+		note(NoteType::ERROR, "subtitle in times differ");
+		same = false;
+	}
+
+	if (out() != other->out()) {
+		note(NoteType::ERROR, "subtitle out times differ");
+		same = false;
+	}
+
+	if (h_position() != other->h_position()) {
+		note(NoteType::ERROR, "subtitle horizontal positions differ");
+		same = false;
+	}
+
+	if (h_align() != other->h_align()) {
+		note(NoteType::ERROR, "subtitle horizontal alignments differ");
+		same = false;
+	}
+
+	if (v_position() != other->v_position()) {
+		note(NoteType::ERROR, "subtitle vertical positions differ");
+		same = false;
+	}
+
+	if (v_align() != other->v_align()) {
+		note(NoteType::ERROR, "subtitle vertical alignments differ");
+		same = false;
+	}
+
+	if (fade_up_time() != other->fade_up_time()) {
+		note(NoteType::ERROR, "subtitle fade-up times differ");
+		same = false;
+	}
+
+	if (fade_down_time() != other->fade_down_time()) {
+		note(NoteType::ERROR, "subtitle fade-down times differ");
+		same = false;
+	}
+
+	return same;
+}
diff --git a/src/subtitle.h b/src/subtitle.h
index c6a628b8..2a8e0322 100644
--- a/src/subtitle.h
+++ b/src/subtitle.h
@@ -117,6 +117,7 @@ public:
 		_fade_down_time = t;
 	}
 
+	virtual bool equals(std::shared_ptr<const dcp::Subtitle> other, EqualityOptions options, NoteHandler note) const;
 
 protected:
 
diff --git a/src/subtitle_asset.cc b/src/subtitle_asset.cc
index bd8bb993..271199ef 100644
--- a/src/subtitle_asset.cc
+++ b/src/subtitle_asset.cc
@@ -558,8 +558,7 @@ SubtitleAsset::equals (shared_ptr<const Asset> other_asset, EqualityOptions opti
 			return false;
 		}
 
-		if (string_i && *string_i != *string_j) {
-			note (NoteType::ERROR, String::compose("subtitles differ in text or metadata: %1 vs %2", string_i->text(), string_j->text()));
+		if (string_i && !string_i->equals(string_j, options, note)) {
 			return false;
 		}
 
diff --git a/src/subtitle_image.cc b/src/subtitle_image.cc
index b281d754..5f51f05e 100644
--- a/src/subtitle_image.cc
+++ b/src/subtitle_image.cc
@@ -41,6 +41,7 @@
 #include "util.h"
 
 
+using std::dynamic_pointer_cast;
 using std::ostream;
 using std::string;
 using std::shared_ptr;
@@ -128,8 +129,18 @@ dcp::operator!= (SubtitleImage const & a, SubtitleImage const & b)
 
 
 bool
-SubtitleImage::equals (shared_ptr<SubtitleImage> other, EqualityOptions options, NoteHandler note)
+SubtitleImage::equals(shared_ptr<const Subtitle> other_sub, EqualityOptions options, NoteHandler note) const
 {
+	if (!Subtitle::equals(other_sub, options, note)) {
+		return false;
+	}
+
+	auto other = dynamic_pointer_cast<const SubtitleImage>(other_sub);
+	if (!other) {
+		note(NoteType::ERROR, "Subtitle types differ: string vs image");
+		return false;
+	}
+
 	if (png_image() != other->png_image()) {
 		note (NoteType::ERROR, "subtitle image PNG data differs");
 		if (options.export_differing_subtitles) {
@@ -149,46 +160,6 @@ SubtitleImage::equals (shared_ptr<SubtitleImage> other, EqualityOptions options,
 		return false;
 	}
 
-	if (in() != other->in()) {
-		note (NoteType::ERROR, "subtitle in times differ");
-		return false;
-	}
-
-	if (out() != other->out()) {
-		note (NoteType::ERROR, "subtitle out times differ");
-		return false;
-	}
-
-	if (h_position() != other->h_position()) {
-		note (NoteType::ERROR, "subtitle horizontal positions differ");
-		return false;
-	}
-
-	if (h_align() != other->h_align()) {
-		note (NoteType::ERROR, "subtitle horizontal alignments differ");
-		return false;
-	}
-
-	if (v_position() != other->v_position()) {
-		note (NoteType::ERROR, "subtitle vertical positions differ");
-		return false;
-	}
-
-	if (v_align() != other->v_align()) {
-		note (NoteType::ERROR, "subtitle vertical alignments differ");
-		return false;
-	}
-
-	if (fade_up_time() != other->fade_up_time()) {
-		note (NoteType::ERROR, "subtitle fade-up times differ");
-		return false;
-	}
-
-	if (fade_down_time() != other->fade_down_time()) {
-		note (NoteType::ERROR, "subtitle fade-down times differ");
-		return false;
-	}
-
 	return true;
 }
 
diff --git a/src/subtitle_image.h b/src/subtitle_image.h
index 595320f7..9d74d18c 100644
--- a/src/subtitle_image.h
+++ b/src/subtitle_image.h
@@ -103,7 +103,7 @@ public:
 		return _file;
 	}
 
-	bool equals (std::shared_ptr<dcp::SubtitleImage> other, EqualityOptions options, NoteHandler note);
+	bool equals(std::shared_ptr<const dcp::Subtitle> other_sub, EqualityOptions options, NoteHandler note) const override;
 
 private:
 	ArrayData _png_image;
diff --git a/src/subtitle_string.cc b/src/subtitle_string.cc
index e70a849f..7f7c74a5 100644
--- a/src/subtitle_string.cc
+++ b/src/subtitle_string.cc
@@ -37,14 +37,17 @@
  */
 
 
+#include "compose.hpp"
 #include "subtitle_string.h"
 #include "xml.h"
 #include <cmath>
 
 
+using std::dynamic_pointer_cast;
 using std::max;
 using std::min;
 using std::ostream;
+using std::shared_ptr;
 using std::string;
 using boost::optional;
 using namespace dcp;
@@ -171,3 +174,83 @@ dcp::operator<< (ostream& s, SubtitleString const & sub)
 
 	return s;
 }
+
+
+bool
+SubtitleString::equals(shared_ptr<const Subtitle> other_sub, EqualityOptions options, NoteHandler note) const
+{
+	if (!Subtitle::equals(other_sub, options, note)) {
+		return false;
+	}
+
+	auto other = dynamic_pointer_cast<const SubtitleString>(other_sub);
+	if (!other) {
+		note(NoteType::ERROR, "Subtitle types differ: string vs image");
+		return false;
+	}
+
+	bool same = true;
+
+	if (_font != other->_font) {
+		note(NoteType::ERROR, String::compose("subtitle font differs: %1 vs %2", _font.get_value_or("[none]"), other->_font.get_value_or("[none]")));
+		same = false;
+	}
+
+	if (_italic != other->_italic) {
+		note(NoteType::ERROR, String::compose("subtitle italic flag differs: %1 vs %2", _italic ? "true" : "false", other->_italic ? "true" : "false"));
+		same = false;
+	}
+
+	if (_bold != other->_bold) {
+		note(NoteType::ERROR, String::compose("subtitle bold flag differs: %1 vs %2", _bold ? "true" : "false", other->_bold ? "true" : "false"));
+		same = false;
+	}
+
+	if (_underline != other->_underline) {
+		note(NoteType::ERROR, String::compose("subtitle underline flag differs: %1 vs %2", _underline ? "true" : "false", other->_underline ? "true" : "false"));
+		same = false;
+	}
+
+	if (_colour != other->_colour) {
+		note(NoteType::ERROR, String::compose("subtitle colour differs: %1 vs %2", _colour.to_rgb_string(), other->_colour.to_rgb_string()));
+		same = false;
+	}
+
+	if (_size != other->_size) {
+		note(NoteType::ERROR, String::compose("subtitle size differs: %1 vs %2", _size, other->_size));
+		same = false;
+	}
+
+	if (_aspect_adjust != other->_aspect_adjust) {
+		note(NoteType::ERROR, String::compose("subtitle aspect_adjust differs: %1 vs %2", _aspect_adjust, other->_aspect_adjust));
+		same = false;
+	}
+
+	if (_direction != other->_direction) {
+		note(NoteType::ERROR, String::compose("subtitle direction differs: %1 vs %2", direction_to_string(_direction), direction_to_string(other->_direction)));
+		same = false;
+	}
+
+	if (_text != other->_text) {
+		note(NoteType::ERROR, String::compose("subtitle text differs: %1 vs %2", _text, other->_text));
+		same = false;
+	}
+
+	if (_effect != other->_effect) {
+		note(NoteType::ERROR, String::compose("subtitle effect differs: %1 vs %2", effect_to_string(_effect), effect_to_string(other->_effect)));
+		same = false;
+	}
+
+	if (_effect_colour != other->_effect_colour) {
+		note(NoteType::ERROR, String::compose("subtitle effect colour differs: %1 vs %2", _effect_colour.to_rgb_string(), other->_effect_colour.to_rgb_string()));
+		same = false;
+	}
+
+	if (_space_before != other->_space_before) {
+		note(NoteType::ERROR, String::compose("subtitle space before differs: %1 vs %2", _space_before, other->_space_before));
+		same = false;
+	}
+
+	return same;
+}
+
diff --git a/src/subtitle_string.h b/src/subtitle_string.h
index f6c39836..0ffa6ec5 100644
--- a/src/subtitle_string.h
+++ b/src/subtitle_string.h
@@ -188,6 +188,8 @@ public:
 		_effect_colour = c;
 	}
 
+	bool equals(std::shared_ptr<const dcp::Subtitle> other_sub, EqualityOptions options, NoteHandler node) const override;
+
 private:
 	/** font ID */
 	boost::optional<std::string> _font;