Opened 3 years ago

Closed 2 years ago

#5991 closed enhancement (fixed)

[PATCH] Adding support for ISCE image format

Reported by: mazhe Owned by: warmerdam
Priority: normal Milestone: 2.1.0
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords: isce driver
Cc: matthieu.volat@…

Description

Hello,

I would like to submit a new driver to support the image format used by the Jet Propulsion Laboratory's ISCE (Interferometric synthetic aperture radar Scientific Computing Environment) project.

It is the successor to the ROI_PAC project, which support was added at the beginning of the year.

The driver read compatible images, storing metadata in the ISCE domain. Consider it an initial support to be improved, as georeferencing is still absent (I have to find proper documentation).

The patch also include a minimal testsuite.

Thank you very much for your time and feedback.

Attachments (3)

gdal_autotest_isce.diff (6.6 KB) - added by mazhe 3 years ago.
Patch for the test suite.
gdal_isce.diff (32.1 KB) - added by mazhe 3 years ago.
All reviews fix integrated, add support for byte order
gdal_autotest__gdrivers__data__isce.slc (960 bytes) - added by mazhe 2 years ago.
ISCE data test file.

Download all attachments as: .zip

Change History (11)

Changed 3 years ago by mazhe

Attachment: gdal_autotest_isce.diff added

Patch for the test suite.

comment:1 Changed 3 years ago by Even Rouault

A few remarks :

  • poDriver->pfnIdentify = ISCEDataset::Identify; should be removed, since the Identify() method isn't specific enough (would identify any foo.bar accompanied with a foo.bar.xml). And the content of the Identify() method moved into Open() to avoid confusion.
  • if ( psNode == NULL || CPLGetXMLNode( psNode, "=imageFile" ) == NULL ) will leak psNode if is not NULL but there's no imageFile node
  • CPLXMLNode *papszCur : wrong use of hungarian notation. Should be psCur instead
  • papszISCE2GDALDatatypes, papszGDAL2ISCEDatatypes, papszSchemeNames should be static
  • the SCHEME creation option should be advertized in the GDAL_DMD_CREATIONOPTIONLIST driver metadata item (with a 'select-string' type for the enumerated list). See frmts/raw/envidataset.cpp

comment:2 Changed 3 years ago by mazhe

Thank you very much,

Regarding Identify(), I assumed that opening the XML and looking for "<imageFile" would be a no-go, do you agree?

I am applying the other remarks.

comment:3 Changed 3 years ago by mazhe

Cc: matthieu.volat@… added

comment:4 in reply to:  2 Changed 3 years ago by Even Rouault

Regarding Identify(), I assumed that opening the XML and looking for "<imageFile" would be a no-go, do you agree?

Well, Identify() is supposed to be super fast, so ideally we shouldn't have to open side-car files. I can see 3 possibilities :

  • make Identify() open the XML file and check for "<imageFile" in the first bytes of the file
  • keep things like they are, but no longer advertize Identify() at the driver level
  • make the driver open the XML file as the main file.

comment:5 Changed 3 years ago by mazhe

I had the same idea as no1, but I was afraid of someday finding an image with a large comment and/or whitespace padding at the beginning of the image.

I'll go with solution no2 until I can figure out something better.

Changed 3 years ago by mazhe

Attachment: gdal_isce.diff added

All reviews fix integrated, add support for byte order

comment:6 Changed 3 years ago by Even Rouault

gdal_autotest_isce.diff doesn't contain gdrivers/data/isce.slc. Should be added as attachment rather than a diff I think

Changed 2 years ago by mazhe

ISCE data test file.

comment:7 Changed 2 years ago by mazhe

Oops, sorry, here it is.

comment:8 Changed 2 years ago by Even Rouault

Milestone: 2.1.0
Resolution: fixed
Status: newclosed

trunk r29352 "Add ISCE raster driver (patch by Matthieu Volat, #5991)"

trunk r29353 "ISCE: fix Windows build"

Note: See TracTickets for help on using tickets.