For blocks bigger than the CacheMax GetBlockRef returns an error

[ilucena]

The INGR driver is producing a GetBlockRef Error when reading CCITT4 compressed images that produce a block in memory larger than the CacheMax.

That is what I found:

The comment on that line: http://trac.osgeo.org/gdal/browser/trunk/gdal/gcore/gdalrasterblock.cpp?rev=13965#L379 says "don't flush this block!" but that is exactly what is happening 7 lines after that.

I made that change and the "GetBlockRef Error" problem with INGR, CCITT4 disappear:

        if (nLockCount == 0)
        {
            if (!GDALFlushCacheBlock())
            {
                eErr = CE_Failure;
                break;
            }
        }

The GDALFlushCacheBlock() was added my Even in 2/3/2008. I would suggest a revision.

Regards.

[warmerdam]

I would like to correct this before preparing 1.5.1 if practical.

[Even Rouault]

Hum, my change was made in trunk only, so it will not affect 1.5 branch for sure.

Ivan, I think that your fix is not the right to do. It just disables completely flushing the cache as nLockCount will be always != 0 because of the previous AddLock().

Could you try to revert it and instead make the little following change :

int GDALRasterBlock::FlushCacheBlock()

{
    int nXOff, nYOff;
    GDALRasterBand *poBand;

    {
        CPLMutexHolderD( &hRBMutex );
        GDALRasterBlock *poTarget = (GDALRasterBlock *) poOldest;

        while( poTarget != NULL && poTarget->GetLockCount() > 0 ) 
            poTarget = poTarget->poPrevious;
        
        if( poTarget == NULL )
            return TRUE; /* Even Rouault : return TRUE instead of FALSE */

        poTarget->Detach();

        nXOff = poTarget->GetXOff();
        nYOff = poTarget->GetYOff();
        poBand = poTarget->GetBand();
    }

    return (poBand->FlushBlock( nXOff, nYOff ) == CE_None);
}

I think I need to elaborate more on the reason for the commit r13690. The problem was that GDAL utilities are sometimes not reporting I/O errors that are differed to the end of the process because of the block cache mechanism. That can be annoying. So I followed the link of calls and tested for error codes and propagated them. Here, the semantics of the return code of GDALRasterBlock::FlushCacheBlock is a bit hard to define. I think that an error occuring in poBand->FlushBlock (that can originate from an error in some driver's ::IWriteBlock method) should be reported to the caller of FlushCacheBlock. But in your situation, I think you're hitting the 'if (poTarget == NULL)' case, that means that no block is elligible to be flushed. My fix would be to report TRUE in that situation instead of FALSE. So the return code of FlushCacheBlock should be interpreted as the presence/absence of errors when flushing blocks, not the fact that some blocks have been flushed or not. But as GDALFlushCacheBlock is part of the C API, that might be disturbing ?

Hum, Frank, any idea about this ?

[ilucena]

Even, I did not commit my quick-fix code and I will feel more comfortable if you commit that correction. I can test it on my local build of GDAL and if you need data or any details to replicate the problem, please let me know. Thank you.

[Even Rouault]

I'm a bit shy at the moment to commit my one-word fix. I don't want to cause regressions elsewhere. Yeah, providing data to replicate the problem would be usefull of course

[Even Rouault]

Ivan, I finally reverted the whole r13690 commit (in r13986) as it can has the unwanted effect of reporting IWriteBlock errors when someone is loading a block for reading for example. In fact, now I remember that I essentially made this 'fix' because a driver (unfortunatelly I can't remember which one) returned CE_Failure without issuing a CPLError(), which should be fixed.

[ilucena]

Even, I tested your changes (the current reverted status of the code) and it is working fine for me. I mean, I can gdal_translate the same INGR CCITT4 files with no problem now. Thanks.