From 1848fd236c88d28fdfa72b671aac3096157ed59d Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Fri, 17 Jul 2026 05:24:16 -0500 Subject: [PATCH 1/2] Accept AVIF lsel property with layer_id 0xffff Currently any lsel (LayerSelectorProperty) is treated as an unsupported essential property, so its item is forbidden and consumers reject the whole image. A layer_id of 0xffff, however, does not select a specific spatial layer. It only enables (without requiring) 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 it wrong. Such a property therefore should not make the item unsupported. Parse the lsel layer_id (previously the box was skipped) and, when it is 0xffff, accept the item and don't report lsel as an unsupported feature. Other layer_id values remain unsupported as before, and the requirement that lsel be marked essential is still enforced for all layer_id values. Adds a test and fixture (tests/lsel-layer-id-ffff.avif, a green image carrying an essential lsel with layer_id 0xffff). See https://aomediacodec.github.io/av1-avif/#layer-selector-property --- mp4parse/src/lib.rs | 69 ++++++++++++++++++------- mp4parse/tests/lsel-layer-id-ffff.avif | Bin 0 -> 305 bytes mp4parse/tests/public.rs | 25 +++++++++ 3 files changed, 74 insertions(+), 20 deletions(-) create mode 100644 mp4parse/tests/lsel-layer-id-ffff.avif 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 0000000000000000000000000000000000000000..fd26120376cf50cb43fd03015b2e37fe6e8ee39f GIT binary patch literal 305 zcmXv`y>7xV5IzS{B7_jC3Ze@NLwC}N6}1BcEWCjgV)dlBoDwU7u>$r!Tc4zF0N(-m zth?{eHW4K*9K_b_2vGLyQqR_{7wH1gzt4l!4w!yzi}EgsgtO4-hWo<7-;BYYaIwqO zeG(ZQ-DJk#SIispx6=?%8p7t$!`XV4sw`Pb0dH8c9r%SS$+Z9;AJRa0&l { + 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); From c5b30f75e6ccc91f1873604fc3c3e76b81e4255c Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Sat, 18 Jul 2026 11:12:02 +1200 Subject: [PATCH 2/2] cargo fmt --- mp4parse/tests/public.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/mp4parse/tests/public.rs b/mp4parse/tests/public.rs index 6376d3af..82f858a6 100644 --- a/mp4parse/tests/public.rs +++ b/mp4parse/tests/public.rs @@ -1295,19 +1295,22 @@ fn public_avif_lsel_missing_essential() { // 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), - }); + 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]