[gdal-dev] How to consider data types when implementing IWriteBlock

[Even Rouault]

Le lundi 18 avril 2016 10:51:06, jramm a écrit :
> Hi
> I have implemented the write support in the NWT_GRD format, having to make
> a few compromises. I'd like to go through them and see if they are
> reasonable. I have also attached the diff.
> 
> 1. The existing read driver reports 4 bands for a GRD dataset. The first 3
> are essentially 'virtual' - they are RGB bands which are interpreted from
> the single band of data actually on disk using color info in the header.
> When opening in write mode, I have prevented more than 1 band being
> created. Therefore in update mode, you cannot read from the 'virtual'
> bands; it is created with 1 band to which you write the actual data. So
> you have some slightly odd behaviour in that, you open it in `update`
> mode, it has 1 band, you then close and reopen in `read only` mode, it has
> 4 bands. I wonder if it would be better for the read driver to utilize the
> GDAL ColorTable stuff somehow, instead of creating these 'virtual' bands?

GDALColorTable can only be used for index values >= 0, so this is not suitable 
for DEM values.
Perhaps an alternate option would be to expose the color table as a Raster 
Attribute Table, but this is generally less easy to handle (rarely supported 
in user code or GDAL utilities themselves)

I've no strong opinion on if the current behaviour in read-only mode must be 
changed or not. One could imagine an open option to decide it...

> 
> 2. The no data value is fixed on disk as 0; this is then converted to
> -1.e37f by the read driver (as per the vertical mapper grid standard) upon
> reading. On writing, I have allowed setting the no data value to anything,
> and these values are treated as no data when writing. So you may create a
> dataset having set the no data value to e.g. -999. However, when you close
> it and open it in 'read' mode, the nodata value will be reported as -1.e37f

Sounds good.

> 
> 3. The GRD format supports data being written on disk as either 16bit
> unsigned ints or 32bit unsigned ints, In the NWT_GRDRasterBand constructor,
> allowance is made for this, however in the read driver, values are always
> read as unsigned short (suggesting that if data were 32bit ints, we might
> encounter over/underflow).

I guess read support is wrong if indeed values might be serialized as 32bit 
unsigned ints, and 32bit samples should be read in that case... (I don't know 
the format, but that makes sense). I guess collecting a sample dataset might 
be helpful.

> I have done the same in the IWriteBlock, but I
> am wondering if their should be a check to see whether the written value
> should be an `unsigned short` or an `unsigned int`

Skimming through the code, it seems you must provide zmin / zmax to do the 
scaling of floating point elevations to integral values. So either it is user 
provided, or you do a prior statistics computation in CreateCopy() if the 
input dataset hasn't min/max metadata.
In any case, checking overflow and warning if it occurs might be a good idea.

> 
> 4. Spatial Reference. The spatial reference is stored on disk in the
> 'MITAB' format and a conversion function is used in the existing
> GetProjectionRef. A function to convert the other way exists and I use
> this in SetProjection. However, some stuff is obviously lost in the
> conversion as setting the projection with e.g. EPSG:27700 and then reading
> it back loses the EPSG information. Is there a way (using PamDataset
> maybe?) to store the GDAL projection so that GDAL will always recognise it
> correctly, while still writing the NWT_GRD format projection in the
> correct place in the tab/grd file?

Yes using PAM might be an option. Basically our SetProjection() should also 
call GDALPamDataset::SetProjection(). And on reading you first check if 
GDALPamDataset::GetProjectionRef() returns a non-empty string, in which case 
you return it. Otherwise you return the MITAB decoded one.

> 
> Are these reasonable compromises or can it be done better?
> 
> Thanks
> James
> 
> 
> 
> --
> View this message in context:
> http://osgeo-org.1560.x6.nabble.com/How-to-consider-data-types-when-implem
> enting-IWriteBlock-tp5260901p5261675.html Sent from the GDAL - Dev mailing
> list archive at Nabble.com.
> _______________________________________________
> gdal-dev mailing list
> gdal-dev at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com