From 595d4fbfee788edfad7f9f8dfe7e76ee634c1a94 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 22 Jul 2014 00:49:15 +0100 Subject: [PATCH] Various attempts to clean up DCP comparison code. --- src/asset.cc | 2 +- src/content.cc | 10 --------- src/content.h | 6 ----- src/cpl.cc | 7 +++--- src/dcp.cc | 26 +++++++++++++--------- src/mono_picture_mxf.cc | 5 +++++ src/mxf.cc | 7 +++--- src/reel.cc | 6 ++--- src/reel_asset.cc | 47 +++++++++++++++++++++++++++++++++++++++ src/reel_asset.h | 10 +-------- src/reel_picture_asset.cc | 26 ++++++++++++++++++++++ src/reel_picture_asset.h | 1 + src/types.h | 3 +++ tools/dcpdiff.cc | 11 +++------ 14 files changed, 113 insertions(+), 54 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 other, EqualityOptions, function 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 other, EqualityOptions opt, boost::function 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 other, - EqualityOptions opt, - boost::function - ) 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 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 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 > a = cpls (); + list > 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 >::const_iterator a = _assets.begin (); - list >::const_iterator b = other._assets.begin (); + bool r = true; + + for (list >::const_iterator i = a.begin(); i != a.end(); ++i) { + list >::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 other, EqualityOptions opt, boost::function note) const { + if (!dynamic_pointer_cast (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 other, EqualityOptions opt, boost::function shared_ptr other_mxf = dynamic_pointer_cast (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 other, EqualityOptions opt, boost::function 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 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 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 other, EqualityOptions opt, boost::function 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); virtual void write_to_cpl (xmlpp::Node* node, Standard standard) const; - - virtual bool equals ( - boost::shared_ptr, - EqualityOptions, - boost::function) - const { - - return false; - } + virtual bool equals (boost::shared_ptr, EqualityOptions, boost::function) const; /** @return a Ref to our actual content */ Ref& 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 other, EqualityOptions opt, boost::function note) const +{ + if (!ReelAsset::equals (other, opt, note)) { + return false; + } + + shared_ptr rpa = dynamic_pointer_cast (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); virtual void write_to_cpl (xmlpp::Node* node, Standard standard) const; + virtual bool equals (boost::shared_ptr, EqualityOptions, boost::function) const; boost::shared_ptr mxf () { return boost::dynamic_pointer_cast (_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 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 3f19ee24..bd6ab3d4 100644 --- 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); } -- 2.30.2