Ignore descr props after transf props

CHANGELOG.md

@@ -55,6 +55,8 @@ The changes are relative to the previous release, unless the baseline is specifi
 * Deprecate avifCropRectConvertCleanApertureBox() and
   avifCleanApertureBoxConvertCropRect(). Replace them with
   avifCropRectFromCleanApertureBox() and avifCleanApertureBoxFromCropRect().
+* Ignore descriptive properties associated after transformative ones.
+* Reject non-essential transformative properties.
 
 ## [1.1.1] - 2024-07-30
 

src/read.c

@@ -2731,15 +2731,21 @@ static avifResult avifParseItemPropertyAssociation(avifMeta * meta, const uint8_
 
         uint8_t associationCount;
         AVIF_CHECKERR(avifROStreamRead(&s, &associationCount, 1), AVIF_RESULT_BMFF_PARSE_FAILED);
+        avifBool transformativePropertySeen = AVIF_FALSE;
         for (uint8_t associationIndex = 0; associationIndex < associationCount; ++associationIndex) {
             uint8_t essential;
             AVIF_CHECKERR(avifROStreamReadBitsU8(&s, &essential, /*bitCount=*/1), AVIF_RESULT_BMFF_PARSE_FAILED); // bit(1) essential;
             uint32_t propertyIndex;
             AVIF_CHECKERR(avifROStreamReadBitsU32(&s, &propertyIndex, /*bitCount=*/propertyIndexIsU15 ? 15 : 7),
                           AVIF_RESULT_BMFF_PARSE_FAILED); // unsigned int(7/15) property_index;
 
+            // ISO/IEC 14496-12 Section 8.11.14.3:
+            //   0 indicating that no property is associated (the essential indicator shall also be 0)
             if (propertyIndex == 0) {
-                // Not associated with any item
+                if (essential) {
+                    avifDiagnosticsPrintf(diag, "Box[ipma] for item ID [%u] contains an illegal essential property index 0", itemID);
+                    return AVIF_RESULT_BMFF_PARSE_FAILED;
+                }
                 continue;
             }
             --propertyIndex; // 1-indexed
@@ -2756,6 +2762,23 @@ static avifResult avifParseItemPropertyAssociation(avifMeta * meta, const uint8_
             // Copy property to item
             const avifProperty * srcProp = &meta->properties.prop[propertyIndex];
 
+            // ISO/IEC 23000-22:2019/Amd. 2:2021 Section 7.3.9:
+            //   All transformative properties associated with coded and derived images shall be marked as essential,
+            //   and shall be from the set defined in 7.3.6.7 or the applicable MIAF profile. No other essential
+            //   transformative property shall be associated with such images.
+            const avifBool isTransformative = !memcmp(srcProp->type, "clap", 4) || !memcmp(srcProp->type, "irot", 4) ||
+                                              !memcmp(srcProp->type, "imir", 4);
+            // ISO/IEC 23008-12:2022 Section 3.1.28:
+            //   item property: descriptive or transformative information
+            const avifBool isDescriptive = !isTransformative;
+            // ISO/IEC 23008-12:2022 Section 6.5.1:
+            //   Readers shall allow and ignore descriptive properties following the first transformative or
+            //   unrecognized property, whichever is earlier, in the sequence associating properties with an item.
+            // No need to check for unrecognized properties as they cannot be transformative according to MIAF.
+            if (transformativePropertySeen && isDescriptive) {
+                continue;
+            }
+
             // Some properties are supported and parsed by libavif.
             // Other properties are forwarded to the user as opaque blobs.
             const avifBool supportedType = !srcProp->isOpaque;
@@ -2790,7 +2813,16 @@ static avifResult avifParseItemPropertyAssociation(avifMeta * meta, const uint8_
                         "a1op",
 
                         // HEIF: Section 6.5.11.1: "essential shall be equal to 1 for an 'lsel' item property."
-                        "lsel"
+                        "lsel",
+
+                        // MIAF 2019/Amd. 2:2021: Section 7.3.9:
+                        //   All transformative properties associated with coded and derived images shall be
+                        //   marked as essential
+                        // It makes no sense to allow for non-essential crop/orientation associated with an item
+                        // that is not a coded or derived image, so for simplicity 'item' is not checked here.
+                        "clap",
+                        "irot",
+                        "imir"
 
                     };
                     size_t essentialTypesCount = sizeof(essentialTypes) / sizeof(essentialTypes[0]);
@@ -2811,6 +2843,15 @@ static avifResult avifParseItemPropertyAssociation(avifMeta * meta, const uint8_
                 *dstProp = *srcProp;
             } else {
                 if (essential) {
+                    // ISO/IEC 23008-12 Section 10.2.1:
+                    //   Under any brand, the primary item (or an alternative if alternative support is required)
+                    //   shall be processable by a reader implementing only the required features of that brand.
+                    //   Specifically, given that each brand has a set of properties that a reader is required to
+                    //   support: the item shall not have properties that are marked as essential and are outside
+                    //   this set.
+                    // It is assumed that this rule also applies to items the primary item depends on (such as
+                    // the cells of a grid).
+
                     // Discovered an essential item property that libavif doesn't support!
                     // Make a note to ignore this item later.
                     item->hasUnsupportedEssentialProperty = AVIF_TRUE;
@@ -2825,6 +2866,11 @@ static avifResult avifParseItemPropertyAssociation(avifMeta * meta, const uint8_
                 AVIF_CHECKRES(
                     avifRWDataSet(&dstProp->u.opaque.boxPayload, srcProp->u.opaque.boxPayload.data, srcProp->u.opaque.boxPayload.size));
             }
+
+            if (isTransformative) {
+                AVIF_ASSERT_OR_RETURN(supportedType);
+                transformativePropertySeen = AVIF_TRUE;
+            }
         }
     }
     return AVIF_RESULT_OK;

tests/CMakeLists.txt

@@ -146,6 +146,7 @@ if(AVIF_GTEST)
     add_avif_gtest_with_data(avifsize0test)
     add_avif_internal_gtest(avifstreamtest)
     add_avif_internal_gtest(aviftilingtest)
+    add_avif_gtest_with_data(aviftransformtest)
     add_avif_internal_gtest(avifutilstest)
     add_avif_gtest(avify4mtest)
 

tests/data/README.md

@@ -35,6 +35,27 @@ License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LIC
 Source: `avifenc circle-trns-after-plte.png` with custom properties added in
 `avifRWStreamWriteProperties()`: FullBox `1234`, Box `abcd` and Box `uuid`.
 
+### File [clap_irot_imir_non_essential.avif](clap_irot_imir_non_essential.avif)
+
+![](clap_irot_imir_non_essential.avif)
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source: File generated with `TransformTest.ClapIrotImir` from
+`aviftransformtest.cc`, where `clap`, `irot` and `imir` are tagged as
+non-essential.
+
+### File [clop_irot_imor.avif](clop_irot_imor.avif)
+
+![](clop_irot_imor.avif)
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source: File generated with `TransformTest.ClapIrotImir` from
+`aviftransformtest.cc`, where essential `clap` property was replaced by made-up
+non-essential `clop` property and essential `imir` property was replaced by
+made-up non-essential `imor` property.
+
 ### File [draw_points.png](draw_points.png)
 
 ![](draw_points.png)

tests/gtest/aviftransformtest.cc

@@ -0,0 +1,110 @@
+// Copyright 2025 Google LLC
+// SPDX-License-Identifier: BSD-2-Clause
+
+#include "avif/avif.h"
+#include "aviftest_helpers.h"
+#include "gtest/gtest.h"
+
+namespace avif {
+namespace {
+
+// Used to pass the data folder path to the GoogleTest suites.
+const char* data_path = nullptr;
+
+//------------------------------------------------------------------------------
+
+TEST(TransformTest, ClapIrotImir) {
+  if (!testutil::Av1EncoderAvailable() || !testutil::Av1DecoderAvailable()) {
+    GTEST_SKIP() << "AV1 codec unavailable, skip test.";
+  }
+
+  avifDiagnostics diag{};
+  ImagePtr image =
+      testutil::CreateImage(/*width=*/12, /*height=*/34, /*depth=*/10,
+                            AVIF_PIXEL_FORMAT_YUV444, AVIF_PLANES_ALL);
+  ASSERT_NE(image, nullptr);
+  testutil::FillImageGradient(image.get());  // The pixels do not matter.
+
+  image->transformFlags |= AVIF_TRANSFORM_CLAP;
+  const avifCropRect rect{/*x=*/4, /*y=*/6, /*width=*/8, /*height=*/10};
+  ASSERT_TRUE(avifCleanApertureBoxFromCropRect(
+      &image->clap, &rect, image->width, image->height, &diag));
+
+  image->transformFlags |= AVIF_TRANSFORM_IROT;
+  image->irot.angle = 1;
+  image->transformFlags |= AVIF_TRANSFORM_IMIR;
+  image->imir.axis = 1;
+
+  // Encode.
+  EncoderPtr encoder(avifEncoderCreate());
+  ASSERT_NE(encoder, nullptr);
+  encoder->speed = AVIF_SPEED_FASTEST;
+  testutil::AvifRwData encoded;
+  ASSERT_EQ(avifEncoderWrite(encoder.get(), image.get(), &encoded),
+            AVIF_RESULT_OK);
+
+  // Decode.
+  ImagePtr decoded(avifImageCreateEmpty());
+  ASSERT_NE(decoded, nullptr);
+  DecoderPtr decoder(avifDecoderCreate());
+  ASSERT_NE(decoder, nullptr);
+  ASSERT_EQ(avifDecoderReadMemory(decoder.get(), decoded.get(), encoded.data,
+                                  encoded.size),
+            AVIF_RESULT_OK);
+
+  EXPECT_EQ(decoded->transformFlags, image->transformFlags);
+  EXPECT_EQ(decoded->clap.widthN, image->clap.widthN);
+  EXPECT_EQ(decoded->clap.widthD, image->clap.widthD);
+  EXPECT_EQ(decoded->clap.heightN, image->clap.heightN);
+  EXPECT_EQ(decoded->clap.heightD, image->clap.heightD);
+  EXPECT_EQ(decoded->clap.horizOffN, image->clap.horizOffN);
+  EXPECT_EQ(decoded->clap.horizOffD, image->clap.horizOffD);
+  EXPECT_EQ(decoded->clap.vertOffN, image->clap.vertOffN);
+  EXPECT_EQ(decoded->clap.vertOffD, image->clap.vertOffD);
+  EXPECT_EQ(decoded->irot.angle, image->irot.angle);
+  EXPECT_EQ(decoded->imir.axis, image->imir.axis);
+}
+
+TEST(TransformTest, ClapIrotImirNonEssential) {
+  // Invalid file with non-essential transformative properties.
+  const std::string path =
+      std::string(data_path) + "clap_irot_imir_non_essential.avif";
+  DecoderPtr decoder(avifDecoderCreate());
+  ASSERT_NE(decoder, nullptr);
+  ASSERT_EQ(avifDecoderSetIOFile(decoder.get(), path.c_str()), AVIF_RESULT_OK);
+  ASSERT_EQ(avifDecoderParse(decoder.get()), AVIF_RESULT_BMFF_PARSE_FAILED);
+}
+
+TEST(TransformTest, ClopIrotImor) {
+  // File with a non-essential unrecognized property 'clop', an essential
+  // transformation property 'irot', and a non-essential unrecognized property
+  // 'imor'.
+  const std::string path = std::string(data_path) + "clop_irot_imor.avif";
+  DecoderPtr decoder(avifDecoderCreate());
+  ASSERT_NE(decoder, nullptr);
+  ASSERT_EQ(avifDecoderSetIOFile(decoder.get(), path.c_str()), AVIF_RESULT_OK);
+  ASSERT_EQ(avifDecoderParse(decoder.get()), AVIF_RESULT_OK);
+
+  // 'imor' shall be ignored by libavif as it is after a transformative property
+  // in the 'ipma' association order.
+  ASSERT_EQ(decoder->image->numProperties, 1u);
+  const avifImageItemProperty& clop = decoder->image->properties[0];
+  EXPECT_EQ(std::string(clop.boxtype, clop.boxtype + 4), "clop");
+}
+
+//------------------------------------------------------------------------------
+
+}  // namespace
+}  // namespace avif
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  if (argc != 2) {
+    std::cerr << "There must be exactly one argument containing the path to "
+                 "the test data folder"
+              << std::endl;
+    return 1;
+  }
+  avif::data_path = argv[1];
+  return RUN_ALL_TESTS();
+}