diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index 4aade002..8fb39403 100644 --- a/mp4parse/src/lib.rs +++ b/mp4parse/src/lib.rs @@ -324,7 +324,7 @@ impl TryFrom<&ItemProperty> for Feature { ItemProperty::Colour(_) => Self::Colr, ItemProperty::ImageSpatialExtents(_) => Self::Ispe, ItemProperty::LayeredImageIndexing => Self::A1lx, - ItemProperty::LayerSelection => Self::Lsel, + ItemProperty::LayerSelection(_) => Self::Lsel, ItemProperty::Mirroring(_) => Self::Imir, ItemProperty::OperatingPointSelector => Self::A1op, ItemProperty::PixelAspectRatio(_) => Self::Pasp, @@ -3024,6 +3024,14 @@ fn read_iprp( let mut association_entries = TryVec::::new(); let mut forbidden_items = TryVec::new(); + // A LayerSelectorProperty with this layer_id enables, but does not require, + // progressive rendering: a client may render progressively or just show the + // final image (which is also what ignoring the property does), so no client + // can get this wrong. It therefore does not need to be treated as an + // unsupported essential property, and its item is accepted and processed. + // See + const LSEL_LAYER_ID_NO_SELECTION: u16 = 0xffff; + while let Some(mut b) = iter.next_box()? { if b.head.name != BoxType::ItemPropertyAssociationBox { return Status::IprpBadChild.into(); @@ -3089,16 +3097,25 @@ fn read_iprp( assert!(brand == MIF1_BRAND); let feature = Feature::try_from(property); - let property_supported = match feature { - Ok(feature) => { - if feature.supported() { - true - } else { - unsupported_features.insert(feature); - false + let property_supported = if matches!( + property, + ItemProperty::LayerSelection(layer_id) + if *layer_id == LSEL_LAYER_ID_NO_SELECTION + ) { + // Not an unsupported feature; see LSEL_LAYER_ID_NO_SELECTION. + true + } else { + match feature { + Ok(feature) => { + if feature.supported() { + true + } else { + unsupported_features.insert(feature); + false + } } + Err(_) => false, } - Err(_) => false, }; if !property_supported { @@ -3169,18 +3186,22 @@ fn read_iprp( } } - ItemProperty::LayerSelection => { - assert!(feature.is_ok() && unsupported_features.contains(feature?)); - if a.essential { - assert!( - forbidden_items.contains(&association_entry.item_id) - || strictness == ParseStrictness::Permissive - ); - } else { + ItemProperty::LayerSelection(layer_id) => { + if !a.essential { + // lsel shall be marked as essential regardless of its + // layer_id. fail_with_status_if( strictness != ParseStrictness::Permissive, Status::LselNoEssential, )?; + } else if *layer_id != LSEL_LAYER_ID_NO_SELECTION { + // A specific layer was requested; selecting a layer is + // unsupported, so the item shall not be processed. + assert!(feature.is_ok() && unsupported_features.contains(feature?)); + assert!( + forbidden_items.contains(&association_entry.item_id) + || strictness == ParseStrictness::Permissive + ); } } @@ -3266,7 +3287,7 @@ pub enum ItemProperty { Colour(ColourInformation), ImageSpatialExtents(ImageSpatialExtentsProperty), LayeredImageIndexing, - LayerSelection, + LayerSelection(u16), Mirroring(ImageMirror), OperatingPointSelector, PixelAspectRatio(PixelAspectRatio), @@ -3283,7 +3304,7 @@ impl From<&ItemProperty> for BoxType { ItemProperty::CleanAperture => BoxType::CleanApertureBox, ItemProperty::Colour(_) => BoxType::ColourInformationBox, ItemProperty::LayeredImageIndexing => BoxType::AV1LayeredImageIndexingProperty, - ItemProperty::LayerSelection => BoxType::LayerSelectorProperty, + ItemProperty::LayerSelection(_) => BoxType::LayerSelectorProperty, ItemProperty::Mirroring(_) => BoxType::ImageMirror, ItemProperty::OperatingPointSelector => BoxType::OperatingPointSelectorProperty, ItemProperty::PixelAspectRatio(_) => BoxType::PixelAspectRatioBox, @@ -3653,6 +3674,7 @@ 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)?), other_box_type => { // Even if we didn't do anything with other property types, we still store @@ -3661,7 +3683,6 @@ fn read_ipco( let item_property = match other_box_type { BoxType::AV1LayeredImageIndexingProperty => ItemProperty::LayeredImageIndexing, BoxType::CleanApertureBox => ItemProperty::CleanAperture, - BoxType::LayerSelectorProperty => ItemProperty::LayerSelection, BoxType::OperatingPointSelectorProperty => ItemProperty::OperatingPointSelector, _ => { warn!("No ItemProperty variant for {other_box_type:?}"); @@ -3687,6 +3708,14 @@ fn read_ipco( Ok(properties) } +/// Parse a LayerSelectorProperty, returning its layer_id. +/// +/// See +fn read_lsel(src: &mut BMFFBox) -> Result { + let layer_id = be_u16(src)?; + Ok(layer_id) +} + #[repr(C)] #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct ImageSpatialExtentsProperty { diff --git a/mp4parse/tests/lsel-layer-id-ffff.avif b/mp4parse/tests/lsel-layer-id-ffff.avif new file mode 100644 index 00000000..fd261203 Binary files /dev/null and b/mp4parse/tests/lsel-layer-id-ffff.avif differ diff --git a/mp4parse/tests/public.rs b/mp4parse/tests/public.rs index 632deabe..82f858a6 100644 --- a/mp4parse/tests/public.rs +++ b/mp4parse/tests/public.rs @@ -82,6 +82,11 @@ static AVIF_TEST_DIRS: &[&str] = &["tests", "av1-avif/testFiles", "link-u-avif-s static AVIF_A1OP: &str = "tests/a1op.avif"; static AVIF_A1LX: &str = "tests/a1lx.avif"; static AVIF_LSEL: &str = "tests/lsel.avif"; +// This is tests/valid.avif with an essential lsel property whose layer_id is +// 0xffff added to the primary item. That layer_id selects no particular layer, +// so the item is still accepted rather than treated as having an unsupported +// essential property. +static AVIF_LSEL_LAYER_ID_FFFF: &str = "tests/lsel-layer-id-ffff.avif"; static AVIF_CLAP: &str = "tests/clap-basic-1_3x3-to-1x1.avif"; static AVIF_GRID: &str = "av1-avif/testFiles/Microsoft/Summer_in_Tomsk_720p_5x4_grid.avif"; @@ -1285,6 +1290,29 @@ fn public_avif_lsel_missing_essential() { assert_avif_shall(IMAGE_AVIF_LSEL_MISSING_ESSENTIAL, Status::LselNoEssential); } +// An lsel property with layer_id 0xffff does not select a specific layer, so the +// item is still accepted (decodable, and lsel not reported as an unsupported +// feature) regardless of strictness. +#[test] +fn public_avif_lsel_no_layer_selection() { + for_strictness_result( + AVIF_LSEL_LAYER_ID_FFFF, + |_strictness, result| match result { + Ok(context) => { + assert!( + !context.unsupported_features.contains(mp4::Feature::Lsel), + "lsel with layer_id 0xffff should not be an unsupported feature" + ); + assert!( + context.primary_item_coded_data().is_some(), + "primary item associated with an lsel of layer_id 0xffff should be decodable" + ); + } + r => panic!("Expected Ok, found {:?}", r), + }, + ); +} + #[test] fn public_avif_clap() { assert_unsupported_essential(AVIF_CLAP, mp4::Feature::Clap);