X-Git-Url: https://main.carlh.net/gitweb/?a=blobdiff_plain;f=src%2Fverify.cc;h=0082cfcdbd8a22191194af58ce12f2a3a175eb5e;hb=14d1f9d76f289a6447a58f03813c771c86c7d8af;hp=8f04f799b712e6397270749e3d0a7378a4009375;hpb=50ec5a50ac9a1701607eee0796201a793f00fddf;p=libdcp.git

diff --git a/src/verify.cc b/src/verify.cc
index 8f04f799..0082cfcd 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -31,6 +31,12 @@
     files in the program, then also delete it here.
 */
 
+
+/** @file  src/verify.cc
+ *  @brief dcp::verify() method and associated code
+ */
+
+
 #include "verify.h"
 #include "dcp.h"
 #include "cpl.h"
@@ -47,6 +53,7 @@
 #include "exceptions.h"
 #include "compose.hpp"
 #include "raw_convert.h"
+#include "reel_markers_asset.h"
 #include "smpte_subtitle_asset.h"
 #include <xercesc/util/PlatformUtils.hpp>
 #include <xercesc/parsers/XercesDOMParser.hpp>
@@ -66,27 +73,30 @@
 #include <xercesc/dom/DOMErrorHandler.hpp>
 #include <xercesc/framework/LocalFileInputSource.hpp>
 #include <xercesc/framework/MemBufInputSource.hpp>
-#include <boost/noncopyable.hpp>
 #include <boost/algorithm/string.hpp>
 #include <map>
-#include <list>
 #include <vector>
 #include <iostream>
 
+
 using std::list;
 using std::vector;
 using std::string;
 using std::cout;
 using std::map;
 using std::max;
+using std::set;
 using std::shared_ptr;
+using std::make_shared;
 using boost::optional;
 using boost::function;
 using std::dynamic_pointer_cast;
 
+
 using namespace dcp;
 using namespace xercesc;
 
+
 static
 string
 xml_ch_to_string (XMLCh const * a)
@@ -97,6 +107,7 @@ xml_ch_to_string (XMLCh const * a)
 	return o;
 }
 
+
 class XMLValidationError
 {
 public:
@@ -182,7 +193,8 @@ private:
 	list<XMLValidationError> _errors;
 };
 
-class StringToXMLCh : public boost::noncopyable
+
+class StringToXMLCh
 {
 public:
 	StringToXMLCh (string a)
@@ -190,6 +202,9 @@ public:
 		_buffer = XMLString::transcode(a.c_str());
 	}
 
+	StringToXMLCh (StringToXMLCh const&) = delete;
+	StringToXMLCh& operator= (StringToXMLCh const&) = delete;
+
 	~StringToXMLCh ()
 	{
 		XMLString::release (&_buffer);
@@ -203,6 +218,7 @@ private:
 	XMLCh* _buffer;
 };
 
+
 class LocalFileResolver : public EntityResolver
 {
 public:
@@ -262,7 +278,7 @@ parse (XercesDOMParser& parser, boost::filesystem::path xml)
 
 
 static void
-parse (XercesDOMParser& parser, std::string xml)
+parse (XercesDOMParser& parser, string xml)
 {
 	xercesc::MemBufInputSource buf(reinterpret_cast<unsigned char const*>(xml.c_str()), xml.size(), "");
 	parser.parse(buf);
@@ -271,7 +287,7 @@ parse (XercesDOMParser& parser, std::string xml)
 
 template <class T>
 void
-validate_xml (T xml, boost::filesystem::path xsd_dtd_directory, list<VerificationNote>& notes)
+validate_xml (T xml, boost::filesystem::path xsd_dtd_directory, vector<VerificationNote>& notes)
 {
 	try {
 		XMLPlatformUtils::Initialize ();
@@ -341,40 +357,38 @@ validate_xml (T xml, boost::filesystem::path xsd_dtd_directory, list<Verificatio
 	XMLPlatformUtils::Terminate ();
 
 	for (auto i: error_handler.errors()) {
-		notes.push_back (
-			VerificationNote(
-				VerificationNote::VERIFY_ERROR,
-				VerificationNote::XML_VALIDATION_ERROR,
-				i.message(),
-				boost::trim_copy(i.public_id() + " " + i.system_id()),
-				i.line()
-				)
-			);
+		notes.push_back ({
+			VerificationNote::Type::ERROR,
+			VerificationNote::Code::INVALID_XML,
+			i.message(),
+			boost::trim_copy(i.public_id() + " " + i.system_id()),
+			i.line()
+		});
 	}
 }
 
 
-enum VerifyAssetResult {
-	VERIFY_ASSET_RESULT_GOOD,
-	VERIFY_ASSET_RESULT_CPL_PKL_DIFFER,
-	VERIFY_ASSET_RESULT_BAD
+enum class VerifyAssetResult {
+	GOOD,
+	CPL_PKL_DIFFER,
+	BAD
 };
 
 
 static VerifyAssetResult
-verify_asset (shared_ptr<const DCP> dcp, shared_ptr<const ReelMXF> reel_mxf, function<void (float)> progress)
+verify_asset (shared_ptr<const DCP> dcp, shared_ptr<const ReelFileAsset> reel_file_asset, function<void (float)> progress)
 {
-	auto const actual_hash = reel_mxf->asset_ref()->hash(progress);
+	auto const actual_hash = reel_file_asset->asset_ref()->hash(progress);
 
 	auto pkls = dcp->pkls();
 	/* We've read this DCP in so it must have at least one PKL */
 	DCP_ASSERT (!pkls.empty());
 
-	auto asset = reel_mxf->asset_ref().asset();
+	auto asset = reel_file_asset->asset_ref().asset();
 
 	optional<string> pkl_hash;
 	for (auto i: pkls) {
-		pkl_hash = i->hash (reel_mxf->asset_ref()->id());
+		pkl_hash = i->hash (reel_file_asset->asset_ref()->id());
 		if (pkl_hash) {
 			break;
 		}
@@ -382,35 +396,35 @@ verify_asset (shared_ptr<const DCP> dcp, shared_ptr<const ReelMXF> reel_mxf, fun
 
 	DCP_ASSERT (pkl_hash);
 
-	auto cpl_hash = reel_mxf->hash();
+	auto cpl_hash = reel_file_asset->hash();
 	if (cpl_hash && *cpl_hash != *pkl_hash) {
-		return VERIFY_ASSET_RESULT_CPL_PKL_DIFFER;
+		return VerifyAssetResult::CPL_PKL_DIFFER;
 	}
 
 	if (actual_hash != *pkl_hash) {
-		return VERIFY_ASSET_RESULT_BAD;
+		return VerifyAssetResult::BAD;
 	}
 
-	return VERIFY_ASSET_RESULT_GOOD;
+	return VerifyAssetResult::GOOD;
 }
 
 
 void
-verify_language_tag (string tag, list<VerificationNote>& notes)
+verify_language_tag (string tag, vector<VerificationNote>& notes)
 {
 	try {
-		dcp::LanguageTag test (tag);
-	} catch (dcp::LanguageTagError &) {
-		notes.push_back (VerificationNote(VerificationNote::VERIFY_BV21_ERROR, VerificationNote::BAD_LANGUAGE, tag));
+		LanguageTag test (tag);
+	} catch (LanguageTagError &) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_LANGUAGE, tag});
 	}
 }
 
 
-enum VerifyPictureAssetResult
+enum class VerifyPictureAssetResult
 {
-	VERIFY_PICTURE_ASSET_RESULT_GOOD,
-	VERIFY_PICTURE_ASSET_RESULT_FRAME_NEARLY_TOO_LARGE,
-	VERIFY_PICTURE_ASSET_RESULT_BAD,
+	GOOD,
+	FRAME_NEARLY_TOO_LARGE,
+	BAD,
 };
 
 
@@ -429,9 +443,9 @@ biggest_frame_size (shared_ptr<const StereoPictureFrame> frame)
 
 template <class A, class R, class F>
 optional<VerifyPictureAssetResult>
-verify_picture_asset_type (shared_ptr<const ReelMXF> reel_mxf, function<void (float)> progress)
+verify_picture_asset_type (shared_ptr<const ReelFileAsset> reel_file_asset, function<void (float)> progress)
 {
-	auto asset = dynamic_pointer_cast<A>(reel_mxf->asset_ref().asset());
+	auto asset = dynamic_pointer_cast<A>(reel_file_asset->asset_ref().asset());
 	if (!asset) {
 		return optional<VerifyPictureAssetResult>();
 	}
@@ -448,21 +462,21 @@ verify_picture_asset_type (shared_ptr<const ReelMXF> reel_mxf, function<void (fl
 	static const int max_frame =   rint(250 * 1000000 / (8 * asset->edit_rate().as_float()));
 	static const int risky_frame = rint(230 * 1000000 / (8 * asset->edit_rate().as_float()));
 	if (biggest_frame > max_frame) {
-		return VERIFY_PICTURE_ASSET_RESULT_BAD;
+		return VerifyPictureAssetResult::BAD;
 	} else if (biggest_frame > risky_frame) {
-		return VERIFY_PICTURE_ASSET_RESULT_FRAME_NEARLY_TOO_LARGE;
+		return VerifyPictureAssetResult::FRAME_NEARLY_TOO_LARGE;
 	}
 
-	return VERIFY_PICTURE_ASSET_RESULT_GOOD;
+	return VerifyPictureAssetResult::GOOD;
 }
 
 
 static VerifyPictureAssetResult
-verify_picture_asset (shared_ptr<const ReelMXF> reel_mxf, function<void (float)> progress)
+verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, function<void (float)> progress)
 {
-	auto r = verify_picture_asset_type<MonoPictureAsset, MonoPictureAssetReader, MonoPictureFrame>(reel_mxf, progress);
+	auto r = verify_picture_asset_type<MonoPictureAsset, MonoPictureAssetReader, MonoPictureFrame>(reel_file_asset, progress);
 	if (!r) {
-		r = verify_picture_asset_type<StereoPictureAsset, StereoPictureAssetReader, StereoPictureFrame>(reel_mxf, progress);
+		r = verify_picture_asset_type<StereoPictureAsset, StereoPictureAssetReader, StereoPictureFrame>(reel_file_asset, progress);
 	}
 
 	DCP_ASSERT (r);
@@ -476,7 +490,7 @@ verify_main_picture_asset (
 	shared_ptr<const ReelPictureAsset> reel_asset,
 	function<void (string, optional<boost::filesystem::path>)> stage,
 	function<void (float)> progress,
-	list<VerificationNote>& notes
+	vector<VerificationNote>& notes
 	)
 {
 	auto asset = reel_asset->asset();
@@ -484,19 +498,15 @@ verify_main_picture_asset (
 	stage ("Checking picture asset hash", file);
 	auto const r = verify_asset (dcp, reel_asset, progress);
 	switch (r) {
-		case VERIFY_ASSET_RESULT_BAD:
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_ERROR, VerificationNote::PICTURE_HASH_INCORRECT, file
-					)
-				);
+		case VerifyAssetResult::BAD:
+			notes.push_back ({
+				VerificationNote::Type::ERROR, VerificationNote::Code::INCORRECT_PICTURE_HASH, file
+			});
 			break;
-		case VERIFY_ASSET_RESULT_CPL_PKL_DIFFER:
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_PICTURE_HASHES_DIFFER, file
-					)
-				);
+		case VerifyAssetResult::CPL_PKL_DIFFER:
+			notes.push_back ({
+				VerificationNote::Type::ERROR, VerificationNote::Code::MISMATCHED_PICTURE_HASHES, file
+			});
 			break;
 		default:
 			break;
@@ -504,19 +514,15 @@ verify_main_picture_asset (
 	stage ("Checking picture frame sizes", asset->file());
 	auto const pr = verify_picture_asset (reel_asset, progress);
 	switch (pr) {
-		case VERIFY_PICTURE_ASSET_RESULT_BAD:
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_ERROR, VerificationNote::PICTURE_FRAME_TOO_LARGE_IN_BYTES, file
-					)
-				);
+		case VerifyPictureAssetResult::BAD:
+			notes.push_back ({
+				VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+			});
 			break;
-		case VERIFY_PICTURE_ASSET_RESULT_FRAME_NEARLY_TOO_LARGE:
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_WARNING, VerificationNote::PICTURE_FRAME_NEARLY_TOO_LARGE_IN_BYTES, file
-					)
-				);
+		case VerifyPictureAssetResult::FRAME_NEARLY_TOO_LARGE:
+			notes.push_back ({
+				VerificationNote::Type::WARNING, VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+			});
 			break;
 		default:
 			break;
@@ -524,57 +530,50 @@ verify_main_picture_asset (
 
 	/* Only flat/scope allowed by Bv2.1 */
 	if (
-		asset->size() != dcp::Size(2048, 858) &&
-		asset->size() != dcp::Size(1998, 1080) &&
-		asset->size() != dcp::Size(4096, 1716) &&
-		asset->size() != dcp::Size(3996, 2160)) {
-		notes.push_back(
-			VerificationNote(
-				VerificationNote::VERIFY_BV21_ERROR,
-				VerificationNote::PICTURE_ASSET_INVALID_SIZE_IN_PIXELS,
-				String::compose("%1x%2", asset->size().width, asset->size().height),
-				file
-				)
-			);
+		asset->size() != Size(2048, 858) &&
+		asset->size() != Size(1998, 1080) &&
+		asset->size() != Size(4096, 1716) &&
+		asset->size() != Size(3996, 2160)) {
+		notes.push_back({
+			VerificationNote::Type::BV21_ERROR,
+			VerificationNote::Code::INVALID_PICTURE_SIZE_IN_PIXELS,
+			String::compose("%1x%2", asset->size().width, asset->size().height),
+			file
+		});
 	}
 
 	/* Only 24, 25, 48fps allowed for 2K */
 	if (
-		(asset->size() == dcp::Size(2048, 858) || asset->size() == dcp::Size(1998, 1080)) &&
-		(asset->edit_rate() != dcp::Fraction(24, 1) && asset->edit_rate() != dcp::Fraction(25, 1) && asset->edit_rate() != dcp::Fraction(48, 1))
+		(asset->size() == Size(2048, 858) || asset->size() == Size(1998, 1080)) &&
+		(asset->edit_rate() != Fraction(24, 1) && asset->edit_rate() != Fraction(25, 1) && asset->edit_rate() != Fraction(48, 1))
 	   ) {
-		notes.push_back(
-			VerificationNote(
-				VerificationNote::VERIFY_BV21_ERROR,
-				VerificationNote::PICTURE_ASSET_INVALID_FRAME_RATE_FOR_2K,
-				String::compose("%1/%2", asset->edit_rate().numerator, asset->edit_rate().denominator),
-				file
-				)
-			);
+		notes.push_back({
+			VerificationNote::Type::BV21_ERROR,
+			VerificationNote::Code::INVALID_PICTURE_FRAME_RATE_FOR_2K,
+			String::compose("%1/%2", asset->edit_rate().numerator, asset->edit_rate().denominator),
+			file
+		});
 	}
 
-	if (asset->size() == dcp::Size(4096, 1716) || asset->size() == dcp::Size(3996, 2160)) {
+	if (asset->size() == Size(4096, 1716) || asset->size() == Size(3996, 2160)) {
 		/* Only 24fps allowed for 4K */
-		if (asset->edit_rate() != dcp::Fraction(24, 1)) {
-			notes.push_back(
-				VerificationNote(
-					VerificationNote::VERIFY_BV21_ERROR,
-					VerificationNote::PICTURE_ASSET_INVALID_FRAME_RATE_FOR_4K,
-					String::compose("%1/%2", asset->edit_rate().numerator, asset->edit_rate().denominator),
-					file
-					)
-				);
+		if (asset->edit_rate() != Fraction(24, 1)) {
+			notes.push_back({
+				VerificationNote::Type::BV21_ERROR,
+				VerificationNote::Code::INVALID_PICTURE_FRAME_RATE_FOR_4K,
+				String::compose("%1/%2", asset->edit_rate().numerator, asset->edit_rate().denominator),
+				file
+			});
 		}
 
 		/* Only 2D allowed for 4K */
 		if (dynamic_pointer_cast<const StereoPictureAsset>(asset)) {
-			notes.push_back(
-				VerificationNote(
-					VerificationNote::VERIFY_BV21_ERROR,
-					VerificationNote::PICTURE_ASSET_4K_3D,
-					file
-					)
-				);
+			notes.push_back({
+				VerificationNote::Type::BV21_ERROR,
+				VerificationNote::Code::INVALID_PICTURE_ASSET_RESOLUTION_FOR_3D,
+				String::compose("%1/%2", asset->edit_rate().numerator, asset->edit_rate().denominator),
+				file
+			});
 
 		}
 	}
@@ -588,26 +587,18 @@ verify_main_sound_asset (
 	shared_ptr<const ReelSoundAsset> reel_asset,
 	function<void (string, optional<boost::filesystem::path>)> stage,
 	function<void (float)> progress,
-	list<VerificationNote>& notes
+	vector<VerificationNote>& notes
 	)
 {
 	auto asset = reel_asset->asset();
 	stage ("Checking sound asset hash", asset->file());
 	auto const r = verify_asset (dcp, reel_asset, progress);
 	switch (r) {
-		case VERIFY_ASSET_RESULT_BAD:
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_ERROR, VerificationNote::SOUND_HASH_INCORRECT, *asset->file()
-					)
-				);
+		case VerifyAssetResult::BAD:
+			notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::INCORRECT_SOUND_HASH, *asset->file()});
 			break;
-		case VERIFY_ASSET_RESULT_CPL_PKL_DIFFER:
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_SOUND_HASHES_DIFFER, *asset->file()
-					)
-				);
+		case VerifyAssetResult::CPL_PKL_DIFFER:
+			notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::MISMATCHED_SOUND_HASHES, *asset->file()});
 			break;
 		default:
 			break;
@@ -616,26 +607,41 @@ verify_main_sound_asset (
 	stage ("Checking sound asset metadata", asset->file());
 
 	verify_language_tag (asset->language(), notes);
+	if (asset->sampling_rate() != 48000) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_SOUND_FRAME_RATE, raw_convert<string>(asset->sampling_rate()), *asset->file()});
+	}
 }
 
 
 static void
-verify_main_subtitle_reel (shared_ptr<const ReelSubtitleAsset> reel_asset, list<VerificationNote>& notes)
+verify_main_subtitle_reel (shared_ptr<const ReelSubtitleAsset> reel_asset, vector<VerificationNote>& notes)
 {
 	/* XXX: is Language compulsory? */
 	if (reel_asset->language()) {
 		verify_language_tag (*reel_asset->language(), notes);
 	}
+
+	if (!reel_asset->entry_point()) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_SUBTITLE_ENTRY_POINT, reel_asset->id() });
+	} else if (reel_asset->entry_point().get()) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INCORRECT_SUBTITLE_ENTRY_POINT, reel_asset->id() });
+	}
 }
 
 
 static void
-verify_closed_caption_reel (shared_ptr<const ReelClosedCaptionAsset> reel_asset, list<VerificationNote>& notes)
+verify_closed_caption_reel (shared_ptr<const ReelClosedCaptionAsset> reel_asset, vector<VerificationNote>& notes)
 {
 	/* XXX: is Language compulsory? */
 	if (reel_asset->language()) {
 		verify_language_tag (*reel_asset->language(), notes);
 	}
+
+	if (!reel_asset->entry_point()) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_CLOSED_CAPTION_ENTRY_POINT, reel_asset->id() });
+	} else if (reel_asset->entry_point().get()) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INCORRECT_CLOSED_CAPTION_ENTRY_POINT, reel_asset->id() });
+	}
 }
 
 
@@ -645,12 +651,57 @@ struct State
 };
 
 
+
+void
+verify_smpte_subtitle_asset (
+	shared_ptr<const SMPTESubtitleAsset> asset,
+	vector<VerificationNote>& notes,
+	State& state
+	)
+{
+	if (asset->language()) {
+		auto const language = *asset->language();
+		verify_language_tag (language, notes);
+		if (!state.subtitle_language) {
+			state.subtitle_language = language;
+		} else if (state.subtitle_language != language) {
+			notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISMATCHED_SUBTITLE_LANGUAGES });
+		}
+	} else {
+		notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE, *asset->file() });
+	}
+	auto const size = boost::filesystem::file_size(asset->file().get());
+	if (size > 115 * 1024 * 1024) {
+		notes.push_back (
+			{ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_TIMED_TEXT_SIZE_IN_BYTES, raw_convert<string>(size), *asset->file() }
+			);
+	}
+	/* XXX: I'm not sure what Bv2.1_7.2.1 means when it says "the font resource shall not be larger than 10MB"
+	 * but I'm hoping that checking for the total size of all fonts being <= 10MB will do.
+	 */
+	auto fonts = asset->font_data ();
+	int total_size = 0;
+	for (auto i: fonts) {
+		total_size += i.second.size();
+	}
+	if (total_size > 10 * 1024 * 1024) {
+		notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_TIMED_TEXT_FONT_SIZE_IN_BYTES, raw_convert<string>(total_size), asset->file().get() });
+	}
+
+	if (!asset->start_time()) {
+		notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_SUBTITLE_START_TIME, asset->file().get() });
+	} else if (asset->start_time() != Time()) {
+		notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_SUBTITLE_START_TIME, asset->file().get() });
+	}
+}
+
+
 static void
 verify_subtitle_asset (
 	shared_ptr<const SubtitleAsset> asset,
 	function<void (string, optional<boost::filesystem::path>)> stage,
 	boost::filesystem::path xsd_dtd_directory,
-	list<VerificationNote>& notes,
+	vector<VerificationNote>& notes,
 	State& state
 	)
 {
@@ -662,59 +713,7 @@ verify_subtitle_asset (
 
 	auto smpte = dynamic_pointer_cast<const SMPTESubtitleAsset>(asset);
 	if (smpte) {
-		if (smpte->language()) {
-			auto const language = *smpte->language();
-			verify_language_tag (language, notes);
-			if (!state.subtitle_language) {
-				state.subtitle_language = language;
-			} else if (state.subtitle_language != language) {
-				notes.push_back (
-					VerificationNote(
-						VerificationNote::VERIFY_BV21_ERROR, VerificationNote::SUBTITLE_LANGUAGES_DIFFER, *asset->file()
-						)
-					);
-			}
-		} else {
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_BV21_ERROR, VerificationNote::MISSING_SUBTITLE_LANGUAGE, *asset->file()
-					)
-				);
-		}
-		if (boost::filesystem::file_size(*asset->file()) > 115 * 1024 * 1024) {
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_BV21_ERROR, VerificationNote::TIMED_TEXT_ASSET_TOO_LARGE_IN_BYTES, *asset->file()
-					)
-				);
-		}
-		/* XXX: I'm not sure what Bv2.1_7.2.1 means when it says "the font resource shall not be larger than 10MB"
-		 * but I'm hoping that checking for the total size of all fonts being <= 10MB will do.
-		 */
-		auto fonts = asset->font_data ();
-		int total_size = 0;
-		for (auto i: fonts) {
-			total_size += i.second.size();
-		}
-		if (total_size > 10 * 1024 * 1024) {
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_BV21_ERROR, VerificationNote::TIMED_TEXT_FONTS_TOO_LARGE_IN_BYTES, *asset->file()
-					)
-				);
-		}
-
-		if (!smpte->start_time()) {
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_BV21_ERROR, VerificationNote::MISSING_SUBTITLE_START_TIME, *asset->file())
-				);
-		} else if (smpte->start_time() != dcp::Time()) {
-			notes.push_back (
-				VerificationNote(
-					VerificationNote::VERIFY_BV21_ERROR, VerificationNote::SUBTITLE_START_TIME_NON_ZERO, *asset->file())
-				);
-		}
+		verify_smpte_subtitle_asset (smpte, notes, state);
 	}
 }
 
@@ -724,23 +723,331 @@ verify_closed_caption_asset (
 	shared_ptr<const SubtitleAsset> asset,
 	function<void (string, optional<boost::filesystem::path>)> stage,
 	boost::filesystem::path xsd_dtd_directory,
-	list<VerificationNote>& notes,
+	vector<VerificationNote>& notes,
 	State& state
 	)
 {
 	verify_subtitle_asset (asset, stage, xsd_dtd_directory, notes, state);
 
 	if (asset->raw_xml().size() > 256 * 1024) {
-		notes.push_back (
-			VerificationNote(
-				VerificationNote::VERIFY_BV21_ERROR, VerificationNote::CLOSED_CAPTION_XML_TOO_LARGE_IN_BYTES, *asset->file()
-				)
-			);
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_CLOSED_CAPTION_XML_SIZE_IN_BYTES, raw_convert<string>(asset->raw_xml().size()), *asset->file()});
 	}
 }
 
 
-list<VerificationNote>
+static
+void
+verify_text_timing (
+	vector<shared_ptr<Reel>> reels,
+	optional<int> picture_frame_rate,
+	vector<VerificationNote>& notes,
+	std::function<bool (shared_ptr<Reel>)> check,
+	std::function<string (shared_ptr<Reel>)> xml,
+	std::function<int64_t (shared_ptr<Reel>)> duration
+	)
+{
+	/* end of last subtitle (in editable units) */
+	optional<int64_t> last_out;
+	auto too_short = false;
+	auto too_close = false;
+	auto too_early = false;
+	/* current reel start time (in editable units) */
+	int64_t reel_offset = 0;
+
+	std::function<void (cxml::ConstNodePtr, int, int, bool)> parse;
+	parse = [&parse, &last_out, &too_short, &too_close, &too_early, &reel_offset](cxml::ConstNodePtr node, int tcr, int pfr, bool first_reel) {
+		if (node->name() == "Subtitle") {
+			Time in (node->string_attribute("TimeIn"), tcr);
+			Time out (node->string_attribute("TimeOut"), tcr);
+			if (first_reel && in < Time(0, 0, 4, 0, tcr)) {
+				too_early = true;
+			}
+			auto length = out - in;
+			if (length.as_editable_units(pfr) < 15) {
+				too_short = true;
+			}
+			if (last_out) {
+				/* XXX: this feels dubious - is it really what Bv2.1 means? */
+				auto distance = reel_offset + in.as_editable_units(pfr) - *last_out;
+				if (distance >= 0 && distance < 2) {
+					too_close = true;
+				}
+			}
+			last_out = reel_offset + out.as_editable_units(pfr);
+		} else {
+			for (auto i: node->node_children()) {
+				parse(i, tcr, pfr, first_reel);
+			}
+		}
+	};
+
+	for (auto i = 0U; i < reels.size(); ++i) {
+		if (!check(reels[i])) {
+			continue;
+		}
+
+		/* We need to look at <Subtitle> instances in the XML being checked, so we can't use the subtitles
+		 * read in by libdcp's parser.
+		 */
+
+		auto doc = make_shared<cxml::Document>("SubtitleReel");
+		doc->read_string (xml(reels[i]));
+		auto const tcr = doc->number_child<int>("TimeCodeRate");
+		parse (doc, tcr, picture_frame_rate.get_value_or(24), i == 0);
+		reel_offset += duration(reels[i]);
+	}
+
+	if (too_early) {
+		notes.push_back({
+			VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME
+		});
+	}
+
+	if (too_short) {
+		notes.push_back ({
+			VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_DURATION
+		});
+	}
+
+	if (too_close) {
+		notes.push_back ({
+			VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_SPACING
+		});
+	}
+}
+
+
+struct LinesCharactersResult
+{
+	bool warning_length_exceeded = false;
+	bool error_length_exceeded = false;
+	bool line_count_exceeded = false;
+};
+
+
+static
+void
+verify_text_lines_and_characters (
+	shared_ptr<SubtitleAsset> asset,
+	int warning_length,
+	int error_length,
+	LinesCharactersResult* result
+	)
+{
+	class Event
+	{
+	public:
+		Event (Time time_, float position_, int characters_)
+			: time (time_)
+			, position (position_)
+			, characters (characters_)
+		{}
+
+		Event (Time time_, shared_ptr<Event> start_)
+			: time (time_)
+			, start (start_)
+		{}
+
+		Time time;
+		int position; //< position from 0 at top of screen to 100 at bottom
+		int characters;
+		shared_ptr<Event> start;
+	};
+
+	vector<shared_ptr<Event>> events;
+
+	auto position = [](shared_ptr<const SubtitleString> sub) {
+		switch (sub->v_align()) {
+		case VAlign::TOP:
+			return lrintf(sub->v_position() * 100);
+		case VAlign::CENTER:
+			return lrintf((0.5f + sub->v_position()) * 100);
+		case VAlign::BOTTOM:
+			return lrintf((1.0f - sub->v_position()) * 100);
+		}
+
+		return 0L;
+	};
+
+	for (auto j: asset->subtitles()) {
+		auto text = dynamic_pointer_cast<const SubtitleString>(j);
+		if (text) {
+			auto in = make_shared<Event>(text->in(), position(text), text->text().length());
+			events.push_back(in);
+			events.push_back(make_shared<Event>(text->out(), in));
+		}
+	}
+
+	std::sort(events.begin(), events.end(), [](shared_ptr<Event> const& a, shared_ptr<Event>const& b) {
+		return a->time < b->time;
+	});
+
+	map<int, int> current;
+	for (auto i: events) {
+		if (current.size() > 3) {
+			result->line_count_exceeded = true;
+		}
+		for (auto j: current) {
+			if (j.second >= warning_length) {
+				result->warning_length_exceeded = true;
+			}
+			if (j.second >= error_length) {
+				result->error_length_exceeded = true;
+			}
+		}
+
+		if (i->start) {
+			/* end of a subtitle */
+			DCP_ASSERT (current.find(i->start->position) != current.end());
+			if (current[i->start->position] == i->start->characters) {
+				current.erase(i->start->position);
+			} else {
+				current[i->start->position] -= i->start->characters;
+			}
+		} else {
+			/* start of a subtitle */
+			if (current.find(i->position) == current.end()) {
+				current[i->position] = i->characters;
+			} else {
+				current[i->position] += i->characters;
+			}
+		}
+	}
+}
+
+
+static
+void
+verify_text_timing (vector<shared_ptr<Reel>> reels, vector<VerificationNote>& notes)
+{
+	if (reels.empty()) {
+		return;
+	}
+
+	optional<int> picture_frame_rate;
+	if (reels[0]->main_picture()) {
+		picture_frame_rate = reels[0]->main_picture()->frame_rate().numerator;
+	}
+
+	if (reels[0]->main_subtitle()) {
+		verify_text_timing (reels, picture_frame_rate, notes,
+			[](shared_ptr<Reel> reel) {
+				return static_cast<bool>(reel->main_subtitle());
+			},
+			[](shared_ptr<Reel> reel) {
+				return reel->main_subtitle()->asset()->raw_xml();
+			},
+			[](shared_ptr<Reel> reel) {
+				return reel->main_subtitle()->actual_duration();
+			}
+		);
+	}
+
+	for (auto i = 0U; i < reels[0]->closed_captions().size(); ++i) {
+		verify_text_timing (reels, picture_frame_rate, notes,
+			[i](shared_ptr<Reel> reel) {
+				return i < reel->closed_captions().size();
+			},
+			[i](shared_ptr<Reel> reel) {
+				return reel->closed_captions()[i]->asset()->raw_xml();
+			},
+			[i](shared_ptr<Reel> reel) {
+				return reel->closed_captions()[i]->actual_duration();
+			}
+		);
+	}
+}
+
+
+void
+verify_extension_metadata (shared_ptr<CPL> cpl, vector<VerificationNote>& notes)
+{
+	DCP_ASSERT (cpl->file());
+	cxml::Document doc ("CompositionPlaylist");
+	doc.read_file (cpl->file().get());
+
+	auto missing = false;
+	string malformed;
+
+	if (auto reel_list = doc.node_child("ReelList")) {
+		auto reels = reel_list->node_children("Reel");
+		if (!reels.empty()) {
+			if (auto asset_list = reels[0]->optional_node_child("AssetList")) {
+				if (auto metadata = asset_list->optional_node_child("CompositionMetadataAsset")) {
+					if (auto extension_list = metadata->optional_node_child("ExtensionMetadataList")) {
+						missing = true;
+						for (auto extension: extension_list->node_children("ExtensionMetadata")) {
+							if (extension->optional_string_attribute("scope").get_value_or("") != "http://isdcf.com/ns/cplmd/app") {
+								continue;
+							}
+							missing = false;
+							if (auto name = extension->optional_node_child("Name")) {
+								if (name->content() != "Application") {
+									malformed = "<Name> should be 'Application'";
+								}
+							}
+							if (auto property_list = extension->optional_node_child("PropertyList")) {
+								if (auto property = property_list->optional_node_child("Property")) {
+									if (auto name = property->optional_node_child("Name")) {
+										if (name->content() != "DCP Constraints Profile") {
+											malformed = "<Name> property should be 'DCP Constraints Profile'";
+										}
+									}
+									if (auto value = property->optional_node_child("Value")) {
+										if (value->content() != "SMPTE-RDD-52:2020-Bv2.1") {
+											malformed = "<Value> property should be 'SMPTE-RDD-52:2020-Bv2.1'";
+										}
+									}
+								}
+							}
+						}
+					} else {
+						missing = true;
+					}
+				}
+			}
+		}
+	}
+
+	if (missing) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_EXTENSION_METADATA, cpl->id(), cpl->file().get()});
+	} else if (!malformed.empty()) {
+		notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_EXTENSION_METADATA, malformed, cpl->file().get()});
+	}
+}
+
+
+bool
+pkl_has_encrypted_assets (shared_ptr<DCP> dcp, shared_ptr<PKL> pkl)
+{
+	vector<string> encrypted;
+	for (auto i: dcp->cpls()) {
+		for (auto j: i->reel_file_assets()) {
+			if (j->asset_ref().resolved()) {
+				/* It's a bit surprising / broken but Interop subtitle assets are represented
+				 * in reels by ReelSubtitleAsset which inherits ReelFileAsset, so it's possible for
+				 * ReelFileAssets to have assets which are not MXFs.
+				 */
+				if (auto asset = dynamic_pointer_cast<MXF>(j->asset_ref().asset())) {
+					if (asset->encrypted()) {
+						encrypted.push_back(j->asset_ref().id());
+					}
+				}
+			}
+		}
+	}
+
+	for (auto i: pkl->asset_list()) {
+		if (find(encrypted.begin(), encrypted.end(), i->id()) != encrypted.end()) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
+
+vector<VerificationNote>
 dcp::verify (
 	vector<boost::filesystem::path> directories,
 	function<void (string, optional<boost::filesystem::path>)> stage,
@@ -750,10 +1057,10 @@ dcp::verify (
 {
 	xsd_dtd_directory = boost::filesystem::canonical (xsd_dtd_directory);
 
-	list<VerificationNote> notes;
-	State state;
+	vector<VerificationNote> notes;
+	State state{};
 
-	list<shared_ptr<DCP>> dcps;
+	vector<shared_ptr<DCP>> dcps;
 	for (auto i: directories) {
 		dcps.push_back (shared_ptr<DCP> (new DCP (i)));
 	}
@@ -763,48 +1070,117 @@ dcp::verify (
 		try {
 			dcp->read (&notes);
 		} catch (ReadError& e) {
-			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
+			notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::FAILED_READ, string(e.what())});
 		} catch (XMLError& e) {
-			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
+			notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::FAILED_READ, string(e.what())});
 		} catch (MXFFileError& e) {
-			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
+			notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::FAILED_READ, string(e.what())});
 		} catch (cxml::Error& e) {
-			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
+			notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::FAILED_READ, string(e.what())});
 		}
 
-		if (dcp->standard() != dcp::SMPTE) {
-			notes.push_back (VerificationNote(VerificationNote::VERIFY_BV21_ERROR, VerificationNote::NOT_SMPTE));
+		if (dcp->standard() != Standard::SMPTE) {
+			notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_STANDARD});
 		}
 
 		for (auto cpl: dcp->cpls()) {
 			stage ("Checking CPL", cpl->file());
 			validate_xml (cpl->file().get(), xsd_dtd_directory, notes);
 
+			if (cpl->any_encrypted() && !cpl->all_encrypted()) {
+				notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::PARTIALLY_ENCRYPTED});
+			}
+
 			for (auto const& i: cpl->additional_subtitle_languages()) {
 				verify_language_tag (i, notes);
 			}
 
 			if (cpl->release_territory()) {
-				verify_language_tag (cpl->release_territory().get(), notes);
+				if (!cpl->release_territory_scope() || cpl->release_territory_scope().get() != "http://www.smpte-ra.org/schemas/429-16/2014/CPL-Metadata#scope/release-territory/UNM49") {
+					auto terr = cpl->release_territory().get();
+					/* Must be a valid region tag, or "001" */
+					try {
+						LanguageTag::RegionSubtag test (terr);
+					} catch (...) {
+						if (terr != "001") {
+							notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_LANGUAGE, terr});
+						}
+					}
+				}
+			}
+
+			if (dcp->standard() == Standard::SMPTE) {
+				if (!cpl->annotation_text()) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_CPL_ANNOTATION_TEXT, cpl->id(), cpl->file().get()});
+				} else if (cpl->annotation_text().get() != cpl->content_title_text()) {
+					notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISMATCHED_CPL_ANNOTATION_TEXT, cpl->id(), cpl->file().get()});
+				}
 			}
 
-			/* Check that the CPL's hash corresponds to the PKL */
 			for (auto i: dcp->pkls()) {
+				/* Check that the CPL's hash corresponds to the PKL */
 				optional<string> h = i->hash(cpl->id());
 				if (h && make_digest(ArrayData(*cpl->file())) != *h) {
-					notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::CPL_HASH_INCORRECT));
+					notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::MISMATCHED_CPL_HASHES, cpl->id(), cpl->file().get()});
+				}
+
+				/* Check that any PKL with a single CPL has its AnnotationText the same as the CPL's ContentTitleText */
+				optional<string> required_annotation_text;
+				for (auto j: i->asset_list()) {
+					/* See if this is a CPL */
+					for (auto k: dcp->cpls()) {
+						if (j->id() == k->id()) {
+							if (!required_annotation_text) {
+								/* First CPL we have found; this is the required AnnotationText unless we find another */
+								required_annotation_text = cpl->content_title_text();
+							} else {
+								/* There's more than one CPL so we don't care what the PKL's AnnotationText is */
+								required_annotation_text = boost::none;
+							}
+						}
+					}
+				}
+
+				if (required_annotation_text && i->annotation_text() != required_annotation_text) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, i->id(), i->file().get()});
 				}
 			}
 
+			/* set to true if any reel has a MainSubtitle */
+			auto have_main_subtitle = false;
+			/* set to true if any reel has no MainSubtitle */
+			auto have_no_main_subtitle = false;
+			/* fewest number of closed caption assets seen in a reel */
+			size_t fewest_closed_captions = SIZE_MAX;
+			/* most number of closed caption assets seen in a reel */
+			size_t most_closed_captions = 0;
+			map<Marker, Time> markers_seen;
+
 			for (auto reel: cpl->reels()) {
 				stage ("Checking reel", optional<boost::filesystem::path>());
 
 				for (auto i: reel->assets()) {
 					if (i->duration() && (i->duration().get() * i->edit_rate().denominator / i->edit_rate().numerator) < 1) {
-						notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::DURATION_TOO_SMALL, i->id()));
+						notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_DURATION, i->id()});
 					}
 					if ((i->intrinsic_duration() * i->edit_rate().denominator / i->edit_rate().numerator) < 1) {
-						notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::INTRINSIC_DURATION_TOO_SMALL, i->id()));
+						notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_INTRINSIC_DURATION, i->id()});
+					}
+					auto file_asset = dynamic_pointer_cast<ReelFileAsset>(i);
+					if (file_asset && !file_asset->hash()) {
+						notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_HASH, i->id()});
+					}
+				}
+
+				if (dcp->standard() == Standard::SMPTE) {
+					boost::optional<int64_t> duration;
+					for (auto i: reel->assets()) {
+						if (!duration) {
+							duration = i->actual_duration();
+						} else if (*duration != i->actual_duration()) {
+							notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISMATCHED_ASSET_DURATION});
+							break;
+						}
 					}
 				}
 
@@ -819,7 +1195,11 @@ dcp::verify (
 					     frame_rate.numerator != 50 &&
 					     frame_rate.numerator != 60 &&
 					     frame_rate.numerator != 96)) {
-						notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::INVALID_PICTURE_FRAME_RATE));
+						notes.push_back ({
+							VerificationNote::Type::ERROR,
+							VerificationNote::Code::INVALID_PICTURE_FRAME_RATE,
+							String::compose("%1/%2", frame_rate.numerator, frame_rate.denominator)
+						});
 					}
 					/* Check asset */
 					if (reel->main_picture()->asset_ref().resolved()) {
@@ -836,6 +1216,9 @@ dcp::verify (
 					if (reel->main_subtitle()->asset_ref().resolved()) {
 						verify_subtitle_asset (reel->main_subtitle()->asset(), stage, xsd_dtd_directory, notes, state);
 					}
+					have_main_subtitle = true;
+				} else {
+					have_no_main_subtitle = true;
 				}
 
 				for (auto i: reel->closed_captions()) {
@@ -844,90 +1227,325 @@ dcp::verify (
 						verify_closed_caption_asset (i->asset(), stage, xsd_dtd_directory, notes, state);
 					}
 				}
+
+				if (reel->main_markers()) {
+					for (auto const& i: reel->main_markers()->get()) {
+						markers_seen.insert (i);
+					}
+				}
+
+				fewest_closed_captions = std::min (fewest_closed_captions, reel->closed_captions().size());
+				most_closed_captions = std::max (most_closed_captions, reel->closed_captions().size());
+			}
+
+			if (dcp->standard() == Standard::SMPTE) {
+
+				if (have_main_subtitle && have_no_main_subtitle) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_MAIN_SUBTITLE_FROM_SOME_REELS});
+				}
+
+				if (fewest_closed_captions != most_closed_captions) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISMATCHED_CLOSED_CAPTION_ASSET_COUNTS});
+				}
+
+				if (cpl->content_kind() == ContentKind::FEATURE) {
+					if (markers_seen.find(Marker::FFEC) == markers_seen.end()) {
+						notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_FFEC_IN_FEATURE});
+					}
+					if (markers_seen.find(Marker::FFMC) == markers_seen.end()) {
+						notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_FFMC_IN_FEATURE});
+					}
+				}
+
+				auto ffoc = markers_seen.find(Marker::FFOC);
+				if (ffoc == markers_seen.end()) {
+					notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSING_FFOC});
+				} else if (ffoc->second.e != 1) {
+					notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::INCORRECT_FFOC, raw_convert<string>(ffoc->second.e)});
+				}
+
+				auto lfoc = markers_seen.find(Marker::LFOC);
+				if (lfoc == markers_seen.end()) {
+					notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSING_LFOC});
+				} else {
+					auto lfoc_time = lfoc->second.as_editable_units(lfoc->second.tcr);
+					if (lfoc_time != (cpl->reels().back()->duration() - 1)) {
+						notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::INCORRECT_LFOC, raw_convert<string>(lfoc_time)});
+					}
+				}
+
+				verify_text_timing (cpl->reels(), notes);
+
+				LinesCharactersResult result;
+				for (auto reel: cpl->reels()) {
+					if (reel->main_subtitle() && reel->main_subtitle()->asset()) {
+						verify_text_lines_and_characters (reel->main_subtitle()->asset(), 52, 79, &result);
+					}
+				}
+
+				if (result.line_count_exceeded) {
+					notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_LINE_COUNT});
+				}
+				if (result.error_length_exceeded) {
+					notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_LINE_LENGTH});
+				} else if (result.warning_length_exceeded) {
+					notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::NEARLY_INVALID_SUBTITLE_LINE_LENGTH});
+				}
+
+				result = LinesCharactersResult();
+				for (auto reel: cpl->reels()) {
+					for (auto i: reel->closed_captions()) {
+						if (i->asset()) {
+							verify_text_lines_and_characters (i->asset(), 32, 32, &result);
+						}
+					}
+				}
+
+				if (result.line_count_exceeded) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_CLOSED_CAPTION_LINE_COUNT});
+				}
+				if (result.error_length_exceeded) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_CLOSED_CAPTION_LINE_LENGTH});
+				}
+
+				if (!cpl->full_content_title_text()) {
+					/* Since FullContentTitleText is assumed always to exist if there's a CompositionMetadataAsset we
+					 * can use it as a proxy for CompositionMetadataAsset's existence.
+					 */
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get()});
+				} else if (!cpl->version_number()) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_CPL_METADATA_VERSION_NUMBER, cpl->id(), cpl->file().get()});
+				}
+
+				verify_extension_metadata (cpl, notes);
+
+				if (cpl->any_encrypted()) {
+					cxml::Document doc ("CompositionPlaylist");
+					DCP_ASSERT (cpl->file());
+					doc.read_file (cpl->file().get());
+					if (!doc.optional_node_child("Signature")) {
+						notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::UNSIGNED_CPL_WITH_ENCRYPTED_CONTENT, cpl->id(), cpl->file().get()});
+					}
+				}
 			}
 		}
 
 		for (auto pkl: dcp->pkls()) {
 			stage ("Checking PKL", pkl->file());
 			validate_xml (pkl->file().get(), xsd_dtd_directory, notes);
+			if (pkl_has_encrypted_assets(dcp, pkl)) {
+				cxml::Document doc ("PackingList");
+				doc.read_file (pkl->file().get());
+				if (!doc.optional_node_child("Signature")) {
+					notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT, pkl->id(), pkl->file().get()});
+				}
+			}
 		}
 
 		if (dcp->asset_map_path()) {
 			stage ("Checking ASSETMAP", dcp->asset_map_path().get());
 			validate_xml (dcp->asset_map_path().get(), xsd_dtd_directory, notes);
 		} else {
-			notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::MISSING_ASSETMAP));
+			notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::MISSING_ASSETMAP});
 		}
 	}
 
 	return notes;
 }
 
+
 string
-dcp::note_to_string (dcp::VerificationNote note)
+dcp::note_to_string (VerificationNote note)
 {
+	/** These strings should say what is wrong, incorporating any extra details (ID, filenames etc.).
+   	 *
+	 *  e.g. "ClosedCaption asset has no <EntryPoint> tag.",
+	 *  not "ClosedCaption assets must have an <EntryPoint> tag."
+	 *
+	 *  It's OK to use XML tag names where they are clear.
+	 *  If both ID and filename are available, use only the ID.
+	 *  End messages with a full stop.
+	 *  Messages should not mention whether or not their errors are a part of Bv2.1.
+	 */
 	switch (note.code()) {
-	case dcp::VerificationNote::GENERAL_READ:
+	case VerificationNote::Code::FAILED_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_DIFFER:
-		return dcp::String::compose("The PKL and CPL hashes differ for the picture asset %1.", note.file()->filename());
-	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_DIFFER:
-		return dcp::String::compose("The PKL and CPL hashes differ for the sound asset %1.", note.file()->filename());
-	case dcp::VerificationNote::EMPTY_ASSET_PATH:
+	case VerificationNote::Code::MISMATCHED_CPL_HASHES:
+		return String::compose("The hash of the CPL %1 in the PKL does not agree with the CPL file.", note.note().get());
+	case VerificationNote::Code::INVALID_PICTURE_FRAME_RATE:
+		return String::compose("The picture in a reel has an invalid frame rate %1.", note.note().get());
+	case VerificationNote::Code::INCORRECT_PICTURE_HASH:
+		return String::compose("The hash of the picture asset %1 does not agree with the PKL file.", note.file()->filename());
+	case VerificationNote::Code::MISMATCHED_PICTURE_HASHES:
+		return String::compose("The PKL and CPL hashes differ for the picture asset %1.", note.file()->filename());
+	case VerificationNote::Code::INCORRECT_SOUND_HASH:
+		return String::compose("The hash of the sound asset %1 does not agree with the PKL file.", note.file()->filename());
+	case VerificationNote::Code::MISMATCHED_SOUND_HASHES:
+		return String::compose("The PKL and CPL hashes differ for the sound asset %1.", note.file()->filename());
+	case VerificationNote::Code::EMPTY_ASSET_PATH:
 		return "The asset map contains an empty asset path.";
-	case dcp::VerificationNote::MISSING_ASSET:
-		return String::compose("The file for an asset in the asset map cannot be found; missing file is %1.", note.file()->filename());
-	case dcp::VerificationNote::MISMATCHED_STANDARD:
+	case VerificationNote::Code::MISSING_ASSET:
+		return String::compose("The file %1 for an asset in the asset map cannot be found.", note.file()->filename());
+	case VerificationNote::Code::MISMATCHED_STANDARD:
 		return "The DCP contains both SMPTE and Interop parts.";
-	case dcp::VerificationNote::XML_VALIDATION_ERROR:
+	case VerificationNote::Code::INVALID_XML:
 		return String::compose("An XML file is badly formed: %1 (%2:%3)", note.note().get(), note.file()->filename(), note.line().get());
-	case dcp::VerificationNote::MISSING_ASSETMAP:
+	case VerificationNote::Code::MISSING_ASSETMAP:
 		return "No ASSETMAP or ASSETMAP.xml was found.";
-	case dcp::VerificationNote::INTRINSIC_DURATION_TOO_SMALL:
-		return String::compose("The intrinsic duration of an asset is less than 1 second long: %1", note.note().get());
-	case dcp::VerificationNote::DURATION_TOO_SMALL:
-		return String::compose("The duration of an asset is less than 1 second long: %1", note.note().get());
-	case dcp::VerificationNote::PICTURE_FRAME_TOO_LARGE_IN_BYTES:
+	case VerificationNote::Code::INVALID_INTRINSIC_DURATION:
+		return String::compose("The intrinsic duration of the asset %1 is less than 1 second long.", note.note().get());
+	case VerificationNote::Code::INVALID_DURATION:
+		return String::compose("The duration of the asset %1 is less than 1 second long.", note.note().get());
+	case VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES:
 		return String::compose("The instantaneous bit rate of the picture asset %1 is larger than the limit of 250Mbit/s in at least one place.", note.file()->filename());
-	case dcp::VerificationNote::PICTURE_FRAME_NEARLY_TOO_LARGE_IN_BYTES:
+	case VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES:
 		return String::compose("The instantaneous bit rate of the picture asset %1 is close to the limit of 250Mbit/s in at least one place.", note.file()->filename());
-	case dcp::VerificationNote::EXTERNAL_ASSET:
-		return String::compose("An asset that this DCP refers to is not included in the DCP.  It may be a VF.  Missing asset is %1.", note.note().get());
-	case dcp::VerificationNote::NOT_SMPTE:
-		return "This DCP does not use the SMPTE standard, which is required for Bv2.1 compliance.";
-	case dcp::VerificationNote::BAD_LANGUAGE:
+	case VerificationNote::Code::EXTERNAL_ASSET:
+		return String::compose("The asset %1 that this DCP refers to is not included in the DCP.  It may be a VF.", note.note().get());
+	case VerificationNote::Code::INVALID_STANDARD:
+		return "This DCP does not use the SMPTE standard.";
+	case VerificationNote::Code::INVALID_LANGUAGE:
 		return String::compose("The DCP specifies a language '%1' which does not conform to the RFC 5646 standard.", note.note().get());
-	case dcp::VerificationNote::PICTURE_ASSET_INVALID_SIZE_IN_PIXELS:
-		return String::compose("A picture asset's size (%1) is not one of those allowed by Bv2.1 (2048x858, 1998x1080, 4096x1716 or 3996x2160)", note.note().get());
-	case dcp::VerificationNote::PICTURE_ASSET_INVALID_FRAME_RATE_FOR_2K:
-		return String::compose("A picture asset's frame rate (%1) is not one of those allowed for 2K DCPs by Bv2.1 (24, 25 or 48fps)", note.note().get());
-	case dcp::VerificationNote::PICTURE_ASSET_INVALID_FRAME_RATE_FOR_4K:
-		return String::compose("A picture asset's frame rate (%1) is not 24fps as required for 4K DCPs by Bv2.1", note.note().get());
-	case dcp::VerificationNote::PICTURE_ASSET_4K_3D:
-		return "3D 4K DCPs are not allowed by Bv2.1";
-	case dcp::VerificationNote::CLOSED_CAPTION_XML_TOO_LARGE_IN_BYTES:
-		return String::compose("The XML for the closed caption asset %1 is longer than the 256KB maximum required by Bv2.1", note.file()->filename());
-	case dcp::VerificationNote::TIMED_TEXT_ASSET_TOO_LARGE_IN_BYTES:
-		return String::compose("The total size of the timed text asset %1 is larger than the 115MB maximum required by Bv2.1", note.file()->filename());
-	case dcp::VerificationNote::TIMED_TEXT_FONTS_TOO_LARGE_IN_BYTES:
-		return String::compose("The total size of the fonts in timed text asset %1 is larger than the 10MB maximum required by Bv2.1", note.file()->filename());
-	case dcp::VerificationNote::MISSING_SUBTITLE_LANGUAGE:
-		return String::compose("The XML for a SMPTE subtitle asset has no <Language> tag, which is required by Bv2.1", note.file()->filename());
-	case dcp::VerificationNote::SUBTITLE_LANGUAGES_DIFFER:
-		return String::compose("Some subtitle assets have different <Language> tags than others", note.file()->filename());
-	case dcp::VerificationNote::MISSING_SUBTITLE_START_TIME:
-		return String::compose("The XML for a SMPTE subtitle asset has no <StartTime> tag, which is required by Bv2.1", note.file()->filename());
-	case dcp::VerificationNote::SUBTITLE_START_TIME_NON_ZERO:
-		return String::compose("The XML for a SMPTE subtitle asset has a non-zero <StartTime> tag, which is disallowed by Bv2.1", note.file()->filename());
+	case VerificationNote::Code::INVALID_PICTURE_SIZE_IN_PIXELS:
+		return String::compose("The size %1 of picture asset %2 is not allowed.", note.note().get(), note.file()->filename());
+	case VerificationNote::Code::INVALID_PICTURE_FRAME_RATE_FOR_2K:
+		return String::compose("The frame rate %1 of picture asset %2 is not allowed for 2K DCPs.", note.note().get(), note.file()->filename());
+	case VerificationNote::Code::INVALID_PICTURE_FRAME_RATE_FOR_4K:
+		return String::compose("The frame rate %1 of picture asset %2 is not allowed for 4K DCPs.", note.note().get(), note.file()->filename());
+	case VerificationNote::Code::INVALID_PICTURE_ASSET_RESOLUTION_FOR_3D:
+		return "3D 4K DCPs are not allowed.";
+	case VerificationNote::Code::INVALID_CLOSED_CAPTION_XML_SIZE_IN_BYTES:
+		return String::compose("The size %1 of the closed caption asset %2 is larger than the 256KB maximum.", note.note().get(), note.file()->filename());
+	case VerificationNote::Code::INVALID_TIMED_TEXT_SIZE_IN_BYTES:
+		return String::compose("The size %1 of the timed text asset %2 is larger than the 115MB maximum.", note.note().get(), note.file()->filename());
+	case VerificationNote::Code::INVALID_TIMED_TEXT_FONT_SIZE_IN_BYTES:
+		return String::compose("The size %1 of the fonts in timed text asset %2 is larger than the 10MB maximum.", note.note().get(), note.file()->filename());
+	case VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE:
+		return String::compose("The XML for the SMPTE subtitle asset %1 has no <Language> tag.", note.file()->filename());
+	case VerificationNote::Code::MISMATCHED_SUBTITLE_LANGUAGES:
+		return "Some subtitle assets have different <Language> tags than others";
+	case VerificationNote::Code::MISSING_SUBTITLE_START_TIME:
+		return String::compose("The XML for the SMPTE subtitle asset %1 has no <StartTime> tag.", note.file()->filename());
+	case VerificationNote::Code::INVALID_SUBTITLE_START_TIME:
+		return String::compose("The XML for a SMPTE subtitle asset %1 has a non-zero <StartTime> tag.", note.file()->filename());
+	case VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME:
+		return "The first subtitle or closed caption is less than 4 seconds from the start of the DCP.";
+	case VerificationNote::Code::INVALID_SUBTITLE_DURATION:
+		return "At least one subtitle lasts less than 15 frames.";
+	case VerificationNote::Code::INVALID_SUBTITLE_SPACING:
+		return "At least one pair of subtitles is separated by less than 2 frames.";
+	case VerificationNote::Code::INVALID_SUBTITLE_LINE_COUNT:
+		return "There are more than 3 subtitle lines in at least one place in the DCP.";
+	case VerificationNote::Code::NEARLY_INVALID_SUBTITLE_LINE_LENGTH:
+		return "There are more than 52 characters in at least one subtitle line.";
+	case VerificationNote::Code::INVALID_SUBTITLE_LINE_LENGTH:
+		return "There are more than 79 characters in at least one subtitle line.";
+	case VerificationNote::Code::INVALID_CLOSED_CAPTION_LINE_COUNT:
+		return "There are more than 3 closed caption lines in at least one place.";
+	case VerificationNote::Code::INVALID_CLOSED_CAPTION_LINE_LENGTH:
+		return "There are more than 32 characters in at least one closed caption line.";
+	case VerificationNote::Code::INVALID_SOUND_FRAME_RATE:
+		return String::compose("The sound asset %1 has a sampling rate of %2", note.file()->filename(), note.note().get());
+	case VerificationNote::Code::MISSING_CPL_ANNOTATION_TEXT:
+		return String::compose("The CPL %1 has no <AnnotationText> tag.", note.note().get());
+	case VerificationNote::Code::MISMATCHED_CPL_ANNOTATION_TEXT:
+		return String::compose("The CPL %1 has an <AnnotationText> which differs from its <ContentTitleText>", note.note().get());
+	case VerificationNote::Code::MISMATCHED_ASSET_DURATION:
+		return "All assets in a reel do not have the same duration.";
+	case VerificationNote::Code::MISSING_MAIN_SUBTITLE_FROM_SOME_REELS:
+		return "At least one reel contains a subtitle asset, but some reel(s) do not";
+	case VerificationNote::Code::MISMATCHED_CLOSED_CAPTION_ASSET_COUNTS:
+		return "At least one reel has closed captions, but reels have different numbers of closed caption assets.";
+	case VerificationNote::Code::MISSING_SUBTITLE_ENTRY_POINT:
+		return String::compose("The subtitle asset %1 has no <EntryPoint> tag.", note.note().get());
+	case VerificationNote::Code::INCORRECT_SUBTITLE_ENTRY_POINT:
+		return String::compose("The subtitle asset %1 has an <EntryPoint> other than 0.", note.note().get());
+	case VerificationNote::Code::MISSING_CLOSED_CAPTION_ENTRY_POINT:
+		return String::compose("The closed caption asset %1 has no <EntryPoint> tag.", note.note().get());
+	case VerificationNote::Code::INCORRECT_CLOSED_CAPTION_ENTRY_POINT:
+		return String::compose("The closed caption asset %1 has an <EntryPoint> other than 0.", note.note().get());
+	case VerificationNote::Code::MISSING_HASH:
+		return String::compose("The asset %1 has no <Hash> tag in the CPL.", note.note().get());
+	case VerificationNote::Code::MISSING_FFEC_IN_FEATURE:
+		return "The DCP is marked as a Feature but there is no FFEC (first frame of end credits) marker";
+	case VerificationNote::Code::MISSING_FFMC_IN_FEATURE:
+		return "The DCP is marked as a Feature but there is no FFMC (first frame of moving credits) marker";
+	case VerificationNote::Code::MISSING_FFOC:
+		return "There should be a FFOC (first frame of content) marker";
+	case VerificationNote::Code::MISSING_LFOC:
+		return "There should be a LFOC (last frame of content) marker";
+	case VerificationNote::Code::INCORRECT_FFOC:
+		return String::compose("The FFOC marker is %1 instead of 1", note.note().get());
+	case VerificationNote::Code::INCORRECT_LFOC:
+		return String::compose("The LFOC marker is %1 instead of 1 less than the duration of the last reel.", note.note().get());
+	case VerificationNote::Code::MISSING_CPL_METADATA:
+		return String::compose("The CPL %1 has no <CompositionMetadataAsset> tag.", note.note().get());
+	case VerificationNote::Code::MISSING_CPL_METADATA_VERSION_NUMBER:
+		return String::compose("The CPL %1 has no <VersionNumber> in its <CompositionMetadataAsset>.", note.note().get());
+	case VerificationNote::Code::MISSING_EXTENSION_METADATA:
+		return String::compose("The CPL %1 has no <ExtensionMetadata> in its <CompositionMetadataAsset>.", note.note().get());
+	case VerificationNote::Code::INVALID_EXTENSION_METADATA:
+		return String::compose("The CPL %1 has a malformed <ExtensionMetadata> (%2).", note.file()->filename(), note.note().get());
+	case VerificationNote::Code::UNSIGNED_CPL_WITH_ENCRYPTED_CONTENT:
+		return String::compose("The CPL %1, which has encrypted content, is not signed.", note.note().get());
+	case VerificationNote::Code::UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT:
+		return String::compose("The PKL %1, which has encrypted content, is not signed.", note.note().get());
+	case VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL:
+		return String::compose("The PKL %1 has only one CPL but its <AnnotationText> does not match the CPL's <ContentTitleText>.", note.note().get());
+	case VerificationNote::Code::PARTIALLY_ENCRYPTED:
+		return "Some assets are encrypted but some are not.";
+	case VerificationNote::Code::INVALID_JPEG2000_CODESTREAM:
+		return String::compose("The JPEG2000 codestream for at least one frame is invalid (%1)", note.note().get());
+	case VerificationNote::Code::INVALID_JPEG2000_GUARD_BITS_FOR_2K:
+		return String::compose("The JPEG2000 codestream uses %1 guard bits in a 2K image instead of 1.", note.note().get());
+	case VerificationNote::Code::INVALID_JPEG2000_GUARD_BITS_FOR_4K:
+		return String::compose("The JPEG2000 codestream uses %1 guard bits in a 4K image instead of 2.", note.note().get());
+	case VerificationNote::Code::INVALID_JPEG2000_TILE_SIZE:
+		return "The JPEG2000 tile size is not the same as the image size.";
+	case VerificationNote::Code::INVALID_JPEG2000_CODE_BLOCK_WIDTH:
+		return String::compose("The JPEG2000 codestream uses a code block width of %1 instead of 32.", note.note().get());
+	case VerificationNote::Code::INVALID_JPEG2000_CODE_BLOCK_HEIGHT:
+		return String::compose("The JPEG2000 codestream uses a code block height of %1 instead of 32.", note.note().get());
+	case VerificationNote::Code::INCORRECT_JPEG2000_POC_MARKER_COUNT_FOR_2K:
+		return String::compose("%1 POC markers found in 2K JPEG2000 codestream instead of 0.", note.note().get());
+	case VerificationNote::Code::INCORRECT_JPEG2000_POC_MARKER_COUNT_FOR_4K:
+		return String::compose("%1 POC markers found in 4K JPEG2000 codestream instead of 1.", note.note().get());
+	case VerificationNote::Code::INCORRECT_JPEG2000_POC_MARKER:
+		return String::compose("Incorrect POC marker content found (%1)", note.note().get());
+	case VerificationNote::Code::INVALID_JPEG2000_POC_MARKER_LOCATION:
+		return "POC marker found outside main header";
+	case VerificationNote::Code::INVALID_JPEG2000_TILE_PARTS_FOR_2K:
+		return String::compose("The JPEG2000 codestream has %1 tile parts in a 2K image instead of 3.", note.note().get());
+	case VerificationNote::Code::INVALID_JPEG2000_TILE_PARTS_FOR_4K:
+		return String::compose("The JPEG2000 codestream has %1 tile parts in a 4K image instead of 6.", note.note().get());
+	case VerificationNote::Code::MISSING_JPEG200_TLM_MARKER:
+		return "No TLM marker was found in a JPEG2000 codestream.";
 	}
 
 	return "";
 }
+
+
+bool
+dcp::operator== (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
+{
+	return a.type() == b.type() && a.code() == b.code() && a.note() == b.note() && a.file() == b.file() && a.line() == b.line();
+}
+
+
+std::ostream&
+dcp::operator<< (std::ostream& s, dcp::VerificationNote const& note)
+{
+	s << note_to_string (note);
+	if (note.note()) {
+		s << " [" << note.note().get() << "]";
+	}
+	if (note.file()) {
+		s << " [" << note.file().get() << "]";
+	}
+	if (note.line()) {
+		s << " [" << note.line().get() << "]";
+	}
+	return s;
+}
+