Verify that main picture active area is valid (even, and not too big) (#2392).v1.8.40

3 files changed, 110 insertions, 3 deletions

diff --git a/src/verify.cc b/src/verify.cc
index d4d63902..197e5183 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -1389,6 +1389,24 @@ dcp::verify (
 			size_t most_closed_captions = 0;
 			map<Marker, Time> markers_seen;
 
+			auto const main_picture_active_area = cpl->main_picture_active_area();
+			if (main_picture_active_area && (main_picture_active_area->width % 2)) {
+				notes.push_back({
+						VerificationNote::Type::ERROR,
+						VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA,
+						String::compose("width %1 is not a multiple of 2", main_picture_active_area->width),
+						cpl->file().get()
+					});
+			}
+			if (main_picture_active_area && (main_picture_active_area->height % 2)) {
+				notes.push_back({
+						VerificationNote::Type::ERROR,
+						VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA,
+						String::compose("height %1 is not a multiple of 2", main_picture_active_area->height),
+						cpl->file().get()
+					});
+			}
+
 			for (auto reel: cpl->reels()) {
 				stage ("Checking reel", optional<boost::filesystem::path>());
 
@@ -1437,6 +1455,25 @@ dcp::verify (
 					/* Check asset */
 					if (reel->main_picture()->asset_ref().resolved()) {
 						verify_main_picture_asset (dcp, reel->main_picture(), stage, progress, notes);
+						auto const asset_size = reel->main_picture()->asset()->size();
+						if (main_picture_active_area) {
+							if (main_picture_active_area->width > asset_size.width) {
+								notes.push_back({
+										VerificationNote::Type::ERROR,
+										VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA,
+										String::compose("width %1 is bigger than the asset width %2", main_picture_active_area->width, asset_size.width),
+										cpl->file().get()
+										});
+							}
+							if (main_picture_active_area->height > asset_size.height) {
+								notes.push_back({
+										VerificationNote::Type::ERROR,
+										VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA,
+										String::compose("height %1 is bigger than the asset height %2", main_picture_active_area->height, asset_size.height),
+										cpl->file().get()
+										});
+							}
+						}
 					}
 				}
 
@@ -1785,6 +1822,8 @@ dcp::note_to_string (VerificationNote note)
 		return "There is an <Duration> node inside a <MainMarkers>.";
 	case VerificationNote::Code::INVALID_CONTENT_KIND:
 		return String::compose("<ContentKind> has an invalid value %1.", note.note().get());
+	case VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA:
+		return String::compose("<MainPictureActiveaArea> has an invalid value: %1", note.note().get());
 	}
 
 	return "";
diff --git a/src/verify.h b/src/verify.h
index baa36138..c7c4b6c8 100644
--- a/src/verify.h
+++ b/src/verify.h
@@ -398,7 +398,12 @@ public:
 		/** Some <MainMarkers> asset has an <Duration> that should not be there */
 		UNEXPECTED_DURATION,
 		/** A <ContentKind> has been specified with either no scope or the SMPTE 429-7 scope, but which is not one of those allowed */
-		INVALID_CONTENT_KIND
+		INVALID_CONTENT_KIND,
+		/** Either the width or height of a <MainPictureActiveArea> in a CPL is either not an even number, or bigger than the corresponding asset dimension.
+		 *  note contains details of what is wrong
+		 *  file contains the CPL filename
+		 */
+		INVALID_MAIN_PICTURE_ACTIVE_AREA,
 	};
 
 	VerificationNote (Type type, Code code)
diff --git a/test/verify_test.cc b/test/verify_test.cc
index 52d774bd..1122f73b 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -1192,8 +1192,8 @@ check_picture_size (int width, int height, int frame_rate, bool three_d)
 	cpl->set_issue_date ("2012-07-17T04:45:18+00:00");
 	cpl->set_main_sound_configuration ("L,C,R,Lfe,-,-");
 	cpl->set_main_sound_sample_rate (48000);
-	cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
-	cpl->set_main_picture_active_area (dcp::Size(1998, 1080));
+	cpl->set_main_picture_stored_area(dcp::Size(width, height));
+	cpl->set_main_picture_active_area(dcp::Size(width, height));
 	cpl->set_version_number (1);
 
 	auto reel = make_shared<dcp::Reel>();
@@ -3280,3 +3280,66 @@ BOOST_AUTO_TEST_CASE(verify_valid_content_kind)
 		});
 
 }
+
+
+BOOST_AUTO_TEST_CASE(verify_invalid_main_picture_active_area_1)
+{
+	path dir = "build/test/verify_invalid_main_picture_active_area_1";
+	prepare_directory(dir);
+	auto dcp = make_simple(dir, 1, 24);
+	dcp->write_xml();
+
+	auto constexpr area = "<meta:MainPictureActiveArea>";
+
+	{
+		Editor e(find_cpl(dir));
+		e.delete_lines_after(area, 2);
+		e.insert(area, "<meta:Height>4080</meta:Height>");
+		e.insert(area, "<meta:Width>1997</meta:Width>");
+	}
+
+	dcp::PKL pkl(find_pkl(dir));
+	dcp::CPL cpl(find_cpl(dir));
+
+	check_verify_result(
+		{ dir },
+		{
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::MISMATCHED_CPL_HASHES, cpl.id(), canonical(find_cpl(dir)) },
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, pkl.id(), canonical(find_pkl(dir)), },
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA, "width 1997 is not a multiple of 2", canonical(find_cpl(dir)) },
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA, "height 4080 is bigger than the asset height 1080", canonical(find_cpl(dir)) },
+		});
+}
+
+
+BOOST_AUTO_TEST_CASE(verify_invalid_main_picture_active_area_2)
+{
+	path dir = "build/test/verify_invalid_main_picture_active_area_2";
+	prepare_directory(dir);
+	auto dcp = make_simple(dir, 1, 24);
+	dcp->write_xml();
+
+	auto constexpr area = "<meta:MainPictureActiveArea>";
+
+	{
+		Editor e(find_cpl(dir));
+		e.delete_lines_after(area, 2);
+		e.insert(area, "<meta:Height>5125</meta:Height>");
+		e.insert(area, "<meta:Width>9900</meta:Width>");
+	}
+
+	dcp::PKL pkl(find_pkl(dir));
+	dcp::CPL cpl(find_cpl(dir));
+
+	check_verify_result(
+		{ dir },
+		{
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::MISMATCHED_CPL_HASHES, cpl.id(), canonical(find_cpl(dir)) },
+			{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, pkl.id(), canonical(find_pkl(dir)), },
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA, "height 5125 is not a multiple of 2", canonical(find_cpl(dir)) },
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA, "width 9900 is bigger than the asset width 1998", canonical(find_cpl(dir)) },
+			{ dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_MAIN_PICTURE_ACTIVE_AREA, "height 5125 is bigger than the asset height 1080", canonical(find_cpl(dir)) },
+		});
+}
+
+