From ddfc82f832f9441d4a309c708e6df89533aac39e Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 16 Sep 2026 17:19:42 +0100 Subject: [PATCH 1/2] Parse a1lx and expose item extents for layered AVIF images A layered (progressive) AVIF image item holds up to four AV1 frames concatenated in its payload, each a better rendering of the same picture. `a1lx` gives the byte sizes of the first three, and is the only signal saying where one layer ends and the next begins. It was previously recorded as an unsupported feature and skipped without reading its payload, so a caller had no way to find a layer boundary and could only decode the item whole. Parse it, and give callers what they need to slice the payload themselves: - `ItemProperty::LayeredImageIndexing` carries the layer sizes, and `Feature::A1lx` becomes supported. - `AvifItem` retains its `iloc` extents and construction method, reached through `primary_item_extents`, `alpha_item_extents` and the `*_is_file_construction` predicates. A caller decoding an item incrementally needs to know which of its bytes have arrived, which the existing item-data copy cannot tell it -- and for a multi-extent item that copy would have captured the whole payload anyway. - `primary_item_a1lx`, `alpha_item_a1lx` and `primary_item_lsel` report the properties that decide whether an item is progressively renderable. An alpha auxiliary item can be layered too, with its own `a1lx`. A malformed `a1lx` -- truncated, overlong, or with non-zero reserved bits -- is deliberately not fatal. It is recorded as present with no layer sizes, which reads as "not a layered image". Such a file decoded fine when the property was skipped, and failing it now would be a regression. The C API gains the matching `Mp4parseAvifInfo` fields, plus `Mp4parseItemExtents` as a borrowed slice of `ItemExtent`, valid for the lifetime of the parser. Because `a1lx` is no longer an unsupported feature, the files whose only unsupported property it was now parse with an empty `unsupported_features` set, so they come off `AVIF_UNSUPPORTED_IMAGES`. `animals_00_multilayer_grid_a1lx` and `quebec_3layer_op2` stay, since `grid` and `a1op` are still unsupported. --- mp4parse/src/lib.rs | 352 +++++++++++++++++++++++++------ mp4parse/tests/public.rs | 44 +++- mp4parse_capi/cbindgen.toml | 2 + mp4parse_capi/src/lib.rs | 82 +++++++ mp4parse_capi/tests/test_avis.rs | 7 + 5 files changed, 414 insertions(+), 73 deletions(-) diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index 7116e6af..6b01a7e2 100644 --- a/mp4parse/src/lib.rs +++ b/mp4parse/src/lib.rs @@ -298,7 +298,8 @@ pub enum Feature { impl Feature { fn supported(self) -> bool { match self { - Self::Auxc + Self::A1lx + | Self::Auxc | Self::Av1c | Self::Avis | Self::Colr @@ -307,7 +308,7 @@ impl Feature { | Self::Ispe | Self::Pasp | Self::Pixi => true, - Self::A1lx | Self::A1op | Self::Clap | Self::Grid | Self::Ipro | Self::Lsel => false, + Self::A1op | Self::Clap | Self::Grid | Self::Ipro | Self::Lsel => false, } } } @@ -323,7 +324,7 @@ impl TryFrom<&ItemProperty> for Feature { ItemProperty::CleanAperture => Self::Clap, ItemProperty::Colour(_) => Self::Colr, ItemProperty::ImageSpatialExtents(_) => Self::Ispe, - ItemProperty::LayeredImageIndexing => Self::A1lx, + ItemProperty::LayeredImageIndexing(_) => Self::A1lx, ItemProperty::LayerSelection(_) => Self::Lsel, ItemProperty::Mirroring(_) => Self::Imir, ItemProperty::OperatingPointSelector => Self::A1op, @@ -1624,6 +1625,43 @@ impl fmt::Debug for IsobmffItem { } } +/// A region of the file holding part of an item's payload, as given by the +/// `extent_offset` and `extent_length` of an `iloc` entry. +/// +/// `offset` already has the entry's `base_offset` added, so for +/// [`ConstructionMethod::File`] it is an absolute file offset. See ISOBMFF +/// (ISO 14496-12:2020) § 8.11.3. +/// +/// This is the `repr(C)`-friendly form of [`Extent`], retained so that callers +/// which need to know *where* an item's bytes live (rather than just reading +/// them) can be told. `extent_length` may be zero, which per § 8.11.3.3 means +/// the extent runs to the end of the enclosing box; that is reported as +/// `len == 0` with `to_end == true`. +#[repr(C)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct ItemExtent { + pub offset: u64, + pub len: u64, + pub to_end: bool, +} + +impl From<&Extent> for ItemExtent { + fn from(extent: &Extent) -> Self { + match extent { + Extent::WithLength { offset, len } => Self { + offset: *offset, + len: *len as u64, + to_end: false, + }, + Extent::ToEnd { offset } => Self { + offset: *offset, + len: 0, + to_end: true, + }, + } + } +} + #[derive(Debug)] struct AvifItem { /// The `item_ID` from ISOBMFF (ISO 14496-12:2020) § 8.11.3 @@ -1633,6 +1671,21 @@ struct AvifItem { /// AV1 Image Item per image_data: IsobmffItem, + + /// The `construction_method` from ISOBMFF (ISO 14496-12:2020) § 8.11.3, + /// which says whether `extents` index the file, the `idat` box or another + /// item. See [`ConstructionMethod`]. + construction_method: ConstructionMethod, + + /// The `iloc` extents making up this item's payload, in payload order. + /// Retained (rather than being folded into `image_data`) so that callers + /// decoding an item incrementally can tell which of its bytes have arrived, + /// and so that they can cut the payload at the layer boundaries an `a1lx` + /// describes -- which is what the property is for, per + /// : + /// it "enables determining the byte ranges required to process one or more + /// layers of an Operating Point". + extents: TryVec, } impl AvifItem { @@ -1640,6 +1693,8 @@ impl AvifItem { Self { id, image_data: IsobmffItem::Data(TryVec::new()), + construction_method: ConstructionMethod::File, + extents: TryVec::new(), } } } @@ -1707,6 +1762,86 @@ impl AvifContext { .map(|item| self.image_bits_per_channel(item.id)) } + /// The layer sizes from the item's AV1LayeredImageIndexingProperty, or + /// `None` if the item has no `a1lx`. + /// + /// An `a1lx` "should not be associated with AV1 Image Items consisting of + /// only one layer", so its presence is the practical signal that an item is + /// layered. See + /// . + pub fn primary_item_a1lx(&self) -> Option<&AV1LayeredImageIndexing> { + self.item_a1lx(self.primary_item.as_ref()?) + } + + pub fn alpha_item_a1lx(&self) -> Option<&AV1LayeredImageIndexing> { + self.item_a1lx(self.alpha_item.as_ref()?) + } + + fn item_a1lx(&self, item: &AvifItem) -> Option<&AV1LayeredImageIndexing> { + match self + .item_properties + .get(item.id, BoxType::AV1LayeredImageIndexingProperty) + { + Ok(Some(ItemProperty::LayeredImageIndexing(a1lx))) => Some(a1lx), + _ => None, + } + } + + /// The `lsel` layer_id for the primary item, or `None` if it has no + /// LayerSelectorProperty. + /// + /// Per + /// the value "shall be between 0 and 3, or the special value 0xFFFF", and a + /// value in 0..=3 "indicates the value of the spatial_id to render", which + /// pins the item to one layer. `0xFFFF` instead means progressive decoding + /// is allowed, so an absent `lsel` and an `lsel` of `0xFFFF` are equivalent + /// to a caller looking for a progressively renderable item. + pub fn primary_item_lsel(&self) -> Option { + match self.item_properties.get( + self.primary_item.as_ref()?.id, + BoxType::LayerSelectorProperty, + ) { + Ok(Some(ItemProperty::LayerSelection(layer_id))) => Some(*layer_id), + _ => None, + } + } + + /// The `iloc` extents making up the item's payload, in payload order, or + /// `None` if the item isn't present. + /// + /// The order matters: an `a1lx` documents layer sizes "in increasing order + /// of spatial_id" within the item payload, so cutting the payload at those + /// sizes means walking the extents in this order. See + /// . + /// + /// Only meaningful for items whose `construction_method` is `File`; see + /// `primary_item_is_file_construction`. + pub fn primary_item_extents(&self) -> Option<&[ItemExtent]> { + Some(self.primary_item.as_ref()?.extents.as_slice()) + } + + pub fn alpha_item_extents(&self) -> Option<&[ItemExtent]> { + Some(self.alpha_item.as_ref()?.extents.as_slice()) + } + + /// Whether the item's extents are offsets into the file, rather than into + /// the `idat` box or another item. + /// + /// MIAF (ISO 23000-22:2019) § 7.2.1.7 restricts `construction_method` to 0 + /// (file) or 1 (idat), but only the former lets a caller map an extent onto + /// a file offset it can wait for. + pub fn primary_item_is_file_construction(&self) -> bool { + self.primary_item + .as_ref() + .is_some_and(|item| item.construction_method == ConstructionMethod::File) + } + + pub fn alpha_item_is_file_construction(&self) -> bool { + self.alpha_item + .as_ref() + .is_some_and(|item| item.construction_method == ConstructionMethod::File) + } + fn image_bits_per_channel(&self, item_id: ItemId) -> Result<&[u8]> { match self .item_properties @@ -2605,69 +2740,85 @@ pub fn read_avif(f: &mut T, strictness: ParseStrictness) -> Result Result { - if let Some(extent_slice) = dat.get(extent) { - match item { - None => { - trace!("Using IsobmffItem::Location"); - *item = Some(AvifItem { - id: item_id, - image_data: dat.location(extent), - }); - } - Some(AvifItem { - image_data: IsobmffItem::Data(bytes), - .. - }) => { - trace!("Using IsobmffItem::Data"); - // We could potentially optimize memory usage by trying to avoid reading - // or storing dat boxes which aren't used by our API, but for now it seems - // like unnecessary complexity - bytes.extend_from_slice(extent_slice)?; + // Scoped so the closure's mutable borrow of `item` ends before we + // record where the item's bytes came from, below. + { + // Generalize the process of connecting items to their data; returns + // true if the extent is successfully added to the AvifItem + let mut find_and_add_to_item = |extent: &Extent, dat: &DataBox| -> Result { + if let Some(extent_slice) = dat.get(extent) { + match item { + None => { + trace!("Using IsobmffItem::Location"); + *item = Some(AvifItem { + id: item_id, + image_data: dat.location(extent), + // Both filled in by the caller once every extent + // has been located. + construction_method: ConstructionMethod::File, + extents: TryVec::new(), + }); + } + Some(AvifItem { + image_data: IsobmffItem::Data(bytes), + .. + }) => { + trace!("Using IsobmffItem::Data"); + // We could potentially optimize memory usage by trying to avoid reading + // or storing dat boxes which aren't used by our API, but for now it seems + // like unnecessary complexity + bytes.extend_from_slice(extent_slice)?; + } + _ => unreachable!(), } - _ => unreachable!(), + return Ok(true); } - return Ok(true); - } - Ok(false) - }; + Ok(false) + }; - match loc.construction_method { - ConstructionMethod::File => { - for extent in loc.extents { - let mut found = false; - // try to find an mdat which contains the extent - for mdat in media_storage.iter() { - if find_and_add_to_item(&extent, mdat)? { - found = true; - break; + match loc.construction_method { + ConstructionMethod::File => { + for extent in &loc.extents { + let mut found = false; + // try to find an mdat which contains the extent + for mdat in media_storage.iter() { + if find_and_add_to_item(extent, mdat)? { + found = true; + break; + } } - } - if !found { - return Status::IlocNotFound.into(); - } - } - } - ConstructionMethod::Idat => { - if let Some(idat) = &item_data_box { - for extent in loc.extents { - let found = find_and_add_to_item(&extent, idat)?; if !found { return Status::IlocNotFound.into(); } } - } else { - return Status::IdatMissing.into(); + } + ConstructionMethod::Idat => { + if let Some(idat) = &item_data_box { + for extent in &loc.extents { + let found = find_and_add_to_item(extent, idat)?; + if !found { + return Status::IlocNotFound.into(); + } + } + } else { + return Status::IdatMissing.into(); + } + } + ConstructionMethod::Item => { + fail_with_status_if( + strictness != ParseStrictness::Permissive, + Status::ConstructionMethod, + )?; } } - ConstructionMethod::Item => { - fail_with_status_if( - strictness != ParseStrictness::Permissive, - Status::ConstructionMethod, - )?; + } + + if let Some(item) = item { + item.construction_method = loc.construction_method; + item.extents = TryVec::with_capacity(loc.extents.len())?; + for extent in &loc.extents { + item.extents.push(ItemExtent::from(extent))?; } } @@ -3216,10 +3367,7 @@ fn read_iprp( } } - // The following properties are unsupported, but we still enforce that - // they've been correctly marked as essential or not. - ItemProperty::LayeredImageIndexing => { - assert!(feature.is_ok() && unsupported_features.contains(feature?)); + ItemProperty::LayeredImageIndexing(_) => { if a.essential { fail_with_status_if( strictness != ParseStrictness::Permissive, @@ -3228,6 +3376,8 @@ fn read_iprp( } } + // The following properties are unsupported, but we still enforce that + // they've been correctly marked as essential or not. ItemProperty::LayerSelection(layer_id) => { if !a.essential { // lsel shall be marked as essential regardless of its @@ -3328,7 +3478,7 @@ pub enum ItemProperty { CleanAperture, Colour(ColourInformation), ImageSpatialExtents(ImageSpatialExtentsProperty), - LayeredImageIndexing, + LayeredImageIndexing(AV1LayeredImageIndexing), LayerSelection(u16), Mirroring(ImageMirror), OperatingPointSelector, @@ -3345,7 +3495,7 @@ impl From<&ItemProperty> for BoxType { ItemProperty::AV1Config(_) => BoxType::AV1CodecConfigurationBox, ItemProperty::CleanAperture => BoxType::CleanApertureBox, ItemProperty::Colour(_) => BoxType::ColourInformationBox, - ItemProperty::LayeredImageIndexing => BoxType::AV1LayeredImageIndexingProperty, + ItemProperty::LayeredImageIndexing(_) => BoxType::AV1LayeredImageIndexingProperty, ItemProperty::LayerSelection(_) => BoxType::LayerSelectorProperty, ItemProperty::Mirroring(_) => BoxType::ImageMirror, ItemProperty::OperatingPointSelector => BoxType::OperatingPointSelectorProperty, @@ -3717,13 +3867,27 @@ fn read_ipco( BoxType::PixelAspectRatioBox => ItemProperty::PixelAspectRatio(read_pasp(&mut b)?), BoxType::PixelInformationBox => ItemProperty::Channels(read_pixi(&mut b)?), BoxType::LayerSelectorProperty => ItemProperty::LayerSelection(read_lsel(&mut b)?), + BoxType::AV1LayeredImageIndexingProperty => { + // Trouble reading the property leaves it recorded as present + // but with no layer sizes, which callers read as "not a layered + // image". An `a1lx` that is truncated, overlong, or has garbage + // in its reserved bits is no reason to fail an image which + // would otherwise decode: the property "shall not be marked as + // essential" per + // , + // so a reader is entitled to ignore it entirely and still + // render the item -- it only ever documents where the layers + // are, never how to decode them. + let a1lx = read_a1lx(&mut b).unwrap_or_default(); + skip_box_remain(&mut b)?; + ItemProperty::LayeredImageIndexing(a1lx) + } other_box_type => { // Even if we didn't do anything with other property types, we still store // a record at the index to identify invalid indices in ipma boxes skip_box_remain(&mut b)?; let item_property = match other_box_type { - BoxType::AV1LayeredImageIndexingProperty => ItemProperty::LayeredImageIndexing, BoxType::CleanApertureBox => ItemProperty::CleanAperture, BoxType::OperatingPointSelectorProperty => ItemProperty::OperatingPointSelector, _ => { @@ -3758,6 +3922,70 @@ fn read_lsel(src: &mut BMFFBox) -> Result { Ok(layer_id) } +/// The sizes, in bytes, of the layers making up a layered (progressive) AV1 +/// image item, as given by its AV1LayeredImageIndexingProperty. +/// +/// `layer_sizes` documents every layer except the last, in increasing order of +/// `spatial_id`, so an item holds at most 4 layers. The last one occupies +/// whatever remains of the item: "the size of the last layer can be determined +/// by subtracting the sum of the sizes of all layers indicated in this property +/// from the entire item size". +/// +/// A zero entry terminates the list -- "a value of zero means that all the +/// layers except the last one have been documented and following values shall +/// be 0" -- so a 2-layer item reports `[size_of_layer_0, 0, 0]` and an all-zero +/// property describes a single layer, i.e. nothing layered at all. +/// +/// Note that an index into `layer_sizes` is not a `spatial_id`: "the spatial_id +/// for the first layer does not necessarily match the index in the array that +/// provides the size". +/// +/// See +/// . +#[repr(C)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct AV1LayeredImageIndexing { + pub layer_sizes: [u32; 3], +} + +/// Parse an AV1LayeredImageIndexingProperty. +/// +/// Not a FullBox, so there is no version or flags field to read before the +/// payload: +/// +/// ```text +/// class AV1LayeredImageIndexingProperty extends ItemProperty('a1lx') { +/// unsigned int(7) reserved = 0; +/// unsigned int(1) large_size; +/// FieldLength = (large_size + 1) * 16; +/// unsigned int(FieldLength) layer_size[3]; +/// } +/// ``` +/// +/// See +fn read_a1lx(src: &mut BMFFBox) -> Result { + let flags = src.read_u8()?; + // unsigned int(7) reserved = 0; warned about rather than rejected, since + // the property is non-essential and this is the only field we would be + // guessing about. + if flags & 0xfe != 0 { + warn!("a1lx reserved bits are not zero: {flags:#x}"); + } + // unsigned int(1) large_size; FieldLength = (large_size + 1) * 16 + let large_size = flags & 1 == 1; + + let mut layer_sizes = [0u32; 3]; + for layer_size in &mut layer_sizes { + *layer_size = if large_size { + be_u32(src)? + } else { + be_u16(src)?.into() + }; + } + + Ok(AV1LayeredImageIndexing { layer_sizes }) +} + #[repr(C)] #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct ImageSpatialExtentsProperty { diff --git a/mp4parse/tests/public.rs b/mp4parse/tests/public.rs index c273171e..32718ca1 100644 --- a/mp4parse/tests/public.rs +++ b/mp4parse/tests/public.rs @@ -104,14 +104,12 @@ static AVIF_AVIS_NO_LOOP: &str = "tests/loop_none.avif"; static AVIF_AVIS_LOOP_FOREVER: &str = "tests/loop_forever.avif"; static AVIF_NO_PIXI_IMAGES: &[&str] = &[IMAGE_AVIF_NO_PIXI, IMAGE_AVIF_NO_ALPHA_PIXI]; static AVIF_UNSUPPORTED_IMAGES: &[&str] = &[ - AVIF_A1LX, AVIF_A1OP, AVIF_CLAP, IMAGE_AVIF_CLAP_MISSING_ESSENTIAL, AVIF_GRID, AVIF_GRID_A1LX, AVIF_LSEL, - "av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_a1lx.avif", "av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_a1op.avif", "av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_a1op_lsel.avif", "av1-avif/testFiles/Apple/multilayer_examples/animals_00_multilayer_lsel.avif", @@ -121,11 +119,7 @@ static AVIF_UNSUPPORTED_IMAGES: &[&str] = &[ "av1-avif/testFiles/Microsoft/Chimera_10bit_cropped_to_1920x1008.avif", "av1-avif/testFiles/Microsoft/Chimera_10bit_cropped_to_1920x1008_with_HDR_metadata.avif", "av1-avif/testFiles/Microsoft/Chimera_8bit_cropped_480x256.avif", - "av1-avif/testFiles/Xiph/abandoned_filmgrain.avif", - "av1-avif/testFiles/Xiph/fruits_2layer_thumbsize.avif", "av1-avif/testFiles/Xiph/quebec_3layer_op2.avif", - "av1-avif/testFiles/Xiph/tiger_3layer_1res.avif", - "av1-avif/testFiles/Xiph/tiger_3layer_3res.avif", "link-u-avif-sample-images/kimono.crop.avif", "link-u-avif-sample-images/kimono.mirror-vertical.rotate270.crop.avif", ]; @@ -1255,17 +1249,45 @@ fn assert_unsupported(path: &str, feature: mp4::Feature, essential: bool) { }); } -fn assert_unsupported_nonessential(path: &str, feature: mp4::Feature) { - assert_unsupported(path, feature, false); -} - fn assert_unsupported_essential(path: &str, feature: mp4::Feature) { assert_unsupported(path, feature, true); } #[test] fn public_avif_a1lx() { - assert_unsupported_nonessential(AVIF_A1LX, mp4::Feature::A1lx); + // `a1lx` is parsed rather than recorded as unsupported: its layer sizes are + // what lets a caller work out the byte ranges of a layered item's layers, + // which is what the property is for per + // . + for_strictness_result(AVIF_A1LX, |strictness, result| { + let context = result.unwrap_or_else(|e| { + panic!( + "{} failed to parse with {:?} strictness: {:?}", + AVIF_A1LX, strictness, e + ) + }); + assert!(!context.unsupported_features.contains(mp4::Feature::A1lx)); + + // A single layer boundary, i.e. the `[X,0,0]` shape the spec gives for a + // 2-layer item: the first layer is X bytes and the second is + // ItemSize - X. See + // . + let a1lx = context + .primary_item_a1lx() + .expect("primary item should have an a1lx"); + assert_eq!(a1lx.layer_sizes, [122336, 0, 0]); + + // No `lsel`, so nothing pins the item to one layer. + assert_eq!(context.primary_item_lsel(), None); + + // The extents are where the layer payloads are cut from. + assert!(context.primary_item_is_file_construction()); + let extents = context + .primary_item_extents() + .expect("primary item should be present"); + assert!(!extents.is_empty()); + assert!(extents.iter().all(|e| !e.to_end && e.len > 0)); + }); } #[test] diff --git a/mp4parse_capi/cbindgen.toml b/mp4parse_capi/cbindgen.toml index 858d0992..5b2c8eb0 100644 --- a/mp4parse_capi/cbindgen.toml +++ b/mp4parse_capi/cbindgen.toml @@ -43,3 +43,5 @@ include = ["Status", "Feature"] "ImageMirror" = "Mp4parseImir" "Indice" = "Mp4parseIndice" "NclxColourInformation" = "Mp4parseNclxColourInformation" +"AV1LayeredImageIndexing" = "Mp4parseA1lx" +"ItemExtent" = "Mp4parseItemExtent" diff --git a/mp4parse_capi/src/lib.rs b/mp4parse_capi/src/lib.rs index 6d8650ea..baa35655 100644 --- a/mp4parse_capi/src/lib.rs +++ b/mp4parse_capi/src/lib.rs @@ -157,6 +157,42 @@ impl Mp4parseByteData { } } +/// A borrowed slice of [`mp4parse::ItemExtent`], valid for the lifetime of the +/// parser it was obtained from. +/// +/// The extents are in payload order, which is what a caller cutting the payload +/// at `a1lx` layer boundaries needs; see ISOBMFF (ISO 14496-12:2020) § 8.11.3 +/// and +/// . +#[repr(C)] +#[derive(Debug)] +pub struct Mp4parseItemExtents { + pub length: usize, + pub extents: *const mp4parse::ItemExtent, +} + +impl Mp4parseItemExtents { + fn with_extents(slice: &[mp4parse::ItemExtent]) -> Self { + Self { + length: slice.len(), + extents: if slice.is_empty() { + std::ptr::null() + } else { + slice.as_ptr() + }, + } + } +} + +impl Default for Mp4parseItemExtents { + fn default() -> Self { + Self { + length: 0, + extents: std::ptr::null(), + } + } +} + impl Default for Mp4parseByteData { fn default() -> Self { Self { @@ -410,6 +446,36 @@ pub struct Mp4parseAvifInfo { /// Bit depth for the alpha item used by the `pitm`, or 0 if values are inconsistent. pub alpha_item_bit_depth: u8, + /// The layer sizes from the primary item's `a1lx`, or null if it has none. + /// + /// An `a1lx` "should not be associated with AV1 Image Items consisting of + /// only one layer", so a non-null value here means the item is layered. See + /// . + pub primary_item_a1lx: *const mp4parse::AV1LayeredImageIndexing, + /// As `primary_item_a1lx`, but for the alpha item. + pub alpha_item_a1lx: *const mp4parse::AV1LayeredImageIndexing, + /// The primary item's `lsel` layer_id, or 0xFFFF if it has no + /// LayerSelectorProperty. + /// + /// Collapsing the two cases is deliberate: per + /// a + /// value in 0..=3 names the single `spatial_id` to render, while 0xFFFF + /// means progressive decoding is allowed -- the same freedom a caller has + /// when no `lsel` is associated at all. + pub primary_item_lsel_layer_id: u16, + /// The `iloc` extents making up the primary item's payload, in payload + /// order. Only meaningful when `primary_item_is_file_construction` is true. + pub primary_item_extents: Mp4parseItemExtents, + /// As `primary_item_extents`, but for the alpha item. + pub alpha_item_extents: Mp4parseItemExtents, + /// Whether the primary item's extents are offsets into the file rather + /// than into an `idat` box or another item, i.e. whether + /// `construction_method` is 0. See ISOBMFF (ISO 14496-12:2020) § 8.11.3 and + /// MIAF (ISO 23000-22:2019) § 7.2.1.7. + pub primary_item_is_file_construction: bool, + /// As `primary_item_is_file_construction`, but for the alpha item. + pub alpha_item_is_file_construction: bool, + /// Whether there is a sequence. Can be true with no primary image. pub has_sequence: bool, /// Indicates whether the EditListBox requests that the image be looped. @@ -1279,6 +1345,22 @@ fn mp4parse_avif_get_info_safe(context: &AvifContext) -> mp4parse::Result Mp4parseAvifInfo { primary_item_bit_depth: Default::default(), has_alpha_item: Default::default(), alpha_item_bit_depth: Default::default(), + primary_item_a1lx: std::ptr::null(), + alpha_item_a1lx: std::ptr::null(), + primary_item_lsel_layer_id: 0xffff, + primary_item_extents: Default::default(), + alpha_item_extents: Default::default(), + primary_item_is_file_construction: Default::default(), + alpha_item_is_file_construction: Default::default(), has_sequence: Default::default(), loop_mode: Default::default(), loop_count: Default::default(), From 8acfdbba7718c4b931e5a938ce628c9f3c9fd046 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Thu, 17 Sep 2026 12:03:25 +0100 Subject: [PATCH 2/2] Keep the C stuff in the C file --- mp4parse/src/lib.rs | 113 +++++++++++++----------------- mp4parse/tests/public.rs | 9 ++- mp4parse_capi/cbindgen.toml | 1 - mp4parse_capi/src/lib.rs | 116 +++++++++++++++++++++++++------ mp4parse_capi/tests/test_avis.rs | 74 ++++++++++++++++++++ 5 files changed, 224 insertions(+), 89 deletions(-) diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index 6b01a7e2..03834eb2 100644 --- a/mp4parse/src/lib.rs +++ b/mp4parse/src/lib.rs @@ -1625,43 +1625,6 @@ impl fmt::Debug for IsobmffItem { } } -/// A region of the file holding part of an item's payload, as given by the -/// `extent_offset` and `extent_length` of an `iloc` entry. -/// -/// `offset` already has the entry's `base_offset` added, so for -/// [`ConstructionMethod::File`] it is an absolute file offset. See ISOBMFF -/// (ISO 14496-12:2020) § 8.11.3. -/// -/// This is the `repr(C)`-friendly form of [`Extent`], retained so that callers -/// which need to know *where* an item's bytes live (rather than just reading -/// them) can be told. `extent_length` may be zero, which per § 8.11.3.3 means -/// the extent runs to the end of the enclosing box; that is reported as -/// `len == 0` with `to_end == true`. -#[repr(C)] -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] -pub struct ItemExtent { - pub offset: u64, - pub len: u64, - pub to_end: bool, -} - -impl From<&Extent> for ItemExtent { - fn from(extent: &Extent) -> Self { - match extent { - Extent::WithLength { offset, len } => Self { - offset: *offset, - len: *len as u64, - to_end: false, - }, - Extent::ToEnd { offset } => Self { - offset: *offset, - len: 0, - to_end: true, - }, - } - } -} - #[derive(Debug)] struct AvifItem { /// The `item_ID` from ISOBMFF (ISO 14496-12:2020) § 8.11.3 @@ -1685,7 +1648,7 @@ struct AvifItem { /// : /// it "enables determining the byte ranges required to process one or more /// layers of an Operating Point". - extents: TryVec, + extents: TryVec, } impl AvifItem { @@ -1814,32 +1777,29 @@ impl AvifContext { /// sizes means walking the extents in this order. See /// . /// - /// Only meaningful for items whose `construction_method` is `File`; see - /// `primary_item_is_file_construction`. - pub fn primary_item_extents(&self) -> Option<&[ItemExtent]> { + /// What the offsets are relative to depends on + /// [`AvifContext::primary_item_construction_method`]. + pub fn primary_item_extents(&self) -> Option<&[Extent]> { Some(self.primary_item.as_ref()?.extents.as_slice()) } - pub fn alpha_item_extents(&self) -> Option<&[ItemExtent]> { + pub fn alpha_item_extents(&self) -> Option<&[Extent]> { Some(self.alpha_item.as_ref()?.extents.as_slice()) } - /// Whether the item's extents are offsets into the file, rather than into - /// the `idat` box or another item. + /// What the item's extents are offsets into, or `None` if the item isn't + /// present. /// - /// MIAF (ISO 23000-22:2019) § 7.2.1.7 restricts `construction_method` to 0 - /// (file) or 1 (idat), but only the former lets a caller map an extent onto - /// a file offset it can wait for. - pub fn primary_item_is_file_construction(&self) -> bool { - self.primary_item - .as_ref() - .is_some_and(|item| item.construction_method == ConstructionMethod::File) + /// MIAF (ISO 23000-22:2019) § 7.2.1.7 restricts `construction_method` to + /// [`ConstructionMethod::File`] or [`ConstructionMethod::Idat`], but only + /// the former lets a caller map an extent onto a file offset it can wait + /// for. + pub fn primary_item_construction_method(&self) -> Option { + Some(self.primary_item.as_ref()?.construction_method) } - pub fn alpha_item_is_file_construction(&self) -> bool { - self.alpha_item - .as_ref() - .is_some_and(|item| item.construction_method == ConstructionMethod::File) + pub fn alpha_item_construction_method(&self) -> Option { + Some(self.alpha_item.as_ref()?.construction_method) } fn image_bits_per_channel(&self, item_id: ItemId) -> Result<&[u8]> { @@ -2077,8 +2037,8 @@ impl DataBox { /// referencing data within this type of box. fn location(&self, extent: &Extent) -> IsobmffItem { match self.metadata { - DataBoxMetadata::Idat => IsobmffItem::IdatLocation(extent.clone()), - DataBoxMetadata::Mdat { .. } => IsobmffItem::MdatLocation(extent.clone()), + DataBoxMetadata::Idat => IsobmffItem::IdatLocation(*extent), + DataBoxMetadata::Mdat { .. } => IsobmffItem::MdatLocation(*extent), } } @@ -2206,9 +2166,14 @@ struct ItemLocationBoxItem { /// > — `construction_method` shall be equal to 0 for MIAF image items that are coded image items.
/// > — `construction_method` shall be equal to 0 or 1 for MIAF image items that are derived image items. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum ConstructionMethod { +pub enum ConstructionMethod { + /// The extents are offsets into the file, so a caller can map an extent + /// onto a file offset it can read or wait for. File = 0, + /// The extents are offsets into the `idat` box, whose bytes this crate has + /// already read. Idat = 1, + /// The extents name other items; not implemented, see [`Status::ConstructionMethod`]. Item = 2, } @@ -2220,9 +2185,23 @@ enum ConstructionMethod { /// `usize::MAX` can be used in a successful indexing operation in rust. /// `extent_index` is omitted since it's only used for ConstructionMethod::Item which /// is currently not implemented. -#[derive(Clone, Debug)] -enum Extent { - WithLength { offset: u64, len: usize }, +/// +/// `offset` in either variant already has the `iloc` entry's `base_offset` +/// added, so for [`ConstructionMethod::File`] it is an absolute file offset. +/// See ISOBMFF (ISO 14496-12:2020) § 8.11.3. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Extent { + WithLength { + offset: u64, + /// Never zero: an `extent_length` of zero means "the entire length of + /// the source is implied" per § 8.11.3.1, which is [`Extent::ToEnd`]. + len: usize, + }, + /// The `iloc` entry gave no length, which per § 8.11.3.1 means "the entire + /// length of the source is implied". This crate resolves that against + /// whichever box the bytes were found in, so the extent ends at the end of + /// the enclosing `idat` or `mdat` -- not at the end of the file, even for + /// [`ConstructionMethod::File`]. ToEnd { offset: u64 }, } @@ -2818,7 +2797,7 @@ pub fn read_avif(f: &mut T, strictness: ParseStrictness) -> Result fn read_a1lx(src: &mut BMFFBox) -> Result { - let flags = src.read_u8()?; + // Not a FullBox, so this leading byte is the reserved bits and large_size, + // not a version/flags word. + let reserved_and_large_size = src.read_u8()?; // unsigned int(7) reserved = 0; warned about rather than rejected, since // the property is non-essential and this is the only field we would be // guessing about. - if flags & 0xfe != 0 { - warn!("a1lx reserved bits are not zero: {flags:#x}"); + if reserved_and_large_size & 0xfe != 0 { + warn!("a1lx reserved bits are not zero: {reserved_and_large_size:#x}"); } // unsigned int(1) large_size; FieldLength = (large_size + 1) * 16 - let large_size = flags & 1 == 1; + let large_size = reserved_and_large_size & 1 == 1; let mut layer_sizes = [0u32; 3]; for layer_size in &mut layer_sizes { diff --git a/mp4parse/tests/public.rs b/mp4parse/tests/public.rs index 32718ca1..493c27e5 100644 --- a/mp4parse/tests/public.rs +++ b/mp4parse/tests/public.rs @@ -1281,12 +1281,17 @@ fn public_avif_a1lx() { assert_eq!(context.primary_item_lsel(), None); // The extents are where the layer payloads are cut from. - assert!(context.primary_item_is_file_construction()); + assert_eq!( + context.primary_item_construction_method(), + Some(mp4::ConstructionMethod::File) + ); let extents = context .primary_item_extents() .expect("primary item should be present"); assert!(!extents.is_empty()); - assert!(extents.iter().all(|e| !e.to_end && e.len > 0)); + assert!(extents + .iter() + .all(|e| matches!(e, mp4::Extent::WithLength { .. }))); }); } diff --git a/mp4parse_capi/cbindgen.toml b/mp4parse_capi/cbindgen.toml index 5b2c8eb0..56372353 100644 --- a/mp4parse_capi/cbindgen.toml +++ b/mp4parse_capi/cbindgen.toml @@ -44,4 +44,3 @@ include = ["Status", "Feature"] "Indice" = "Mp4parseIndice" "NclxColourInformation" = "Mp4parseNclxColourInformation" "AV1LayeredImageIndexing" = "Mp4parseA1lx" -"ItemExtent" = "Mp4parseItemExtent" diff --git a/mp4parse_capi/src/lib.rs b/mp4parse_capi/src/lib.rs index baa35655..23835ea6 100644 --- a/mp4parse_capi/src/lib.rs +++ b/mp4parse_capi/src/lib.rs @@ -157,7 +157,48 @@ impl Mp4parseByteData { } } -/// A borrowed slice of [`mp4parse::ItemExtent`], valid for the lifetime of the +/// A region of the file holding part of an item's payload, as given by the +/// `extent_offset` and `extent_length` of an `iloc` entry. +/// +/// `offset` already has the entry's `base_offset` added, so when the item's +/// `construction_method` is 0 it is an absolute file offset. See ISOBMFF +/// (ISO 14496-12:2020) § 8.11.3. +/// +/// This is the flattened, `repr(C)` form of [`mp4parse::Extent`], which cannot +/// cross the FFI boundary as the enum it is. Only two of the representable +/// states occur: `to_end == false` with `len > 0`, and `to_end == true` with +/// `len == 0`. +/// +/// `to_end` is an `extent_length` of zero, which per § 8.11.3.1 means "the +/// entire length of the source is implied". The parser resolves that against +/// the enclosing `mdat` or `idat`, whose end this struct cannot name, so a +/// caller cutting byte ranges gets a start but no end for such an extent. +#[repr(C)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct Mp4parseItemExtent { + pub offset: u64, + pub len: u64, + pub to_end: bool, +} + +impl From for Mp4parseItemExtent { + fn from(extent: mp4parse::Extent) -> Self { + match extent { + mp4parse::Extent::WithLength { offset, len } => Self { + offset, + len: len as u64, + to_end: false, + }, + mp4parse::Extent::ToEnd { offset } => Self { + offset, + len: 0, + to_end: true, + }, + } + } +} + +/// A borrowed slice of [`Mp4parseItemExtent`], valid for the lifetime of the /// parser it was obtained from. /// /// The extents are in payload order, which is what a caller cutting the payload @@ -168,11 +209,11 @@ impl Mp4parseByteData { #[derive(Debug)] pub struct Mp4parseItemExtents { pub length: usize, - pub extents: *const mp4parse::ItemExtent, + pub extents: *const Mp4parseItemExtent, } impl Mp4parseItemExtents { - fn with_extents(slice: &[mp4parse::ItemExtent]) -> Self { + fn with_extents(slice: &[Mp4parseItemExtent]) -> Self { Self { length: slice.len(), extents: if slice.is_empty() { @@ -193,6 +234,25 @@ impl Default for Mp4parseItemExtents { } } +/// Whether an item's extents are offsets into the file rather than into an +/// `idat` box or another item. Absent items report false, matching the +/// `has_primary_item`/`has_alpha_item` flags the caller checks first. +fn is_file_construction(method: Option) -> bool { + matches!(method, Some(mp4parse::ConstructionMethod::File)) +} + +/// Flatten an item's extents into the `repr(C)` form the C API exposes. +fn flatten_extents( + extents: Option<&[mp4parse::Extent]>, +) -> mp4parse::Result> { + let extents = extents.unwrap_or(&[]); + let mut flattened = TryVec::with_capacity(extents.len())?; + for &extent in extents { + flattened.push(Mp4parseItemExtent::from(extent))?; + } + Ok(flattened) +} + impl Default for Mp4parseByteData { fn default() -> Self { Self { @@ -448,8 +508,15 @@ pub struct Mp4parseAvifInfo { /// The layer sizes from the primary item's `a1lx`, or null if it has none. /// + /// Non-null means only that the property is *present*. It gives no layer + /// boundary when `layer_sizes[0] == 0`, which covers both a spec-legal + /// single-layer `a1lx` and a malformed one, since a malformed `a1lx` is + /// recorded as present with no sizes rather than failing the parse. A + /// caller asking "is this item layered" therefore has to check + /// `layer_sizes[0] != 0` too. + /// /// An `a1lx` "should not be associated with AV1 Image Items consisting of - /// only one layer", so a non-null value here means the item is layered. See + /// only one layer". See /// . pub primary_item_a1lx: *const mp4parse::AV1LayeredImageIndexing, /// As `primary_item_a1lx`, but for the alpha item. @@ -511,7 +578,7 @@ where { type Context; - fn with_context(context: Self::Context) -> Self; + fn with_context(context: Self::Context) -> mp4parse::Result; fn read(io: &mut T, strictness: ParseStrictness) -> mp4parse::Result; } @@ -529,11 +596,11 @@ impl Mp4parseParser { impl ContextParser for Mp4parseParser { type Context = MediaContext; - fn with_context(context: Self::Context) -> Self { - Self { + fn with_context(context: Self::Context) -> mp4parse::Result { + Ok(Self { context, ..Default::default() - } + }) } fn read(io: &mut T, strictness: ParseStrictness) -> mp4parse::Result { @@ -547,6 +614,12 @@ impl ContextParser for Mp4parseParser { pub struct Mp4parseAvifParser { context: AvifContext, sample_table: TryHashMap>, + // `mp4parse::Extent` is an enum, so it can't be handed to C as-is. The + // flattened form is built once here rather than per `mp4parse_avif_get_info` + // call, because the pointers `Mp4parseAvifInfo` exposes have to stay valid + // for as long as the parser does. + primary_item_extents: TryVec, + alpha_item_extents: TryVec, } trait CacheInsertExt { @@ -576,11 +649,13 @@ impl Mp4parseAvifParser { impl ContextParser for Mp4parseAvifParser { type Context = AvifContext; - fn with_context(context: Self::Context) -> Self { - Self { + fn with_context(context: Self::Context) -> mp4parse::Result { + Ok(Self { + primary_item_extents: flatten_extents(context.primary_item_extents())?, + alpha_item_extents: flatten_extents(context.alpha_item_extents())?, context, ..Default::default() - } + }) } fn read(io: &mut T, strictness: ParseStrictness) -> mp4parse::Result { @@ -692,7 +767,7 @@ fn mp4parse_new_common_safe( strictness: ParseStrictness, ) -> Result<*mut P, Mp4parseStatus> { P::read(io, strictness) - .map(P::with_context) + .and_then(P::with_context) .and_then(|x| TryBox::try_new(x).map_err(mp4parse::Error::from)) .map(TryBox::into_raw) .map_err(Mp4parseStatus::from) @@ -1316,7 +1391,7 @@ pub unsafe extern "C" fn mp4parse_avif_get_info( return Mp4parseStatus::BadArg; } - if let Ok(info) = mp4parse_avif_get_info_safe((*parser).context()) { + if let Ok(info) = mp4parse_avif_get_info_safe(&*parser) { *avif_info = info; Mp4parseStatus::Ok } else { @@ -1324,7 +1399,8 @@ pub unsafe extern "C" fn mp4parse_avif_get_info( } } -fn mp4parse_avif_get_info_safe(context: &AvifContext) -> mp4parse::Result { +fn mp4parse_avif_get_info_safe(parser: &Mp4parseAvifParser) -> mp4parse::Result { + let context = parser.context(); let info = Mp4parseAvifInfo { premultiplied_alpha: context.premultiplied_alpha, major_brand: context.major_brand.value, @@ -1352,14 +1428,14 @@ fn mp4parse_avif_get_info_safe(context: &AvifContext) -> mp4parse::Result 0); + assert!(!info1.primary_item_extents.extents.is_null()); + assert_slice_pointer_is_readable( + info1.primary_item_extents.extents, + info1.primary_item_extents.length, + ); + + // No alpha item, so no extents; empty slices use null pointers. + assert!(!info1.has_alpha_item); + assert_eq!(info1.alpha_item_extents.length, 0); + assert!(info1.alpha_item_extents.extents.is_null()); + + let extents1: &[Mp4parseItemExtent] = std::slice::from_raw_parts( + info1.primary_item_extents.extents, + info1.primary_item_extents.length, + ); + // This file's extents are all bounded, i.e. `Extent::WithLength`. + assert!(extents1.iter().all(|e| !e.to_end && e.len > 0)); + + let mut info2 = default_avif_info(); + let rv = mp4parse_avif_get_info(parser, &mut info2); + assert_eq!(rv, Mp4parseStatus::Ok); + assert_eq!( + info1.primary_item_extents.length, + info2.primary_item_extents.length + ); + assert_eq!( + info1.primary_item_extents.extents, + info2.primary_item_extents.extents + ); + + let extents2: &[Mp4parseItemExtent] = std::slice::from_raw_parts( + info2.primary_item_extents.extents, + info2.primary_item_extents.length, + ); + assert_eq!(extents1, extents2); + + mp4parse_avif_free(parser); + } +} + +/// Both `mp4parse::Extent` variants flatten into exactly one of the two states +/// `Mp4parseItemExtent` is documented to take. No test file produces a +/// zero-`extent_length` `iloc` entry, so cover the mapping directly. +#[test] +fn item_extents_flatten_both_extent_variants() { + assert_eq!( + Mp4parseItemExtent::from(mp4parse::Extent::WithLength { offset: 42, len: 7 }), + Mp4parseItemExtent { + offset: 42, + len: 7, + to_end: false, + } + ); + assert_eq!( + Mp4parseItemExtent::from(mp4parse::Extent::ToEnd { offset: 42 }), + Mp4parseItemExtent { + offset: 42, + len: 0, + to_end: true, + } + ); +} + #[test] fn empty_avif_byte_slices_use_null_pointers() { let (parser, info) = unsafe { parse_file_and_get_info("tests/no_edts.avif") };