Skip to content

Commit

Permalink
Do not store item pointers until all items are created
Browse files Browse the repository at this point in the history
Calling avifMetaFindOrCreateItem() could invalidate all the
existing item pointers that are being stored by the caller (since
the function could resize the item array).

This patch fixes avifDecoderReset by storing the item indices
instead of item pointers until all the items are either created
or found.
  • Loading branch information
vigneshvg committed Nov 14, 2023
1 parent b293d7e commit 2041109
Showing 1 changed file with 77 additions and 51 deletions.
128 changes: 77 additions & 51 deletions src/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,8 @@ static avifResult avifCheckItemID(const char * boxFourcc, uint32_t itemID, avifD
return AVIF_RESULT_OK;
}

// CAUTION: This function could potentially resize the meta->items array thereby invalidating all existing pointers that are being
// stored locally. So if this function is being called, exercise caution in the caller to not use invalid pointers.
static avifResult avifMetaFindOrCreateItem(avifMeta * meta, uint32_t itemID, avifDecoderItem ** item)
{
*item = NULL;
Expand Down Expand Up @@ -1935,11 +1937,12 @@ static avifBool avifParseToneMappedImageBox(avifGainMapMetadata * metadata, cons
// isItemInInput must be false if the item is a made-up structure
// (and thus not part of the parseable input bitstream).
static avifResult avifDecoderItemReadAndParse(const avifDecoder * decoder,
avifDecoderItem * item,
int itemIndex,
avifBool isItemInInput,
avifImageGrid * grid,
avifCodecType * codecType)
{
avifDecoderItem * item = &decoder->data->meta->items.item[itemIndex];
if (!memcmp(item->type, "grid", 4)) {
if (isItemInInput) {
avifROData readData;
Expand Down Expand Up @@ -4222,19 +4225,19 @@ static avifBool avifDecoderItemShouldBeSkipped(const avifDecoderItem * item)
(avifGetCodecType(item->type) == AVIF_CODEC_TYPE_UNKNOWN && memcmp(item->type, "grid", 4)) || item->thumbnailForID != 0;
}

// Returns the primary color item if found, or NULL.
static avifDecoderItem * avifMetaFindColorItem(avifMeta * meta)
// Returns the index of the primary color item if found, or -1.
static int avifMetaFindColorItem(avifMeta * meta)
{
for (uint32_t itemIndex = 0; itemIndex < meta->items.count; ++itemIndex) {
avifDecoderItem * item = &meta->items.item[itemIndex];
if (avifDecoderItemShouldBeSkipped(item)) {
continue;
}
if (item->id == meta->primaryItemID) {
return item;
return itemIndex;
}
}
return NULL;
return -1;
}

// Returns AVIF_TRUE if item is an alpha auxiliary item of the parent color
Expand All @@ -4247,39 +4250,40 @@ static avifBool avifDecoderItemIsAlphaAux(avifDecoderItem * item, uint32_t color
return auxCProp && isAlphaURN(auxCProp->u.auxC.auxType);
}

// Finds the alpha item whose parent item is colorItem and sets it in the alphaItem output parameter. Returns AVIF_RESULT_OK on
// success. Note that *alphaItem can be NULL even if the return value is AVIF_RESULT_OK. If the colorItem is a grid and the alpha
// item is represented as a set of auxl items to each color tile, then a fake item will be created and *isAlphaItemInInput will be
// set to AVIF_FALSE. In this case, the alpha item merely exists to hold the locations of the alpha tile items. The data of this
// item need not be read and the pixi property cannot be validated. Otherwise, *isAlphaItemInInput will be set to AVIF_TRUE when
// *alphaItem is not NULL.
// Finds the alpha item whose parent item is the color item and sets its index in the alphaItemIndex output parameter. Returns
// AVIF_RESULT_OK on success. Note that *alphaItemIndex can be -1 even if the return value is AVIF_RESULT_OK. If the color item is
// a grid and the alpha item is represented as a set of auxl items to each color tile, then a fake item will be created and
// *isAlphaItemInInput will be set to AVIF_FALSE. In this case, the alpha item merely exists to hold the locations of the alpha
// tile items. The data of this item need not be read and the pixi property cannot be validated. Otherwise, *isAlphaItemInInput
// will be set to AVIF_TRUE when *alphaItemIndex is not -1.
static avifResult avifMetaFindAlphaItem(avifMeta * meta,
const avifDecoderItem * colorItem,
int colorItemIndex,
const avifTileInfo * colorInfo,
avifDecoderItem ** alphaItem,
int * alphaItemIndex,
avifTileInfo * alphaInfo,
avifBool * isAlphaItemInInput)
{
const avifDecoderItem * colorItem = &meta->items.item[colorItemIndex];
for (uint32_t itemIndex = 0; itemIndex < meta->items.count; ++itemIndex) {
avifDecoderItem * item = &meta->items.item[itemIndex];
if (avifDecoderItemShouldBeSkipped(item)) {
continue;
}
if (avifDecoderItemIsAlphaAux(item, colorItem->id)) {
*alphaItem = item;
*alphaItemIndex = itemIndex;
*isAlphaItemInInput = AVIF_TRUE;
return AVIF_RESULT_OK;
}
}
if (memcmp(colorItem->type, "grid", 4)) {
*alphaItem = NULL;
*alphaItemIndex = -1;
*isAlphaItemInInput = AVIF_FALSE;
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) {
*alphaItem = NULL;
*alphaItemIndex = -1;
*isAlphaItemInInput = AVIF_FALSE;
return AVIF_RESULT_OK;
}
Expand All @@ -4304,22 +4308,27 @@ static avifResult avifMetaFindAlphaItem(avifMeta * meta,
if (alphaItemCount != colorItemCount) {
// Not all the color items had an alpha auxiliary attached to it. Report this case as an image without alpha channel.
avifFree(alphaItemIndices);
*alphaItem = NULL;
*alphaItemIndex = -1;
*isAlphaItemInInput = AVIF_FALSE;
return AVIF_RESULT_OK;
}
const avifResult result = avifMetaFindOrCreateItem(meta, maxItemID + 1, alphaItem); // Create new empty item.
avifDecoderItem * alphaItem;
const avifResult result = avifMetaFindOrCreateItem(meta, maxItemID + 1, &alphaItem); // Create new empty item.
if (result != AVIF_RESULT_OK) {
avifFree(alphaItemIndices);
*isAlphaItemInInput = AVIF_FALSE;
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;
*alphaItemIndex = meta->items.count - 1;
// avifMetaFindOrCreateItem() could invalidate all existing item pointers. So reset the colorItem pointer.
colorItem = &meta->items.item[colorItemIndex];

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;
item->dimgForID = alphaItem->id;
}
avifFree(alphaItemIndices);
*isAlphaItemInInput = AVIF_FALSE;
Expand Down Expand Up @@ -4378,41 +4387,50 @@ static avifResult avifDecoderDataFindToneMappedImageItem(const avifDecoderData *
return AVIF_RESULT_OK;
}

// Finds a 'tmap' (tone mapped image item) box associated with the given 'colorItem',
// then finds the associated gain map image.
// If found, fills 'toneMappedImageItem', 'gainMapItem' and 'gainMapCodecType'.
// Otherwise, sets 'toneMappedImageItem' and 'gainMapItem' to NULL.
// Finds a 'tmap' (tone mapped image item) box associated with the given color item at index 'colorItemIndex', then finds the
// associated gain map image.
// If found, fills 'toneMappedImageItem', 'gainMapItemIndex' and 'gainMapCodecType'.
// Otherwise, sets 'toneMappedImageItem' to NULL and 'gainMapItemIndex' to -1.
// Returns AVIF_RESULT_OK if no errors were encountered (whether or not a gain map was found).
// Assumes that there is a single tmap item, and not, e.g., a grid of tmap items.
static avifResult avifDecoderFindGainMapItem(const avifDecoder * decoder,
const avifDecoderItem * colorItem,
int colorItemIndex,
avifDecoderItem ** toneMappedImageItem,
avifDecoderItem ** gainMapItem,
int * gainMapItemIndex,
avifCodecType * gainMapCodecType)
{
*toneMappedImageItem = NULL;
*gainMapItem = NULL;
*gainMapItemIndex = -1;
*gainMapCodecType = AVIF_CODEC_TYPE_UNKNOWN;

avifDecoderData * data = decoder->data;

uint32_t gainMapItemID;
avifDecoderItem * toneMappedImageItemTmp;
AVIF_CHECKRES(avifDecoderDataFindToneMappedImageItem(data, colorItem, &toneMappedImageItemTmp, &gainMapItemID));
AVIF_CHECKRES(
avifDecoderDataFindToneMappedImageItem(data, &data->meta->items.item[colorItemIndex], &toneMappedImageItemTmp, &gainMapItemID));
if (!toneMappedImageItemTmp) {
return AVIF_RESULT_OK;
}

assert(gainMapItemID != 0);
avifDecoderItem * gainMapItemTmp;
AVIF_CHECKRES(avifMetaFindOrCreateItem(data->meta, gainMapItemID, &gainMapItemTmp));
if (avifDecoderItemShouldBeSkipped(gainMapItemTmp)) {
avifDecoderItem * gainMapItem;
AVIF_CHECKRES(avifMetaFindOrCreateItem(data->meta, gainMapItemID, &gainMapItem));
if (avifDecoderItemShouldBeSkipped(gainMapItem)) {
avifDiagnosticsPrintf(data->diag, "Box[tmap] gain map item %d is not a supported image type", gainMapItemID);
return AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE;
}
int gainMapItemIndexTmp = -1;
for (uint32_t itemIndex = 0; itemIndex < data->meta->items.count; ++itemIndex) {
if (gainMapItem->id == data->meta->items.item[itemIndex].id) {
gainMapItemIndexTmp = itemIndex;
break;
}
}
assert(gainMapItemIndexTmp >= 0);

AVIF_CHECKRES(avifDecoderItemReadAndParse(decoder,
gainMapItemTmp,
gainMapItemIndexTmp,
/*isItemInInput=*/AVIF_TRUE,
&data->tileInfos[AVIF_ITEM_GAIN_MAP].grid,
gainMapCodecType));
Expand All @@ -4421,8 +4439,8 @@ static avifResult avifDecoderFindGainMapItem(const avifDecoder * decoder,
decoder->image->gainMap.image = avifImageCreateEmpty();

// Look for a colr nclx box. Other colr box types (e.g. ICC) are not supported.
for (uint32_t propertyIndex = 0; propertyIndex < gainMapItemTmp->properties.count; ++propertyIndex) {
avifProperty * prop = &gainMapItemTmp->properties.prop[propertyIndex];
for (uint32_t propertyIndex = 0; propertyIndex < gainMapItem->properties.count; ++propertyIndex) {
avifProperty * prop = &gainMapItem->properties.prop[propertyIndex];
if (!memcmp(prop->type, "colr", 4) && prop->u.colr.hasNCLX) {
decoder->image->gainMap.image->colorPrimaries = prop->u.colr.colorPrimaries;
decoder->image->gainMap.image->transferCharacteristics = prop->u.colr.transferCharacteristics;
Expand All @@ -4444,7 +4462,7 @@ static avifResult avifDecoderFindGainMapItem(const avifDecoder * decoder,

// Only set the output parameters after everything has been validated.
*toneMappedImageItem = toneMappedImageItemTmp;
*gainMapItem = gainMapItemTmp;
*gainMapItemIndex = gainMapItemIndexTmp;
return AVIF_RESULT_OK;
}
#endif // AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP
Expand Down Expand Up @@ -4676,38 +4694,38 @@ avifResult avifDecoderReset(avifDecoder * decoder)
}

// Main item of each group category (top-level item such as grid or single tile), if any.
avifDecoderItem * mainItems[AVIF_ITEM_CATEGORY_COUNT];
int mainItemIndices[AVIF_ITEM_CATEGORY_COUNT];
avifCodecType codecType[AVIF_ITEM_CATEGORY_COUNT];
for (int c = 0; c < AVIF_ITEM_CATEGORY_COUNT; ++c) {
mainItems[c] = NULL;
mainItemIndices[c] = -1;
codecType[c] = AVIF_CODEC_TYPE_UNKNOWN;
}

// Mandatory primary color item
mainItems[AVIF_ITEM_COLOR] = avifMetaFindColorItem(data->meta);
if (!mainItems[AVIF_ITEM_COLOR]) {
mainItemIndices[AVIF_ITEM_COLOR] = avifMetaFindColorItem(data->meta);
if (mainItemIndices[AVIF_ITEM_COLOR] == -1) {
avifDiagnosticsPrintf(&decoder->diag, "Primary item not found");
return AVIF_RESULT_MISSING_IMAGE_ITEM;
}
AVIF_CHECKRES(avifDecoderItemReadAndParse(decoder,
mainItems[AVIF_ITEM_COLOR],
mainItemIndices[AVIF_ITEM_COLOR],
/*isItemInInput=*/AVIF_TRUE,
&data->tileInfos[AVIF_ITEM_COLOR].grid,
&codecType[AVIF_ITEM_COLOR]));
colorProperties = &mainItems[AVIF_ITEM_COLOR]->properties;
colorProperties = &data->meta->items.item[mainItemIndices[AVIF_ITEM_COLOR]].properties;
colorCodecType = codecType[AVIF_ITEM_COLOR];

// Optional alpha auxiliary item
avifBool isAlphaItemInInput;
AVIF_CHECKRES(avifMetaFindAlphaItem(data->meta,
mainItems[AVIF_ITEM_COLOR],
mainItemIndices[AVIF_ITEM_COLOR],
&data->tileInfos[AVIF_ITEM_COLOR],
&mainItems[AVIF_ITEM_ALPHA],
&mainItemIndices[AVIF_ITEM_ALPHA],
&data->tileInfos[AVIF_ITEM_ALPHA],
&isAlphaItemInInput));
if (mainItems[AVIF_ITEM_ALPHA]) {
if (mainItemIndices[AVIF_ITEM_ALPHA] != -1) {
AVIF_CHECKRES(avifDecoderItemReadAndParse(decoder,
mainItems[AVIF_ITEM_ALPHA],
mainItemIndices[AVIF_ITEM_ALPHA],
isAlphaItemInInput,
&data->tileInfos[AVIF_ITEM_ALPHA].grid,
&codecType[AVIF_ITEM_ALPHA]));
Expand All @@ -4716,9 +4734,9 @@ avifResult avifDecoderReset(avifDecoder * decoder)
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
avifDecoderItem * toneMappedImageItem;
const avifResult findGainMapResult = avifDecoderFindGainMapItem(decoder,
mainItems[AVIF_ITEM_COLOR],
mainItemIndices[AVIF_ITEM_COLOR],
&toneMappedImageItem,
&mainItems[AVIF_ITEM_GAIN_MAP],
&mainItemIndices[AVIF_ITEM_GAIN_MAP],
&codecType[AVIF_ITEM_GAIN_MAP]);
if (!decoder->enableDecodingGainMap) {
// When ignoring the gain map, we still report whether one is present or not,
Expand All @@ -4729,9 +4747,9 @@ avifResult avifDecoderReset(avifDecoder * decoder)
// Clear diagnostic message.
avifDiagnosticsClearError(data->diag);
}
decoder->gainMapPresent = (mainItems[AVIF_ITEM_GAIN_MAP] != NULL);
decoder->gainMapPresent = (mainItemIndices[AVIF_ITEM_GAIN_MAP] != -1);
// We also ignore the actual item and don't decode it.
mainItems[AVIF_ITEM_GAIN_MAP] = NULL;
mainItemIndices[AVIF_ITEM_GAIN_MAP] = -1;
} else {
AVIF_CHECKRES(findGainMapResult);
}
Expand All @@ -4745,6 +4763,14 @@ avifResult avifDecoderReset(avifDecoder * decoder)
}
#endif // AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP

// When searching for alpha and gainmap items, it is possible that the data->meta->items array is re-sized in calls to
// avifMetaFindOrCreateItem. So it is not safe to store pointers into the data->meta->items array until all the items are
// either found or created. From this point onwards, we do not create any more items, so it is safe to store the pointers.
avifDecoderItem * mainItems[AVIF_ITEM_CATEGORY_COUNT];
for (int c = 0; c < AVIF_ITEM_CATEGORY_COUNT; ++c) {
mainItems[c] = (mainItemIndices[c] == -1) ? NULL : &data->meta->items.item[mainItemIndices[c]];
}

// Find Exif and/or XMP metadata, if any
AVIF_CHECKRES(avifDecoderFindMetadata(decoder, data->meta, decoder->image, mainItems[AVIF_ITEM_COLOR]->id));

Expand Down

0 comments on commit 2041109

Please sign in to comment.