Use new pkl_id for a couple of PKL notes.

3 files changed, 31 insertions, 20 deletions

diff --git a/src/verify.cc b/src/verify.cc
index ade9865c..deebab4a 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -1588,8 +1588,6 @@ verify_cpl(Context& context, shared_ptr<const CPL> cpl)
 		}
 	}
 
-	using VN = dcp::VerificationNote;
-
 	if (cpl->release_territory()) {
 		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();
@@ -1611,8 +1609,6 @@ verify_cpl(Context& context, shared_ptr<const CPL> cpl)
 		}
 	}
 
-	using VN = dcp::VerificationNote;
-
 	for (auto version: cpl->content_versions()) {
 		if (version.label_text.empty()) {
 			context.warning(VerificationNote::Code::EMPTY_CONTENT_VERSION_LABEL_TEXT, cpl->file().get());
@@ -1666,7 +1662,7 @@ verify_cpl(Context& context, shared_ptr<const CPL> cpl)
 		}
 
 		if (required_annotation_text && i->annotation_text() != required_annotation_text) {
-			context.bv21_error(VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, i->id(), i->file().get());
+			context.add_note(VN(VN::Type::BV21_ERROR, VN::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, i->file().get()).set_pkl_id(i->id()));
 		} else {
 			context.ok(VerificationNote::Code::MATCHING_PKL_ANNOTATION_TEXT_WITH_CPL);
 		}
@@ -1834,11 +1830,15 @@ verify_pkl(Context& context, shared_ptr<const PKL> pkl)
 {
 	validate_xml(context, pkl->file().get());
 
+	using VN = dcp::VerificationNote;
+
 	if (pkl_has_encrypted_assets(context.dcp, pkl)) {
 		cxml::Document doc("PackingList");
 		doc.read_file(dcp::filesystem::fix_long_path(pkl->file().get()));
 		if (!doc.optional_node_child("Signature")) {
-			context.bv21_error(VerificationNote::Code::UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT, pkl->id(), pkl->file().get());
+			context.add_note(
+				VN(VN::Type::BV21_ERROR, VN::Code::UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT, pkl->file().get()).set_pkl_id(pkl->id())
+			);
 		}
 	}
 
@@ -2132,9 +2132,9 @@ dcp::note_to_string(VerificationNote note, function<string (string)> process_str
 	case VerificationNote::Code::UNSIGNED_CPL_WITH_ENCRYPTED_CONTENT:
 		return compose("The CPL %1, which has encrypted content, is not signed.", note.cpl_id().get());
 	case VerificationNote::Code::UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT:
-		return compose("The PKL %1, which has encrypted content, is not signed.", note.note().get());
+		return compose("The PKL %1, which has encrypted content, is not signed.", note.pkl_id().get());
 	case VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL:
-		return compose("The PKL %1 has only one CPL but its <AnnotationText> does not match the CPL's <ContentTitleText>.", note.note().get());
+		return compose("The PKL %1 has only one CPL but its <AnnotationText> does not match the CPL's <ContentTitleText>.", note.pkl_id().get());
 	case VerificationNote::Code::MATCHING_PKL_ANNOTATION_TEXT_WITH_CPL:
 		return process_string("The PKL and CPL annotation texts match.");
 	case VerificationNote::Code::ALL_ENCRYPTED:
@@ -2268,6 +2268,7 @@ dcp::operator== (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
 		a.mismatched_asset_id_from_file() == b.mismatched_asset_id_from_file() &&
 		a.frame_rate() == b.frame_rate() &&
 		a.cpl_id() == b.cpl_id() &&
+		a.pkl_id() == b.pkl_id() &&
 		a.reference_hash() == b.reference_hash() &&
 		a.calculated_hash() == b.calculated_hash() &&
 		a.reel_index() == b.reel_index() &&
diff --git a/src/verify.h b/src/verify.h
index ce13cba6..e4a9649b 100644
--- a/src/verify.h
+++ b/src/verify.h
@@ -321,17 +321,17 @@ public:
 		 */
 		INCORRECT_LFOC,
 		/** There must be a _<CompositionMetadataAsset>_
-		 *  note contains the CPL ID
+		 *  cpl_id contains the CPL ID
 		 *  file contains the CPL filename
 		 */
 		MISSING_CPL_METADATA,
 		/** CPL metadata should contain _<VersionNumber>_ of 1, at least
-		 *  note contains the CPL ID
+		 *  cpl_id contains the CPL ID
 		 *  file contains the CPL filename
 		 */
 		MISSING_CPL_METADATA_VERSION_NUMBER,
 		/** There must be an _<ExtensionMetadata>_ in _<CompositionMetadataAsset>_ [Bv2.1_8.6.3]
-		 *  note contains the CPL ID
+		 *  cpl_id contains the CPL ID
 		 *  file contains the CPL filename
 		 */
 		MISSING_EXTENSION_METADATA,
@@ -341,17 +341,17 @@ public:
 		 */
 		INVALID_EXTENSION_METADATA,
 		/** A CPL containing encrypted content is not signed [Bv2.1_8.7]
-		 *  note contains the CPL ID
+		 *  cpl_id contains the CPL ID
 		 *  file contains the CPL filename
 		 */
 		UNSIGNED_CPL_WITH_ENCRYPTED_CONTENT,
 		/** A PKL containing encrypted content is not signed [Bv2.1_8.7]
-		 *  note contains the PKL ID
+		 *  pkl_id contains the PKL ID
 		 *  file contains the PKL filename
 		 */
 		UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT,
 		/** If a PKL has one CPL its _<ContentTitleText>_ must be the same as the PKL's _<AnnotationText>_
-		 *  note contains the PKL ID
+		 *  pkl_id contains the PKL ID
 		 *  file contains the PKL filename
 		 */
 		MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL,
@@ -607,6 +607,7 @@ private:
 		MISMATCHED_ASSET_ID_FROM_FILE,
 		FRAME_RATE,
 		CPL_ID,
+		PKL_ID,
 		CALCULATED_HASH,
 		REFERENCE_HASH,
 		REEL_INDEX,  ///< reel index, counting from 0
@@ -752,6 +753,15 @@ public:
 		return data<std::string>(Data::CPL_ID);
 	}
 
+	VerificationNote& set_pkl_id(std::string id) {
+		_data[Data::PKL_ID] = id;
+		return *this;
+	}
+
+	boost::optional<std::string> pkl_id() const {
+		return data<std::string>(Data::PKL_ID);
+	}
+
 	VerificationNote& set_reel_index(int index) {
 		_data[Data::REEL_INDEX] = index;
 		return *this;
diff --git a/test/verify_test.cc b/test/verify_test.cc
index 94553c9c..69aa756a 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -4636,8 +4636,8 @@ BOOST_AUTO_TEST_CASE (verify_unsigned_cpl_with_encrypted_content)
 				dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::MISMATCHED_CPL_HASHES, canonical(cpl_path)
 				).set_cpl_id(cpl->id()).set_reference_hash(calc.old_hash()).set_calculated_hash(calc.new_hash()),
 			dcp::VerificationNote(
-				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, encryption_test_pkl_id(), canonical(pkl)
-				).set_cpl_id(cpl->id()),
+				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, canonical(pkl)
+				).set_cpl_id(cpl->id()).set_pkl_id(encryption_test_pkl_id()),
 			dcp::VerificationNote(
 				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_FFEC_IN_FEATURE
 				).set_cpl_id(cpl->id()),
@@ -4689,8 +4689,8 @@ BOOST_AUTO_TEST_CASE (verify_unsigned_pkl_with_encrypted_content)
 			ok(dcp::VerificationNote::Code::VALID_PICTURE_FRAME_SIZES_IN_BYTES, canonical(dir / "video.mxf"), cpl).set_reel_index(0),
 			ok(dcp::VerificationNote::Code::MATCHING_CPL_HASHES, cpl),
 			dcp::VerificationNote(
-				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, encryption_test_pkl_id(), canonical(pkl)
-				).set_cpl_id(cpl->id()),
+				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISMATCHED_PKL_ANNOTATION_TEXT_WITH_CPL, canonical(pkl)
+				).set_cpl_id(cpl->id()).set_pkl_id(encryption_test_pkl_id()),
 			dcp::VerificationNote(
 				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_FFEC_IN_FEATURE
 				).set_cpl_id(cpl->id()),
@@ -4707,8 +4707,8 @@ BOOST_AUTO_TEST_CASE (verify_unsigned_pkl_with_encrypted_content)
 				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, canonical(cpl_path)
 				).set_cpl_id(cpl->id()),
 			dcp::VerificationNote(
-				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT, encryption_test_pkl_id(), canonical(pkl)
-				)
+				dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::UNSIGNED_PKL_WITH_ENCRYPTED_CONTENT, canonical(pkl)
+				).set_pkl_id(encryption_test_pkl_id())
 		});
 }