Validate the item properties for made up alpha grid Also address the comments from the review here: https://aomedia-review.googlesource.com/c/libavif/+/175421
diff --git a/src/read.c b/src/read.c index 091b405..3c1c849 100644 --- a/src/read.c +++ b/src/read.c
@@ -3563,20 +3563,20 @@ // Returns AVIF_TRUE if the item should be skipped. Items should be skipped for one of the following reasons: // * Size is 0. // * Has an essential property that isn't supported by libavif. -// * Item is Exif or similar data. +// * Item is Exif or similar metadata. // * Item is a thumbnail. -static avifBool avifDecoderDataShouldSkipItem(avifDecoderItem * item) +static avifBool avifDecoderItemShouldBeSkipped(const avifDecoderItem * item) { return !item->size || item->hasUnsupportedEssentialProperty || (avifGetCodecType(item->type) == AVIF_CODEC_TYPE_UNKNOWN && memcmp(item->type, "grid", 4)) || item->thumbnailForID != 0; } -// Returns the color item if found, or NULL. +// Returns the primary color item if found, or NULL. static avifDecoderItem * avifDecoderDataFindColorItem(avifDecoderData * data) { for (uint32_t itemIndex = 0; itemIndex < data->meta->items.count; ++itemIndex) { avifDecoderItem * item = &data->meta->items.item[itemIndex]; - if (avifDecoderDataShouldSkipItem(item)) { + if (avifDecoderItemShouldBeSkipped(item)) { continue; } if (item->id == data->meta->primaryItemID) { @@ -3586,19 +3586,22 @@ return NULL; } +// Returns AVIF_TRUE if item is an alpha auxiliary item of the parent color +// item. static avifBool avifDecoderItemIsAlphaAux(avifDecoderItem * item, uint32_t colorItemId) { if (item->auxForID != colorItemId) return AVIF_FALSE; - // Is this an alpha auxiliary item of the color item? const avifProperty * auxCProp = avifPropertyArrayFind(&item->properties, "auxC"); 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. Otherwise, isAlphaItemInInput will be set to AVIF_TRUE when *alphaItem is not NULL. +// 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. static avifResult avifDecoderDataFindAlphaItem(avifDecoderData * data, avifDecoderItem * colorItem, avifDecoderItem ** alphaItem, @@ -3606,7 +3609,7 @@ { for (uint32_t itemIndex = 0; itemIndex < data->meta->items.count; ++itemIndex) { avifDecoderItem * item = &data->meta->items.item[itemIndex]; - if (avifDecoderDataShouldSkipItem(item)) { + if (avifDecoderItemShouldBeSkipped(item)) { continue; } if (avifDecoderItemIsAlphaAux(item, colorItem->id)) { @@ -3615,55 +3618,60 @@ return AVIF_RESULT_OK; } } - if (!memcmp(colorItem->type, "grid", 4)) { - // 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 = data->color.grid.rows * data->color.grid.columns; - if (colorItemCount <= 0) { - *alphaItem = NULL; - return AVIF_RESULT_OK; - } - uint32_t * alphaItemIndices = avifAlloc(colorItemCount * sizeof(uint32_t)); - AVIF_CHECKERR(alphaItemIndices, AVIF_RESULT_OUT_OF_MEMORY); - uint32_t alphaItemCount = 0; - uint32_t maxItemID = 0; - for (uint32_t i = 0; i < colorItem->meta->items.count; ++i) { - avifDecoderItem * item = &colorItem->meta->items.item[i]; - if (item->id > maxItemID) { - maxItemID = item->id; - } - if (item->dimgForID == colorItem->id) { - for (uint32_t j = 0; j < colorItem->meta->items.count; ++j) { - avifDecoderItem * auxlItem = &colorItem->meta->items.item[j]; - if (avifDecoderItemIsAlphaAux(auxlItem, item->id)) { - alphaItemIndices[alphaItemCount++] = j; - } - } - } - } - 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; - return AVIF_RESULT_OK; - } - *alphaItem = avifMetaFindItem(colorItem->meta, maxItemID + 1); - if (*alphaItem == NULL) { - avifFree(alphaItemIndices); - return AVIF_RESULT_OUT_OF_MEMORY; - } - memcpy((*alphaItem)->type, "grid", 4); - (*alphaItem)->width = colorItem->width; - (*alphaItem)->height = colorItem->height; - for (uint32_t i = 0; i < alphaItemCount; ++i) { - avifDecoderItem * item = &colorItem->meta->items.item[alphaItemIndices[i]]; - item->dimgForID = (*alphaItem)->id; - } - avifFree(alphaItemIndices); + if (memcmp(colorItem->type, "grid", 4)) { + *alphaItem = NULL; *isAlphaItemInInput = AVIF_FALSE; return AVIF_RESULT_OK; } - *alphaItem = NULL; + // 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 = data->color.grid.rows * data->color.grid.columns; + if (colorItemCount == 0) { + *alphaItem = NULL; + *isAlphaItemInInput = AVIF_FALSE; + return AVIF_RESULT_OK; + } + uint32_t * alphaItemIndices = avifAlloc(colorItemCount * sizeof(uint32_t)); + AVIF_CHECKERR(alphaItemIndices, AVIF_RESULT_OUT_OF_MEMORY); + uint32_t alphaItemCount = 0; + uint32_t maxItemID = 0; + for (uint32_t i = 0; i < colorItem->meta->items.count; ++i) { + avifDecoderItem * item = &colorItem->meta->items.item[i]; + if (item->id > maxItemID) { + maxItemID = item->id; + } + if (item->dimgForID == colorItem->id) { + for (uint32_t j = 0; j < colorItem->meta->items.count; ++j) { + avifDecoderItem * auxlItem = &colorItem->meta->items.item[j]; + if (avifDecoderItemIsAlphaAux(auxlItem, item->id)) { + alphaItemIndices[alphaItemCount++] = j; + } + } + } + } + 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; + *isAlphaItemInInput = AVIF_FALSE; + return AVIF_RESULT_OK; + } + *alphaItem = avifMetaFindItem(colorItem->meta, maxItemID + 1); + if (*alphaItem == NULL) { + avifFree(alphaItemIndices); + *isAlphaItemInInput = AVIF_FALSE; + return AVIF_RESULT_OUT_OF_MEMORY; + } + memcpy((*alphaItem)->type, "grid", 4); + (*alphaItem)->width = colorItem->width; + (*alphaItem)->height = colorItem->height; + for (uint32_t i = 0; i < alphaItemCount; ++i) { + avifDecoderItem * item = &colorItem->meta->items.item[alphaItemIndices[i]]; + item->dimgForID = (*alphaItem)->id; + } + avifFree(alphaItemIndices); + *isAlphaItemInInput = AVIF_FALSE; + data->alpha.grid = data->color.grid; return AVIF_RESULT_OK; } @@ -3901,7 +3909,7 @@ assert(colorCodecType != AVIF_CODEC_TYPE_UNKNOWN); } - avifBool isAlphaItemInInput = AVIF_TRUE; + avifBool isAlphaItemInInput; avifDecoderItem * alphaItem; AVIF_CHECKRES(avifDecoderDataFindAlphaItem(data, colorItem, &alphaItem, &isAlphaItemInInput)); avifCodecType alphaCodecType = AVIF_CODEC_TYPE_UNKNOWN; @@ -3917,9 +3925,6 @@ decoder->imageDimensionLimit, data->diag), AVIF_RESULT_INVALID_IMAGE_GRID); - } else { - // This item was not actually in the input. Just copy the color grid structure to the alpha grid. - data->alpha.grid = data->color.grid; } alphaCodecType = avifDecoderItemGetGridCodecType(alphaItem); if (alphaCodecType == AVIF_CODEC_TYPE_UNKNOWN) { @@ -3975,11 +3980,15 @@ AVIF_CHECKRES( avifDecoderItemValidateProperties(colorItem, avifGetConfigurationPropertyName(colorCodecType), &decoder->diag, decoder->strictFlags)); - if (alphaItem && isAlphaItemInInput) { - AVIF_CHECKRES(avifDecoderItemValidateProperties(alphaItem, - avifGetConfigurationPropertyName(alphaCodecType), - &decoder->diag, - decoder->strictFlags)); + if (alphaItem) { + avifStrictFlags strictFlags = decoder->strictFlags; + if (!isAlphaItemInInput) { + // In this case, the made up grid item will not have a an associated pixi property. So validate everything else + // but the pixi property. + strictFlags &= ~AVIF_STRICT_PIXI_REQUIRED; + } + AVIF_CHECKRES( + avifDecoderItemValidateProperties(alphaItem, avifGetConfigurationPropertyName(alphaCodecType), &decoder->diag, strictFlags)); } }