Ticket #2039 (closed defect: fixed)

Opened 8 months ago

Last modified 5 months ago

RawRasterBand::IWriteBlock not testing for complex

Reported by: JaredRubinAZ Assigned to: mloskot
Priority: normal Milestone: 1.5.1
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords: VRT raw
Cc: warmerdam, rouault

Description (Last modified by warmerdam)

In RawRasterBand::IWriteBlock there is a test for complex data type when copying into the buffer, but no test for complex data at the end of the method when reaccessing the buffer. This looks like a simple fix...unless I don't understand the purpose of rawdataset.cpp

/* -------------------------------------------------------------------- */
/*      Byte swap (if necessary) back into disk order before writing.   */
/* -------------------------------------------------------------------- */
    if( !bNativeOrder && eDataType != GDT_Byte )
    {
        if( GDALDataTypeIsComplex( eDataType ) )
        {

...
...
...



/* -------------------------------------------------------------------- */
/*      Byte swap (if necessary) back into machine order so the         */
/*      buffer is still usable for reading purposes.                    */
/* -------------------------------------------------------------------- */
    if( !bNativeOrder && eDataType != GDT_Byte )
    {

so need to added the following:

       if( GDALDataTypeIsComplex( eDataType ) )
        {
            int nWordSize;

            nWordSize = GDALGetDataTypeSize(eDataType)/16;
            GDALSwapWords( pLineBuffer, nWordSize, nBlockXSize, nPixelOffset );
            GDALSwapWords( ((GByte *) pLineBuffer)+nWordSize, 
                           nWordSize, nBlockXSize, nPixelOffset );
        }

Attachments

vrtrawlink.py (7.4 kB) - added by rouault on 12/02/07 15:21:45.
Modified version with vrtrawlink_5 added
cfloat32.raw (16 bytes) - added by rouault on 12/02/07 15:22:17.
New data file for vrtrawlink_5

Change History

11/30/07 11:10:25 changed by warmerdam

  • description changed.
  • cc set to warmerdam.
  • component changed from default to GDAL_Raster.
  • priority changed from highest to normal.
  • owner changed from warmerdam to mloskot.
  • milestone set to 1.5.0.
  • keywords set to VRT raw.

Mateusz,

Could you address this? We will need to add a test of raw complex files (using the VRT "raw dataset" support would likely be best) for the test suite to know if there is future reversion in handling of complex raw files.

12/02/07 15:20:54 changed by rouault

I've commited the fix in r13200.

I'm attaching a version of rawdatalink.py with a new test (vrtrawlink_5) and a new data file (gdrivers/data/cfloat32.raw) that enable to test the bug. The bug is very difficult to hit indeed. The principle of this test is that : 1) we write a block 2) we exhaust the block cache by writing a huge amount of the data 3) we reread the block written at step 1) by computing its checksum. In fact, it's not reread from disk, but from the temporary buffer maintained by the rawrasterband.

As it, this test is probably not appropriate for autotest, but it demonstrates the correctness of the fix.

12/02/07 15:21:45 changed by rouault

  • attachment vrtrawlink.py added.

Modified version with vrtrawlink_5 added

12/02/07 15:22:17 changed by rouault

  • attachment cfloat32.raw added.

New data file for vrtrawlink_5

12/02/07 15:31:37 changed by rouault

Oh, "of course", if you run vrtrawlink_5 on a big-endian machine, it will not hit the bug. You'd have to change line 162.

12/02/07 15:58:04 changed by rouault

  • cc changed from warmerdam to warmerdam, rouault.

12/19/07 06:23:26 changed by mloskot

If I understand the status correctly, Even has already fixed (Thanks Even!) this bug and committed fix to trunk (r13200). Looks like Even has taken over and cleaned this issue, so I'm not sure what I could add to it. Is it ready to close?

12/20/07 09:17:31 changed by mloskot

  • status changed from new to assigned.

Honestly, I have no idea on how to test this issue well. Obviously, Even's vrtrawlink_5 test does not come in handy for autotest, but as Even explained on the #gdal:

a simple test for complex data could be fine, but will not propably test what was the bug. I've run out of concepts about how to test this issue it properly.

Any comments would be helpful.

03/04/08 16:19:41 changed by warmerdam

  • status changed from assigned to closed.
  • resolution set to fixed.

Ok, we will skip preparing a regression test for this.