Opened 17 years ago

Closed 17 years ago

#1716 closed defect (fixed)

[PATCH] Add more checks to DTED files reading (checksum, horizontal datum)

Reported by: Even Rouault Owned by: warmerdam
Priority: normal Milestone: 1.5.0
Component: GDAL_Raster Version: 1.4.2
Severity: normal Keywords: dted
Cc:

Description

Below a summary of the new functionnalities, copied from the updated version of frmt_dted.html (please review for the correctness of the English, I'm not a native English writer):


Read Issues

If you do not specify any of the following environment variables, you will obtain the same behaviour as GDAL versions prior to 1.4.3.

Georeferencing Issues

The DTED specification (MIL-PRF-89020B) states that horizontal datum shall be the World Geodetic System (WGS 84). However, there are still people using old data files georeferenced in WGS 72. A header field indicates the horizontal datum code, so we can detect and handle this situation. But, you have also other situations where the real geodetic system used for the data grid is WGS 84, but the header indicates WGS72... You may use the following environment variable :

  • DTED_ALWAYS_WGS84 (YES/NO, default is YES) : if YES, the horizontal datum header field is simply ignored, and WGS84 is always used. If NO, the horizontal datum header is read and taken into account (only WGS 84 and WGS 72 are supported, other horizontal datum will default to WGS 84).

Checksum Issues

The default behaviour of the DTED driver is to ignore the checksum while reading data from the files. However, you may use the following environment variables if you want to specify it more precisely :

  • DTED_VERIFY_CHECKSUM (YES/NO, default is NO): the checksum will be computed from the read data and compared with the checksum written in the file. This option must be turned on for the following options to be taken into account. Otherwise, the data is considered as good in any case.
  • DTED_IGNORE_IMPOSSIBLE_CHECKSUM (YES/NO, default is NO) : in some cases, the data producer has not written in the file a sensible checksum value (for example 0xFFFFFFF). If you turn on this option, you agree that the checksum must be ignored and assume the data to be correct.
  • DTED_BEHAVIOUR_WHEN_BAD_CHECKSUM (FAIL/IGNORE/FILL_WITH_NODATA, default is FAIL).
    • FAIL : read is stopped as soon as a checksum mismatch is detected.
    • IGNORE : a warning is issued, but the read will go on.
    • FILL_WITH_NODATA : the whole column where a checksum mismatch occurs is filled with the NODATA value (-32767).

Additionnal notes :

  • in the string returned by GetProjectionRef(), I've also included the AUTHORITY value for the GEOGCS (eg 4326 for WGS84, 4322 for WGS72).
  • I've added a new metadata 'DTED_HorizontalDatum'. In the DTEDMetaDataCode enum, I've added it at the end, in case we must preserve binary compatibility for dted_api.h/c.

Attachments (3)

gdal-1.4.2-dted.patch (12.8 KB ) - added by Even Rouault 17 years ago.
gdal-1.4.2-dted-v2.patch (15.0 KB ) - added by Even Rouault 17 years ago.
gdal-1.4.2-dted-v3.patch (15.0 KB ) - added by Even Rouault 17 years ago.

Download all attachments as: .zip

Change History (8)

by Even Rouault, 17 years ago

Attachment: gdal-1.4.2-dted.patch added

comment:1 by Even Rouault, 17 years ago

Keywords: dted added

Updated version of the patch. The changes from first version are in DTEDCreateCopy :

  • enforces bStrict flag when the cell size is not a standard DTED cell size
  • enforces input SRS to be WGS84, unless bStrict == FALSE

by Even Rouault, 17 years ago

Attachment: gdal-1.4.2-dted-v2.patch added

comment:2 by Even Rouault, 17 years ago

Oops, default value for DTED_VERIFY_CHECKSUM and DTED_IGNORE_IMPOSSIBLE_CHECKSUM was incorrectly put in the code to 'YES', instead of 'NO' as mentionned in the documentation. Patch updated.

by Even Rouault, 17 years ago

Attachment: gdal-1.4.2-dted-v3.patch added

comment:3 by warmerdam, 17 years ago

Component: defaultGDAL_Raster
Milestone: 1.5.0
Status: newassigned

I thought I had written a comment on this yesterday, but apparently not.

I like to minimize use of configuration options. I'd prefer to boil this down to just the DTED_VERIFY_CHECKSUM option.

For the WGS84/WGS72 thing, I'd like to always try and check for WGS72 and if it isn't clearly WGS72 then we report it as WGS84.

We should always ignore impossible checksums.

We should always issue an error on invalid checksums.

Would you be willing to revise the patch in this light?

comment:4 by Even Rouault, 17 years ago

For WGS84/WGS72, here are 2 'real-life' situations I've expericenced :

  • my customer has very very old DTED files that are really encoded in WGS72, so the DTED driver must advertise WGS72 in that case
  • my customer has wrongly formated DTED files that advertise WGS72, but we know that it's not the case... The data producer has done a bad job... And the DTED driver must advertise WGS84.

My use case : the application I'm working on has a DTED import function. When importing a DTED file, I check the 'DTED_HorizontalDatum' metadata. If it's WGS72, I ask the user "Are you really sure it's a WGS72 DTED file and not a wrongly encoded WGS84 DTED file ?". If the user answers it is a real WGS72 DTED file, I go on. If it's a false WGS72 DTED file, a "gdal_translate -not_strict -of DTED ori.dtX dst.dtX" fixes it and I go on.

So, you're right, we can probably get rid of the DTED_ALWAYS_WGS84 environment variable. However this is going to change the behaviour of the DTED driver in comparison to previous GDAL versions (WGS72 was handled as WGS84). Is this OK ?

As for checksums, I'm not sure we should always ignore impossible checksums. Ignoring it means that we are silently accepting the fact that we cannot check the data. In my opinion, it must be a deliberate choice of the user : "yeah, I've impossible checksums, and that's ok because I know what I'm doing. Let's go on".

And for invalid checksums, the behaviour can be discussed. If there are checksums per column, in my opinion, it means that it is possible to invalidate just this column and not the whole file.

So, here the environment variables could help me to provide the user a precise control.

comment:5 by Even Rouault, 17 years ago

Resolution: fixed
Status: assignedclosed

Frank, I finally revised the patch taking into account your remarks. It has been commited in trunk in r12225 and r12226

Note: See TracTickets for help on using tickets.