From 23e1316809b8b6cabf4064fb0c45c9b4ee58c5a3 Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Wed, 10 Apr 2024 15:17:29 +0200 Subject: [PATCH] Apply comments --- include/avif/internal.h | 1 + src/read.c | 16 ++++++++++------ src/write.c | 33 +++++++++++++++++++-------------- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/include/avif/internal.h b/include/avif/internal.h index 690bd365ae..3de27a7884 100644 --- a/include/avif/internal.h +++ b/include/avif/internal.h @@ -197,6 +197,7 @@ typedef struct avifSampleTransformToken { uint8_t type; // avifSampleTransformTokenType int32_t constant; // If value is AVIF_SAMPLE_TRANSFORM_CONSTANT. + // Only 32-bit (bit_depth=2) constants are supported. uint8_t inputImageItemIndex; // If value is AVIF_SAMPLE_TRANSFORM_INPUT_IMAGE_ITEM_INDEX. 1-based. } avifSampleTransformToken; diff --git a/src/read.c b/src/read.c index 1892864522..36fb0cc6bb 100644 --- a/src/read.c +++ b/src/read.c @@ -2024,6 +2024,7 @@ static avifBool avifParseToneMappedImageBox(avifGainMapMetadata * metadata, cons #endif // AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP #if defined(AVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM) +// bit-depth is assumed to be 2 (32-bit). static avifResult avifParseSampleTransformTokens(avifROStream * s, avifSampleTransformExpression * expression) { uint8_t tokenCount; @@ -2036,7 +2037,7 @@ static avifResult avifParseSampleTransformTokens(avifROStream * s, avifSampleTra AVIF_CHECK(avifROStreamRead(s, &token->type, /*size=*/1)); // unsigned int(8) token; if (token->type == AVIF_SAMPLE_TRANSFORM_CONSTANT) { - // TODO(yguyon): Verify two's complement representation is guaranteed here. + // Two's complement representation is assumed here. uint32_t constant; AVIF_CHECK(avifROStreamReadU32(s, &constant)); // signed int(1<<(bit_depth+3)) constant; token->constant = *(int32_t *)&constant; // maybe =(int32_t)constant; is enough @@ -2166,11 +2167,11 @@ static avifResult avifDecoderItemReadAndParse(const avifDecoder * decoder, *codecType = avifGetCodecType(item->type); AVIF_ASSERT_OR_RETURN(*codecType != AVIF_CODEC_TYPE_UNKNOWN); } - // Note: If AVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM is defined, backward-incompatible files - // with a primary 'sato' Sample Transform derived image item could be handled here - // (compared to backward-compatible files with a 'sato' item in the same 'altr' group - // as the primary regular color item which are handled in - // avifDecoderDataFindSampleTransformImageItem() below). + // TODO(yguyon): If AVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM is defined, backward-incompatible + // files with a primary 'sato' Sample Transform derived image item could be + // handled here (compared to backward-compatible files with a 'sato' item in the + // same 'altr' group as the primary regular color item which are handled in + // avifDecoderDataFindSampleTransformImageItem() below). return AVIF_RESULT_OK; } @@ -5155,6 +5156,8 @@ avifResult avifDecoderReset(avifDecoder * decoder) // Input image item order is important because input image items are indexed according to this order. AVIF_CHECKERR(inputImageItem->dimgIdx == data->sampleTransformNumInputImageItems, AVIF_RESULT_NOT_IMPLEMENTED); + AVIF_CHECKERR(data->sampleTransformNumInputImageItems < AVIF_SAMPLE_TRANSFORM_MAX_NUM_INPUT_IMAGE_ITEMS, + AVIF_RESULT_NOT_IMPLEMENTED); avifItemCategory * category = &data->sampleTransformInputImageItems[data->sampleTransformNumInputImageItems]; avifBool foundItem = AVIF_FALSE; uint32_t alphaCategory = AVIF_ITEM_CATEGORY_COUNT; @@ -5646,6 +5649,7 @@ avifResult avifDecoderApplySampleTransform(const avifDecoder * decoder, avifImag } for (avifBool alpha = AVIF_FALSE; alpha <= decoder->alphaPresent ? AVIF_TRUE : AVIF_FALSE; ++alpha) { + AVIF_ASSERT_OR_RETURN(decoder->data->sampleTransformNumInputImageItems <= AVIF_SAMPLE_TRANSFORM_MAX_NUM_INPUT_IMAGE_ITEMS); const avifImage * inputImages[AVIF_SAMPLE_TRANSFORM_MAX_NUM_INPUT_IMAGE_ITEMS]; for (uint32_t i = 0; i < decoder->data->sampleTransformNumInputImageItems; ++i) { avifItemCategory category = decoder->data->sampleTransformInputImageItems[i]; diff --git a/src/write.c b/src/write.c index 6c694786ab..7908d0b8ad 100644 --- a/src/write.c +++ b/src/write.c @@ -1161,6 +1161,24 @@ static const char infeNameGainMap[] = "GMap"; static const char infeNameSampleTransform[] = "SampleTransform"; #endif +static const char * getInfeName(avifItemCategory itemCategory) +{ + if (avifIsAlpha(itemCategory)) { + return infeNameAlpha; + } +#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) + if (itemCategory == AVIF_ITEM_GAIN_MAP) { + return infeNameGainMap; + } +#endif +#if defined(AVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM) + if (itemCategory >= AVIF_SAMPLE_TRANSFORM_MIN_CATEGORY && itemCategory <= AVIF_SAMPLE_TRANSFORM_MAX_CATEGORY) { + return infeNameSampleTransform; + } +#endif + return infeNameColor; +} + // Adds the items for a single cell or a grid of cells. Outputs the topLevelItemID which is // the only item if there is exactly one cell, or the grid item for multiple cells. // Note: The topLevelItemID output argument has the type uint16_t* instead of avifEncoderItem** because @@ -1174,20 +1192,7 @@ static avifResult avifEncoderAddImageItems(avifEncoder * encoder, uint16_t * topLevelItemID) { const uint32_t cellCount = gridCols * gridRows; - const char * infeName; - if (avifIsAlpha(itemCategory)) { - infeName = infeNameAlpha; -#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) - } else if (itemCategory == AVIF_ITEM_GAIN_MAP) { - infeName = infeNameGainMap; -#endif -#if defined(AVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM) - } else if (itemCategory >= AVIF_SAMPLE_TRANSFORM_MIN_CATEGORY && itemCategory <= AVIF_SAMPLE_TRANSFORM_MAX_CATEGORY) { - infeName = infeNameSampleTransform; -#endif - } else { - infeName = infeNameColor; - } + const char * infeName = getInfeName(itemCategory); const size_t infeNameSize = strlen(infeName) + 1; if (cellCount > 1) {