commit cf041601e27ce7aed2ddc805744e60d28320c502
parent 2723117d95d8e234ea20d1425c1be6fba2046400
Author: Gavin Peacock <gpeacock@adobe.com>
Date: Thu, 13 Jul 2023 13:47:08 -0700
Fix for #195 make_test_images missing ingredient references (#254)
* remove e-dat-ca test image
relies on XMP that is no longer written, use XCA instead.
* Remove deduplication logic when adding ingredients.
This caused some errors and is better left to the calling application to optimize
* test image actions can refer to the same ingredient
* update test images
* add source type and softwareagent to c.jpg
* update all generated unit test images
* remove tests for provenance that are no longer valid
* fix xmp modification test
* removes e-dat-ca test image
* update readme and refresh images
Diffstat:
12 files changed, 47 insertions(+), 51 deletions(-)
diff --git a/README.md b/README.md
@@ -98,6 +98,9 @@ Refer to the [CHANGELOG](https://github.com/contentauth/c2pa-rs/blob/main/CHANGE
_14 July 2023_
* (important!) the add_thumbnails feature is no longer tied to file_io, so you will need to specify it or thumbnails will not be generated.
* DataHash and BoxHash SDK support (generates a signed manifest ready to write into a file without writing to the file)
+* The SDK will no longer remove duplicate ingredients based on hash
+* make_test_images updated to fix issue 195, actions without required ingredients
+* updated the test fixtures generated by make_test_images
* Expose CAIRead and CAIWrite traits required by some SDK calls.
* Bug fix for certain BMFF formats (AVIF) that causes images to be unreadable
diff --git a/make_test_images/src/make_test_images.rs b/make_test_images/src/make_test_images.rs
@@ -13,6 +13,7 @@
//! Constructs a set of test images using a configuration script
use std::{
+ collections::HashMap,
fs,
path::{Path, PathBuf},
};
@@ -199,6 +200,8 @@ impl MakeTestImages {
}
let generator = format!("{}/{}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"));
+ let software_agent = format!("{} {}", "Make Test Images", env!("CARGO_PKG_VERSION"));
+
let mut manifest = Manifest::new(generator);
manifest.set_vendor("contentauth".to_owned()); // needed for generating error cases below
@@ -209,6 +212,9 @@ impl MakeTestImages {
manifest.add_assertion(&creative_work)?;
}
+ // keep track of ingredient instances so we don't duplicate them
+ let mut ingredient_table = HashMap::new();
+
// process parent first
let mut img = match src {
Some(src) => {
@@ -216,11 +222,16 @@ impl MakeTestImages {
let parent = Ingredient::from_file_with_options(src_path, &ImageOptions::new())?;
+ let instance_id = parent.instance_id().to_string();
+
actions = actions.add_action(
Action::new(c2pa_action::OPENED).set_instance_id(parent.instance_id()),
);
manifest.set_parent(parent)?;
+ // keep track of all ingredients we add via the instance Id
+ ingredient_table.insert(src, instance_id);
+
// load the image for editing
let mut img =
image::open(src_path).context(format!("opening parent {src_path:?}"))?;
@@ -244,13 +255,14 @@ impl MakeTestImages {
*pixel = image::Rgb([r, 100, b]);
}
}
- actions = actions
- .add_action(Action::new(c2pa_action::CREATED))
- .add_action(
- Action::new(c2pa_action::DRAWING)
- .set_parameter("name".to_owned(), "gradient")?,
- );
-
+ actions = actions.add_action(
+ Action::new(c2pa_action::CREATED)
+ .set_source_type(
+ "http://cv.iptc.org/newscodes/digitalsourcetype/algorithmicMedia",
+ )
+ .set_software_agent(software_agent.as_str())
+ .set_parameter("name".to_owned(), "gradient")?,
+ );
img
}
};
@@ -274,13 +286,21 @@ impl MakeTestImages {
let img_small = img_ingredient.thumbnail(width, height);
image::imageops::overlay(&mut img, &img_small, x, 0);
- // create and add the ingredient
- let ingredient =
- Ingredient::from_file_with_options(ing_path, &ImageOptions::new())?;
- actions = actions.add_action(
- Action::new(c2pa_action::PLACED).set_instance_id(ingredient.instance_id()),
- );
- manifest.add_ingredient(ingredient);
+ // if we have already created an ingredient, get the instanceId, otherwise create a new one
+ let instance_id = match ingredient_table.get(ing.as_str()) {
+ Some(id) => id.to_owned(),
+ None => {
+ let ingredient =
+ Ingredient::from_file_with_options(ing_path, &ImageOptions::new())?;
+ let instance_id = ingredient.instance_id().to_string();
+ ingredient_table.insert(ing, instance_id.clone());
+ manifest.add_ingredient(ingredient);
+ instance_id
+ }
+ };
+
+ actions = actions
+ .add_action(Action::new(c2pa_action::PLACED).set_instance_id(instance_id));
x += width as i64;
}
diff --git a/make_test_images/tests.json b/make_test_images/tests.json
@@ -20,7 +20,6 @@
{ "op": "make", "ingredients": ["CA","CA","CA"], "output": "CICACACA" },
{ "op": "ogp", "parent": "CA", "output": "XCA" },
{ "op": "ogp", "parent": "CI", "output": "XCI" },
- { "op": "dat", "parent": "CA", "output": "E-dat-CA" },
{ "op": "sig", "parent": "CA", "output": "E-sig-CA" },
{ "op": "uri", "parent": "CA", "output": "E-uri-CA" },
{ "op": "clm", "parent": "CAICAI", "output": "E-clm-CAICAI" },
diff --git a/sdk/src/asset_handlers/bmff_io.rs b/sdk/src/asset_handlers/bmff_io.rs
@@ -1660,7 +1660,7 @@ pub mod tests {
use crate::utils::test::{fixture_path, temp_dir_path};
#[cfg(not(target_arch = "wasm32"))]
- #[cfg(file_io)]
+ #[cfg(feature = "file_io")]
#[test]
fn test_read_mp4() {
use crate::{
diff --git a/sdk/src/ingredient.rs b/sdk/src/ingredient.rs
@@ -1372,7 +1372,6 @@ mod tests {
println!("ingredient = {ingredient}");
assert_eq!(&ingredient.title, title);
assert_eq!(ingredient.format(), format);
- assert!(ingredient.provenance().is_some());
assert!(ingredient.manifest_data().is_some());
assert!(ingredient.metadata().is_none());
#[cfg(target_arch = "wasm32")]
@@ -1396,7 +1395,6 @@ mod tests {
println!("ingredient = {ingredient}");
assert_eq!(&ingredient.title, title);
assert_eq!(ingredient.format(), format);
- assert!(ingredient.provenance().is_some());
assert!(ingredient.manifest_data().is_some());
assert!(ingredient.metadata().is_none());
assert!(ingredient.validation_status().is_none());
@@ -1561,7 +1559,6 @@ mod tests_file_io {
assert_eq!(&ingredient.title, MANIFEST_JPEG);
assert_eq!(ingredient.format(), "image/jpeg");
assert!(ingredient.thumbnail().is_some()); // we don't generate this thumbnail
- assert!(ingredient.provenance().is_some());
assert!(ingredient.manifest_data().is_some());
assert!(ingredient.metadata().is_none());
}
@@ -1616,7 +1613,6 @@ mod tests_file_io {
assert_eq!(ingredient.format(), "image/jpeg");
assert!(ingredient.hash().is_some());
assert!(ingredient.thumbnail().is_some()); // always generated
- assert!(ingredient.provenance().is_some());
assert!(ingredient.manifest_data().is_some());
assert!(ingredient.metadata().is_none());
}
@@ -1646,7 +1642,6 @@ mod tests_file_io {
assert_eq!(ingredient.title(), BAD_SIGNATURE_JPEG);
assert_eq!(ingredient.format(), "image/jpeg");
test_thumbnail(&ingredient, "image/jpeg");
- assert!(ingredient.provenance().is_some());
assert!(ingredient.manifest_data().is_some());
assert!(ingredient.validation_status().is_some());
assert!(ingredient
diff --git a/sdk/src/manifest.rs b/sdk/src/manifest.rs
@@ -15,7 +15,7 @@ use std::{borrow::Cow, collections::HashMap, io::Cursor};
#[cfg(feature = "file_io")]
use std::{fs::create_dir_all, path::Path};
-use log::{debug, error, warn};
+use log::{debug, error};
#[cfg(feature = "json_schema")]
use schemars::JsonSchema;
use serde::{de::DeserializeOwned, Deserialize, Serialize};
@@ -303,38 +303,15 @@ impl Manifest {
error!("parent already added");
return Err(Error::BadParam("Parent parent already added".to_owned()));
}
- // if the hash of our new ingredient does not match any of the ingredients
- // then add it
- if !self
- .ingredients
- .iter()
- .any(|i| ingredient.hash().is_some() && i.hash() == ingredient.hash())
- {
- debug!("ingredients:set_is_parent {:?}", ingredient.title());
- ingredient.set_is_parent();
- self.ingredients.insert(0, ingredient);
- } else {
- // dup so just keep the ingredient instead of adding the parent
- warn!("duplicate parent {}", ingredient.title());
- }
+ ingredient.set_is_parent();
+ self.ingredients.insert(0, ingredient);
Ok(self)
}
/// Add an ingredient removing duplicates (consumes the asset)
pub fn add_ingredient(&mut self, ingredient: Ingredient) -> &mut Self {
- // if the hash of the new asset does not match any of the ingredients
- // then add it
- if !self
- .ingredients
- .iter()
- .any(|i| ingredient.hash().is_some() && i.hash() == ingredient.hash())
- {
- debug!("Manifest:add_ingredient {:?}", ingredient.title());
- self.ingredients.push(ingredient);
- } else {
- warn!("duplicate ingredient {}", ingredient.title());
- }
+ self.ingredients.push(ingredient);
self
}
@@ -823,6 +800,7 @@ impl Manifest {
})
.collect();
+ dbg!(&needs_ingredient);
for (index, action) in needs_ingredient {
if let Some(id) = action.instance_id() {
if let Some(hash_url) = ingredient_map.get(id) {
@@ -2201,7 +2179,7 @@ pub(crate) mod tests {
#[actix::test]
#[cfg(feature = "file_io")]
async fn test_box_hash_embeddable_manifest() {
- let asset_bytes = include_bytes!("../tests/fixtures/CA.jpg");
+ let asset_bytes = include_bytes!("../tests/fixtures/boxhash.jpg");
let box_hash_data = include_bytes!("../tests/fixtures/boxhash.json");
let box_hash: crate::assertions::BoxHash = serde_json::from_slice(box_hash_data).unwrap();
@@ -2227,8 +2205,8 @@ pub(crate) mod tests {
)
.await
.unwrap();
+ println!("{manifest_store}");
assert!(!manifest_store.manifests().is_empty());
assert!(manifest_store.validation_status().is_none());
- println!("{manifest_store}");
}
}
diff --git a/sdk/src/store.rs b/sdk/src/store.rs
@@ -3991,7 +3991,7 @@ pub mod tests {
fn test_modify_xmp() {
// modify the XMP (change xmp magic id value) - this should cause a data hash mismatch (OTGP)
let mut report = patch_and_report(
- "C.jpg",
+ "cloud.jpg",
b"W5M0MpCehiHzreSzNTczkc9d",
b"W5M0MpCehiHzreSzNTczkXXX",
);
@@ -4474,6 +4474,7 @@ pub mod tests {
}
#[actix::test]
+ #[cfg(feature = "file_io")]
async fn test_boxhash_embeddable_manifest() {
// test adding to actual image
let ap = fixture_path("boxhash.jpg");
diff --git a/sdk/tests/fixtures/C.jpg b/sdk/tests/fixtures/C.jpg
Binary files differ.
diff --git a/sdk/tests/fixtures/CA.jpg b/sdk/tests/fixtures/CA.jpg
Binary files differ.
diff --git a/sdk/tests/fixtures/CIE-sig-CA.jpg b/sdk/tests/fixtures/CIE-sig-CA.jpg
Binary files differ.
diff --git a/sdk/tests/fixtures/E-sig-CA.jpg b/sdk/tests/fixtures/E-sig-CA.jpg
Binary files differ.
diff --git a/sdk/tests/fixtures/XCA.jpg b/sdk/tests/fixtures/XCA.jpg
Binary files differ.
