Clarify ID in verification note.

4 files changed, 42 insertions, 27 deletions

diff --git a/src/dcp.cc b/src/dcp.cc
index 049cef12..d7245ed8 100644
--- a/src/dcp.cc
+++ b/src/dcp.cc
@@ -270,7 +270,7 @@ DCP::read (vector<dcp::VerificationNote>* notes, bool ignore_incorrect_picture_m
 			bool found_threed_marked_as_twod = false;
 			auto asset = asset_factory(path, ignore_incorrect_picture_mxf_type, &found_threed_marked_as_twod);
 			if (asset->id() != id) {
-				notes->push_back(dcp::VerificationNote{VerificationNote::Code::MISMATCHED_ASSET_MAP_ID}.set_id(id).set_other_id(asset->id()));
+				notes->push_back(dcp::VerificationNote{VerificationNote::Code::MISMATCHED_ASSET_MAP_ID}.set_asset_id(id).set_other_asset_id(asset->id()));
 			}
 			other_assets.push_back(asset);
 			if (found_threed_marked_as_twod && notes) {
diff --git a/src/verify.cc b/src/verify.cc
index 6c0ec96b..5e48e0b3 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -1024,7 +1024,7 @@ verify_text_details (
 		reel_offset = end;
 
 		if (context.dcp->standard() && *context.dcp->standard() == dcp::Standard::SMPTE && has_text && font_ids.empty()) {
-			context.add_note(dcp::VerificationNote(VerificationNote::Code::MISSING_LOAD_FONT).set_id(id(reels[i])));
+			context.add_note(dcp::VerificationNote(VerificationNote::Code::MISSING_LOAD_FONT).set_asset_id(id(reels[i])));
 		}
 	}
 
@@ -1057,7 +1057,7 @@ verify_text_details (
 	}
 
 	if (missing_load_font_id) {
-		context.add_note(dcp::VerificationNote(VerificationNote::Code::MISSING_LOAD_FONT_FOR_FONT).set_id(*missing_load_font_id));
+		context.add_note(dcp::VerificationNote(VerificationNote::Code::MISSING_LOAD_FONT_FOR_FONT).set_load_font_id(*missing_load_font_id));
 	}
 }
 
@@ -2190,11 +2190,11 @@ dcp::note_to_string(VerificationNote note, function<string (string)> process_str
 	case VerificationNote::Code::INCORRECT_SUBTITLE_NAMESPACE_COUNT:
 		return compose("The XML in the subtitle asset %1 has more than one namespace declaration.", note.note().get());
 	case VerificationNote::Code::MISSING_LOAD_FONT_FOR_FONT:
-		return compose("A subtitle or closed caption refers to a font with ID %1 that does not have a corresponding <LoadFont> node", note.id().get());
+		return compose("A subtitle or closed caption refers to a font with ID %1 that does not have a corresponding <LoadFont> node", note.load_font_id().get());
 	case VerificationNote::Code::MISSING_LOAD_FONT:
-		return compose("The SMPTE subtitle asset %1 has <Text> nodes but no <LoadFont> node", note.id().get());
+		return compose("The SMPTE subtitle asset %1 has <Text> nodes but no <LoadFont> node", note.asset_id().get());
 	case VerificationNote::Code::MISMATCHED_ASSET_MAP_ID:
-		return compose("The asset with ID %1 in the asset map actually has an id of %2", note.id().get(), note.other_id().get());
+		return compose("The asset with ID %1 in the asset map actually has an id of %2", note.asset_id().get(), note.other_asset_id().get());
 	case VerificationNote::Code::EMPTY_CONTENT_VERSION_LABEL_TEXT:
 		return compose("The <LabelText> in a <ContentVersion> in CPL %1 is empty", note.cpl_id().get());
 	case VerificationNote::Code::VALID_CONTENT_VERSION_LABEL_TEXT:
@@ -2356,8 +2356,9 @@ dcp::operator== (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
 		a.frame() == b.frame() &&
 		a.component() == b.component() &&
 		a.size() == b.size() &&
-		a.id() == b.id() &&
-		a.other_id() == b.other_id() &&
+		a.load_font_id() == b.load_font_id() &&
+		a.asset_id() == b.asset_id() &&
+		a.other_asset_id() == b.other_asset_id() &&
 		a.frame_rate() == b.frame_rate() &&
 		a.cpl_id() == b.cpl_id() &&
 		a.reference_hash() == b.reference_hash() &&
@@ -2408,12 +2409,16 @@ dcp::operator< (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
 		return a.size().get_value_or(0) < b.size().get_value_or(0);
 	}
 
-	if (a.id() != b.id()) {
-		return a.id().get_value_or("") < b.id().get_value_or("");
+	if (a.load_font_id() != b.load_font_id()) {
+		return a.load_font_id().get_value_or("") < b.load_font_id().get_value_or("");
 	}
 
-	if (a.other_id() != b.other_id()) {
-		return a.other_id().get_value_or("") < b.other_id().get_value_or("");
+	if (a.asset_id() != b.asset_id()) {
+		return a.asset_id().get_value_or("") < b.asset_id().get_value_or("");
+	}
+
+	if (a.other_asset_id() != b.other_asset_id()) {
+		return a.other_asset_id().get_value_or("") < b.other_asset_id().get_value_or("");
 	}
 
 	if (a.cpl_id() != b.cpl_id()) {
diff --git a/src/verify.h b/src/verify.h
index ce097578..af84a77f 100644
--- a/src/verify.h
+++ b/src/verify.h
@@ -624,8 +624,9 @@ private:
 		FRAME,
 		COMPONENT,
 		SIZE,
-		ID,
-		OTHER_ID,
+		LOAD_FONT_ID,
+		ASSET_ID,
+		OTHER_ASSET_ID,
 		FRAME_RATE,
 		CPL_ID,
 		CALCULATED_HASH,
@@ -683,22 +684,31 @@ public:
 		return data<int>(Data::SIZE);
 	}
 
-	VerificationNote& set_id(std::string id) {
-		_data[Data::ID] = id;
+	VerificationNote& set_load_font_id(std::string id) {
+		_data[Data::LOAD_FONT_ID] = id;
 		return *this;
 	}
 
-	boost::optional<std::string> id() const {
-		return data<std::string>(Data::ID);
+	boost::optional<std::string> load_font_id() const {
+		return data<std::string>(Data::LOAD_FONT_ID);
 	}
 
-	VerificationNote& set_other_id(std::string other_id) {
-		_data[Data::OTHER_ID] = other_id;
+	VerificationNote& set_asset_id(std::string id) {
+		_data[Data::ASSET_ID] = id;
 		return *this;
 	}
 
-	boost::optional<std::string> other_id() const {
-		return data<std::string>(Data::OTHER_ID);
+	boost::optional<std::string> asset_id() const {
+		return data<std::string>(Data::ASSET_ID);
+	}
+
+	VerificationNote& set_other_asset_id(std::string other_id) {
+		_data[Data::OTHER_ASSET_ID] = other_id;
+		return *this;
+	}
+
+	boost::optional<std::string> other_asset_id() const {
+		return data<std::string>(Data::OTHER_ASSET_ID);
 	}
 
 	VerificationNote& set_frame_rate(int frame_rate) {
diff --git a/test/verify_test.cc b/test/verify_test.cc
index 89981dbe..5472f38e 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -254,8 +254,8 @@ to_string(dcp::VerificationNote const& note)
 
 	s += dcp::String::compose(
 		"%1 %2 %3 %4 %5]\n",
-		note.id().get_value_or("<none>"),
-		note.other_id().get_value_or("<none>"),
+		note.asset_id().get_value_or("<none>"),
+		note.other_asset_id().get_value_or("<none>"),
 		note.cpl_id().get_value_or("<none>"),
 		note.reference_hash().get_value_or("<none>"),
 		note.calculated_hash().get_value_or("<none>")
@@ -5605,7 +5605,7 @@ BOOST_AUTO_TEST_CASE(verify_missing_load_font_for_font)
 			note(VC::VALID_CONTENT_KIND, string{"trailer"}, cpl),
 			note(VC::VALID_CONTENT_VERSION_LABEL_TEXT, cpl->content_version()->label_text, cpl),
 			{ VC::INVALID_STANDARD },
-			VN(VC::MISSING_LOAD_FONT_FOR_FONT).set_id("theFontId").set_cpl_id(cpl->id())
+			VN(VC::MISSING_LOAD_FONT_FOR_FONT).set_load_font_id("theFontId").set_cpl_id(cpl->id())
 		});
 
 }
@@ -5670,7 +5670,7 @@ BOOST_AUTO_TEST_CASE(verify_missing_load_font)
 			note(VC::VALID_CONTENT_VERSION_LABEL_TEXT, cpl->content_version()->label_text, cpl),
 			note(VC::CORRECT_PICTURE_HASH, canonical(dir / "video.mxf"), cpl).set_reel_index(0),
 			note(VC::VALID_PICTURE_FRAME_SIZES_IN_BYTES, canonical(dir / "video.mxf"), cpl).set_reel_index(0),
-			VN(VC::MISSING_LOAD_FONT).set_id(reel_subs->id()).set_cpl_id(cpl->id()).set_reel_index(0)
+			VN(VC::MISSING_LOAD_FONT).set_asset_id(reel_subs->id()).set_cpl_id(cpl->id()).set_reel_index(0)
 		});
 }
 
@@ -5711,7 +5711,7 @@ BOOST_AUTO_TEST_CASE(verify_spots_wrong_asset)
 			note(VC::VALID_CONTENT_KIND, string{"trailer"}, cpl),
 			note(VC::VALID_CPL_ANNOTATION_TEXT, string{"A Test DCP"}, cpl),
 			note(VC::VALID_CONTENT_VERSION_LABEL_TEXT, cpl->content_version()->label_text, cpl),
-			VN(VC::MISMATCHED_ASSET_MAP_ID).set_id(asset_1).set_other_id(asset_2)
+			VN(VC::MISMATCHED_ASSET_MAP_ID).set_asset_id(asset_1).set_other_asset_id(asset_2)
 		});
 }