Opened 8 years ago

Last modified 7 years ago

#5653 closed defect

Stride calculation error on GDALRegenerateOverviewsMultiBand — at Version 1

Reported by: Lucian Plesea Owned by: warmerdam
Priority: high Milestone: 2.0.0
Component: GDAL_Raster Version: svn-trunk
Severity: major Keywords:
Cc: antonio

Description (last modified by Lucian Plesea)

GDALRegenerateOverviewsMultiBand is used when paged and compressed TIF overviews are built. The source relative stride calculation is sometimes wrong, leading to inefficient code execution and partial tiles being generated multiple tiles, particularly for large rasters, where the errors accumulate. Sometimes there are visible artifacts in the output, but not always.

This is due to the following two lines in gcore/overview.cpp, in GDALRegenerateOverviewsMultiBand(), lines 1993 and 1994 currently:

int nFullResXChunk = (nDstBlockXSize * nSrcWidth) / nDstWidth;

int nFullResYChunk = (nDstBlockYSize * nSrcHeight) / nDstHeight;

Most of the time the result is correct, nSrcWidth/nDstWidth is commonly 2/1. When nSrcWidth is odd, nDstWidth is rounded up, thus the ratio is a bit under 2. Since all variables are integers, the end result is rounded down, the result ends up being off by one. For example (512*1023)/512 results in 1023 instead of the correct 1024. The brackets makes this error less likely. It escaped detection because it is relatively infrequent and the effect is noticeable only when the sizes are large, the stepping error having time to accumulate.

The proposed fix is to use floating point and round up to an integer, which also takes care of the potential integer overflow in the original formula. This calculation is not in an inner loop, so efficiency is not an issue.

int nFullResXChunk = int( double(nDstBlockXSize) * nSrcWidth / nDstWidth + 0.5);

int nFullResYChunk = int( double(nDstBlockYSize) * nSrcHeight / nDstHeight + 0.5;

Change History (1)

comment:1 by Lucian Plesea, 8 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.