Ticket #4468 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

Corrupted TIFF with GA_Update and image smaller than 64x64

Reported by: cleo Owned by: warmerdam
Priority: normal Milestone: 1.9.1
Component: GDAL_Raster Version: 1.9.0
Severity: major Keywords: gtiff
Cc:

Description

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);

Attachments

test_raster_32bit.tif Download (1.0 KB) - added by cleo 3 years ago.
original
test_raster_32bit_corrupted.tif Download (1.0 KB) - added by cleo 3 years ago.
corrupted

Change History

Changed 3 years ago by 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

Changed 3 years ago by 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.

Changed 3 years ago by cleo

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

Changed 3 years ago by 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.

Changed 3 years ago by cleo

original

Changed 3 years ago by cleo

corrupted

Changed 3 years ago by 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.

Changed 3 years ago by warmerdam

  • keywords gtiff added
  • status changed from new to assigned

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.

Changed 3 years ago by 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.

Changed 3 years ago by 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...

Changed 3 years ago by 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.

Changed 3 years ago by warmerdam

  • status changed from assigned to closed
  • resolution set to fixed
  • milestone set to 1.9.1

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.

Changed 3 years ago by 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!

Changed 3 years ago by cleo

Everything works!

Note: See TracTickets for help on using tickets.