Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

props: write out any properties set on the image #2510

Merged
merged 22 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
86fced7
props: write out any properties set on the image
bradh Nov 27, 2024
f6ebb58
properties: clean up test suite for review comments
bradh Nov 27, 2024
7f4afe2
properties: do not test property encoding on AVM
bradh Nov 27, 2024
e3bb17e
properties: add extra usage documentation
bradh Nov 27, 2024
45c2076
properties: sanity check boxtype and UUID values
bradh Nov 30, 2024
2df40e8
properties: update documentation for review comment
bradh Nov 30, 2024
7463d28
properties: updates for review comments
bradh Dec 2, 2024
b03e405
properties: simplify property decoding test
bradh Dec 4, 2024
651eb2a
properties: explain rationale for property checks
bradh Dec 4, 2024
35d413a
properties: fix oversize constants breaking windows build
bradh Dec 4, 2024
8e16606
properties: try additional init syntax
bradh Dec 4, 2024
852810b
properties: update structure comments based on review
bradh Dec 4, 2024
eab8f17
properties: remove unused test code
bradh Dec 5, 2024
713cdff
properties: updates for review comments
bradh Dec 14, 2024
8237099
properties: fix opaque property reading for the gain map image
bradh Dec 14, 2024
ee1e7e3
properties: initial fuzzer framework
bradh Dec 17, 2024
0cf3434
properties: add recommendation for avifParse() to check encoding
bradh Dec 17, 2024
3fc27af
properties: fix property fuzzer formatting
bradh Dec 17, 2024
77d6b0e
properties: address some review comments
bradh Dec 19, 2024
811d0a8
properties: formatting update
bradh Dec 19, 2024
eb139c2
properties: group calls to avifImagePushProperty()
bradh Dec 19, 2024
593ad22
properties: note order of application of properties
bradh Dec 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ The changes are relative to the previous release, unless the baseline is specifi

### Added since 1.1.1
* Add the properties and numProperties fields to avifImage. They are filled by
the avifDecoder instance with the properties unrecognized by libavif.
the avifDecoder instance with the properties unrecognized by libavif. They are
written by the avifEncoder.

### Changed since 1.1.1
* avifenc: Allow large images to be encoded.
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ set(AVIF_SRCS
src/io.c
src/mem.c
src/obu.c
src/properties.c
src/rawdata.c
src/read.c
src/reformat.c
Expand Down
19 changes: 18 additions & 1 deletion include/avif/avif.h
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,8 @@ typedef struct avifImage

// Other properties attached to this image item (primary or gainmap).
// At decoding: Forwarded here as opaque byte sequences by the avifDecoder.
// At encoding: Ignored.
// At encoding: Set using avifImageAddOpaqueProperty() or avifImageAddUUIDProperty() and written by the avifEncoder
// in the order that they are added to the image.
avifImageItemProperty * properties; // NULL only if numProperties is 0.
size_t numProperties;

Expand Down Expand Up @@ -853,6 +854,22 @@ AVIF_API avifResult avifImageAllocatePlanes(avifImage * image, avifPlanesFlags p
AVIF_API void avifImageFreePlanes(avifImage * image, avifPlanesFlags planes); // Ignores already-freed planes
AVIF_API void avifImageStealPlanes(avifImage * dstImage, avifImage * srcImage, avifPlanesFlags planes);

// Add arbitrary (opaque) properties to the image.
// Note: This is an advanced usage, intended for users with specific requirements who are familiar with the
// HEIF and ISO BMFF standards. Use of these functions for properties and boxes that are handled by
// libavif (e.g. ispe or meta) will likely result in invalid files, and should be avoided.
// If creating an ItemFullProperty, the version and flags values should be provided as the first four bytes of
// the data argument, and those four bytes included in the dataSize.
// Any properties will be added after the usual libavif descriptive properties, and before the libavif
// transformative properties (e.g. irot, imir, clap). Be aware that readers will apply transformative
// properties in the order they occur.
// Users of this API should consider calling avifParse() on the resulting file (i.e. the encoder output) to
// check that the arbitrary properties have not resulted in an invalid file.
AVIF_API avifResult avifImageAddOpaqueProperty(avifImage * image, const uint8_t boxtype[4], const uint8_t * data, size_t dataSize);
// This version adds an ItemProperty (or ItemFullProperty if version and flags are provided in data argument), using
// the user extension (uuid) mechanism, see ISO/IEC 14496-12:2022 Section 4.2. The box type is set to 'uuid'.
AVIF_API avifResult avifImageAddUUIDProperty(avifImage * image, const uint8_t uuid[16], const uint8_t * data, size_t dataSize);
bradh marked this conversation as resolved.
Show resolved Hide resolved

// ---------------------------------------------------------------------------
// Understanding maxThreads
//
Expand Down
5 changes: 5 additions & 0 deletions include/avif/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ AVIF_API avifResult avifImagePushProperty(avifImage * image,
const uint8_t * boxPayload,
size_t boxPayloadSize);

// Check if the FourCC property value is a known value
AVIF_NODISCARD avifBool avifIsKnownPropertyType(const uint8_t boxtype[4]);
// Check if the extended property (UUID) is valid
AVIF_NODISCARD avifBool avifIsValidUUID(const uint8_t uuid[16]);

// ---------------------------------------------------------------------------

#if defined(AVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM)
Expand Down
20 changes: 20 additions & 0 deletions src/avif.c
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,26 @@ avifResult avifImagePushProperty(avifImage * image, const uint8_t boxtype[4], co
return AVIF_RESULT_OK;
}

avifResult avifImageAddOpaqueProperty(avifImage * image, const uint8_t boxtype[4], const uint8_t * data, size_t dataSize)
{
const uint8_t uuid[16] = { 0 };
// Do not allow adding properties that are also handled by libavif
if (avifIsKnownPropertyType(boxtype)) {
return AVIF_RESULT_INVALID_ARGUMENT;
}
return avifImagePushProperty(image, boxtype, uuid, data, dataSize);
}

avifResult avifImageAddUUIDProperty(avifImage * image, const uint8_t uuid[16], const uint8_t * data, size_t dataSize)
{
const uint8_t boxtype[4] = { 'u', 'u', 'i', 'd' };
// Do not allow adding invalid UUIDs, or using uuid representation of properties that are also handled by libavif
if (!avifIsValidUUID(uuid)) {
return AVIF_RESULT_INVALID_ARGUMENT;
}
return avifImagePushProperty(image, boxtype, uuid, data, dataSize);
}

avifResult avifImageAllocatePlanes(avifImage * image, avifPlanesFlags planes)
{
if (image->width == 0 || image->height == 0) {
Expand Down
69 changes: 69 additions & 0 deletions src/properties.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2024 Brad Hards. All rights reserved.
// SPDX-License-Identifier: BSD-2-Clause

#include "avif/internal.h"
bradh marked this conversation as resolved.
Show resolved Hide resolved

#include <string.h>

struct avifKnownProperty
{
uint8_t fourcc[4];
};

static const struct avifKnownProperty knownProperties[] = {
{ { 'f', 't', 'y', 'p' } }, { { 'u', 'u', 'i', 'd' } }, { { 'm', 'e', 't', 'a' } }, { { 'h', 'd', 'l', 'r' } },
{ { 'p', 'i', 't', 'm' } }, { { 'd', 'i', 'n', 'f' } }, { { 'd', 'r', 'e', 'f' } }, { { 'i', 'd', 'a', 't' } },
{ { 'i', 'l', 'o', 'c' } }, { { 'i', 'i', 'n', 'f' } }, { { 'i', 'n', 'f', 'e' } }, { { 'i', 'p', 'r', 'p' } },
{ { 'i', 'p', 'c', 'o' } }, { { 'a', 'v', '1', 'C' } }, { { 'a', 'v', '2', 'C' } }, { { 'i', 's', 'p', 'e' } },
{ { 'p', 'i', 'x', 'i' } }, { { 'p', 'a', 's', 'p' } }, { { 'c', 'o', 'l', 'r' } }, { { 'a', 'u', 'x', 'C' } },
{ { 'c', 'l', 'a', 'p' } }, { { 'i', 'r', 'o', 't' } }, { { 'i', 'm', 'i', 'r' } }, { { 'c', 'l', 'l', 'i' } },
{ { 'c', 'c', 'l', 'v' } }, { { 'm', 'd', 'c', 'v' } }, { { 'a', 'm', 'v', 'e' } }, { { 'r', 'e', 'v', 'e' } },
{ { 'n', 'd', 'w', 't' } }, { { 'a', '1', 'o', 'p' } }, { { 'l', 's', 'e', 'l' } }, { { 'a', '1', 'l', 'x' } },
{ { 'c', 'm', 'i', 'n' } }, { { 'c', 'm', 'e', 'x' } }, { { 'i', 'p', 'm', 'a' } }, { { 'i', 'r', 'e', 'f' } },
{ { 'a', 'u', 'x', 'l' } }, { { 't', 'h', 'm', 'b' } }, { { 'd', 'i', 'm', 'g' } }, { { 'p', 'r', 'e', 'm' } },
{ { 'c', 'd', 's', 'c' } }, { { 'g', 'r', 'p', 'l' } }, { { 'a', 'l', 't', 'r' } }, { { 's', 't', 'e', 'r' } },
{ { 'm', 'd', 'a', 't' } },
};

static const size_t numKnownProperties = sizeof(knownProperties) / sizeof(knownProperties[0]);

static const size_t FOURCC_BYTES = 4;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: The integer literal 4 is already used in several places (e.g., line 9 and line 34) in the pull request, so it doesn't seem necessary to define this constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it made the code a little easier to read.

The other usages were in array bounds, which is only supported by GCC as an extension.

I can change it to be a #define if that would be preferred.

static const size_t UUID_BYTES = 16;

static const uint8_t ISO_UUID_SUFFIX[12] = { 0x00, 0x01, 0x00, 0x10, 0x80, 0x00, 0x00, 0xAA, 0x00, 0x38, 0x9b, 0x71 };

avifBool avifIsKnownPropertyType(const uint8_t boxtype[4])
bradh marked this conversation as resolved.
Show resolved Hide resolved
{
for (size_t i = 0; i < numKnownProperties; i++) {
if (memcmp(knownProperties[i].fourcc, boxtype, FOURCC_BYTES) == 0) {
return AVIF_TRUE;
}
}
return AVIF_FALSE;
}

avifBool avifIsValidUUID(const uint8_t uuid[16])
y-guyon marked this conversation as resolved.
Show resolved Hide resolved
{
// This check is to reject encoding a known property via the UUID mechanism
// See ISO/IEC 14496-12 Section 4.2.3
for (size_t i = 0; i < numKnownProperties; i++) {
if ((memcmp(knownProperties[i].fourcc, uuid, FOURCC_BYTES) == 0) &&
(memcmp(ISO_UUID_SUFFIX, uuid + FOURCC_BYTES, UUID_BYTES - FOURCC_BYTES) == 0)) {
return AVIF_FALSE;
}
}
// This check rejects UUIDs with unexpected variant field values, including Nil UUID and Max UUID.
// See RFC 9562 Section 4.1
uint8_t variant = uuid[8] >> 4;
if ((variant < 0x08) || (variant > 0x0b)) {
return AVIF_FALSE;
}
// This check rejects UUIDs with unexpected version field values.
// See RFC 9562 Section 4.2
uint8_t version = uuid[6] >> 4;
if ((version < 1) || (version > 8)) {
return AVIF_FALSE;
}
// The rest of a UUID is pretty much a bucket of bits, so assume its OK.
return AVIF_TRUE;
}
12 changes: 12 additions & 0 deletions src/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -6052,6 +6052,18 @@ avifResult avifDecoderReset(avifDecoder * decoder)
}
}

if (gainMapProperties) {
for (size_t i = 0; i < gainMapProperties->count; ++i) {
const avifProperty * property = &gainMapProperties->prop[i];
if (property->isOpaque) {
AVIF_CHECKRES(avifImagePushProperty(decoder->image->gainMap->image,
property->type,
property->u.opaque.usertype,
property->u.opaque.boxPayload.data,
property->u.opaque.boxPayload.size));
}
}
}
return AVIF_RESULT_OK;
}

Expand Down
14 changes: 14 additions & 0 deletions src/write.c
Original file line number Diff line number Diff line change
Expand Up @@ -3003,6 +3003,20 @@ static avifResult avifRWStreamWriteProperties(avifItemPropertyDedup * const dedu
// see https://github.com/AOMediaCodec/libavif/pull/2429
}

// write out any opaque properties from avifImageAddOpaqueProperty() or avifImageAddUUIDProperty()
for (size_t i = 0; i < itemMetadata->numProperties; i++) {
avifItemPropertyDedupStart(dedup);
const avifImageItemProperty * prop = &itemMetadata->properties[i];
avifBoxMarker propMarker;
AVIF_CHECKRES(avifRWStreamWriteBox(&dedup->s, (const char *)prop->boxtype, AVIF_BOX_SIZE_TBD, &propMarker));
y-guyon marked this conversation as resolved.
Show resolved Hide resolved
if (memcmp(prop->boxtype, "uuid", 4) == 0) {
AVIF_CHECKRES(avifRWStreamWrite(&dedup->s, prop->usertype, 16));
}
AVIF_CHECKRES(avifRWStreamWrite(&dedup->s, prop->boxPayload.data, prop->boxPayload.size));
avifRWStreamFinishBox(&dedup->s, propMarker);
AVIF_CHECKRES(avifItemPropertyDedupFinish(dedup, s, &item->ipma, AVIF_FALSE));
}

// Also write the transformative properties.

if (item->itemCategory == AVIF_ITEM_COLOR) {
Expand Down
3 changes: 3 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ if(AVIF_GTEST)
add_avif_gtest_with_data(avifpng16bittest)
add_avif_gtest_with_data(avifprogressivetest)
add_avif_gtest_with_data(avifpropertytest)
add_avif_internal_gtest(avifpropinternaltest)
add_avif_gtest(avifrangetest)
add_avif_gtest_with_data(avifreadimagetest)
add_avif_internal_gtest(avifrgbtest)
Expand Down Expand Up @@ -217,6 +218,7 @@ if(AVIF_ENABLE_FUZZTEST)
add_avif_fuzztest(avif_fuzztest_enc_dec)
add_avif_fuzztest(avif_fuzztest_enc_dec_anim)
add_avif_fuzztest(avif_fuzztest_enc_dec_incr gtest/avifincrtest_helpers.cc)
add_avif_fuzztest(avif_fuzztest_properties)
add_avif_fuzztest(avif_fuzztest_read_image)
add_avif_fuzztest(avif_fuzztest_yuvrgb)
else()
Expand Down Expand Up @@ -358,6 +360,7 @@ if(AVIF_CODEC_AVM_ENABLED)
avifiostatstest
avifmetadatatest
avifprogressivetest
avifpropertytest
avifrangetest
avify4mtest
PROPERTIES DISABLED True
Expand Down
109 changes: 109 additions & 0 deletions tests/gtest/avif_fuzztest_properties.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright 2024 Google LLC
// SPDX-License-Identifier: BSD-2-Clause

#include <array>
#include <cstdint>
#include <cstring>
#include <memory>
#include <vector>

#include "avif/avif.h"
#include "avif/internal.h"
#include "avif_fuzztest_helpers.h"
#include "aviftest_helpers.h"
#include "fuzztest/fuzztest.h"
#include "gtest/gtest.h"

namespace avif {
namespace testutil {
namespace {

struct TestProp {
std::vector<uint8_t> fourcc;
std::vector<uint8_t> uuid;
std::vector<uint8_t> body;
};

static std::vector<uint8_t> UUID_4CC = {'u', 'u', 'i', 'd'};

void PropsValid(ImagePtr image, EncoderPtr encoder, DecoderPtr decoder,
std::vector<TestProp> testProps) {
ImagePtr decoded_image(avifImageCreateEmpty());
ASSERT_NE(image.get(), nullptr);
ASSERT_NE(encoder.get(), nullptr);
ASSERT_NE(decoder.get(), nullptr);
ASSERT_NE(decoded_image.get(), nullptr);

for (TestProp testProp : testProps) {
if (testProp.fourcc == UUID_4CC) {
ASSERT_EQ(
avifImageAddUUIDProperty(image.get(), testProp.uuid.data(),
testProp.body.data(), testProp.body.size()),
AVIF_RESULT_OK);
} else {
ASSERT_EQ(avifImageAddOpaqueProperty(image.get(), testProp.fourcc.data(),
testProp.body.data(),
testProp.body.size()),
AVIF_RESULT_OK);
}
}

AvifRwData encoded_data;
const avifResult encoder_result =
avifEncoderWrite(encoder.get(), image.get(), &encoded_data);
ASSERT_EQ(encoder_result, AVIF_RESULT_OK)
<< avifResultToString(encoder_result);

const avifResult decoder_result = avifDecoderReadMemory(
decoder.get(), decoded_image.get(), encoded_data.data, encoded_data.size);
ASSERT_EQ(decoder_result, AVIF_RESULT_OK)
<< avifResultToString(decoder_result);

ASSERT_EQ(decoder->image->numProperties, testProps.size());
for (size_t i = 0; i < testProps.size(); i++) {
TestProp testProp = testProps[i];
const avifImageItemProperty& decodeProp = decoder->image->properties[i];
EXPECT_EQ(std::string(decodeProp.boxtype, decodeProp.boxtype + 4),
std::string(testProp.fourcc.data(), testProp.fourcc.data() + 4));
EXPECT_EQ(std::vector<uint8_t>(
decodeProp.boxPayload.data,
decodeProp.boxPayload.data + decodeProp.boxPayload.size),
testProp.body);
}
}

inline auto ArbitraryProp() {
auto fourcc = fuzztest::Arbitrary<std::vector<uint8_t>>().WithSize(4);
auto uuid =
fuzztest::Arbitrary<std::vector<uint8_t>>().WithSize(16); // ignored
auto body = fuzztest::Arbitrary<std::vector<uint8_t>>();
// Don't return known properties.
return fuzztest::Filter(
[](TestProp prop) {
return !avifIsKnownPropertyType(prop.fourcc.data());
},
fuzztest::StructOf<TestProp>(fourcc, uuid, body));
}

inline auto ArbitraryUUIDProp() {
auto fourcc = fuzztest::Just(UUID_4CC);
auto uuid = fuzztest::Arbitrary<std::vector<uint8_t>>().WithSize(16);
auto body = fuzztest::Arbitrary<std::vector<uint8_t>>();
// Don't use invalid UUIDs
return fuzztest::Filter(
[](TestProp prop) { return avifIsValidUUID(prop.uuid.data()); },
fuzztest::StructOf<TestProp>(fourcc, uuid, body));
}

inline auto ArbitraryProps() {
return fuzztest::VectorOf(
fuzztest::OneOf(ArbitraryProp(), ArbitraryUUIDProp()));
}

FUZZ_TEST(PropertiesAvifFuzzTest, PropsValid)
.WithDomains(ArbitraryAvifImage(), ArbitraryAvifEncoder(),
ArbitraryAvifDecoder(), ArbitraryProps());

} // namespace
} // namespace testutil
} // namespace avif
Loading
Loading