Corrupted TIFF with GA_Update and image smaller than 64x64

[cleo]

This happens in 1.8.0 as well.

Take any image 24 or 32 bit (haven't tested other bit depths yet), resize to 63x63 or less in GIMP for example and save it out. This will create a strips, interleaved pixel image. Save as TIFF using any compression or no compression. Doesn't matter. Open file with GDALOpen(filename, GA_Update); Do RasterIO on either the file or band. Flush the cache and the image is corrupted.

I'm opening, reading one pixel, writing that pixel back out and flushing the cache, then closing the file. That's it. Works fine with images 64x64 or larger. Reads also work fine.

I'm on Win32, MSVC 9 (2008). Internal libs. Default settings. With 1.9, never crashes but corrupts file. With 1.8 and external libs, it crashes at all image sizes on flush in external lib (in deflate reset for example) used for respective compression though it's possible that's something to do on our end, yet reads work fine. With no compression, doesn't crash, but corrupts the file.

    //Register all formats.
    GDALAllRegister();

    // Create a copy that we can write to.
    // uses boosts' copy_file() method.
    copyFile(sourceFilename, destFilename); 

    // Open
    GDALDataset *pDataSet = (GDALDataset *) GDALOpen( destFilename.c_str(), GA_Update);
    CPPUNIT_ASSERT( pDataSet != NULL);

    // Read one pixel
    unsigned int pixel;
    CPLErr err = pDataSet->RasterIO(
        GF_Read,
        0, 0,		
        1, 1,
        &pixel,
        1, 1,
        GDT_Byte,
        pDataSet->GetRasterCount(),
        NULL,
        pDataSet->GetRasterCount(),
        pDataSet->GetRasterCount(),
        1);
    CPPUNIT_ASSERT (err == CE_None);

    // Write one pixel
    err = pDataSet->RasterIO(
        GF_Write,
        0, 0,		
        1, 1,
        &pixel,
        1, 1,
        GDT_Byte,
        pDataSet->GetRasterCount(),
        NULL,
        pDataSet->GetRasterCount(),
        pDataSet->GetRasterCount(),
        1);
    CPPUNIT_ASSERT (err == CE_None);

    // Flush
    pDataSet->FlushCache();

    // Close
    GDALClose(pDataSet);

[Even Rouault]

I already see something incorrect in your code. You use "unsigned int pixel" and GDT_Byte. That doesn't match. You should rather use "unsigned char pixel".

The crashes must be due to something else. Perhaps a mismatch of debugging compilation flags. GDAL setup in this regard is a bit unusual : see the comments about OPTFLAGS in http://trac.osgeo.org/gdal/wiki/BuildingOnWindows

[cleo]

This is GDALDataset::RasterIO, so I'm interleaving 4 values to form an RGBA pixel. If you read one band (with an unsigned char) at a time, the result is the same.

[cleo]

I'll take a look at OPTFLAGS and report back. Hopefully, that'll fix it. Thanks.

[warmerdam]

Hi,

Assuming changing build options doesn't fix this, could you attach a before and after TIFF file demonstrating the corruption I/we could try with? It will be this evening (pacific time) before I can try this on windows.

[cleo]

original

[cleo]

corrupted

[cleo]

Tried correct settings for debug mode (also, MSVC sets _DEBUG, not DEBUG)... but no change. I'll try release mode tomorrow just to see and ask someone else to take a look as well to make doubly sure everything is correct.

I've added some test images. But any image will do the same. The corruption is different depending on the source image and the compression, etc. Sometimes, it's all black. But most of the time, GIMP says it's missing image data.

[warmerdam]

I see the TIFF file is compressed. Can you also try it with an uncompressed image and see if the same thing happens? The problem may be platform independent - I'll try it on linux.

[warmerdam]

I have confirmed the problem also happens on linux, and that it goes away with an uncompressed image. I have lots to go on. I'll let you know when I have a fix.

In general inplace update of compressed TIFF files has been a source of bugs in the past though I had thought things were in good shape.

[warmerdam]

I observe that if I comment this out in WriteEncodedStrip() then the problem does not occur:

    if( (int) ((nStripWithinBand+1) * nRowsPerStrip) > GetRasterYSize() )
    {
        cc = (cc / nRowsPerStrip)
            * (GetRasterYSize() - nStripWithinBand * nRowsPerStrip);
        CPLDebug( "GTiff", "Adjusted bytes to write from %d to %d.", 
                  (int) TIFFStripSize(hTIFF), cc );
    }

When it occurs, the problem reported is:

RROR 1: ZIPDecode:Not enough data at scanline 0 (short 2048 bytes)
ERROR 1: TIFFReadEncodedStrip() failed.

So the problem seems to be that on writing we believe we need to trim the last strip if it is partial, but the zip reading code seems to expect a whole strip. In this particular case it is interesting to see that tiffinfo reports 64 lines per strip even though there are only 32 lines in the file. I presume if rows per strip did not exceed the file size this issue would not occur. Digging further...

[warmerdam]

Oh, evil. The problem closely relates to RawsPerStrip being larger than the image size. It seems that TIFFStripSize() knows to trim the strip size if the rowsperstrip is larger than image ysize; however, the "partial last block" code above does *not* know about this logic in TIFFStripSize(). Grr.

The problem would not occur if a reasonable TIFF file was produced, but it seems clear that GIMP and perhaps other apps are doing this wrong so I'll try to work around it.

[warmerdam]

Problem is fixed in trunk (r23800) and 1.9 (r23802). I also added a regression test using the provided sample data file in trunk (r23801).

Thanks for the report. Feel free to graft r23800 into your local code.

[cleo]

That's great! Thanks for the very quick response. BTW, I've had the same problem with no compression (as you say, perhaps it's an issue with GIMP?). But I'll try out the fix and I'll report back if there are any issues.

Thanks again!

[cleo]

Everything works!