Use VerificationNote for non-fatal errors in DCP::read.

16 files changed, 105 insertions, 186 deletions

diff --git a/run/tests b/run/tests
index 5c294dab..48745f1d 100755
--- a/run/tests
+++ b/run/tests
@@ -91,7 +91,7 @@ fi
 rm -f $work/info.log
 for d in `find $private/metadata -mindepth 1 -maxdepth 1 -type d | sort -f -d`; do
     if [ `basename $d` != ".git" ]; then
-        $dcpinfo --ignore-missing-assets -k -s $d >> $work/info.log
+        $dcpinfo --ignore-missing-assets -s $d >> $work/info.log
         if [ "$?" != "0" ]; then
             echo "FAIL: dcpinfo failed for $d"
             exit 1
@@ -116,7 +116,7 @@ cp -r $private/* $work/private
 for d in `find $work/private/metadata -mindepth 1 -maxdepth 1 -type d | sort -f -d`; do
     if [ `basename $d` != ".git" ]; then
         $work/rewrite_subs $d
-        $dcpinfo --ignore-missing-assets -k -s $d >> $work/info2.log
+        $dcpinfo --ignore-missing-assets -s $d >> $work/info2.log
     fi
 done
 
diff --git a/src/dcp.cc b/src/dcp.cc
index adb33320..7f4540fa 100644
--- a/src/dcp.cc
+++ b/src/dcp.cc
@@ -58,6 +58,7 @@
 #include "font_asset.h"
 #include "pkl.h"
 #include "asset_factory.h"
+#include "verify.h"
 #include <asdcp/AS_DCP.h>
 #include <xmlsec/xmldsig.h>
 #include <xmlsec/app.h>
@@ -95,21 +96,19 @@ DCP::DCP (boost::filesystem::path directory)
 	_directory = boost::filesystem::canonical (_directory);
 }
 
-/** Call this instead of throwing an exception if the error can be tolerated */
-template<class T> void
-survivable_error (bool keep_going, dcp::DCP::ReadErrors* errors, T const & e)
-{
-	if (keep_going) {
-		if (errors) {
-			errors->push_back (shared_ptr<T> (new T (e)));
-		}
-	} else {
-		throw e;
-	}
-}
-
+/** Read a DCP.  This method does not do any deep checking of the DCP's validity, but
+ *  if it comes across any bad things it will do one of two things.
+ *
+ *  Errors that are so serious that they prevent the method from working will result
+ *  in an exception being thrown.  For example, a missing ASSETMAP means that the DCP
+ *  can't be read without a lot of guesswork, so this will throw.
+ *
+ *  Errors that are not fatal will be added to notes, if it's non-0.  For example,
+ *  if the DCP contains a mixture of Interop and SMPTE elements this will result
+ *  in a note being added to the list.
+ */
 void
-DCP::read (bool keep_going, ReadErrors* errors, bool ignore_incorrect_picture_mxf_type)
+DCP::read (list<dcp::VerificationNote>* notes, bool ignore_incorrect_picture_mxf_type)
 {
 	/* Read the ASSETMAP and PKL */
 
@@ -193,12 +192,16 @@ DCP::read (bool keep_going, ReadErrors* errors, bool ignore_incorrect_picture_mx
 			   been seen in the wild with a DCP that
 			   claims to come from ClipsterDCI 5.10.0.5.
 			*/
-			survivable_error (keep_going, errors, EmptyAssetPathError(i->first));
+			if (notes) {
+				notes->push_back (VerificationNote(VerificationNote::VERIFY_WARNING, VerificationNote::EMPTY_ASSET_PATH));
+			}
 			continue;
 		}
 
 		if (!boost::filesystem::exists(path)) {
-			survivable_error (keep_going, errors, MissingAssetError (path));
+			if (notes) {
+				notes->push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::MISSING_ASSET));
+			}
 			continue;
 		}
 
@@ -227,13 +230,13 @@ DCP::read (bool keep_going, ReadErrors* errors, bool ignore_incorrect_picture_mx
 
 			if (root == "CompositionPlaylist") {
 				shared_ptr<CPL> cpl (new CPL (path));
-				if (_standard && cpl->standard() && cpl->standard().get() != _standard.get()) {
-					survivable_error (keep_going, errors, MismatchedStandardError ());
+				if (_standard && cpl->standard() && cpl->standard().get() != _standard.get() && notes) {
+					notes->push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::MISMATCHED_STANDARD));
 				}
 				_cpls.push_back (cpl);
 			} else if (root == "DCSubtitle") {
 				if (_standard && _standard.get() == SMPTE) {
-					survivable_error (keep_going, errors, MismatchedStandardError ());
+					notes->push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::MISMATCHED_STANDARD));
 				}
 				other_assets.push_back (shared_ptr<InteropSubtitleAsset> (new InteropSubtitleAsset (path)));
 			}
diff --git a/src/dcp.h b/src/dcp.h
index b47daf55..34611fb7 100644
--- a/src/dcp.h
+++ b/src/dcp.h
@@ -43,6 +43,7 @@
 #include "certificate.h"
 #include "metadata.h"
 #include "name_format.h"
+#include "verify.h"
 #include <boost/shared_ptr.hpp>
 #include <boost/signals2.hpp>
 #include <string>
@@ -83,16 +84,13 @@ public:
 	 */
 	explicit DCP (boost::filesystem::path directory);
 
-	typedef std::list<boost::shared_ptr<DCPReadError> > ReadErrors;
-
 	/** Read the DCP's structure into this object.
-	 *  @param keep_going true to try to keep going in the face of (some) errors.
-	 *  @param errors List of errors that will be added to if keep_going is true.
+	 *  @param notes List of notes that will be added to if non-0.
 	 *  @param ignore_incorrect_picture_mxf_type true to try loading MXF files marked as monoscopic
 	 *  as stereoscopic if the monoscopic load fails; fixes problems some 3D DCPs that (I think)
 	 *  have an incorrect descriptor in their MXF.
 	 */
-	void read (bool keep_going = false, ReadErrors* errors = 0, bool ignore_incorrect_picture_mxf_type = false);
+	void read (std::list<VerificationNote>* notes = 0, bool ignore_incorrect_picture_mxf_type = false);
 
 	/** Compare this DCP with another, according to various options.
 	 *  @param other DCP to compare this one to.
diff --git a/src/exceptions.cc b/src/exceptions.cc
index 19422090..a49c7004 100644
--- a/src/exceptions.cc
+++ b/src/exceptions.cc
@@ -62,17 +62,6 @@ TimeFormatError::TimeFormatError (string bad_time)
 
 }
 
-MissingAssetError::MissingAssetError (boost::filesystem::path path, AssetType type)
-	: DCPReadError (
-		type == MAIN_PICTURE    ? String::compose ("Missing asset %1 for main picture", path.filename().string()) :
-		(type == MAIN_SOUND     ? String::compose ("Missing asset %1 for main sound", path.filename().string()) :
-		 (type == MAIN_SUBTITLE ? String::compose ("Missing asset %1 for main subtitle", path.filename().string()) :
-		  String::compose ("Missing asset %1", path.filename().string()))))
-	, _path (path)
-{
-
-}
-
 BadContentKindError::BadContentKindError (string content_kind)
 	: DCPReadError (String::compose("Bad content kind '%1'", content_kind))
 {
@@ -92,12 +81,6 @@ ProgrammingError::ProgrammingError (string file, int line)
 
 }
 
-MismatchedStandardError::MismatchedStandardError ()
-	: DCPReadError ("DCP contains both Interop and SMPTE parts")
-{
-
-}
-
 KDMDecryptionError::KDMDecryptionError (std::string message, int cipher_length, int modulus_dmax)
 	: runtime_error (String::compose ("Could not decrypt KDM (%1) (%2/%3)", message, cipher_length, modulus_dmax))
 {
@@ -130,15 +113,6 @@ MissingSubtitleImageError::MissingSubtitleImageError (string id)
 
 }
 
-/** The <Path> in the ASSETMAP is empty for asset.  I can't see how this is valid,
-    but it's been seen in the wild with a DCP that claims to come from ClipsterDCI 5.10.0.5.
- */
-EmptyAssetPathError::EmptyAssetPathError (string id)
-	: DCPReadError (String::compose("Asset map path is empty for asset %1", id))
-{
-
-}
-
 BadKDMDateError::BadKDMDateError (bool starts_too_early)
 	: runtime_error (
 		starts_too_early ?
diff --git a/src/exceptions.h b/src/exceptions.h
index 17b18eb2..ccf7a081 100644
--- a/src/exceptions.h
+++ b/src/exceptions.h
@@ -119,30 +119,6 @@ private:
 	boost::optional<std::string> _detail;
 };
 
-/** @class MissingAssetError
- *  @brief An error of a missing asset.
- */
-class MissingAssetError : public DCPReadError
-{
-public:
-	enum AssetType {
-		MAIN_PICTURE,  //< main picture is missing
-		MAIN_SOUND,    //< main sound is missing
-		MAIN_SUBTITLE, //< main subtitle is missing
-		UNKNOWN        //< something is missing but we don't know what
-	};
-
-	MissingAssetError (boost::filesystem::path, AssetType = UNKNOWN);
-	~MissingAssetError () throw () {}
-
-	boost::filesystem::path path () const {
-		return _path;
-	}
-
-private:
-	boost::filesystem::path _path;
-};
-
 class BadContentKindError : public DCPReadError
 {
 public:
@@ -198,12 +174,6 @@ public:
 	ProgrammingError (std::string file, int line);
 };
 
-class MismatchedStandardError : public DCPReadError
-{
-public:
-	MismatchedStandardError ();
-};
-
 class KDMDecryptionError : public std::runtime_error
 {
 public:
@@ -228,12 +198,6 @@ public:
 	MissingSubtitleImageError (std::string id);
 };
 
-class EmptyAssetPathError : public DCPReadError
-{
-public:
-	EmptyAssetPathError (std::string id);
-};
-
 class BadKDMDateError : public std::runtime_error
 {
 public:
diff --git a/src/verify.cc b/src/verify.cc
index 809e9e11..279de556 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -105,19 +105,14 @@ dcp::verify (vector<boost::filesystem::path> directories, function<void (string,
 
 	BOOST_FOREACH (shared_ptr<DCP> dcp, dcps) {
 		stage ("Checking DCP", dcp->directory());
-		DCP::ReadErrors errors;
 		try {
-			dcp->read (true, &errors);
+			dcp->read (&notes);
 		} catch (DCPReadError& e) {
 			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
 		} catch (XMLError& e) {
 			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
 		}
 
-		BOOST_FOREACH (shared_ptr<DCPReadError> i, errors) {
-			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(i->what())));
-		}
-
 		BOOST_FOREACH (shared_ptr<CPL> cpl, dcp->cpls()) {
 			stage ("Checking CPL", cpl->file());
 
@@ -188,3 +183,33 @@ dcp::verify (vector<boost::filesystem::path> directories, function<void (string,
 
 	return notes;
 }
+
+string
+dcp::note_to_string (dcp::VerificationNote note)
+{
+	switch (note.code()) {
+	case dcp::VerificationNote::GENERAL_READ:
+		return *note.note();
+	case dcp::VerificationNote::CPL_HASH_INCORRECT:
+		return "The hash of the CPL in the PKL does not agree with the CPL file";
+	case dcp::VerificationNote::INVALID_PICTURE_FRAME_RATE:
+		return "The picture in a reel has an invalid frame rate";
+	case dcp::VerificationNote::PICTURE_HASH_INCORRECT:
+		return dcp::String::compose("The hash of the picture asset %1 does not agree with the PKL file", note.file()->filename());
+	case dcp::VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE:
+		return "The PKL and CPL hashes disagree for a picture asset.";
+	case dcp::VerificationNote::SOUND_HASH_INCORRECT:
+		return dcp::String::compose("The hash of the sound asset %1 does not agree with the PKL file", note.file()->filename());
+	case dcp::VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE:
+		return "The PKL and CPL hashes disagree for a sound asset.";
+	case dcp::VerificationNote::EMPTY_ASSET_PATH:
+		return "The asset map contains an empty asset path.";
+	case dcp::VerificationNote::MISSING_ASSET:
+		return "The file for an asset in the asset map cannot be found.";
+	case dcp::VerificationNote::MISMATCHED_STANDARD:
+		return "The DCP contains both SMPTE and Interop parts.";
+	}
+
+	return "";
+}
+
diff --git a/src/verify.h b/src/verify.h
index 4b967d1d..e219c6ec 100644
--- a/src/verify.h
+++ b/src/verify.h
@@ -69,6 +69,12 @@ public:
 		SOUND_HASH_INCORRECT,
 		/** The hash of a main sound is different in the CPL and PKL */
 		PKL_CPL_SOUND_HASHES_DISAGREE,
+		/** An assetmap's <Path> entry is empty */
+		EMPTY_ASSET_PATH,
+		/** An file mentioned in an assetmap cannot be found */
+		MISSING_ASSET,
+		/** The DCP contains both SMPTE and Interop-standard components */
+		MISMATCHED_STANDARD,
 	};
 
 	VerificationNote (Type type, Code code)
@@ -117,6 +123,8 @@ std::list<VerificationNote> verify (
 	boost::function<void (float)> progress
 	);
 
+std::string note_to_string (dcp::VerificationNote note);
+
 }
 
 #endif
diff --git a/test/exception_test.cc b/test/exception_test.cc
index 2ae6cdca..43b1d021 100644
--- a/test/exception_test.cc
+++ b/test/exception_test.cc
@@ -43,7 +43,4 @@ BOOST_AUTO_TEST_CASE (exception_test)
 	BOOST_CHECK_EQUAL (string (dcp::UnresolvedRefError ("foo").what()), "Unresolved reference to asset id foo");
 	BOOST_CHECK_EQUAL (string (dcp::NotEncryptedError ("foo").what()), "foo is not encrypted");
 	BOOST_CHECK_EQUAL (string (dcp::ProgrammingError ("foo", 42).what()), "Programming error at foo:42");
-	BOOST_CHECK_EQUAL (string (dcp::MissingAssetError ("foo", dcp::MissingAssetError::MAIN_PICTURE).what()), "Missing asset foo for main picture");
-	BOOST_CHECK_EQUAL (string (dcp::MissingAssetError ("foo", dcp::MissingAssetError::MAIN_SOUND).what()), "Missing asset foo for main sound");
-	BOOST_CHECK_EQUAL (string (dcp::MissingAssetError ("foo", dcp::MissingAssetError::MAIN_SUBTITLE).what()), "Missing asset foo for main subtitle");
 }
diff --git a/test/rewrite_subs.cc b/test/rewrite_subs.cc
index fe21f651..d8ec4eea 100644
--- a/test/rewrite_subs.cc
+++ b/test/rewrite_subs.cc
@@ -57,7 +57,7 @@ main (int argc, char* argv[])
 		}
 
 		DCP* dcp = new DCP (argv[1]);
-		dcp->read (true);
+		dcp->read ();
 
 		list<shared_ptr<CPL> > cpls = dcp->cpls ();
 		for (list<boost::shared_ptr<CPL> >::iterator i = cpls.begin(); i != cpls.end(); ++i) {
diff --git a/test/verify_test.cc b/test/verify_test.cc
index 98f37635..9b028729 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -233,9 +233,7 @@ BOOST_AUTO_TEST_CASE (verify_test6)
 
 	BOOST_REQUIRE_EQUAL (notes.size(), 1);
 	BOOST_CHECK_EQUAL (notes.front().type(), dcp::VerificationNote::VERIFY_ERROR);
-	BOOST_CHECK_EQUAL (notes.front().code(), dcp::VerificationNote::GENERAL_READ);
-	BOOST_REQUIRE (static_cast<bool>(notes.front().note()));
-	BOOST_REQUIRE_EQUAL (notes.front().note().get(), "Missing asset video.mxf");
+	BOOST_CHECK_EQUAL (notes.front().code(), dcp::VerificationNote::Code::MISSING_ASSET);
 }
 
 /* Empty asset filename in ASSETMAP */
@@ -253,9 +251,7 @@ BOOST_AUTO_TEST_CASE (verify_test7)
 	list<dcp::VerificationNote> notes = dcp::verify (directories, &stage, &progress);
 
 	BOOST_REQUIRE_EQUAL (notes.size(), 1);
-	BOOST_CHECK_EQUAL (notes.front().type(), dcp::VerificationNote::VERIFY_ERROR);
-	BOOST_CHECK_EQUAL (notes.front().code(), dcp::VerificationNote::GENERAL_READ);
-	BOOST_REQUIRE (static_cast<bool>(notes.front().note()));
-	BOOST_REQUIRE_EQUAL (notes.front().note().get(), "Asset map path is empty for asset 1fab8bb0-cfaf-4225-ad6d-01768bc10470");
+	BOOST_CHECK_EQUAL (notes.front().type(), dcp::VerificationNote::VERIFY_WARNING);
+	BOOST_CHECK_EQUAL (notes.front().code(), dcp::VerificationNote::Code::EMPTY_ASSET_PATH);
 }
 
diff --git a/tools/common.cc b/tools/common.cc
index cda6817b..7ae3dae0 100644
--- a/tools/common.cc
+++ b/tools/common.cc
@@ -39,15 +39,15 @@ using boost::shared_ptr;
 using boost::dynamic_pointer_cast;
 
 void
-dcp::filter_errors (dcp::DCP::ReadErrors& errors, bool ignore_missing_assets)
+dcp::filter_notes (list<dcp::VerificationNote>& notes, bool ignore_missing_assets)
 {
-	for (DCP::ReadErrors::iterator i = errors.begin(); i != errors.end(); ) {
+	for (list<dcp::VerificationNote>::iterator i = notes.begin(); i != notes.end(); ) {
 
-		DCP::ReadErrors::iterator tmp = i;
+		list<dcp::VerificationNote>::iterator tmp = i;
 		++tmp;
 
-		if (ignore_missing_assets && dynamic_pointer_cast<MissingAssetError> (*i)) {
-			errors.erase (i);
+		if (ignore_missing_assets && i->code() == dcp::VerificationNote::Code::MISSING_ASSET) {
+			notes.erase (i);
 		}
 
 		i = tmp;
diff --git a/tools/common.h b/tools/common.h
index cea183db..0ef0b081 100644
--- a/tools/common.h
+++ b/tools/common.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2014 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2014-2019 Carl Hetherington <cth@carlh.net>
 
     This file is part of libdcp.
 
@@ -17,10 +17,10 @@
     along with libdcp.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-#include "exceptions.h"
+#include "verify.h"
 
 namespace dcp {
 
-extern void filter_errors (std::list<boost::shared_ptr<DCPReadError> >& errors, bool ignore_missing_assets);
+extern void filter_notes (std::list<dcp::VerificationNote>& notes, bool ignore_missing_assets);
 
 }
diff --git a/tools/dcpdiff.cc b/tools/dcpdiff.cc
index 5e0560a0..976a2919 100644
--- a/tools/dcpdiff.cc
+++ b/tools/dcpdiff.cc
@@ -39,6 +39,7 @@
 #include <boost/optional.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/filesystem.hpp>
+#include <boost/foreach.hpp>
 #include <iostream>
 #include <list>
 
@@ -67,7 +68,6 @@ help (string 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"
 	     << "      --key                    hexadecimal key to use to decrypt MXFs\n"
-	     << "  -k, --keep-going             carry on in the event of errors, if possible\n"
 	     << "      --ignore-missing-assets  ignore missing asset files\n"
 	     << "\n"
 	     << "The <DCP>s are the DCP directories to compare.\n"
@@ -84,17 +84,18 @@ note (NoteType t, string n)
 	}
 }
 
+static
 DCP *
-load_dcp (boost::filesystem::path path, bool keep_going, bool ignore_missing_assets, optional<string> key)
+load_dcp (boost::filesystem::path path, bool ignore_missing_assets, optional<string> key)
 {
 	DCP* dcp = 0;
 	try {
 		dcp = new DCP (path);
-		DCP::ReadErrors errors;
-		dcp->read (keep_going, &errors);
-		filter_errors (errors, ignore_missing_assets);
-		for (DCP::ReadErrors::const_iterator i = errors.begin(); i != errors.end(); ++i) {
-			cerr << (*i)->what() << "\n";
+		list<dcp::VerificationNote> notes;
+		dcp->read (&notes);
+		filter_notes (notes, ignore_missing_assets);
+		BOOST_FOREACH (dcp::VerificationNote i, notes) {
+			cerr << dcp::note_to_string(i) << "\n";
 		}
 
 		if (key) {
@@ -123,7 +124,6 @@ main (int argc, char* argv[])
 	options.max_std_dev_pixel_error = 5;
 	options.reel_hashes_can_differ = true;
 	options.reel_annotation_texts_can_differ = false;
-	options.keep_going = false;
 	bool ignore_missing_assets = false;
 	optional<string> key;
 
@@ -135,7 +135,6 @@ main (int argc, char* argv[])
 			{ "verbose", no_argument, 0, 'v'},
 			{ "mean-pixel", required_argument, 0, 'm'},
 			{ "std-dev-pixel", required_argument, 0, 's'},
-			{ "keep-going", no_argument, 0, 'k'},
 			{ "annotation-texts", no_argument, 0, 'a'},
 			{ "issue-dates", no_argument, 0, 'd'},
 			/* From here we're using random capital letters for the short option */
@@ -146,7 +145,7 @@ main (int argc, char* argv[])
 			{ 0, 0, 0, 0 }
 		};
 
-		int c = getopt_long (argc, argv, "Vhvm:s:kadACD:E", long_options, &option_index);
+		int c = getopt_long (argc, argv, "Vhvm:s:adACD:E", long_options, &option_index);
 
 		if (c == -1) {
 			break;
@@ -168,9 +167,6 @@ main (int argc, char* argv[])
 		case 's':
 			options.max_std_dev_pixel_error = atof (optarg);
 			break;
-		case 'k':
-			options.keep_going = true;
-			break;
 		case 'a':
 			options.cpl_annotation_texts_can_differ = options.reel_annotation_texts_can_differ = true;
 			break;
@@ -208,8 +204,8 @@ main (int argc, char* argv[])
 		exit (EXIT_FAILURE);
 	}
 
-	DCP* a = load_dcp (argv[optind], options.keep_going, ignore_missing_assets, key);
-	DCP* b = load_dcp (argv[optind + 1], options.keep_going, ignore_missing_assets, key);
+	DCP* a = load_dcp (argv[optind], ignore_missing_assets, key);
+	DCP* b = load_dcp (argv[optind + 1], ignore_missing_assets, key);
 
 	/* I think this is just below the LSB at 16-bits (ie the 8th most significant bit at 24-bit) */
 	options.max_audio_sample_error = 255;
diff --git a/tools/dcpinfo.cc b/tools/dcpinfo.cc
index d06d2068..7010bbab 100644
--- a/tools/dcpinfo.cc
+++ b/tools/dcpinfo.cc
@@ -76,7 +76,6 @@ help (string n)
 	     << "  -s, --subtitles              list all subtitles\n"
 	     << "  -p, --picture                analyse picture\n"
 	     << "  -d, --decompress             decompress picture when analysing (this is slow)\n"
-	     << "  -k, --keep-going             carry on in the event of errors, if possible\n"
 	     << "      --kdm                    KDM to decrypt DCP\n"
 	     << "      --private-key            private key for the certificate that the KDM is targeted at\n"
 	     << "      --ignore-missing-assets  ignore missing asset files\n";
@@ -217,7 +216,6 @@ int
 main (int argc, char* argv[])
 {
 	bool subtitles = false;
-	bool keep_going = false;
 	bool picture = false;
 	bool decompress = false;
 	bool ignore_missing_assets = false;
@@ -230,7 +228,6 @@ main (int argc, char* argv[])
 			{ "version", no_argument, 0, 'v' },
 			{ "help", no_argument, 0, 'h' },
 			{ "subtitles", no_argument, 0, 's' },
-			{ "keep-going", no_argument, 0, 'k' },
 			{ "picture", no_argument, 0, 'p' },
 			{ "decompress", no_argument, 0, 'd' },
 			{ "ignore-missing-assets", no_argument, 0, 'A' },
@@ -239,7 +236,7 @@ main (int argc, char* argv[])
 			{ 0, 0, 0, 0 }
 		};
 
-		int c = getopt_long (argc, argv, "vhskpdAB:C:", long_options, &option_index);
+		int c = getopt_long (argc, argv, "vhspdAB:C:", long_options, &option_index);
 
 		if (c == -1) {
 			break;
@@ -255,9 +252,6 @@ main (int argc, char* argv[])
 		case 's':
 			subtitles = true;
 			break;
-		case 'k':
-			keep_going = true;
-			break;
 		case 'p':
 			picture = true;
 			break;
@@ -289,10 +283,10 @@ main (int argc, char* argv[])
 	list<shared_ptr<CPL> > cpls;
 	if (boost::filesystem::is_directory(argv[optind])) {
 		DCP* dcp = 0;
-		DCP::ReadErrors errors;
+		list<dcp::VerificationNote> notes;
 		try {
 			dcp = new DCP (argv[optind]);
-			dcp->read (keep_going, &errors);
+			dcp->read (&notes);
 			if (kdm && private_key) {
 				dcp->add(DecryptedKDM(EncryptedKDM(file_to_string(*kdm)), file_to_string(*private_key)));
 			}
@@ -309,15 +303,14 @@ main (int argc, char* argv[])
 
 		cout << "DCP: " << boost::filesystem::path(argv[optind]).string() << "\n";
 
-		dcp::filter_errors (errors, ignore_missing_assets);
-		for (DCP::ReadErrors::const_iterator i = errors.begin(); i != errors.end(); ++i) {
-			cerr << "Error: " << (*i)->what() << "\n";
+		dcp::filter_notes (notes, ignore_missing_assets);
+		BOOST_FOREACH (dcp::VerificationNote i, notes) {
+			cerr << "Error: " << note_to_string(i) << "\n";
 		}
 
 		cpls = dcp->cpls ();
 	} else {
 		cpls.push_back (shared_ptr<CPL>(new CPL(boost::filesystem::path(argv[optind]))));
-		keep_going = true;
 		ignore_missing_assets = true;
 	}
 
@@ -331,36 +324,24 @@ main (int argc, char* argv[])
 			try {
 				main_picture (j, picture, decompress);
 			} catch (UnresolvedRefError& e) {
-				if (keep_going) {
-					if (!ignore_missing_assets) {
-						cerr << e.what() << " (for main picture)\n";
-					}
-				} else {
-					throw;
+				if (!ignore_missing_assets) {
+					cerr << e.what() << " (for main picture)\n";
 				}
 			}
 
 			try {
 				main_sound (j);
 			} catch (UnresolvedRefError& e) {
-				if (keep_going) {
-					if (!ignore_missing_assets) {
-						cerr << e.what() << " (for main sound)\n";
-					}
-				} else {
-					throw;
+				if (!ignore_missing_assets) {
+					cerr << e.what() << " (for main sound)\n";
 				}
 			}
 
 			try {
 				main_subtitle (j, subtitles);
 			} catch (UnresolvedRefError& e) {
-				if (keep_going) {
-					if (!ignore_missing_assets) {
-						cerr << e.what() << " (for main subtitle)\n";
-					}
-				} else {
-					throw;
+				if (!ignore_missing_assets) {
+					cerr << e.what() << " (for main subtitle)\n";
 				}
 			}
 
diff --git a/tools/dcprecover.cc b/tools/dcprecover.cc
index b747a5da..eb3b0d82 100644
--- a/tools/dcprecover.cc
+++ b/tools/dcprecover.cc
@@ -100,15 +100,15 @@ main (int argc, char* argv[])
 	/* Try to read it and report errors */
 
 	dcp::DCP dcp (dcp_dir);
-	dcp::DCP::ReadErrors errors;
+	list<dcp::VerificationNote> notes;
 	try {
-		dcp.read (true, &errors, true);
+		dcp.read (&notes, true);
 	} catch (dcp::DCPReadError& e) {
 		cout << "Error:" <<  e.what() << "\n";
 	}
 
-	BOOST_FOREACH (shared_ptr<dcp::DCPReadError> i, errors) {
-		cout << "Error: " << i->what() << "\n";
+	BOOST_FOREACH (dcp::VerificationNote i, notes) {
+		cout << "Error: " << dcp::note_to_string(i) << "\n";
 	}
 
 	/* Look for a CPL */
diff --git a/tools/dcpverify.cc b/tools/dcpverify.cc
index 675b7522..0c69ba78 100644
--- a/tools/dcpverify.cc
+++ b/tools/dcpverify.cc
@@ -73,29 +73,6 @@ progress ()
 
 }
 
-std::string
-note_to_string (dcp::VerificationNote note)
-{
-	switch (note.code()) {
-	case dcp::VerificationNote::GENERAL_READ:
-		return *note.note();
-	case dcp::VerificationNote::CPL_HASH_INCORRECT:
-		return "The hash of the CPL in the PKL does not agree with the CPL file";
-	case dcp::VerificationNote::INVALID_PICTURE_FRAME_RATE:
-		return "The picture in a reel has an invalid frame rate";
-	case dcp::VerificationNote::PICTURE_HASH_INCORRECT:
-		return dcp::String::compose("The hash of the picture asset %1 does not agree with the PKL file", note.file()->filename());
-	case dcp::VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE:
-		return "The PKL and CPL hashes disagree for a picture asset.";
-	case dcp::VerificationNote::SOUND_HASH_INCORRECT:
-		return dcp::String::compose("The hash of the sound asset %1 does not agree with the PKL file", note.file()->filename());
-	case dcp::VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE:
-		return "The PKL and CPL hashes disagree for a sound asset.";
-	}
-
-	return "";
-}
-
 int
 main (int argc, char* argv[])
 {