Skip to content

Add new wrappers for image-rs #446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
amyspark wants to merge 2 commits into
base: master
Choose a base branch
from
+205 −31
Open

Add new wrappers for image-rs #446

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca7a660
Add safe wrappers for AVIF metadata boxes
amyspark Apr 9, 2026
4575afd
Read and expose Exif and XMP metadata in the AvifContext
amyspark Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
236 changes: 205 additions & 31 deletions mp4parse/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ pub enum Status {
IlocOffsetOverflow,
ImageItemType,
InfeFlagsNonzero,
InfeStringNoNul,
InfeStringNotUtf8,
InvalidUtf8,
IpcoIndexOverflow,
IpmaBadIndex,
Expand Down Expand Up @@ -611,6 +613,14 @@ impl From<Status> for &str {
"'infe' flags field shall be 0 \
per ISOBMFF (ISO 14496-12:2020) § 8.11.6.2"
}
Status::InfeStringNoNul => {
"'infe' strings shall be null-terminated \
per ISOBMFF (ISO 14496-12:2020) § 8.11.6.3"
}
Status::InfeStringNotUtf8 => {
"'infe' strings field shall be valid utf8 \
per ISOBMFF (ISO 14496-12:2020) § 8.11.6.3"
}
Status::InvalidUtf8 => {
"invalid utf8"
}
Expand Down Expand Up @@ -1594,6 +1604,10 @@ pub struct AvifContext {
pub sequence: Option<MediaContext>,
/// A collection of unsupported features encountered during the parse
pub unsupported_features: UnsupportedFeatures,
/// AVIF box containing the Exif metadata, if any
exif_metadata: Option<AvifItem>,
/// AVIF box containing the XMP metadata, if any
xmp_metadata: Option<AvifItem>,
}

impl AvifContext {
Expand Down Expand Up @@ -1640,6 +1654,49 @@ impl AvifContext {
}
}

pub fn av1_config(&self) -> Result<&AV1ConfigBox> {
if let Some(primary_item) = &self.primary_item {
match self
.item_properties
.get(primary_item.id, BoxType::AV1CodecConfigurationBox)?
{
Some(ItemProperty::AV1Config(av1c)) => Ok(av1c),
Some(other_property) => panic!("property key mismatch: {:?}", other_property),
None => Err(Error::from(Status::Av1cMissing)),
}
} else {
Err(Error::from(Status::PitmMissing))
}
}


pub fn exif_metadata(&self) -> Option<&[u8]> {
self.exif_metadata
.as_ref()
.map(|item| self.item_as_slice(item))
}

pub fn xmp_metadata(&self) -> Option<&[u8]> {
self.xmp_metadata
.as_ref()
.map(|item| self.item_as_slice(item))
}

pub fn spatial_extents(&self) -> Result<&ImageSpatialExtentsProperty> {
Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The duplication in these additions with the *_ptr() variants needs to be addressed.

Copy link
Copy Markdown
Author

@amyspark amyspark Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how to address it, since using the existing _ptr APIs requires unsafe blocks:

error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block
  --> src\codecs\avif\decoder.rs:99:21
   |
99 |             height: (*dimensions).image_height,
   |                     ^^^^^^^^^^^^^ dereference of raw pointer
   |
   = note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior

As per #444 (comment) you've been relying on cbindgen to use this crate, so didn't want to modify them.

Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

fn primary_property<'a, T>(
        &'a self,
        box_type: BoxType,
        extract: impl Fn(&'a ItemProperty) -> Option<&'a T>,
    ) -> Result<Option<&'a T>> {
        let primary = self.primary_item.as_ref().ok_or(Error::from(Status::PitmMissing))?;
        match self.item_properties.get(primary.id, box_type)? {
            Some(prop) => Ok(Some(extract(prop).expect("property key mismatch"))),
            None => Ok(None),
        }
    }

The safe and _ptr variants can then be rewritten as:

  pub fn image_mirror(&self) -> Result<&ImageMirror> {
      self.primary_property(BoxType::ImageMirror, |p| match p {
          ItemProperty::Mirroring(v) => Some(v),
          _ => None,
      })?
      .ok_or_else(|| Status::ImirMissing.into())
  }

  pub fn image_mirror_ptr(&self) -> Result<*const ImageMirror> {
      match self.image_mirror() {
          Ok(v) => Ok(v),
          Err(Error::InvalidData(Status::PitmMissing | Status::ImirMissing)) => Ok(std::ptr::null()),
          Err(e) => Err(e),
      }
  }

for example. The spatial_extents change needs to preserve the fail_with_status_if check.

if let Some(primary_item) = &self.primary_item {
match self
.item_properties
.get(primary_item.id, BoxType::ImageSpatialExtentsProperty)?
{
Some(ItemProperty::ImageSpatialExtents(ispe)) => Ok(ispe),
Some(other_property) => panic!("property key mismatch: {:?}", other_property),
None => Err(Error::from(Status::IspeMissing)),
}
} else {
Err(Error::from(Status::PitmMissing))
}
}

pub fn spatial_extents_ptr(&self) -> Result<*const ImageSpatialExtentsProperty> {
if let Some(primary_item) = &self.primary_item {
match self
Expand All @@ -1661,6 +1718,21 @@ impl AvifContext {
}
}

pub fn colour_information(&self) -> Result<&ColourInformation> {
Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add this as-is. item_properties.get() errors with Status::IprpConflict when multiple colr boxes are associated with the primary item but both an nclx and an ICC colr on one item is legal per HEIF (and common in real AVIFs that want to specify both display primaries and a profile). Add safe variants of the existing nclx_colour_information_ptr and use icc_colour_information directly.

if let Some(primary_item) = &self.primary_item {
match self
.item_properties
.get(primary_item.id, BoxType::ColourInformationBox)?
{
Some(ItemProperty::Colour(v)) => Ok(v),
Some(other_property) => panic!("property key mismatch: {:?}", other_property),
None => Err(Error::from(Status::ItemTypeMissing)),
}
} else {
Err(Error::from(Status::PitmMissing))
}
}

/// Returns None if there is no primary item or it has no associated NCLX colour boxes.
pub fn nclx_colour_information_ptr(&self) -> Option<Result<*const NclxColourInformation>> {
if let Some(primary_item) = &self.primary_item {
Expand Down Expand Up @@ -1722,6 +1794,21 @@ impl AvifContext {
}
}

pub fn image_mirror(&self) -> Result<&ImageMirror> {
if let Some(primary_item) = &self.primary_item {
match self
.item_properties
.get(primary_item.id, BoxType::ImageMirror)?
{
Some(ItemProperty::Mirroring(imir)) => Ok(imir),
Some(other_property) => panic!("property key mismatch: {:?}", other_property),
None => Err(Error::from(Status::ItemTypeMissing)),
Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error type should be Status::ImirMissing.

}
} else {
Err(Error::from(Status::PitmMissing))
}
}

pub fn image_mirror_ptr(&self) -> Result<*const ImageMirror> {
if let Some(primary_item) = &self.primary_item {
match self
Expand All @@ -1737,6 +1824,21 @@ impl AvifContext {
}
}

pub fn pixel_aspect_ratio(&self) -> Result<&PixelAspectRatio> {
if let Some(primary_item) = &self.primary_item {
match self
.item_properties
.get(primary_item.id, BoxType::PixelAspectRatioBox)?
{
Some(ItemProperty::PixelAspectRatio(pasp)) => Ok(pasp),
Some(other_property) => panic!("property key mismatch: {:?}", other_property),
None => Err(Error::from(Status::ItemTypeMissing)),
Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error type should be Status::PaspMissing.

}
} else {
Err(Error::from(Status::PitmMissing))
}
}

pub fn pixel_aspect_ratio_ptr(&self) -> Result<*const PixelAspectRatio> {
if let Some(primary_item) = &self.primary_item {
match self
Expand Down Expand Up @@ -1894,7 +1996,7 @@ impl DataBox {
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
struct PropertyIndex(u16);
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd)]
struct ItemId(u32);
pub struct ItemId(u32);
Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this and ItemInfoEntry made pub?


impl ItemId {
fn read(src: &mut impl ReadBytesExt, version: u8) -> Result<ItemId> {
Expand All @@ -1910,9 +2012,13 @@ impl ItemId {
/// See ISOBMFF (ISO 14496-12:2020) § 8.11.6
/// Only versions {2, 3} are supported
#[derive(Debug)]
struct ItemInfoEntry {
item_id: ItemId,
item_type: u32,
pub struct ItemInfoEntry {
pub item_id: ItemId,
pub item_type: u32,
Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be FourCC.

pub item_name: TryVec<u8>,
pub content_type: Option<TryVec<u8>>,
pub content_encoding: Option<TryVec<u8>>,
pub uri_type: Option<TryVec<u8>>,
}

/// See ISOBMFF (ISO 14496-12:2020) § 8.11.12
Expand Down Expand Up @@ -2501,24 +2607,17 @@ pub fn read_avif<T: Read>(f: &mut T, strictness: ParseStrictness) -> Result<Avif
debug!("alpha_item_id: {alpha_item_id:?}");
let mut primary_item = None;
let mut alpha_item = None;
let mut items: TryHashMap<ItemId, AvifItem> = Default::default();

// store data or record location of relevant items
for (item_id, loc) in iloc_items {
let item = if Some(item_id) == primary_item_id {
&mut primary_item
} else if Some(item_id) == alpha_item_id {
&mut alpha_item
} else {
continue;
};

assert!(item.is_none());
let mut item: Option<AvifItem> = None;
Copy link
Copy Markdown
Collaborator

@kinetiknz kinetiknz Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert these changes entirely. This can be done more efficiently with:

Before the iloc_items loop:

  let exif_item_id = item_infos.iter()
      .find(|i| i.item_type.to_be_bytes() == *b"Exif")
      .map(|i| i.item_id);
  let xmp_item_id = item_infos.iter()
      .find(|i| i.item_type.to_be_bytes() == *b"mime"
          && i.content_type.as_ref()
              .is_some_and(|ct| ct.as_slice() == b"application/rdf+xml"))
      .map(|i| i.item_id);

Inside the loop:

  let item = if Some(item_id) == primary_item_id {
      &mut primary_item
  } else if Some(item_id) == alpha_item_id {
      &mut alpha_item
  } else if Some(item_id) == exif_item_id {
      &mut exif_metadata
  } else if Some(item_id) == xmp_item_id {
      &mut xmp_metadata
  } else {
      continue;
  };


// If our item is spread over multiple extents, we'll need to copy it
// into a contiguous buffer. Otherwise, we can just store the extent
// and return a pointer into the mdat/idat later to avoid the copy.
if loc.extents.len() > 1 {
*item = Some(AvifItem::with_inline_data(item_id))
item = Some(AvifItem::with_inline_data(item_id))
}

trace!(
Expand All @@ -2531,10 +2630,10 @@ pub fn read_avif<T: Read>(f: &mut T, strictness: ParseStrictness) -> Result<Avif
// true if the extent is successfully added to the AvifItem
let mut find_and_add_to_item = |extent: &Extent, dat: &DataBox| -> Result<bool> {
if let Some(extent_slice) = dat.get(extent) {
match item {
match &mut item {
None => {
trace!("Using IsobmffItem::Location");
*item = Some(AvifItem {
item = Some(AvifItem {
id: item_id,
image_data: dat.location(extent),
});
Expand Down Expand Up @@ -2594,6 +2693,14 @@ pub fn read_avif<T: Read>(f: &mut T, strictness: ParseStrictness) -> Result<Avif
}

assert!(item.is_some());

if Some(item_id) == primary_item_id {
primary_item = item;
} else if Some(item_id) == alpha_item_id {
alpha_item = item;
} else {
items.insert(item_id, item.unwrap())?;
};
}

if (primary_item_id.is_some() && primary_item.is_none())
Expand Down Expand Up @@ -2697,6 +2804,21 @@ pub fn read_avif<T: Read>(f: &mut T, strictness: ParseStrictness) -> Result<Avif
check_image_item(&mut primary_item)?;
check_image_item(&mut alpha_item)?;

let mut exif_metadata = None;
let mut xmp_metadata = None;

item_infos.into_iter().for_each(|item| {
if &item.item_type.to_be_bytes() == b"Exif" {
exif_metadata = items.remove(&item.item_id);
} else if &item.item_type.to_be_bytes() == b"mime"
&& item
.content_type
.is_some_and(|v| v.as_slice() == b"application/rdf+xml")
{
xmp_metadata = items.remove(&item.item_id);
}
});

Ok(AvifContext {
strictness,
media_storage,
Expand All @@ -2707,6 +2829,8 @@ pub fn read_avif<T: Read>(f: &mut T, strictness: ParseStrictness) -> Result<Avif
item_properties,
major_brand,
sequence: image_sequence,
exif_metadata,
xmp_metadata,
unsupported_features,
})
}
Expand Down Expand Up @@ -2881,6 +3005,39 @@ impl std::fmt::Display for U32BE {
}
}

fn read_infe_string<T: Read>(
src: &mut BMFFBox<T>,
strictness: ParseStrictness,
) -> Result<TryVec<u8>> {
let mut s = TryVec::new();
loop {
match src.read_u8() {
Ok(v) => {
s.push(v)?;
if v == 0 {
break;
}
},
Err(_) => break,
}
}
match std::str::from_utf8(&s) {
Ok(s) => {
if !s.bytes().any(|b| b == b'\0') {
fail_with_status_if(
strictness != ParseStrictness::Permissive,
Status::InfeStringNoNul,
)?;
}
}
Err(_) => fail_with_status_if(
strictness != ParseStrictness::Permissive,
Status::InfeStringNotUtf8,
)?,
}
Ok(s)
}

/// Parse an Item Info Entry
/// See ISOBMFF (ISO 14496-12:2020) § 8.11.6.2
fn read_infe<T: Read>(
Expand Down Expand Up @@ -2912,15 +3069,32 @@ fn read_infe<T: Read>(
let item_type = be_u32(src)?;
debug!("infe {:?} item_type: {}", item_id, U32BE(item_type));

// There are some additional fields here, but they're not of interest to us
skip_box_remain(src)?;

if item_protection_index != 0 {
unsupported_features.insert(Feature::Ipro);
Ok(None)
} else {
Ok(Some(ItemInfoEntry { item_id, item_type }))
skip_box_remain(src)?;
return Ok(None);
}

let item_name = read_infe_string(src, strictness)?;

let (content_type, content_encoding, uri_type) = if &item_type.to_be_bytes() == b"mime" {
(
Some(read_infe_string(src, strictness)?),
Some(read_infe_string(src, strictness)?),
None
)
} else {
(None, None, Some(read_infe_string(src, strictness)?))
};

Ok(Some(ItemInfoEntry {
item_id,
item_type,
item_name,
content_type,
content_encoding,
uri_type
}))
}

/// Parse an Item Reference Box
Expand Down Expand Up @@ -3645,8 +3819,8 @@ fn read_ipco<T: Read>(
#[repr(C)]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct ImageSpatialExtentsProperty {
image_width: u32,
image_height: u32,
pub image_width: u32,
pub image_height: u32,
}

/// Parse image spatial extents property
Expand All @@ -3669,8 +3843,8 @@ fn read_ispe<T: Read>(src: &mut BMFFBox<T>) -> Result<ImageSpatialExtentsPropert
#[repr(C)]
#[derive(Debug)]
pub struct PixelAspectRatio {
h_spacing: u32,
v_spacing: u32,
pub h_spacing: u32,
pub v_spacing: u32,
}

/// Parse pixel aspect ratio property
Expand Down Expand Up @@ -3722,16 +3896,16 @@ fn read_pixi<T: Read>(src: &mut BMFFBox<T>) -> Result<PixelInformation> {
#[repr(C)]
#[derive(Debug)]
pub struct NclxColourInformation {
colour_primaries: u8,
transfer_characteristics: u8,
matrix_coefficients: u8,
full_range_flag: bool,
pub colour_primaries: u8,
pub transfer_characteristics: u8,
pub matrix_coefficients: u8,
pub full_range_flag: bool,
}

/// The raw bytes of the ICC profile
#[repr(C)]
pub struct IccColourInformation {
bytes: TryVec<u8>,
pub bytes: TryVec<u8>,
}

impl fmt::Debug for IccColourInformation {
Expand Down
Loading