Fix out-of-order dimg grid associations (#2312)
Add sofa_grid1x5_420_reversed_dimg_order.avif and a test.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index fb9a284..7ade277 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,8 @@
* Fix aom link flags so that transitive library link flags are included when
aom is a static library
https://github.com/AOMediaCodec/libavif/issues/2274.
+* Fix out-of-order 'dimg' grid associations
+ https://github.com/AOMediaCodec/libavif/issues/2311.
## [1.1.0] - 2024-07-11
diff --git a/src/read.c b/src/read.c
index 12996cf..46f6b42 100644
--- a/src/read.c
+++ b/src/read.c
@@ -1420,17 +1420,42 @@
return AVIF_CODEC_TYPE_UNKNOWN;
}
-static avifResult avifDecoderGenerateImageGridTiles(avifDecoder * decoder, avifImageGrid * grid, avifDecoderItem * gridItem, avifItemCategory itemCategory)
+// Fills the dimgIdxToItemIdx array with a mapping from each 0-based tile index in the 'dimg' reference
+// to its corresponding 0-based index in the avifMeta::items array.
+static avifResult avifFillDimgIdxToItemIdxArray(uint32_t * dimgIdxToItemIdx, uint32_t numExpectedTiles, const avifDecoderItem * gridItem)
{
- // Count number of dimg for this item, bail out if it doesn't match perfectly
- unsigned int tilesAvailable = 0;
+ const uint32_t itemIndexNotSet = UINT32_MAX;
+ for (uint32_t dimgIdx = 0; dimgIdx < numExpectedTiles; ++dimgIdx) {
+ dimgIdxToItemIdx[dimgIdx] = itemIndexNotSet;
+ }
+ uint32_t numTiles = 0;
+ for (uint32_t i = 0; i < gridItem->meta->items.count; ++i) {
+ if (gridItem->meta->items.item[i]->dimgForID == gridItem->id) {
+ const uint32_t tileItemDimgIdx = gridItem->meta->items.item[i]->dimgIdx;
+ AVIF_CHECKERR(tileItemDimgIdx < numExpectedTiles, AVIF_RESULT_INVALID_IMAGE_GRID);
+ AVIF_CHECKERR(dimgIdxToItemIdx[tileItemDimgIdx] == itemIndexNotSet, AVIF_RESULT_INVALID_IMAGE_GRID);
+ dimgIdxToItemIdx[tileItemDimgIdx] = i;
+ ++numTiles;
+ }
+ }
+ // The number of tiles has been verified in avifDecoderItemReadAndParse().
+ AVIF_ASSERT_OR_RETURN(numTiles == numExpectedTiles);
+ return AVIF_RESULT_OK;
+}
+
+// Creates the tiles and associate them to the items in the order of the 'dimg' association.
+static avifResult avifDecoderGenerateImageGridTiles(avifDecoder * decoder,
+ avifDecoderItem * gridItem,
+ avifItemCategory itemCategory,
+ const uint32_t * dimgIdxToItemIdx,
+ uint32_t numTiles)
+{
avifDecoderItem * firstTileItem = NULL;
avifBool progressive = AVIF_TRUE;
- for (uint32_t i = 0; i < gridItem->meta->items.count; ++i) {
- avifDecoderItem * item = gridItem->meta->items.item[i];
- if (item->dimgForID != gridItem->id) {
- continue;
- }
+ for (uint32_t dimgIdx = 0; dimgIdx < numTiles; ++dimgIdx) {
+ const uint32_t itemIdx = dimgIdxToItemIdx[dimgIdx];
+ AVIF_ASSERT_OR_RETURN(itemIdx < gridItem->meta->items.count);
+ avifDecoderItem * item = gridItem->meta->items.item[itemIdx];
// According to HEIF (ISO 14496-12), Section 6.6.2.3.1, the SingleItemTypeReferenceBox of type 'dimg'
// identifies the input images of the derived image item of type 'grid'. Since the reference_count
@@ -1505,22 +1530,11 @@
if (!item->progressive) {
progressive = AVIF_FALSE;
}
- ++tilesAvailable;
}
if (itemCategory == AVIF_ITEM_COLOR && progressive) {
// If all the items that make up the grid are progressive, then propagate that status to the top-level grid item.
gridItem->progressive = AVIF_TRUE;
}
-
- if (tilesAvailable != grid->rows * grid->columns) {
- avifDiagnosticsPrintf(&decoder->diag,
- "Grid image of dimensions %ux%u requires %u tiles, but %u were found",
- grid->columns,
- grid->rows,
- grid->rows * grid->columns,
- tilesAvailable);
- return AVIF_RESULT_INVALID_IMAGE_GRID;
- }
return AVIF_RESULT_OK;
}
@@ -4650,14 +4664,19 @@
return AVIF_RESULT_OK;
}
// If color item is a grid, check if there is an alpha channel which is represented as an auxl item to each color tile item.
- uint32_t colorItemCount = colorInfo->grid.rows * colorInfo->grid.columns;
- if (colorItemCount == 0) {
+ const uint32_t tileCount = colorInfo->grid.rows * colorInfo->grid.columns;
+ if (tileCount == 0) {
*alphaItem = NULL;
*isAlphaItemInInput = AVIF_FALSE;
return AVIF_RESULT_OK;
}
- uint32_t * alphaItemIndices = (uint32_t *)avifAlloc(colorItemCount * sizeof(uint32_t));
- AVIF_CHECKERR(alphaItemIndices, AVIF_RESULT_OUT_OF_MEMORY);
+ // Keep the same 'dimg' order as it defines where each tile is located in the reconstructed image.
+ uint32_t * dimgIdxToAlphaItemIdx = (uint32_t *)avifAlloc(tileCount * sizeof(uint32_t));
+ AVIF_CHECKERR(dimgIdxToAlphaItemIdx != NULL, AVIF_RESULT_OUT_OF_MEMORY);
+ const uint32_t itemIndexNotSet = UINT32_MAX;
+ for (uint32_t dimgIdx = 0; dimgIdx < tileCount; ++dimgIdx) {
+ dimgIdxToAlphaItemIdx[dimgIdx] = itemIndexNotSet;
+ }
uint32_t alphaItemCount = 0;
for (uint32_t i = 0; i < meta->items.count; ++i) {
const avifDecoderItem * const item = meta->items.item[i];
@@ -4666,27 +4685,34 @@
for (uint32_t j = 0; j < meta->items.count; ++j) {
avifDecoderItem * auxlItem = meta->items.item[j];
if (avifDecoderItemIsAlphaAux(auxlItem, item->id)) {
- if (seenAlphaForCurrentItem || auxlItem->dimgForID != 0) {
+ if (seenAlphaForCurrentItem || auxlItem->dimgForID != 0 || item->dimgIdx >= tileCount ||
+ dimgIdxToAlphaItemIdx[item->dimgIdx] != itemIndexNotSet) {
// One of the following invalid cases:
// * Multiple items are claiming to be the alpha auxiliary of the current item.
// * Alpha auxiliary is dimg for another item.
- avifFree(alphaItemIndices);
+ // * There are too many items in the dimg array (also checked later in avifFillDimgIdxToItemIdxArray()).
+ // * There is a repetition in the dimg array (also checked later in avifFillDimgIdxToItemIdxArray()).
+ avifFree(dimgIdxToAlphaItemIdx);
return AVIF_RESULT_INVALID_IMAGE_GRID;
}
- alphaItemIndices[alphaItemCount++] = j;
+ dimgIdxToAlphaItemIdx[item->dimgIdx] = j;
+ ++alphaItemCount;
seenAlphaForCurrentItem = AVIF_TRUE;
}
}
if (!seenAlphaForCurrentItem) {
// No alpha auxiliary item was found for the current item. Treat this as an image without alpha.
- avifFree(alphaItemIndices);
+ avifFree(dimgIdxToAlphaItemIdx);
*alphaItem = NULL;
*isAlphaItemInInput = AVIF_FALSE;
return AVIF_RESULT_OK;
}
}
}
- AVIF_ASSERT_OR_RETURN(alphaItemCount == colorItemCount);
+ if (alphaItemCount != tileCount) {
+ avifFree(dimgIdxToAlphaItemIdx);
+ return AVIF_RESULT_INVALID_IMAGE_GRID;
+ }
// Find an unused ID.
avifResult result;
if (meta->items.count >= UINT32_MAX - 1) {
@@ -4708,17 +4734,22 @@
result = avifMetaFindOrCreateItem(meta, newItemID, alphaItem); // Create new empty item.
}
if (result != AVIF_RESULT_OK) {
- avifFree(alphaItemIndices);
+ avifFree(dimgIdxToAlphaItemIdx);
return result;
}
memcpy((*alphaItem)->type, "grid", 4); // Make it a grid and register alpha items as its tiles.
(*alphaItem)->width = colorItem->width;
(*alphaItem)->height = colorItem->height;
- for (uint32_t i = 0; i < alphaItemCount; ++i) {
- avifDecoderItem * item = meta->items.item[alphaItemIndices[i]];
- item->dimgForID = (*alphaItem)->id;
+ for (uint32_t dimgIdx = 0; dimgIdx < tileCount; ++dimgIdx) {
+ if (dimgIdxToAlphaItemIdx[dimgIdx] >= meta->items.count) {
+ avifFree(dimgIdxToAlphaItemIdx);
+ AVIF_ASSERT_OR_RETURN(AVIF_FALSE);
+ }
+ avifDecoderItem * alphaTileItem = meta->items.item[dimgIdxToAlphaItemIdx[dimgIdx]];
+ alphaTileItem->dimgForID = (*alphaItem)->id;
+ alphaTileItem->dimgIdx = dimgIdx;
}
- avifFree(alphaItemIndices);
+ avifFree(dimgIdxToAlphaItemIdx);
*isAlphaItemInInput = AVIF_FALSE;
alphaInfo->grid = colorInfo->grid;
return AVIF_RESULT_OK;
@@ -5014,7 +5045,16 @@
{
const uint32_t previousTileCount = decoder->data->tiles.count;
if ((info->grid.rows > 0) && (info->grid.columns > 0)) {
- AVIF_CHECKRES(avifDecoderGenerateImageGridTiles(decoder, &info->grid, item, itemCategory));
+ // The number of tiles was verified in avifDecoderItemReadAndParse().
+ const uint32_t numTiles = info->grid.rows * info->grid.columns;
+ uint32_t * dimgIdxToItemIdx = (uint32_t *)avifAlloc(numTiles * sizeof(uint32_t));
+ AVIF_CHECKERR(dimgIdxToItemIdx != NULL, AVIF_RESULT_OUT_OF_MEMORY);
+ avifResult result = avifFillDimgIdxToItemIdxArray(dimgIdxToItemIdx, numTiles, item);
+ if (result == AVIF_RESULT_OK) {
+ result = avifDecoderGenerateImageGridTiles(decoder, item, itemCategory, dimgIdxToItemIdx, numTiles);
+ }
+ avifFree(dimgIdxToItemIdx);
+ AVIF_CHECKRES(result);
} else {
AVIF_CHECKERR(item->size != 0, AVIF_RESULT_MISSING_IMAGE_ITEM);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 5739276..be18478 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -114,6 +114,7 @@
add_avif_internal_gtest_with_data(avifcolrconverttest)
add_avif_internal_gtest(avifcolrtest)
add_avif_gtest_with_data(avifdecodetest)
+ add_avif_gtest_with_data(avifdimgtest avifincrtest_helpers)
add_avif_gtest_with_data(avifencodetest)
if(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
@@ -127,7 +128,12 @@
add_avif_gtest(avifgridapitest)
add_avif_gtest_with_data(avifilocextenttest)
add_avif_gtest(avifimagetest)
- add_avif_gtest_with_data(avifincrtest avifincrtest_helpers)
+
+ add_executable(avifincrtest gtest/avifincrtest.cc)
+ target_link_libraries(avifincrtest aviftest_helpers avifincrtest_helpers)
+ add_test(NAME avifincrtest COMMAND avifincrtest ${CMAKE_CURRENT_SOURCE_DIR}/data/)
+ register_test_for_coverage(avifincrtest ${CMAKE_CURRENT_SOURCE_DIR}/data/)
+
add_avif_gtest_with_data(avifiostatstest)
add_avif_gtest_with_data(avifkeyframetest)
add_avif_gtest_with_data(aviflosslesstest)
@@ -342,6 +348,7 @@
avifchangesettingtest
avifcllitest
avifcolrconverttest
+ avifdimgtest
avifencodetest
avifgridapitest
avifincrtest
diff --git a/tests/data/README.md b/tests/data/README.md
index 8ed0c6c..2996eb9 100644
--- a/tests/data/README.md
+++ b/tests/data/README.md
@@ -363,6 +363,15 @@
Source: Personal photo converted with `avifenc --grid 1x5 --yuv 420` at
commit [632d131](https://github.com/AOMediaCodec/libavif/commit/632d13188f9b7faa40f20d870e792174b8b5b8e6).
+### File [sofa_grid1x5_420_reversed_dimg_order.avif](sofa_grid1x5_420_reversed_dimg_order.avif)
+
+
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source: `sofa_grid1x5_420.avif` manually edited so that the `dimg` order is
+6,5,4,3,2 instead of 2,3,4,5,6.
+
### File [color_grid_alpha_nogrid.avif](color_grid_alpha_nogrid.avif)
diff --git a/tests/data/sofa_grid1x5_420_reversed_dimg_order.avif b/tests/data/sofa_grid1x5_420_reversed_dimg_order.avif
new file mode 100644
index 0000000..4b83ed6
--- /dev/null
+++ b/tests/data/sofa_grid1x5_420_reversed_dimg_order.avif
Binary files differ
diff --git a/tests/gtest/avifdimgtest.cc b/tests/gtest/avifdimgtest.cc
new file mode 100644
index 0000000..c166c5b
--- /dev/null
+++ b/tests/gtest/avifdimgtest.cc
@@ -0,0 +1,69 @@
+// Copyright 2024 Google LLC
+// SPDX-License-Identifier: BSD-2-Clause
+
+#include <algorithm>
+#include <iostream>
+#include <string>
+
+#include "avif/avif.h"
+#include "avifincrtest_helpers.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(IncrementalTest, Dimg) {
+ testutil::AvifRwData avif =
+ testutil::ReadFile(std::string(data_path) + "sofa_grid1x5_420.avif");
+ ASSERT_NE(avif.size, 0u);
+ ImagePtr decoded(avifImageCreateEmpty());
+ ASSERT_NE(decoded, nullptr);
+ DecoderPtr decoder(avifDecoderCreate());
+ ASSERT_NE(decoder, nullptr);
+ ASSERT_EQ(
+ avifDecoderReadMemory(decoder.get(), decoded.get(), avif.data, avif.size),
+ AVIF_RESULT_OK);
+
+ testutil::AvifRwData avif_reversed_dimg_order = testutil::ReadFile(
+ std::string(data_path) + "sofa_grid1x5_420_reversed_dimg_order.avif");
+ ImagePtr decoded_reversed_dimg_order(avifImageCreateEmpty());
+ ASSERT_NE(decoded_reversed_dimg_order, nullptr);
+ ASSERT_EQ(avifDecoderReadMemory(decoder.get(), decoded.get(),
+ avif_reversed_dimg_order.data,
+ avif_reversed_dimg_order.size),
+ AVIF_RESULT_OK);
+ EXPECT_FALSE(
+ testutil::AreImagesEqual(*decoded, *decoded_reversed_dimg_order));
+
+ // Verify that it works incrementally.
+ // enable_fine_incremental_check is false because the tiles are out-of-order.
+ ASSERT_EQ(testutil::DecodeIncrementally(
+ avif_reversed_dimg_order, decoder.get(), /*is_persistent=*/true,
+ /*give_size_hint=*/true,
+ /*use_nth_image_api=*/false, *decoded_reversed_dimg_order,
+ /*cell_height=*/154, /*enable_fine_incremental_check=*/false),
+ AVIF_RESULT_OK);
+}
+
+//------------------------------------------------------------------------------
+
+} // namespace
+} // namespace avif
+
+int main(int argc, char** argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+ if (argc < 2) {
+ std::cerr
+ << "The path to the test data folder must be provided as an argument"
+ << std::endl;
+ return 1;
+ }
+ avif::data_path = argv[1];
+ return RUN_ALL_TESTS();
+}
