Various attempts to clean up DCP comparison code.

diff --git a/src/asset.cc b/src/asset.cc
index e55dca2dab05999c58d9c65d93efe330f9c1acf0..96196b41e3a999a284ca9db7803bd93471c5ce32 100644 (file)
--- 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 265778bff5461ee5b4ca57d76055781d2c8d5c76..3842ec09a230a2675e8ed4f1247d8fc1a4e70e8f 100644 (file)
--- 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 27939e8318f4ad3a6be53c0db39b7478f9cc754d..fcbfc25dc96c3f8b90ea7fbdf1f59467a4dbc2f4 100644 (file)
--- 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 9c8c865bcf1cdd5d15bbcc11a4a328db9410efb9..3599231daad1420b1acb201568667f6237514623 100644 (file)
--- 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 c92a14b883b1e2e6e36a37029cc75346d583b878..ca1c23d9a9df0fdaf7e85a75769c380781f4cf05 100644 (file)
--- 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 b2042fa5d0d2c5d58bec3262299a8dba1c59909a..66cfa12cba3a402b30fde50f800f393873bddc97 100644 (file)
--- 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 f4d4a49090da6e6558ba0e60e29a195f86d3f131..6709af4c57a8625f97f1f3d5c15b3fc6669b9eeb 100644 (file)
--- 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 5878fc2ba7ffc5db484e2c74061c05f14e8b52e2..0071de868497a121aa79361d3a5c210dd7101e79 100644 (file)
--- 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 0c7c6e517a8fd6c1813b115438669778ef7a3ace..978f1dee9de4609eab104eeddf33203bd45af6db 100644 (file)
--- 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 84d982210dfb1c096a03c161ce516af1dc39ea0c..61d2b48f0c5c4ec0e397eaac3e0bd16dc80cee3a 100644 (file)
--- 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 4f737a910205d9452cb9323bfb7fb00725f64ade..1fbd635b78e825b096f03ff9388d460c4f7c5f1d 100644 (file)
--- 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 7ab92228f369eadd209d9f5be5b62342c697aa24..76ba22c9e458fbfad9c2361d673908251cf2a36e 100644 (file)
--- 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 040e6197f01f2355040a321ce90f48e8fbd81e19..98244891192b0ddb499ed12ee4235871de5614d8 100644 (file)
--- 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

diff --git a/tools/dcpdiff.cc b/tools/dcpdiff.cc
index 3f19ee24061e9e9f5e40f9d4278fc0b8003e61a6..bd6ab3d4b014afca6ba91bbf4552bab5394ed665 100644 (file)
--- a/tools/dcpdiff.cc
+++ b/tools/dcpdiff.cc
@@ -38,7 +38,6 @@ help (string n)
             << "  -h, --help                   show this help\n"
             << "  -v, --verbose                be verbose\n"
             << "  -n, --names                  allow differing MXF names\n"
-            << "      --cpl-names              allow differing CPL annotation texts (backwards compatible)\n"
             << "      --cpl-annotation-texts   allow differing CPL annotation texts\n"
             << "  -m, --mean-pixel             maximum allowed mean pixel error (default 5)\n"
             << "  -s, --std-dev-pixel          maximum allowed standard deviation of pixel error (default 5)\n"
@@ -84,6 +83,7 @@ main (int argc, char* argv[])
        EqualityOptions options;
        options.max_mean_pixel_error = 5;
        options.max_std_dev_pixel_error = 5;
+       options.reel_hashes_can_differ = true;
        bool keep_going = false;
        bool ignore_missing_assets = false;
        
@@ -99,12 +99,11 @@ main (int argc, char* argv[])
                        { "keep-going", no_argument, 0, 'k'},
                        /* From here we're using random capital letters for the short option */
                        { "ignore-missing-assets", no_argument, 0, 'A'},
-                       { "cpl-names", no_argument, 0, 'B'},
                        { "cpl-annotation-texts", no_argument, 0, 'C'},
                        { 0, 0, 0, 0 }
                };
 
-               int c = getopt_long (argc, argv, "Vhvnm:s:kAB", long_options, &option_index);
+               int c = getopt_long (argc, argv, "Vhvnm:s:kA", long_options, &option_index);
 
                if (c == -1) {
                        break;
@@ -165,9 +164,5 @@ main (int argc, char* argv[])
 
        bool const equals = a->equals (*b, options, boost::bind (note, _1, _2));
 
-       if (equals) {
-               exit (EXIT_SUCCESS);
-       }
-
-       exit (EXIT_FAILURE);
+       exit (equals ? EXIT_SUCCESS : EXIT_FAILURE);
 }