Skip to content

Commit

Permalink
Revert "Simpler itemID finding for alpha. (#1967)"
Browse files Browse the repository at this point in the history 
This reverts commit 4378794.

Validating the ipma requirement of increasing item_ids inside
avifMetaFindOrCreateItem is incorrect since that function is
called when parsing several other boxes where new items could
be added (for e.g. iloc).

A sample avif file that is broken by this change:
https://g-issues.chromium.org/issues/323735410#comment4
  • Loading branch information
@vigneshvg
vigneshvg committed Feb 5, 2024
1 parent 3ead1f3 commit 992b69d
Showing 1 changed file with 31 additions and 37 deletions.
68 changes: 31 additions & 37 deletions src/read.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ static avifResult avifCheckItemID(const char * boxFourcc, uint32_t itemID, avifD
return AVIF_RESULT_OK;
}

static avifResult avifMetaFindOrCreateItem(avifMeta * meta, uint32_t itemID, avifDecoderItem ** item, avifDiagnostics * diag)
static avifResult avifMetaFindOrCreateItem(avifMeta * meta, uint32_t itemID, avifDecoderItem ** item)
{
*item = NULL;
AVIF_ASSERT_OR_RETURN(itemID != 0);
Expand All @@ -799,18 +799,6 @@ static avifResult avifMetaFindOrCreateItem(avifMeta * meta, uint32_t itemID, avi
}
}

if (meta->items.count != 0) {
// ISO/IEC 23008-12, First edition, 2017-12, Section 9.3.1:
// Each ItemPropertyAssociation box shall be ordered by increasing item_ID, and there shall
// be at most one association box for each item_ID, in any ItemPropertyAssociation box.
const uint32_t lastID = meta->items.item[meta->items.count - 1]->id;
if (itemID <= lastID) {
avifBreakOnError();
avifDiagnosticsPrintf(diag, "The added itemID [%u] does not preserve the itemID order", itemID);
return AVIF_RESULT_BMFF_PARSE_FAILED;
}
}

avifDecoderItem ** itemPtr = (avifDecoderItem **)avifArrayPush(&meta->items);
AVIF_CHECKERR(itemPtr != NULL, AVIF_RESULT_OUT_OF_MEMORY);
*item = (avifDecoderItem *)avifAlloc(sizeof(avifDecoderItem));
Expand Down Expand Up @@ -1777,7 +1765,7 @@ static avifResult avifParseItemLocationBox(avifMeta * meta, const uint8_t * raw,
#endif // AVIF_ENABLE_EXPERIMENTAL_AVIR

avifDecoderItem * item;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, itemID, &item, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, itemID, &item));
if (item->extents.count > 0) {
// This item has already been given extents via this iloc box. This is invalid.
avifDiagnosticsPrintf(diag, "Item ID [%u] contains duplicate sets of extents", itemID);
Expand Down Expand Up @@ -2391,7 +2379,7 @@ static avifResult avifParseItemPropertyAssociation(avifMeta * meta, const uint8_
prevItemID = itemID;

avifDecoderItem * item;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, itemID, &item, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, itemID, &item));
if (item->ipmaSeen) {
avifDiagnosticsPrintf(diag, "Duplicate Box[ipma] for item ID [%u]", itemID);
return AVIF_RESULT_BMFF_PARSE_FAILED;
Expand Down Expand Up @@ -2667,7 +2655,7 @@ static avifResult avifParseItemInfoEntry(avifMeta * meta, const uint8_t * raw, s
}
#endif // AVIF_ENABLE_EXPERIMENTAL_AVIR
avifDecoderItem * item;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, itemID, &item, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, itemID, &item));

memcpy(item->type, itemType, sizeof(itemType));
item->contentType = contentType;
Expand Down Expand Up @@ -2736,7 +2724,7 @@ static avifResult avifParseItemReferenceBox(avifMeta * meta, const uint8_t * raw
AVIF_CHECKRES(avifCheckItemID("iref", fromID, diag));

avifDecoderItem * item;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, fromID, &item, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, fromID, &item));
if (!memcmp(irefHeader.type, "dimg", 4)) {
if (item->hasDimgFrom) {
// ISO/IEC 23008-12 (HEIF) 6.6.1: The number of SingleItemTypeReferenceBoxes with the box type 'dimg'
Expand Down Expand Up @@ -2774,7 +2762,7 @@ static avifResult avifParseItemReferenceBox(avifMeta * meta, const uint8_t * raw
} else if (!memcmp(irefHeader.type, "dimg", 4)) {
// derived images refer in the opposite direction
avifDecoderItem * dimg;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, toID, &dimg, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, toID, &dimg));

dimg->dimgForID = fromID;
dimg->dimgIdx = refIndex;
Expand Down Expand Up @@ -3574,7 +3562,7 @@ static avifResult avifParseCondensedImageBox(avifMeta * meta, uint64_t rawOffset

meta->primaryItemID = 1;
avifDecoderItem * colorItem;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, meta->primaryItemID, &colorItem, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, meta->primaryItemID, &colorItem));
memcpy(colorItem->type, "av01", 4);
colorItem->width = width;
colorItem->height = height;
Expand All @@ -3583,7 +3571,7 @@ static avifResult avifParseCondensedImageBox(avifMeta * meta, uint64_t rawOffset

avifDecoderItem * alphaItem = NULL;
if (hasAlpha) {
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/2, &alphaItem, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/2, &alphaItem));
memcpy(alphaItem->type, "av01", 4);
alphaItem->width = width;
alphaItem->height = height;
Expand Down Expand Up @@ -3688,7 +3676,7 @@ static avifResult avifParseCondensedImageBox(avifMeta * meta, uint64_t rawOffset

if (hasExif) {
avifDecoderItem * exifItem;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/3, &exifItem, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/3, &exifItem));
memcpy(exifItem->type, "Exif", 4);
exifItem->descForID = colorItem->id;
colorItem->premByID = alphaIsPremultiplied;
Expand All @@ -3703,7 +3691,7 @@ static avifResult avifParseCondensedImageBox(avifMeta * meta, uint64_t rawOffset

if (hasXMP) {
avifDecoderItem * xmpItem;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/4, &xmpItem, diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/4, &xmpItem));
memcpy(xmpItem->type, "mime", 4);
memcpy(xmpItem->contentType.contentType, xmpContentType, xmpContentTypeSize);
xmpItem->descForID = colorItem->id;
Expand Down Expand Up @@ -4046,7 +4034,7 @@ avifResult avifDecoderNthImageMaxExtent(const avifDecoder * decoder, uint32_t fr
// The data comes from an item. Let avifDecoderItemMaxExtent() do the heavy lifting.

avifDecoderItem * item;
AVIF_CHECKRES(avifMetaFindOrCreateItem(decoder->data->meta, sample->itemID, &item, decoder->data->diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(decoder->data->meta, sample->itemID, &item));
avifResult maxExtentResult = avifDecoderItemMaxExtent(item, sample, &sampleExtent);
if (maxExtentResult != AVIF_RESULT_OK) {
return maxExtentResult;
Expand Down Expand Up @@ -4085,7 +4073,7 @@ static avifResult avifDecoderPrepareSample(avifDecoder * decoder, avifDecodeSamp
// The data comes from an item. Let avifDecoderItemRead() do the heavy lifting.

avifDecoderItem * item;
AVIF_CHECKRES(avifMetaFindOrCreateItem(decoder->data->meta, sample->itemID, &item, decoder->data->diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(decoder->data->meta, sample->itemID, &item));
avifROData itemContents;
if (sample->offset > SIZE_MAX) {
return AVIF_RESULT_BMFF_PARSE_FAILED;
Expand Down Expand Up @@ -4352,8 +4340,7 @@ static avifResult avifMetaFindAlphaItem(avifMeta * meta,
const avifTileInfo * colorInfo,
avifDecoderItem ** alphaItem,
avifTileInfo * alphaInfo,
avifBool * isAlphaItemInInput,
avifDiagnostics * diag)
avifBool * isAlphaItemInInput)
{
for (uint32_t itemIndex = 0; itemIndex < meta->items.count; ++itemIndex) {
avifDecoderItem * item = meta->items.item[itemIndex];
Expand Down Expand Up @@ -4411,17 +4398,25 @@ static avifResult avifMetaFindAlphaItem(avifMeta * meta,
}
}
AVIF_ASSERT_OR_RETURN(alphaItemCount == colorItemCount);
// Figure out the last used itemID.
// Find an unused ID.
avifResult result;
const uint32_t lastID = meta->items.item[meta->items.count - 1]->id;
if (lastID == UINT32_MAX) {
// In the improbable case where the last ID is the maximum one, ids cannot be kept ordered.
avifDiagnosticsPrintf(diag,
"Cannot set an itemID for alpha that fits the increasing "
"order as the maximum possible ID is in use.");
if (meta->items.count >= UINT32_MAX - 1) {
// In the improbable case where all IDs are used.
result = AVIF_RESULT_DECODE_ALPHA_FAILED;
} else {
result = avifMetaFindOrCreateItem(meta, lastID + 1, alphaItem, diag); // Create new empty item.
uint32_t newItemID = 0;
avifBool isUsed;
do {
++newItemID;
isUsed = AVIF_FALSE;
for (uint32_t i = 0; i < meta->items.count; ++i) {
if (meta->items.item[i]->id == newItemID) {
isUsed = AVIF_TRUE;
break;
}
}
} while (isUsed && newItemID != 0);
result = avifMetaFindOrCreateItem(meta, newItemID, alphaItem); // Create new empty item.
}
if (result != AVIF_RESULT_OK) {
avifFree(alphaItemIndices);
Expand Down Expand Up @@ -4565,7 +4560,7 @@ static avifResult avifDecoderFindGainMapItem(const avifDecoder * decoder,

AVIF_ASSERT_OR_RETURN(gainMapItemID != 0);
avifDecoderItem * gainMapItemTmp;
AVIF_CHECKRES(avifMetaFindOrCreateItem(data->meta, gainMapItemID, &gainMapItemTmp, data->diag));
AVIF_CHECKRES(avifMetaFindOrCreateItem(data->meta, gainMapItemID, &gainMapItemTmp));
if (avifDecoderItemShouldBeSkipped(gainMapItemTmp)) {
avifDiagnosticsPrintf(data->diag, "Box[tmap] gain map item %d is not a supported image type", gainMapItemID);
return AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE;
Expand Down Expand Up @@ -4891,8 +4886,7 @@ avifResult avifDecoderReset(avifDecoder * decoder)
&data->tileInfos[AVIF_ITEM_COLOR],
&mainItems[AVIF_ITEM_ALPHA],
&data->tileInfos[AVIF_ITEM_ALPHA],
&isAlphaItemInInput,
data->diag));
&isAlphaItemInInput));
if (mainItems[AVIF_ITEM_ALPHA]) {
AVIF_CHECKRES(avifDecoderItemReadAndParse(decoder,
mainItems[AVIF_ITEM_ALPHA],
Expand Down

0 comments on commit 992b69d

Please sign in to comment.