Call verify_j2k when verifying DCPs.

2 files changed, 85 insertions, 95 deletions

diff --git a/src/verify.cc b/src/verify.cc
index d8a4f37f..17f54e80 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -37,46 +37,47 @@
  */
 
 
-#include "verify.h"
-#include "dcp.h"
+#include "compose.hpp"
 #include "cpl.h"
+#include "dcp.h"
+#include "exceptions.h"
+#include "interop_subtitle_asset.h"
+#include "mono_picture_asset.h"
+#include "mono_picture_frame.h"
+#include "raw_convert.h"
 #include "reel.h"
 #include "reel_closed_caption_asset.h"
+#include "reel_markers_asset.h"
 #include "reel_picture_asset.h"
 #include "reel_sound_asset.h"
 #include "reel_subtitle_asset.h"
-#include "interop_subtitle_asset.h"
-#include "mono_picture_asset.h"
-#include "mono_picture_frame.h"
+#include "smpte_subtitle_asset.h"
 #include "stereo_picture_asset.h"
 #include "stereo_picture_frame.h"
-#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>
-#include <xercesc/parsers/AbstractDOMParser.hpp>
-#include <xercesc/sax/HandlerBase.hpp>
+#include "verify.h"
+#include "verify_j2k.h"
+#include <xercesc/dom/DOMAttr.hpp>
+#include <xercesc/dom/DOMDocument.hpp>
+#include <xercesc/dom/DOMError.hpp>
+#include <xercesc/dom/DOMErrorHandler.hpp>
+#include <xercesc/dom/DOMException.hpp>
 #include <xercesc/dom/DOMImplementation.hpp>
 #include <xercesc/dom/DOMImplementationLS.hpp>
 #include <xercesc/dom/DOMImplementationRegistry.hpp>
 #include <xercesc/dom/DOMLSParser.hpp>
-#include <xercesc/dom/DOMException.hpp>
-#include <xercesc/dom/DOMDocument.hpp>
-#include <xercesc/dom/DOMNodeList.hpp>
-#include <xercesc/dom/DOMError.hpp>
 #include <xercesc/dom/DOMLocator.hpp>
 #include <xercesc/dom/DOMNamedNodeMap.hpp>
-#include <xercesc/dom/DOMAttr.hpp>
-#include <xercesc/dom/DOMErrorHandler.hpp>
+#include <xercesc/dom/DOMNodeList.hpp>
 #include <xercesc/framework/LocalFileInputSource.hpp>
 #include <xercesc/framework/MemBufInputSource.hpp>
+#include <xercesc/parsers/AbstractDOMParser.hpp>
+#include <xercesc/parsers/XercesDOMParser.hpp>
+#include <xercesc/sax/HandlerBase.hpp>
+#include <xercesc/util/PlatformUtils.hpp>
 #include <boost/algorithm/string.hpp>
+#include <iostream>
 #include <map>
 #include <vector>
-#include <iostream>
 
 
 using std::list;
@@ -420,67 +421,56 @@ verify_language_tag (string tag, vector<VerificationNote>& notes)
 }
 
 
-enum class VerifyPictureAssetResult
-{
-	GOOD,
-	FRAME_NEARLY_TOO_LARGE,
-	BAD,
-};
-
-
-int
-biggest_frame_size (shared_ptr<const MonoPictureFrame> frame)
-{
-	return frame->size ();
-}
-
-int
-biggest_frame_size (shared_ptr<const StereoPictureFrame> frame)
+static void
+verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, boost::filesystem::path file, vector<VerificationNote>& notes, function<void (float)> progress)
 {
-	return max(frame->left()->size(), frame->right()->size());
-}
+	int biggest_frame = 0;
+	auto asset = dynamic_pointer_cast<PictureAsset>(reel_file_asset->asset_ref().asset());
+	auto const duration = asset->intrinsic_duration ();
 
+	auto check_and_add = [&notes](vector<VerificationNote> const& j2k_notes) {
+		for (auto i: j2k_notes) {
+			if (find(notes.begin(), notes.end(), i) == notes.end()) {
+				notes.push_back (i);
+			}
+		}
+	};
 
-template <class A, class R, class F>
-optional<VerifyPictureAssetResult>
-verify_picture_asset_type (shared_ptr<const ReelFileAsset> reel_file_asset, function<void (float)> progress)
-{
-	auto asset = dynamic_pointer_cast<A>(reel_file_asset->asset_ref().asset());
-	if (!asset) {
-		return optional<VerifyPictureAssetResult>();
-	}
+	if (auto mono_asset = dynamic_pointer_cast<MonoPictureAsset>(reel_file_asset->asset_ref().asset())) {
+		auto reader = mono_asset->start_read ();
+		for (int64_t i = 0; i < duration; ++i) {
+			auto frame = reader->get_frame (i);
+			biggest_frame = max(biggest_frame, frame->size());
+			vector<VerificationNote> j2k_notes;
+			verify_j2k (frame, j2k_notes);
+			check_and_add (j2k_notes);
+			progress (float(i) / duration);
+		}
+	} else if (auto stereo_asset = dynamic_pointer_cast<StereoPictureAsset>(asset)) {
+		auto reader = stereo_asset->start_read ();
+		for (int64_t i = 0; i < duration; ++i) {
+			auto frame = reader->get_frame (i);
+			biggest_frame = max(biggest_frame, max(frame->left()->size(), frame->right()->size()));
+			vector<VerificationNote> j2k_notes;
+			verify_j2k (frame->left(), j2k_notes);
+			verify_j2k (frame->right(), j2k_notes);
+			check_and_add (j2k_notes);
+			progress (float(i) / duration);
+		}
 
-	int biggest_frame = 0;
-	auto reader = asset->start_read ();
-	auto const duration = asset->intrinsic_duration ();
-	for (int64_t i = 0; i < duration; ++i) {
-		shared_ptr<const F> frame = reader->get_frame (i);
-		biggest_frame = max(biggest_frame, biggest_frame_size(frame));
-		progress (float(i) / duration);
 	}
 
 	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 VerifyPictureAssetResult::BAD;
+		notes.push_back ({
+			VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+		});
 	} else if (biggest_frame > risky_frame) {
-		return VerifyPictureAssetResult::FRAME_NEARLY_TOO_LARGE;
-	}
-
-	return VerifyPictureAssetResult::GOOD;
-}
-
-
-static VerifyPictureAssetResult
-verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, function<void (float)> 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_file_asset, progress);
+		notes.push_back ({
+			VerificationNote::Type::WARNING, VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+		});
 	}
-
-	DCP_ASSERT (r);
-	return *r;
 }
 
 
@@ -512,21 +502,7 @@ verify_main_picture_asset (
 			break;
 	}
 	stage ("Checking picture frame sizes", asset->file());
-	auto const pr = verify_picture_asset (reel_asset, progress);
-	switch (pr) {
-		case VerifyPictureAssetResult::BAD:
-			notes.push_back ({
-				VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
-			});
-			break;
-		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;
-	}
+	verify_picture_asset (reel_asset, file, notes, progress);
 
 	/* Only flat/scope allowed by Bv2.1 */
 	if (
diff --git a/test/verify_test.cc b/test/verify_test.cc
index cb7c539d..53ce1745 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -206,7 +206,6 @@ void
 check_verify_result (vector<path> dir, vector<dcp::VerificationNote> test_notes)
 {
 	auto notes = dcp::verify ({dir}, &stage, &progress, xsd_test);
-	dump_notes (notes);
 	BOOST_REQUIRE_EQUAL (notes.size(), test_notes.size());
 	for (auto i = 0U; i < notes.size(); ++i) {
 		BOOST_REQUIRE_EQUAL (notes[i], test_notes[i]);
@@ -525,10 +524,13 @@ BOOST_AUTO_TEST_CASE (verify_invalid_standard)
 	++st;
 	BOOST_REQUIRE (st == stages.end());
 
-	BOOST_REQUIRE_EQUAL (notes.size(), 1U);
+	BOOST_REQUIRE_EQUAL (notes.size(), 2U);
 	auto i = notes.begin ();
 	BOOST_CHECK_EQUAL (i->type(), dcp::VerificationNote::Type::BV21_ERROR);
 	BOOST_CHECK_EQUAL (i->code(), dcp::VerificationNote::Code::INVALID_STANDARD);
+	++i;
+	BOOST_CHECK_EQUAL (i->type(), dcp::VerificationNote::Type::BV21_ERROR);
+	BOOST_CHECK_EQUAL (i->code(), dcp::VerificationNote::Code::INVALID_JPEG2000_GUARD_BITS_FOR_2K);
 }
 
 /* DCP with a short asset */
@@ -542,7 +544,8 @@ BOOST_AUTO_TEST_CASE (verify_invalid_duration)
 			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_DURATION, string("d7576dcb-a361-4139-96b8-267f5f8d7f91") },
 			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_INTRINSIC_DURATION, string("d7576dcb-a361-4139-96b8-267f5f8d7f91") },
 			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_DURATION, string("a2a87f5d-b749-4a7e-8d0c-9d48a4abf626") },
-			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_INTRINSIC_DURATION, string("a2a87f5d-b749-4a7e-8d0c-9d48a4abf626") }
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_INTRINSIC_DURATION, string("a2a87f5d-b749-4a7e-8d0c-9d48a4abf626") },
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_GUARD_BITS_FOR_2K, string("2") }
 		});
 }
 
@@ -585,6 +588,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_picture_frame_size_in_bytes)
 	check_verify_result (
 		{ dir },
 		{
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte") },
 			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") },
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
 		});
@@ -612,6 +616,7 @@ BOOST_AUTO_TEST_CASE (verify_nearly_invalid_picture_frame_size_in_bytes)
 	check_verify_result (
 		{ dir },
 		{
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte") },
 			{ dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") },
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
 		});
@@ -1047,7 +1052,6 @@ void
 check_picture_size_ok (int width, int height, int frame_rate, bool three_d)
 {
 	auto notes = check_picture_size(width, height, frame_rate, three_d);
-	dump_notes (notes);
 	BOOST_CHECK_EQUAL (notes.size(), 0U);
 }
 
@@ -1115,7 +1119,7 @@ BOOST_AUTO_TEST_CASE (verify_picture_size)
 	check_picture_size_bad_frame_size (2050, 858, 24, false);
 	check_picture_size_bad_frame_size (2048, 658, 25, false);
 	check_picture_size_bad_frame_size (1920, 1080, 48, true);
-	check_picture_size_bad_frame_size (4000, 3000, 24, true);
+	check_picture_size_bad_frame_size (4000, 2000, 24, true);
 
 	/* Bad 2K frame rate */
 	check_picture_size_bad_2k_frame_rate (2048, 858, 26, false);
@@ -2682,6 +2686,8 @@ BOOST_AUTO_TEST_CASE (verify_unsigned_cpl_with_encrypted_content)
 		{
 			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::MISMATCHED_CPL_HASHES, encryption_test_cpl_id, canonical(cpl) },
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, encryption_test_pkl_id, canonical(pkl), },
+			/* It's encrypted so the J2K validity checks will fail */
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte") },
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_FFEC_IN_FEATURE },
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_FFMC_IN_FEATURE },
 			{ dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::MISSING_FFOC },
@@ -2711,6 +2717,8 @@ BOOST_AUTO_TEST_CASE (verify_unsigned_pkl_with_encrypted_content)
 		{dir},
 		{
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, encryption_test_pkl_id, canonical(pkl) },
+			/* It's encrypted so the J2K validity checks will fail */
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte") },
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_FFEC_IN_FEATURE },
 			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_FFMC_IN_FEATURE },
 			{ dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::MISSING_FFOC },
@@ -2793,7 +2801,13 @@ BOOST_AUTO_TEST_CASE (verify_partially_encrypted)
 
 	d.write_xml (dcp::Standard::SMPTE, "OpenDCP 0.0.25", "OpenDCP 0.0.25", "2012-07-17T04:45:18+00:00", "A Test DCP", signer);
 
-	check_verify_result ({dir}, {{dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::PARTIALLY_ENCRYPTED}});
+	check_verify_result (
+		{dir},
+		{
+			{dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::PARTIALLY_ENCRYPTED},
+			/* It's encrypted so the J2K validity checks will fail */
+			{dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte")}
+		});
 }
 
 
@@ -2804,7 +2818,7 @@ BOOST_AUTO_TEST_CASE (verify_jpeg2000_codestream_2k)
 	auto reader = picture.start_read ();
 	auto frame = reader->get_frame (0);
 	verify_j2k (frame, notes);
-	dump_notes (notes);
+	BOOST_REQUIRE_EQUAL (notes.size(), 0U);
 }
 
 
@@ -2815,7 +2829,7 @@ BOOST_AUTO_TEST_CASE (verify_jpeg2000_codestream_4k)
 	auto reader = picture.start_read ();
 	auto frame = reader->get_frame (0);
 	verify_j2k (frame, notes);
-	dump_notes (notes);
+	BOOST_REQUIRE_EQUAL (notes.size(), 0U);
 }
 
 
@@ -2830,6 +2844,6 @@ BOOST_AUTO_TEST_CASE (verify_jpeg2000_codestream_libdcp)
 	auto reader = picture.start_read ();
 	auto frame = reader->get_frame (0);
 	verify_j2k (frame, notes);
-	dump_notes (notes);
+	BOOST_REQUIRE_EQUAL (notes.size(), 0U);
 }