[gdal-dev] proposal for a new GDAL/OGR utility app: gdalsrsinfo

[Even Rouault]

Selon Etienne Tourigny <etourigny.dev at gmail.com>:

> I've committed a slightly modified version to trunk.
>
> I'll also add sometime soon a -srsformat option to gdalinfo with
> similar behaviour.
>
> I'll wait a bit before adding to the 1.8 branch
>
> 	M	gdal/apps/GNUmakefile
> 	M	gdal/apps/gdal_utilities.dox
> 	A	gdal/apps/gdalsrsinfo.c
>
>

Hi Etienne,

A few remarks :

1) apps/makefile.vc also needs to be updated

2) We usually don't commit new features/utilities in stable branches.

3) I think I know why you had to comment /* if ( hOGRDS )  OGR_DS_Destroy(
hOGRDS ); */ at the end. The issue is that the hSRS you got with hSRS =
OGR_L_GetSpatialRef( hLayer ); is managed by the datasource object. So you must
not free it explicitely : it will be freed when you destroy the datasource
object. An alternative would be to clone the SRS you get with
OGR_L_GetSpatialRef() to be able to close the datasource early.

4) Your test to know if the input is a filename that can be opened by GDALOpen()
/ OGROpen() uses file = fopen(pszInput, "r"); . But a lot of GDAL and OGR
drivers can now open VSI virtual files. So I'd suggest using VSIFOpenL() /
VSIFCloseL() instead.

5) The various OSRExportToXXX() allocated a new pszOutput that should be freed.

6) The OSRMorphToESRI( hSRS ); will alter the SRS object. So all code below will
be run on a ESRI'fied SRS. You probably need to clone the SRS before morphing to
ESRI, so it can be used specifically by that part of the code.

7) nTmp = OGR_L_GetFeatureCount(hLayer,TRUE); is dead code

8) I'd encourage you to extent autotest/utilities to test the new utility.

9) I'd suggest doing the cleanup for GDALAllRegister() and OGRRegisterAll()
at the end of the file and not in the middle, to avoid errors, and for nice
visual pairing.

Best regards,

Even


>
> On Sun, Oct 16, 2011 at 11:06 PM, Frank Warmerdam <warmerdam at pobox.com>
> wrote:
> > On Sun, Oct 16, 2011 at 6:02 PM, Etienne Tourigny
> > <etourigny.dev at gmail.com> wrote:
> >> I'll look into testepsg, I thought it was only for EPSG codes (guess I
> >> was wrong). I also overlooked it because it is not in the GDAL
> >> utilities page.
> >
> > Etienne,
> >
> > The most accurate part of the utilities name is "test".  It is not
> > in the utilities page because it is not a supported utility.  It
> > was really just for me to do testing.
> >
> >> Perhaps I can combine testepsg with gdalsrsinfo ?
> >
> > I would imagine gdalsrsinfo superceding testepsg.
> >
> > The gdaltransform utility already does the transformation work
> > done by testepsg.
> >
> > I'm ok with you adding gdalsrsinfo as a supported utility.
> >
> > Best regards,
> > --
> >
> ---------------------------------------+--------------------------------------
> > I set the clouds in motion - turn up   | Frank Warmerdam,
> warmerdam at pobox.com
> > light and sound - activate the windows | http://pobox.com/~warmerdam
> > and watch the world go round - Rush    | Geospatial Software Developer
> >
> _______________________________________________
> gdal-dev mailing list
> gdal-dev at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>