Opened 11 years ago

Closed 9 years ago

#5018 closed enhancement (fixed)

DIMAP driver enhancements

Reported by: mats Owned by: warmerdam
Priority: normal Milestone: 1.11.2
Component: GDAL_Raster Version: 1.9.2
Severity: normal Keywords: DIMAP
Cc:

Description

Attached patch adds some enhancements to the DIMAP driver:

  1. Solves the issue when opening the DIMAP product metadata files, instead of the DIMAP delivery index file, as reported in ticket #4826 (DIMAP driver fails to open Pleiades HR datasets).

When opening the DIMAP delivery index file, only metadata from the first dataset in the the delivery was read, and opening the DIMAP product metadata files failed (as reported in defect #4826).

  1. Read geotransform from underlying raster if not available in metadata, just like it's done with spatial reference.
  2. Up to now it has been possible to access various DIMAP metadata specified in hardcoded translation tables with the DIMAPDataset::GetMetadata method.

The patch adds the ability to access additional metadata by using metadata translation tables specified in csv files. The files should be placed in the GDAL data directory (specified by the GDAL_DATA configuration option).
Example files with metadata translation tables for the DIMAP product metadata file and the DIMAP strip metadata file are attached:
dimap_metadata_translation_dim.csv and dimap_metadata_translation_strip.csv.

Attachments (4)

dimap_metadata_translation_dim.csv (672 bytes ) - added by mats 11 years ago.
dimap_metadata_translation_strip.csv (495 bytes ) - added by mats 11 years ago.
dimapdataset.cpp.patch (5.1 KB ) - added by mats 11 years ago.
frmt_various.html.patch (2.1 KB ) - added by mats 11 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 by warmerdam, 11 years ago

I'm getting lots of failures trying to apply this in trunk:

patch --dry-run < ~/Downloads/dimapdataset.cpp.patch 
(Stripping trailing CRs from patch.)
patching file dimapdataset.cpp
Hunk #1 FAILED at 33.
Hunk #2 FAILED at 78.
Hunk #6 FAILED at 293.
Hunk #7 succeeded at 360 (offset 11 lines).
Hunk #8 succeeded at 378 (offset 11 lines).
Hunk #9 succeeded at 387 (offset 11 lines).
Hunk #10 succeeded at 397 (offset 11 lines).
Hunk #11 succeeded at 513 (offset 11 lines).
Hunk #12 succeeded at 597 (offset 11 lines).
Hunk #13 succeeded at 611 (offset 11 lines).
Hunk #14 succeeded at 630 (offset 11 lines).
Hunk #15 succeeded at 780 (offset 11 lines).
Hunk #16 succeeded at 805 (offset 11 lines).
Hunk #17 succeeded at 857 (offset 11 lines).
Hunk #18 FAILED at 897.
Hunk #19 FAILED at 915.
Hunk #20 succeeded at 940 (offset 11 lines).
Hunk #21 succeeded at 948 (offset 11 lines).
Hunk #22 succeeded at 1060 (offset 11 lines).
Hunk #23 succeeded at 1184 (offset 11 lines).
5 out of 23 hunks FAILED -- saving rejects to file dimapdataset.cpp.rej

Are you sure the patch was prepared against trunk? It also seems to have a lot of white space changes that make it hard to evaluate visually.

The provided CSV files should presumably live in gdal/data, right? Could you elaborate on their purpose? It seems like an override mechanism for people to fix up particular metadata formulations but without more detail it will be of limited value. Ideally the user docs should also be patched.

by mats, 11 years ago

Attachment: dimapdataset.cpp.patch added

in reply to:  1 comment:2 by mats, 11 years ago

Sorry about the problems with the previous path. I have prepared a new patch against trunk to replace the first one. It must have been the tabs and indentation adjustments that messed up the previous path. Since it's my first contribution I thought I'd be good and replaced all tabs with blanks as suggested in the developer guidelines (RFC8).
Regarding the CSV files, after we discovered that some of the DIMAP metadata we wanted (sun azimuth etc) wasn't returned by the driver's GetMetadata method, I noticed that just the metadata defined in the hardcoded metadata translation tables in the source code was returned.
I thought this solution was a (dynamic) way for the users to specify what metadata they wanted to access from the DIMAP dim and strip files. An alternative would have been to use the parameter "xml:dimap" to request the xml code, parse it, and extract the desired values.
My solution was just to make it possible for the users to manipulate which information that would be accessible from the DIMAP xml files, since the metadata translation mechanism was already there.
Regarding the user docs, what docs should be patched?

comment:3 by warmerdam, 11 years ago

There is quite brief DIMAP documentation in gdal/frmts/frmts_various.html that should be extended with any notes about configuration files.

I'm a bit dubious about the .csv file approach. Is it that the items available varies in unpredictable ways for different products?

I do like the idea of exposing the full xml in a special subdomain though ideally users shouldn't need to dig into that in common cases.

comment:4 by mats, 11 years ago

Still a bit dubious about the CSV file approach? We have an application that needs certain metadata from satellite imagery from different sources. For example sun azimuth. When using the GDAL DIMAP driver we saw no easy way to aquire that information, the only available alternative seemed to be to parse the whole dim-file XML. But there were certain accessible metadata, not the one we needed though. So, what metadata was returned? The one specified in some hardcoded translation tables within the DIMAP driver. In our first approach we just expanded the tables with the items we needed and recompiled. Worked just as expected, we got what we wanted. But if our customers demanded other items in coming releases? That's why we put the metadata translation tables in CSV files. To meet future demands in new releases of our software. We left the original metadata translation tables in the driver though. So, as I tried to explain in one of my previous comments, the CSV file approach is a way to determine what metadata should be available to us when building our application whithout being forced to rebuild the GDAL lib as well.

by mats, 11 years ago

Attachment: frmt_various.html.patch added

comment:5 by mats, 11 years ago

Patched the user docs as requested with information about the CSV configuration files.

comment:6 by Even Rouault, 10 years ago

trunk r27221, branches/1.11 r27222 : "DIMAP 2: handle the case where the Raster_Data element is in main file; fix to extract geodetic SRS; fix to extract geotransform from JPEG2000 file if not available in XML (adapted from patch by mats, #5018, #4826)"

comment:7 by Jukka Rahkonen, 9 years ago

Did r27221 and r27222 make this ticket ready for closing?

comment:8 by Jukka Rahkonen, 9 years ago

Milestone: 1.11.2
Resolution: fixed
Status: newclosed

I suppose this issue was resolved by r27221 / r27222.

Note: See TracTickets for help on using tickets.