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

Author: tnikkel

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<T: Read>(
     let mut association_entries = TryVec::<ItemPropertyAssociationEntry>::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 <https://aomediacodec.github.io/av1-avif/#layer-selector-property>
+    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<T: Read>(
                     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<T: Read>(
                             }
                         }
 
-                        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<T: Read>(
             }
             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<T: Read>(
                 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<T: Read>(
     Ok(properties)
 }
 
+/// Parse a LayerSelectorProperty, returning its layer_id.
+///
+/// See <https://aomediacodec.github.io/av1-avif/#layer-selector-property>
+fn read_lsel<T: Read>(src: &mut BMFFBox<T>) -> Result<u16> {
+    let layer_id = be_u16(src)?;
+    Ok(layer_id)
+}
+
 #[repr(C)]
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct ImageSpatialExtentsProperty {

mp4parse/tests/lsel-layer-id-ffff.avif

@@ -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,26 @@ 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);