Various attempts to clean up DCP comparison code.

13 files changed, 110 insertions, 46 deletions

diff --git a/src/asset.cc b/src/asset.cc
index e55dca2d..96196b41 100644
--- a/src/asset.cc
+++ b/src/asset.cc
@@ -105,7 +105,7 @@ bool
 Asset::equals (boost::shared_ptr<const Asset> other, EqualityOptions, function<void (NoteType, string)> note) const
 {
 	if (_hash != other->_hash) {
-		note (DCP_ERROR, "Asset hashes differ");
+		note (DCP_ERROR, "Asset: hashes differ");
 		return false;
 	}
 
diff --git a/src/content.cc b/src/content.cc
index 265778bf..3842ec09 100644
--- a/src/content.cc
+++ b/src/content.cc
@@ -36,13 +36,3 @@ Content::Content (boost::filesystem::path file)
 {
 	
 }
-
-bool
-Content::equals (shared_ptr<const Asset> other, EqualityOptions opt, boost::function<void (NoteType, string)> note) const
-{
-	if (!Asset::equals (other, opt, note)) {
-		return false;
-	}
-
-	return true;
-}
diff --git a/src/content.h b/src/content.h
index 27939e83..fcbfc25d 100644
--- a/src/content.h
+++ b/src/content.h
@@ -58,12 +58,6 @@ public:
 	 */
 	Content (boost::filesystem::path file);
 
-	bool equals (
-		boost::shared_ptr<const Asset> other,
-		EqualityOptions opt,
-		boost::function<void (NoteType, std::string)>
-		) const;
-
 protected:
 	virtual std::string asdcp_kind () const = 0;
 };
diff --git a/src/cpl.cc b/src/cpl.cc
index 9c8c865b..3599231d 100644
--- a/src/cpl.cc
+++ b/src/cpl.cc
@@ -42,6 +42,7 @@ using std::ostream;
 using std::list;
 using std::pair;
 using std::make_pair;
+using std::cout;
 using boost::shared_ptr;
 using boost::optional;
 using boost::dynamic_pointer_cast;
@@ -184,18 +185,18 @@ CPL::equals (shared_ptr<const Asset> other, EqualityOptions opt, boost::function
 	
 	if (_annotation_text != other_cpl->_annotation_text && !opt.cpl_annotation_texts_can_differ) {
 		stringstream s;
-		s << "annotation texts differ: " << _annotation_text << " vs " << other_cpl->_annotation_text << "\n";
+		s << "CPL: annotation texts differ: " << _annotation_text << " vs " << other_cpl->_annotation_text << "\n";
 		note (DCP_ERROR, s.str ());
 		return false;
 	}
 
 	if (_content_kind != other_cpl->_content_kind) {
-		note (DCP_ERROR, "content kinds differ");
+		note (DCP_ERROR, "CPL: content kinds differ");
 		return false;
 	}
 
 	if (_reels.size() != other_cpl->_reels.size()) {
-		note (DCP_ERROR, String::compose ("reel counts differ (%1 vs %2)", _reels.size(), other_cpl->_reels.size()));
+		note (DCP_ERROR, String::compose ("CPL: reel counts differ (%1 vs %2)", _reels.size(), other_cpl->_reels.size()));
 		return false;
 	}
 	
diff --git a/src/dcp.cc b/src/dcp.cc
index c92a14b8..ca1c23d9 100644
--- a/src/dcp.cc
+++ b/src/dcp.cc
@@ -50,6 +50,7 @@
 
 using std::string;
 using std::list;
+using std::cout;
 using std::stringstream;
 using std::ostream;
 using std::make_pair;
@@ -174,23 +175,28 @@ DCP::read (bool keep_going, ReadErrors* errors)
 bool
 DCP::equals (DCP const & other, EqualityOptions opt, boost::function<void (NoteType, string)> note) const
 {
-	if (_assets.size() != other._assets.size()) {
-		note (DCP_ERROR, String::compose ("Asset counts differ: %1 vs %2", _assets.size(), other._assets.size()));
+	list<shared_ptr<CPL> > a = cpls ();
+	list<shared_ptr<CPL> > b = other.cpls ();
+	
+	if (a.size() != b.size()) {
+		note (DCP_ERROR, String::compose ("CPL counts differ: %1 vs %2", a.size(), b.size()));
 		return false;
 	}
 
-	list<shared_ptr<Asset> >::const_iterator a = _assets.begin ();
-	list<shared_ptr<Asset> >::const_iterator b = other._assets.begin ();
+	bool r = true;
+
+	for (list<shared_ptr<CPL> >::const_iterator i = a.begin(); i != a.end(); ++i) {
+		list<shared_ptr<CPL> >::const_iterator j = b.begin ();
+		while (j != b.end() && !(*j)->equals (*i, opt, note)) {
+			++j;
+		}
 
-	while (a != _assets.end ()) {
-		if (!(*a)->equals (*b, opt, note)) {
-			return false;
+		if (j == b.end ()) {
+			r = false;
 		}
-		++a;
-		++b;
 	}
 
-	return true;
+	return r;
 }
 
 void
diff --git a/src/mono_picture_mxf.cc b/src/mono_picture_mxf.cc
index b2042fa5..66cfa12c 100644
--- a/src/mono_picture_mxf.cc
+++ b/src/mono_picture_mxf.cc
@@ -27,6 +27,7 @@
 
 using std::string;
 using std::vector;
+using std::cout;
 using boost::shared_ptr;
 using boost::dynamic_pointer_cast;
 using namespace dcp;
@@ -70,6 +71,10 @@ MonoPictureMXF::get_frame (int n) const
 bool
 MonoPictureMXF::equals (shared_ptr<const Asset> other, EqualityOptions opt, boost::function<void (NoteType, string)> note) const
 {
+	if (!dynamic_pointer_cast<const MonoPictureMXF> (other)) {
+		return false;
+	}
+	
 	if (!MXF::equals (other, opt, note)) {
 		return false;
 	}
diff --git a/src/mxf.cc b/src/mxf.cc
index f4d4a490..6709af4c 100644
--- a/src/mxf.cc
+++ b/src/mxf.cc
@@ -100,22 +100,21 @@ MXF::equals (shared_ptr<const Asset> other, EqualityOptions opt, boost::function
 	
 	shared_ptr<const MXF> other_mxf = dynamic_pointer_cast<const MXF> (other);
 	if (!other_mxf) {
-		note (DCP_ERROR, "comparing an MXF asset with a non-MXF asset");
 		return false;
 	}
 	
 	if (_edit_rate != other_mxf->_edit_rate) {
-		note (DCP_ERROR, "content edit rates differ");
+		note (DCP_ERROR, "MXF: edit rates differ");
 	 	return false;
 	}
 	
 	if (_intrinsic_duration != other_mxf->_intrinsic_duration) {
-	 	note (DCP_ERROR, "asset intrinsic durations differ");
+	 	note (DCP_ERROR, String::compose ("MXF: intrinsic durations differ (%1 vs %2)", _intrinsic_duration, other_mxf->_intrinsic_duration));
 		return false;
 	}
 
 	if (_file != other_mxf->file ()) {
-		note (DCP_ERROR, "MXF names differ");
+		note (DCP_ERROR, "MXF: names differ");
 		if (!opt.mxf_names_can_differ) {
 			return false;
 		}
diff --git a/src/reel.cc b/src/reel.cc
index 5878fc2b..0071de86 100644
--- a/src/reel.cc
+++ b/src/reel.cc
@@ -98,7 +98,7 @@ bool
 Reel::equals (boost::shared_ptr<const Reel> other, EqualityOptions opt, boost::function<void (NoteType, string)> note) const
 {
 	if ((_main_picture && !other->_main_picture) || (!_main_picture && other->_main_picture)) {
-		note (DCP_ERROR, "reel has different assets");
+		note (DCP_ERROR, "Reel: assets differ");
 		return false;
 	}
 	
@@ -107,7 +107,7 @@ Reel::equals (boost::shared_ptr<const Reel> other, EqualityOptions opt, boost::f
 	}
 
 	if ((_main_sound && !other->_main_sound) || (!_main_sound && other->_main_sound)) {
-		note (DCP_ERROR, "reel has different assets");
+		note (DCP_ERROR, "Reel: assets differ");
 		return false;
 	}
 	
@@ -116,7 +116,7 @@ Reel::equals (boost::shared_ptr<const Reel> other, EqualityOptions opt, boost::f
 	}
 
 	if ((_main_subtitle && !other->_main_subtitle) || (!_main_subtitle && other->_main_subtitle)) {
-		note (DCP_ERROR, "reel has different assets");
+		note (DCP_ERROR, "Reel: assets differ");
 		return false;
 	}
 	
diff --git a/src/reel_asset.cc b/src/reel_asset.cc
index 0c7c6e51..978f1dee 100644
--- a/src/reel_asset.cc
+++ b/src/reel_asset.cc
@@ -25,6 +25,7 @@
 
 using std::pair;
 using std::string;
+using std::stringstream;
 using std::make_pair;
 using boost::shared_ptr;
 using namespace dcp;
@@ -97,3 +98,49 @@ ReelAsset::cpl_node_attribute (Standard) const
 {
 	return make_pair ("", "");
 }
+
+bool
+ReelAsset::equals (shared_ptr<const ReelAsset> other, EqualityOptions opt, boost::function<void (NoteType, std::string)> note) const
+{
+	if (_annotation_text != other->_annotation_text) {
+		stringstream s;
+		s << "Reel: annotation texts differ (" << _annotation_text << " vs " << other->_annotation_text << ")\n";
+		note (DCP_ERROR, s.str ());
+		return false;
+	}
+
+	if (_edit_rate != other->_edit_rate) {
+		note (DCP_ERROR, "Reel: edit rates differ");
+		return false;
+	}
+
+	if (_intrinsic_duration != other->_intrinsic_duration) {
+		note (DCP_ERROR, "Reel: intrinsic durations differ");
+		return false;
+	}
+
+	if (_entry_point != other->_entry_point) {
+		note (DCP_ERROR, "Reel: entry points differ");
+		return false;
+	}
+
+	if (_duration != other->_duration) {
+		note (DCP_ERROR, "Reel: durations differ");
+		return false;
+	}
+
+	if (_hash != other->_hash) {
+		if (!opt.reel_hashes_can_differ) {
+			note (DCP_ERROR, "Reel: hashes differ");
+			return false;
+		} else {
+			note (DCP_NOTE, "Reel: hashes differ");
+		}
+	}
+
+	if (_content.resolved () && other->_content.resolved ()) {
+		return _content->equals (other->_content.object (), opt, note);
+	}
+
+	return true;
+}
diff --git a/src/reel_asset.h b/src/reel_asset.h
index 84d98221..61d2b48f 100644
--- a/src/reel_asset.h
+++ b/src/reel_asset.h
@@ -52,15 +52,7 @@ public:
 	ReelAsset (boost::shared_ptr<const cxml::Node>);
 
 	virtual void write_to_cpl (xmlpp::Node* node, Standard standard) const;
-
-	virtual bool equals (
-		boost::shared_ptr<const ReelAsset>,
-		EqualityOptions,
-		boost::function<void (NoteType, std::string)>)
-		const {
-		
-		return false;
-	}
+	virtual bool equals (boost::shared_ptr<const ReelAsset>, EqualityOptions, boost::function<void (NoteType, std::string)>) const;
 
 	/** @return a Ref to our actual content */
 	Ref<Content>& content () {
diff --git a/src/reel_picture_asset.cc b/src/reel_picture_asset.cc
index 4f737a91..1fbd635b 100644
--- a/src/reel_picture_asset.cc
+++ b/src/reel_picture_asset.cc
@@ -32,6 +32,7 @@ using std::bad_cast;
 using std::string;
 using std::stringstream;
 using boost::shared_ptr;
+using boost::dynamic_pointer_cast;
 using namespace dcp;
 
 ReelPictureAsset::ReelPictureAsset ()
@@ -96,3 +97,28 @@ ReelPictureAsset::key_type () const
 {
 	return "MDIK";
 }
+
+bool
+ReelPictureAsset::equals (shared_ptr<const ReelAsset> other, EqualityOptions opt, boost::function<void (NoteType, std::string)> note) const
+{
+	if (!ReelAsset::equals (other, opt, note)) {
+		return false;
+	}
+	
+	shared_ptr<const ReelPictureAsset> rpa = dynamic_pointer_cast<const ReelPictureAsset> (other);
+	if (!rpa) {
+		return false;
+	}
+
+	if (_frame_rate != rpa->_frame_rate) {
+		note (DCP_ERROR, "frame rates differ in reel");
+		return false;
+	}
+
+	if (_screen_aspect_ratio != rpa->_screen_aspect_ratio) {
+		note (DCP_ERROR, "screen aspect ratios differ in reel");
+		return false;
+	}
+
+	return true;
+}
diff --git a/src/reel_picture_asset.h b/src/reel_picture_asset.h
index 7ab92228..76ba22c9 100644
--- a/src/reel_picture_asset.h
+++ b/src/reel_picture_asset.h
@@ -40,6 +40,7 @@ public:
 	ReelPictureAsset (boost::shared_ptr<const cxml::Node>);
 
 	virtual void write_to_cpl (xmlpp::Node* node, Standard standard) const;
+	virtual bool equals (boost::shared_ptr<const ReelAsset>, EqualityOptions, boost::function<void (NoteType, std::string)>) const;
 
 	boost::shared_ptr<PictureMXF> mxf () {
 		return boost::dynamic_pointer_cast<PictureMXF> (_content.object ());
diff --git a/src/types.h b/src/types.h
index 040e6197..98244891 100644
--- a/src/types.h
+++ b/src/types.h
@@ -127,6 +127,7 @@ struct EqualityOptions
 		, max_audio_sample_error (0)
 		, cpl_annotation_texts_can_differ (false)
 		, mxf_names_can_differ (false)
+		, reel_hashes_can_differ (false)
 	{}
 
 	/** The maximum allowable mean difference in pixel value between two images */
@@ -139,6 +140,8 @@ struct EqualityOptions
 	bool cpl_annotation_texts_can_differ;
 	/** true if MXF filenames are allowed to differ */
 	bool mxf_names_can_differ;
+	/** true if <Hash>es in Reels can differ */
+	bool reel_hashes_can_differ;
 };
 
 /* I've been unable to make mingw happy with ERROR as a symbol, so