Ticket #2369 (closed defect: fixed)

Opened 3 months ago

Last modified 3 months ago

short integer overflow with AAGRID when input grid contains some 32bit integers

Reported by: dylan Assigned to: rouault
Priority: normal Milestone: 1.6.0
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords: AAGRID
Cc:

Description

There appears to be a problem with GDAL utilities not recognizing the possibility that an AAGRID file may contain a mixture of 16 and 32 bit integers. This is not surprising, as the AAGRID file format does not contain any hints about its contents. A ticket was started on the GRASS tracker, but the issue appears to be a purely GDAL bug.

One idea for a fix includes an environmental flag which would cause the AAGRID driver to scan the input file in order to properly determine an appropriate data type. Another approach would be to manually set the data type for any input file from the command line.

Example using the attached file: 

gdal_translate  big_int_arc_file.asc test.tiff

the resulting file is produced: 

gdalinfo -stats test.tiff 
Driver: GTiff/GeoTIFF
Files: test.tiff
Size is 7, 7
Coordinate System is `'
Origin = (592000.000000000000000,4926000.000000000000000)
Pixel Size = (1000.000000000000000,-1000.000000000000000)
Image Structure Metadata:
  INTERLEAVE=BAND
Corner Coordinates:
Upper Left  (  592000.000, 4926000.000) 
Lower Left  (  592000.000, 4919000.000) 
Upper Right (  599000.000, 4926000.000) 
Lower Right (  599000.000, 4919000.000) 
Center      (  595500.000, 4922500.000) 
Band 1 Block=7x7 Type=Int16, ColorInterp=Gray
  Minimum=-31293.000, Maximum=32420.000, Mean=216.837, StdDev=18087.589
  NoData Value=-9999
  Metadata:
    STATISTICS_MINIMUM=-31293
    STATISTICS_MAXIMUM=32420
    STATISTICS_MEAN=216.83673469388
    STATISTICS_STDDEV=18087.589140406

Attachments

big_int_arc_file.asc (392 bytes) - added by dylan on 05/13/08 18:06:57.
an AAGRID file with a mix of 16 and 32 bit integers

Change History

05/13/08 18:06:57 changed by dylan

  • attachment big_int_arc_file.asc added.

an AAGRID file with a mix of 16 and 32 bit integers

05/13/08 18:07:40 changed by dylan

  • component changed from default to GDAL_Raster.

05/16/08 14:05:23 changed by rouault

Frank,

is there a particular reason for the AAIGRID driver to default to Int16 type and not Int32 ?

Another question, not directly related to this bug report, but in the ::CreateCopy?, I see that we do the read RasterIO on the source as GDT_CFloat64. But it seems that we only use the real part, so that looks very weird.

05/16/08 14:23:21 changed by warmerdam

Even,

The CFloat64 stuff is pretty scary, and apparently only works because we provide a "step size" that is sizeof(double) so all the imaginary values are getting written over. Please correct this, or let me know you don't plan to and I will do so. This fix ought to go into 1.5 and trunk.

The other issue is data type. The issue I have with defaulting to Int32 is that it produces output files that can't be read with most software after a translation and most grids are 8bit or 16bit values. But since there is no good way of knowing in advance what the range is (and prescanning is not an appealing option) I am ok with changing to GDT_Int32 as the default. This behavior change should only be made in trunk though.

The other possibility is that we just always treat the data as GDT_Float32, which gets rid of the other mess - trying to figure out if a file is floating point or integer. If you would like to change this, it should also only be done in trunk.

05/16/08 14:33:41 changed by dylan

Thanks for looking into this Frank and others. Forcing the read as 32bit int or 32bit float seems like a workable solution, but will this lead to inflexibility later on? In other words, for funky formats like AAGRID it would be nice to have a flag for defining the data type on the fly. Defaulting to Float32 might work well in the general case, but if the user knows that the AAGRID is only an Int16, it would be helpful to only allocate the space that is really needed.

Just an idea.

05/16/08 15:04:56 changed by warmerdam

Generally speaking I don't like users having to provide such information in order to open a dataset, though it could be accomplished via a configuration option.

Note that with gdal_translate you can convert pixel types on the fly, so if you know the data range is suitable for Int16, you could do "gdal_translate -ot Int16 in.grd out.tif" for instance.

05/16/08 16:11:52 changed by rouault

  • status changed from new to assigned.
  • owner changed from warmerdam to rouault.

05/16/08 16:17:48 changed by rouault

Fix wrong data type used to read the source band AAIGCreateCopy in trunk in r14476 and in branches/1.5 in r14477

05/16/08 16:36:03 changed by rouault

  • status changed from assigned to closed.
  • resolution set to fixed.
  • milestone set to 1.6.0.

Switch to Int32 if there are no floating point values in the first 100K chunk and in no data, otherwise choose Float32

Commited in trunk in r14478. Autotest adapted and extended in r14479

05/16/08 18:43:35 changed by dylan

The applied fix works well for the test cases I was using when I had originally created the ticket. Thanks rouault!