From ce9db7727a5ca5c93fac32a55f5a47a800c94a87 Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Fri, 15 Mar 2024 10:04:54 +0100 Subject: [PATCH 1/2] Refactor read write for sample transforms --- src/read.c | 128 +++++++++++++++++++++++++++++----------------------- src/write.c | 46 ++++++++----------- 2 files changed, 91 insertions(+), 83 deletions(-) diff --git a/src/read.c b/src/read.c index e1a652bc9d..02e86331e8 100644 --- a/src/read.c +++ b/src/read.c @@ -1500,27 +1500,45 @@ static avifResult avifDecoderGenerateImageGridTiles(avifDecoder * decoder, avifI return AVIF_RESULT_OK; } -// Allocates the dstImage based on the grid image requirements. Also verifies some spec compliance rules for grids. -static avifResult avifDecoderDataAllocateGridImagePlanes(avifDecoderData * data, const avifTileInfo * info, avifImage * dstImage) +// Allocates the dstImage. Also verifies some spec compliance rules for grids, if relevant. +static avifResult avifDecoderDataAllocateImagePlanes(avifDecoderData * data, const avifTileInfo * info, avifImage * dstImage) { - const avifImageGrid * grid = &info->grid; const avifTile * tile = &data->tiles.tile[info->firstTileIndex]; - - // Validate grid image size and tile size. - // - // HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1: - // The tiled input images shall completely "cover" the reconstructed image grid canvas, ... - if (((tile->image->width * grid->columns) < grid->outputWidth) || ((tile->image->height * grid->rows) < grid->outputHeight)) { - avifDiagnosticsPrintf(data->diag, - "Grid image tiles do not completely cover the image (HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1)"); - return AVIF_RESULT_INVALID_IMAGE_GRID; - } - // Tiles in the rightmost column and bottommost row must overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2. - if (((tile->image->width * (grid->columns - 1)) >= grid->outputWidth) || - ((tile->image->height * (grid->rows - 1)) >= grid->outputHeight)) { - avifDiagnosticsPrintf(data->diag, - "Grid image tiles in the rightmost column and bottommost row do not overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2"); - return AVIF_RESULT_INVALID_IMAGE_GRID; + uint32_t dstWidth; + uint32_t dstHeight; + + if (info->grid.columns > 0 && info->grid.rows > 0) { + const avifImageGrid * grid = &info->grid; + // Validate grid image size and tile size. + // + // HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1: + // The tiled input images shall completely "cover" the reconstructed image grid canvas, ... + if (((tile->image->width * grid->columns) < grid->outputWidth) || ((tile->image->height * grid->rows) < grid->outputHeight)) { + avifDiagnosticsPrintf(data->diag, + "Grid image tiles do not completely cover the image (HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1)"); + return AVIF_RESULT_INVALID_IMAGE_GRID; + } + // Tiles in the rightmost column and bottommost row must overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2. + if (((tile->image->width * (grid->columns - 1)) >= grid->outputWidth) || + ((tile->image->height * (grid->rows - 1)) >= grid->outputHeight)) { + avifDiagnosticsPrintf(data->diag, + "Grid image tiles in the rightmost column and bottommost row do not overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2"); + return AVIF_RESULT_INVALID_IMAGE_GRID; + } + if (!avifAreGridDimensionsValid(tile->image->yuvFormat, + grid->outputWidth, + grid->outputHeight, + tile->image->width, + tile->image->height, + data->diag)) { + return AVIF_RESULT_INVALID_IMAGE_GRID; + } + dstWidth = grid->outputWidth; + dstHeight = grid->outputHeight; + } else { + // Only one tile. Width and height are inherited from the 'ispe' property of the corresponding avifDecoderItem. + dstWidth = tile->width; + dstHeight = tile->height; } avifBool alpha = (tile->input->itemCategory == AVIF_ITEM_ALPHA); @@ -1528,13 +1546,12 @@ static avifResult avifDecoderDataAllocateGridImagePlanes(avifDecoderData * data, // An alpha tile does not contain any YUV pixels. AVIF_ASSERT_OR_RETURN(tile->image->yuvFormat == AVIF_PIXEL_FORMAT_NONE); } - if (!avifAreGridDimensionsValid(tile->image->yuvFormat, grid->outputWidth, grid->outputHeight, tile->image->width, tile->image->height, data->diag)) { - return AVIF_RESULT_INVALID_IMAGE_GRID; - } + + const uint32_t dstDepth = tile->image->depth; // Lazily populate dstImage with the new frame's properties. - const avifBool dimsOrDepthIsDifferent = (dstImage->width != grid->outputWidth) || (dstImage->height != grid->outputHeight) || - (dstImage->depth != tile->image->depth); + const avifBool dimsOrDepthIsDifferent = (dstImage->width != dstWidth) || (dstImage->height != dstHeight) || + (dstImage->depth != dstDepth); const avifBool yuvFormatIsDifferent = !alpha && (dstImage->yuvFormat != tile->image->yuvFormat); if (dimsOrDepthIsDifferent || yuvFormatIsDifferent) { if (alpha) { @@ -1545,9 +1562,9 @@ static avifResult avifDecoderDataAllocateGridImagePlanes(avifDecoderData * data, if (dimsOrDepthIsDifferent) { avifImageFreePlanes(dstImage, AVIF_PLANES_ALL); - dstImage->width = grid->outputWidth; - dstImage->height = grid->outputHeight; - dstImage->depth = tile->image->depth; + dstImage->width = dstWidth; + dstImage->height = dstHeight; + dstImage->depth = dstDepth; } if (yuvFormatIsDifferent) { avifImageFreePlanes(dstImage, AVIF_PLANES_YUV); @@ -1571,15 +1588,14 @@ static avifResult avifDecoderDataAllocateGridImagePlanes(avifDecoderData * data, return AVIF_RESULT_OK; } -// After verifying that the relevant properties of the tile match those of the first tile, copies over the pixels from the tile -// into dstImage. +// Copies over the pixels from the tile into dstImage. +// Verifies that the relevant properties of the tile match those of the first tile in case of a grid. static avifResult avifDecoderDataCopyTileToImage(avifDecoderData * data, const avifTileInfo * info, avifImage * dstImage, const avifTile * tile, unsigned int tileIndex) { - const avifImageGrid * grid = &info->grid; const avifTile * firstTile = &data->tiles.tile[info->firstTileIndex]; if (tile != firstTile) { // Check for tile consistency. All tiles in a grid image should match the first tile in the properties checked below. @@ -1593,30 +1609,25 @@ static avifResult avifDecoderDataCopyTileToImage(avifDecoderData * data, } } - unsigned int rowIndex = tileIndex / info->grid.columns; - unsigned int colIndex = tileIndex % info->grid.columns; avifImage srcView; avifImageSetDefaults(&srcView); avifImage dstView; avifImageSetDefaults(&dstView); - avifCropRect dstViewRect = { - firstTile->image->width * colIndex, firstTile->image->height * rowIndex, firstTile->image->width, firstTile->image->height - }; - if (dstViewRect.x + dstViewRect.width > grid->outputWidth) { - dstViewRect.width = grid->outputWidth - dstViewRect.x; - } - if (dstViewRect.y + dstViewRect.height > grid->outputHeight) { - dstViewRect.height = grid->outputHeight - dstViewRect.y; + avifCropRect dstViewRect = { 0, 0, firstTile->image->width, firstTile->image->height }; + if (info->grid.columns > 0 && info->grid.rows > 0) { + unsigned int rowIndex = tileIndex / info->grid.columns; + unsigned int colIndex = tileIndex % info->grid.columns; + dstViewRect.x = firstTile->image->width * colIndex; + dstViewRect.y = firstTile->image->height * rowIndex; + if (dstViewRect.x + dstViewRect.width > info->grid.outputWidth) { + dstViewRect.width = info->grid.outputWidth - dstViewRect.x; + } + if (dstViewRect.y + dstViewRect.height > info->grid.outputHeight) { + dstViewRect.height = info->grid.outputHeight - dstViewRect.y; + } } const avifCropRect srcViewRect = { 0, 0, dstViewRect.width, dstViewRect.height }; - avifImage * dst = dstImage; -#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) - if (tile->input->itemCategory == AVIF_ITEM_GAIN_MAP) { - AVIF_ASSERT_OR_RETURN(dst->gainMap && dst->gainMap->image); - dst = dst->gainMap->image; - } -#endif - AVIF_ASSERT_OR_RETURN(avifImageSetViewRect(&dstView, dst, &dstViewRect) == AVIF_RESULT_OK && + AVIF_ASSERT_OR_RETURN(avifImageSetViewRect(&dstView, dstImage, &dstViewRect) == AVIF_RESULT_OK && avifImageSetViewRect(&srcView, tile->image, &srcViewRect) == AVIF_RESULT_OK); avifImageCopySamples(&dstView, &srcView, (tile->input->itemCategory == AVIF_ITEM_ALPHA) ? AVIF_PLANES_A : AVIF_PLANES_YUV); @@ -5251,18 +5262,21 @@ static avifResult avifDecoderDecodeTiles(avifDecoder * decoder, uint32_t nextIma ++info->decodedTileCount; - if ((info->grid.rows > 0) && (info->grid.columns > 0)) { - if (tileIndex == 0) { - avifImage * dstImage = decoder->image; + const avifBool isGrid = (info->grid.rows > 0) && (info->grid.columns > 0); + avifBool stealPlanes = !isGrid; + + if (!stealPlanes) { + avifImage * dstImage = decoder->image; #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) - if (tile->input->itemCategory == AVIF_ITEM_GAIN_MAP) { - AVIF_ASSERT_OR_RETURN(dstImage->gainMap && dstImage->gainMap->image); - dstImage = dstImage->gainMap->image; - } + if (tile->input->itemCategory == AVIF_ITEM_GAIN_MAP) { + AVIF_ASSERT_OR_RETURN(dstImage->gainMap && dstImage->gainMap->image); + dstImage = dstImage->gainMap->image; + } #endif - AVIF_CHECKRES(avifDecoderDataAllocateGridImagePlanes(decoder->data, info, dstImage)); + if (tileIndex == 0) { + AVIF_CHECKRES(avifDecoderDataAllocateImagePlanes(decoder->data, info, dstImage)); } - AVIF_CHECKRES(avifDecoderDataCopyTileToImage(decoder->data, info, decoder->image, tile, tileIndex)); + AVIF_CHECKRES(avifDecoderDataCopyTileToImage(decoder->data, info, dstImage, tile, tileIndex)); } else { // Non-grid path. Just steal the planes from the only "tile". AVIF_ASSERT_OR_RETURN(info->tileCount == 1); diff --git a/src/write.c b/src/write.c index 9cbe1f8d0b..8b052b99ab 100644 --- a/src/write.c +++ b/src/write.c @@ -1195,19 +1195,6 @@ static avifResult avifGetErrorForItemCategory(avifItemCategory itemCategory) return (itemCategory == AVIF_ITEM_ALPHA) ? AVIF_RESULT_ENCODE_ALPHA_FAILED : AVIF_RESULT_ENCODE_COLOR_FAILED; } -static avifResult avifValidateImageBasicProperties(const avifImage * avifImage) -{ - if ((avifImage->depth != 8) && (avifImage->depth != 10) && (avifImage->depth != 12)) { - return AVIF_RESULT_UNSUPPORTED_DEPTH; - } - - if (avifImage->yuvFormat == AVIF_PIXEL_FORMAT_NONE) { - return AVIF_RESULT_NO_YUV_FORMAT_SELECTED; - } - - return AVIF_RESULT_OK; -} - static uint32_t avifGridWidth(uint32_t gridCols, const avifImage * firstCell, const avifImage * bottomRightCell) { return (gridCols - 1) * firstCell->width + bottomRightCell->width; @@ -1326,7 +1313,8 @@ static avifResult avifEncoderAddImageInternal(avifEncoder * encoder, const avifImage * firstCell = cellImages[0]; const avifImage * bottomRightCell = cellImages[cellCount - 1]; - AVIF_CHECKRES(avifValidateImageBasicProperties(firstCell)); + AVIF_CHECKERR(firstCell->depth == 8 || firstCell->depth == 10 || firstCell->depth == 12, AVIF_RESULT_UNSUPPORTED_DEPTH); + AVIF_CHECKERR(firstCell->yuvFormat != AVIF_PIXEL_FORMAT_NONE, AVIF_RESULT_NO_YUV_FORMAT_SELECTED); if (!firstCell->width || !firstCell->height || !bottomRightCell->width || !bottomRightCell->height) { return AVIF_RESULT_NO_CONTENT; } @@ -1388,7 +1376,10 @@ static avifResult avifEncoderAddImageInternal(avifEncoder * encoder, } if (hasGainMap) { - AVIF_CHECKRES(avifValidateImageBasicProperties(firstCell->gainMap->image)); + AVIF_CHECKERR(firstCell->gainMap->image->depth == 8 || firstCell->gainMap->image->depth == 10 || + firstCell->gainMap->image->depth == 12, + AVIF_RESULT_UNSUPPORTED_DEPTH); + AVIF_CHECKERR(firstCell->gainMap->image->yuvFormat != AVIF_PIXEL_FORMAT_NONE, AVIF_RESULT_NO_YUV_FORMAT_SELECTED); AVIF_CHECKRES(avifValidateGrid(gridCols, gridRows, cellImages, /*validateGainMap=*/AVIF_TRUE, &encoder->diag)); if (firstCell->gainMap->image->colorPrimaries != AVIF_COLOR_PRIMARIES_UNSPECIFIED || firstCell->gainMap->image->transferCharacteristics != AVIF_TRANSFER_CHARACTERISTICS_UNSPECIFIED) { @@ -1543,6 +1534,7 @@ static avifResult avifEncoderAddImageInternal(avifEncoder * encoder, toneMappedItem->itemCategory = AVIF_ITEM_COLOR; uint16_t toneMappedItemID = toneMappedItem->id; + AVIF_ASSERT_OR_RETURN(encoder->data->alternativeItemIDs.count == 0); uint16_t * alternativeItemID = (uint16_t *)avifArrayPush(&encoder->data->alternativeItemIDs); AVIF_CHECKERR(alternativeItemID != NULL, AVIF_RESULT_OUT_OF_MEMORY); *alternativeItemID = toneMappedItemID; @@ -1642,7 +1634,9 @@ static avifResult avifEncoderAddImageInternal(avifEncoder * encoder, avifEncoderItem * item = &encoder->data->items.item[itemIndex]; if (item->codec) { const avifImage * cellImage = cellImages[item->cellIndex]; + avifImage * cellImagePlaceholder = NULL; // May be used as a temporary, modified cellImage. Left as NULL otherwise. const avifImage * firstCellImage = firstCell; + #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) if (item->itemCategory == AVIF_ITEM_GAIN_MAP) { AVIF_ASSERT_OR_RETURN(cellImage->gainMap && cellImage->gainMap->image); @@ -1651,16 +1645,18 @@ static avifResult avifEncoderAddImageInternal(avifEncoder * encoder, firstCellImage = firstCell->gainMap->image; } #endif - avifImage * paddedCellImage = NULL; + if ((cellImage->width != firstCellImage->width) || (cellImage->height != firstCellImage->height)) { - paddedCellImage = avifImageCreateEmpty(); - AVIF_CHECKERR(paddedCellImage, AVIF_RESULT_OUT_OF_MEMORY); - const avifResult result = avifImageCopyAndPad(paddedCellImage, cellImage, firstCellImage->width, firstCellImage->height); + // Pad the right-most and/or bottom-most tiles so that all tiles share the same dimensions. + cellImagePlaceholder = avifImageCreateEmpty(); + AVIF_CHECKERR(cellImagePlaceholder, AVIF_RESULT_OUT_OF_MEMORY); + const avifResult result = + avifImageCopyAndPad(cellImagePlaceholder, cellImage, firstCellImage->width, firstCellImage->height); if (result != AVIF_RESULT_OK) { - avifImageDestroy(paddedCellImage); + avifImageDestroy(cellImagePlaceholder); return result; } - cellImage = paddedCellImage; + cellImage = cellImagePlaceholder; } const int quantizer = (item->itemCategory == AVIF_ITEM_ALPHA) ? encoder->data->quantizerAlpha #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) @@ -1681,15 +1677,13 @@ static avifResult avifEncoderAddImageInternal(avifEncoder * encoder, /*disableLaggedOutput=*/encoder->data->alphaPresent, addImageFlags, item->encodeOutput); - if (paddedCellImage) { - avifImageDestroy(paddedCellImage); + if (cellImagePlaceholder) { + avifImageDestroy(cellImagePlaceholder); } if (encodeResult == AVIF_RESULT_UNKNOWN_ERROR) { encodeResult = avifGetErrorForItemCategory(item->itemCategory); } - if (encodeResult != AVIF_RESULT_OK) { - return encodeResult; - } + AVIF_CHECKRES(encodeResult); if (itemIndex == 0 && avifEncoderDataShouldForceKeyframeForAlpha(encoder->data, item, addImageFlags)) { addImageFlags |= AVIF_ADD_IMAGE_FLAG_FORCE_KEYFRAME; } From 37762d7e521ed12afd5696c36c0c421237d102e0 Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Thu, 21 Mar 2024 10:54:16 +0100 Subject: [PATCH 2/2] Apply nits --- src/read.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/read.c b/src/read.c index 02e86331e8..86fa61aeec 100644 --- a/src/read.c +++ b/src/read.c @@ -1507,7 +1507,7 @@ static avifResult avifDecoderDataAllocateImagePlanes(avifDecoderData * data, con uint32_t dstWidth; uint32_t dstHeight; - if (info->grid.columns > 0 && info->grid.rows > 0) { + if (info->grid.rows > 0 && info->grid.columns > 0) { const avifImageGrid * grid = &info->grid; // Validate grid image size and tile size. // @@ -1614,7 +1614,7 @@ static avifResult avifDecoderDataCopyTileToImage(avifDecoderData * data, avifImage dstView; avifImageSetDefaults(&dstView); avifCropRect dstViewRect = { 0, 0, firstTile->image->width, firstTile->image->height }; - if (info->grid.columns > 0 && info->grid.rows > 0) { + if (info->grid.rows > 0 && info->grid.columns > 0) { unsigned int rowIndex = tileIndex / info->grid.columns; unsigned int colIndex = tileIndex % info->grid.columns; dstViewRect.x = firstTile->image->width * colIndex; @@ -5263,7 +5263,7 @@ static avifResult avifDecoderDecodeTiles(avifDecoder * decoder, uint32_t nextIma ++info->decodedTileCount; const avifBool isGrid = (info->grid.rows > 0) && (info->grid.columns > 0); - avifBool stealPlanes = !isGrid; + const avifBool stealPlanes = !isGrid; if (!stealPlanes) { avifImage * dstImage = decoder->image; @@ -5278,7 +5278,6 @@ static avifResult avifDecoderDecodeTiles(avifDecoder * decoder, uint32_t nextIma } AVIF_CHECKRES(avifDecoderDataCopyTileToImage(decoder->data, info, dstImage, tile, tileIndex)); } else { - // Non-grid path. Just steal the planes from the only "tile". AVIF_ASSERT_OR_RETURN(info->tileCount == 1); AVIF_ASSERT_OR_RETURN(tileIndex == 0); avifImage * src = tile->image;