Minor fixes and small refactor in libavifinfo Not a security patch. Fixes the parsing of some oddly valid and weirdly invalid AVIF files. Add instructions in README.md. Change-Id: I91339d8c4f462ec6f882e3f54b3acba2087395e9
diff --git a/README.md b/README.md index 83d9a6f..7d17d55 100644 --- a/README.md +++ b/README.md
@@ -6,69 +6,44 @@ See `avifinfo.h` for details on the API and `avifinfo.c` for the implementation. See `tests/avifinfo_demo.cc` for API usage examples. -## Contents - -1. [How to use](#how-to-use) - - 1. [Build](#build) - 2. [Test](#test) - -2. [Development](#development) - - 1. [Coding style](#coding-style) - 2. [Submitting patches](#submitting-patches) - -3. [PHP implementation](#php-implementation) - -4. [Bug reports](#bug-reports) - -## How to use {#how-to-use} +## How to use **libavifinfo** can be used when only a few AVIF features are needed and when linking to or including [libavif](https://github.com/AOMediaCodec/libavif) is not an option. For decoding an image or extracting more features, please rely on [libavif](https://github.com/AOMediaCodec/libavif). -Note: `AvifInfoGetFeatures()` is designed to return the same `avifImage` field -values as +Note: The C implementation `AvifInfoGetFeatures()` is designed to return the +same `avifImage` field values as [`avifDecoderParse()`](https://github.com/AOMediaCodec/libavif/blob/e34204f5370509c72b3b2f065e5ebb2767cbbd48/include/avif/avif.h#L1049). -However **libavifinfo** is more permissive and may return features of images -considered invalid by **libavif**. +However **libavifinfo** is often more permissive and may return features of +images considered invalid by **libavif**. -### Build {#build} +## C implementation -`avifinfo.c` is written in C. To build from this directory: - -```sh -mkdir build && \ -cd build && \ -cmake .. && \ -cmake --build . --config Release -``` - -### Test {#test} - -Tests are written in C++. GoogleTest is required. - -```sh -mkdir build && \ -cd build && \ -cmake .. -DAVIFINFO_BUILD_TESTS=ON && \ -cmake --build . --config Debug && \ -ctest . -``` - -## Development {#development} - -### Coding style {#coding-style} +### Coding style [Google C/C++ Style Guide](https://google.github.io/styleguide/cppguide.html) is used in this project. -### Submitting patches {#submitting-patches} +### Build -If you would like to contribute to **libavifinfo**, please follow the steps for -**libaom** at https://aomedia.googlesource.com/aom/#submitting-patches. +`avifinfo.c` is written in C. To build from this directory: + +```sh +cmake -S . -B build && \ +cmake --build build --config Release +``` + +### Test + +GoogleTest is required for the C++ tests (e.g. `sudo apt install libgtest-dev`). + +```sh +cmake -S . -B build -DAVIFINFO_BUILD_TESTS=ON && \ +cmake --build build --config Debug && \ +ctest --test-dir build build +``` ## PHP implementation @@ -81,7 +56,40 @@ See `avifinfo_test.php` for a usage example. -## Bug reports {#bug-reports} +### PHP test + +```sh +cd tests && \ +php avifinfo_test.php +``` + +## Development + +### Submitting patches {#submitting-patches} + +If you would like to contribute to **libavifinfo**, please follow the steps for +**libaom** at https://aomedia.googlesource.com/aom/#submitting-patches. Summary: + +- Log in at https://aomedia.googlesource.com +- Sign in at https://aomedia-review.googlesource.com +- Execute the contributor agreement if necessary, see http://aomedia.org/license +- Test all implementations as described in this document +- Copy the commit message hook: + `cd libavifinfo && curl -Lo .git/hooks/commit-msg https://chromium-review.googlesource.com/tools/hooks/commit-msg && chmod u+x .git/hooks/commit-msg` +- Create a commit +- `git push https://aomedia.googlesource.com/libavifinfo/ HEAD:refs/for/main` +- Request a review from a project maintainer + +## Known users of libavifinfo + +- [Chromium](https://source.chromium.org/chromium/chromium/src/+/main:third_party/libavifinfo) + uses the C impl to extract the gain map item for HDR tone mapping. +- [PHP-src](https://github.com/php/php-src/tree/master/ext/standard/libavifinfo) + uses the C impl to parse image features without relying on libavif or libheif. +- [WordPress](https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/class-avif-info.php) + uses the PHP impl to support AVIF images with older versions of PHP. + +## Bug reports Bug reports can be filed in the Alliance for Open Media [issue tracker](https://bugs.chromium.org/p/aomedia/issues/list).
diff --git a/avifinfo.c b/avifinfo.c index 62abf28..599c019 100644 --- a/avifinfo.c +++ b/avifinfo.c
@@ -318,11 +318,18 @@ // Read the 32 least-significant bits. box->size = AvifInfoInternalReadBigEndian(data + 4, sizeof(uint32_t)); } else if (box->size == 0) { + // ISO/IEC 14496-12 4.2.2: + // if size is 0, then this box shall be in a top-level box + // (i.e. not contained in another box) + AVIFINFO_CHECK(nesting_level == 0, kInvalid); box->size = num_remaining_bytes; } AVIFINFO_CHECK(box->size >= box_header_size, kInvalid); AVIFINFO_CHECK(box->size <= num_remaining_bytes, kInvalid); + // 16 bytes of usertype should be read here if the box type is 'uuid'. + // 'uuid' boxes are skipped so usertype is part of the skipped body. + const int has_fullbox_header = !memcmp(box->type, "meta", 4) || !memcmp(box->type, "pitm", 4) || !memcmp(box->type, "ipma", 4) || !memcmp(box->type, "ispe", 4) || @@ -483,7 +490,11 @@ if (strcmp(aux_type, kGainmapStr) == 0) { // Note: It is unlikely but it is possible that this gain map // does not belong to the primary item or a tile. Ignore this issue. - features->gainmap_property_index = box_index; + if (box_index <= AVIFINFO_MAX_VALUE) { + features->gainmap_property_index = (uint8_t)box_index; + } else { + features->data_was_skipped = 1; + } } else if (box.content_size >= kAlphaStrLength && memcmp(aux_type, kAlphaStr, kGainmapStrLength) == 0) { // The beginning of the aux type matches the alpha aux type string. @@ -511,7 +522,7 @@ AVIFINFO_RETURN(kNotFound); } -// Parses a 'stream' of an "iprp" box into 'features'. The "ipco" box contain +// Parses a 'stream' of an "iprp" box into 'features'. The "ipco" box contains // the properties which are linked to items by the "ipma" box. static AvifInfoInternalStatus ParseIprp(int nesting_level, AvifInfoInternalStream* stream, @@ -692,38 +703,37 @@ if (!memcmp(box.type, "infe", 4)) { // See ISO/IEC 14496-12:2015(E) 8.11.6.2 - uint32_t num_read_bytes = 0; - const uint32_t num_bytes_per_id = (box.version == 2) ? 2 : 4; const uint8_t* data; // item_ID (16 or 32) + item_protection_index (16) + item_type (32). - AVIFINFO_CHECK((num_bytes_per_id + 6) <= num_remaining_bytes, kInvalid); + AVIFINFO_CHECK(num_bytes_per_id + 2 + 4 <= box.content_size, kInvalid); AVIFINFO_CHECK_FOUND( AvifInfoInternalRead(stream, num_bytes_per_id, &data)); - num_read_bytes += num_bytes_per_id; const uint32_t item_id = AvifInfoInternalReadBigEndian(data, num_bytes_per_id); - // Skip item_protection_index + // Skip item_protection_index. AVIFINFO_CHECK_FOUND(AvifInfoInternalSkip(stream, 2)); - num_read_bytes += 2; const uint8_t* item_type; AVIFINFO_CHECK_FOUND(AvifInfoInternalRead(stream, 4, &item_type)); - num_read_bytes += 4; if (!memcmp(item_type, "tmap", 4)) { // Tone Mapped Image: indicates the presence of a gain map. - features->tone_mapped_item_id = item_id; + if (item_id <= AVIFINFO_MAX_VALUE) { + features->tone_mapped_item_id = (uint8_t)item_id; + } else { + features->data_was_skipped = 1; + } } - AVIFINFO_CHECK_FOUND( - AvifInfoInternalSkip(stream, box.content_size - num_read_bytes)); + AVIFINFO_CHECK_FOUND(AvifInfoInternalSkip( + stream, box.content_size - (num_bytes_per_id + 2 + 4))); } else { AVIFINFO_CHECK_FOUND(AvifInfoInternalSkip(stream, box.content_size)); } num_remaining_bytes -= box.size; - if (num_remaining_bytes <= 0) break; + if (num_remaining_bytes == 0) break; // Ignore entry_count bigger than box. } AVIFINFO_RETURN(kNotFound); } @@ -822,7 +832,6 @@ AVIFINFO_CHECK_FOUND(AvifInfoInternalSkip(stream, box.content_size)); } } - AVIFINFO_RETURN(kInvalid); // No "meta" no good. } //------------------------------------------------------------------------------
diff --git a/avifinfo.php b/avifinfo.php index bebcc35..81269ec 100644 --- a/avifinfo.php +++ b/avifinfo.php
@@ -250,6 +250,10 @@ // Read the 32 least-significant bits. $this->size = read_big_endian( substr( $data, 4, 4 ), 4 ); } else if ( $this->size == 0 ) { + // ISO/IEC 14496-12 4.2.2: + // if size is 0, then this box shall be in a top-level box + // (i.e. not contained in another box) + // Unfortunately the presence of a parent box is unknown here. $this->size = $num_remaining_bytes; } if ( $this->size < $header_size ) { @@ -259,6 +263,9 @@ return INVALID; } + // 16 bytes of usertype should be read here if the box type is 'uuid'. + // 'uuid' boxes are skipped so usertype is part of the skipped body. + $has_fullbox_header = $this->type == 'meta' || $this->type == 'pitm' || $this->type == 'ipma' || $this->type == 'ispe' || $this->type == 'pixi' || $this->type == 'iref' || @@ -477,7 +484,7 @@ /** * Parses an "iprp" box. * - * The "ipco" box contain the properties which are linked to items by the "ipma" box. + * The "ipco" box contains the properties which are linked to items by the "ipma" box. * * @param stream $handle The resource the box will be parsed from. * @param int $num_remaining_bytes The number of bytes that should be available from the resource.