Opened 16 years ago
Closed 16 years ago
#2039 closed defect (fixed)
RawRasterBand::IWriteBlock not testing for complex
Reported by: | JaredRubinAZ | Owned by: | Mateusz Łoskot |
---|---|---|---|
Priority: | normal | Milestone: | 1.5.1 |
Component: | GDAL_Raster | Version: | unspecified |
Severity: | normal | Keywords: | VRT raw |
Cc: | warmerdam, Even Rouault |
Description (last modified by )
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 (2)
Change History (9)
comment:1 by , 16 years ago
Cc: | added |
---|---|
Component: | default → GDAL_Raster |
Description: | modified (diff) |
Keywords: | VRT raw added |
Milestone: | → 1.5.0 |
Owner: | changed from | to
Priority: | highest → normal |
comment:2 by , 16 years ago
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.
comment:3 by , 16 years ago
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.
comment:4 by , 16 years ago
Cc: | added |
---|
comment:5 by , 16 years ago
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?
comment:6 by , 16 years ago
Status: | new → 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.
comment:7 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Ok, we will skip preparing a regression test for this.
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.