Skip to content

Commit

Permalink
Ignore descr props after transf props
Browse files Browse the repository at this point in the history
Also reject non-essential transformative properties.
  • Loading branch information
y-guyon committed Jan 14, 2025
1 parent d980845 commit 881195f
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ The changes are relative to the previous release, unless the baseline is specifi
avifCleanApertureBoxConvertCropRect(). Replace them with
avifCropRectFromCleanApertureBox() and avifCleanApertureBoxFromCropRect().
* Add avifCropRectRequiresUpsampling().
* Ignore descriptive properties associated after transformative ones.
* Reject non-essential transformative properties.

## [1.1.1] - 2024-07-30

Expand Down
50 changes: 48 additions & 2 deletions src/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 Section 6.5.1:
// 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 Section 3.1.29:
// item property: descriptive or transformative information
const avifBool isDescriptive = !isTransformative;
// ISO/IEC 23008-12 Section 7.3.9 :
// 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;
Expand Down Expand Up @@ -2790,7 +2813,15 @@ 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: 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]);
Expand All @@ -2809,8 +2840,23 @@ static avifResult avifParseItemPropertyAssociation(avifMeta * meta, const uint8_
avifProperty * dstProp = (avifProperty *)avifArrayPush(&item->properties);
AVIF_CHECKERR(dstProp != NULL, AVIF_RESULT_OUT_OF_MEMORY);
*dstProp = *srcProp;

if (isTransformative) {
transformativePropertySeen = AVIF_TRUE;
}
} else {
AVIF_ASSERT_OR_RETURN(!isTransformative);

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.
// Assuming this rule also applies to items whose 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;
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
21 changes: 21 additions & 0 deletions tests/data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Binary file added tests/data/clap_irot_imir_non_essential.avif
Binary file not shown.
Binary file added tests/data/clop_irot_imor.avif
Binary file not shown.
106 changes: 106 additions & 0 deletions tests/gtest/aviftransformtest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// 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) {
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();
}

0 comments on commit 881195f

Please sign in to comment.