Opened 9 years ago

Closed 9 years ago

#3355 closed defect (fixed)

Fixes to variations on PDS Image pointer and implementation of LINE_PREFIX_BYTES

Reported by: gaige Owned by: warmerdam
Priority: normal Milestone: 1.7.2
Component: GDAL_Raster Version: 1.7.0
Severity: normal Keywords: PDS
Cc:

Description

This patch fixes a couple of bugs in the current PDS implementation, namely:

  • LINE_SAMPLES is used as the byte width for a row, when RECORD_BYTES is actually the byte width of a row, the LINE_SAMPLES merely describes how many data points there are on the row
  • Add support for LINE_PREFIX_BYTES, which is resident in certain test files that I have found from NASA
  • Modified support for PDS Image Pointer variations (#3177) to comply with the documentation
  • Fixed a bug that caused the pointer variation fix in 3177 not to work due to the quotation marks being removed from the line

I've enclosed the diff files against 1.6.3 and a couple of different test sets. Please feel free to contact if there are any questions.

Attachments (3)

pdsdataset.cpp-diffs (2.2 KB) - added by gaige 9 years ago.
Patches against 1.6.3
PDS-TestFiles.zip (371.0 KB) - added by gaige 9 years ago.
Image file tests
pds-diffs (3.7 KB) - added by gaige 9 years ago.
PDS1.7Diffs

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by gaige

Attachment: pdsdataset.cpp-diffs added

Patches against 1.6.3

Changed 9 years ago by gaige

Attachment: PDS-TestFiles.zip added

Image file tests

comment:1 Changed 9 years ago by warmerdam

Keywords: PDS added
Milestone: 1.6.41.7.1
Resolution: fixed
Status: newclosed

I have manually reapplied the proposed patch against trunk with some adjustments and extended the test suite to test at least some of this logic. The changes are in trunk (r18693) and 1.7 branch (r18694). Let me know if the changes ought to be backported to 1.6 branch.

comment:2 Changed 9 years ago by gaige

Resolution: fixed
Status: closedreopened

I've upgraded to 1.7 here on my test bed and a number of the changes fell out when it was pulled into 1.7 and trunk.

I'm reopening this, and I will provide diffs based on 1.7 and trunk in the near future. In particular, the <BYTES> support fell out, as did support for images which have quotation marks in their pointer specifiers.

I can provide test files for these if you like.

comment:3 Changed 9 years ago by gaige

Version: 1.6.31.7.0

Here are the diffs for 1.7. They address the following problems:

  • N/A units, which I've found in some mars PDS files
  • The 3 image pointer formats IMAGE = ("blah.img") , IMAGE= ("blah.img",1) and IMAGE = ("blah.img", 1 <BYTES> )
  • The previous problem with the use of the IMAGE decoder removing the quotes around the file name, resulting in the file name not being recognized (due to the test for osQube[0]=='"'

-Gaige

Changed 9 years ago by gaige

Attachment: pds-diffs added

PDS1.7Diffs

comment:4 Changed 9 years ago by warmerdam

Unfortunately; this patch does not apply for me in trunk or 1.7 branch:

warmerda@gdal64[53]% patch < ~/Desktop/pds-diffs 
can't find file to patch at input line 2
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|==== //import/GDAL/frmts/pds/pdsdataset.cpp#5 (text) - //depot/GDAL/frmts/pds/p
dsdataset.cpp#8 (text) ==== content
--------------------------
File to patch: pdsdataset.cpp
patching file pdsdataset.cpp
Hunk #3 succeeded at 604 (offset 3 lines).
Hunk #4 FAILED at 610.
Hunk #5 succeeded at 632 (offset 3 lines).
Hunk #6 succeeded at 698 (offset 11 lines).
Hunk #7 FAILED at 715.
Hunk #8 FAILED at 815.
Hunk #9 FAILED at 821.
Hunk #10 FAILED at 827.
Hunk #11 succeeded at 848 (offset 16 lines).
5 out of 11 hunks FAILED -- saving rejects to file pdsdataset.cpp.rej

I've skimmed the patch, but it lacks sufficient context to properly hand apply. I don't really see how the current code could not be working for BYTES. I haven't review the other aspects closely.

comment:5 Changed 9 years ago by gaige

Resolution: fixed
Status: reopenedclosed

My apologies for the confusion. This was reopened in error due to 1.7.0 vs 1.7.1 introduction of these changes.

I've closed it as 1.7.1 appears just fine, thank you.

-Gaige

Note: See TracTickets for help on using tickets.