Opened 12 years ago

Closed 4 years ago

#4011 closed defect (wontfix)

[PATCH]GDALFillNoData(): Assertion triggered by -si option

Reported by: Even Rouault Owned by: warmerdam
Priority: normal Milestone: closed_because_of_github_migration
Component: Algorithms Version: unspecified
Severity: normal Keywords:
Cc: Kyle Shannon

Description (last modified by Kyle Shannon)

python swig/python/scripts/gdal_fillnodata.py byte.asc out.tif -si 1

triggers :

ERROR 7: Assertion `pabyThisTMask[iX]' failed
in file `rasterfill.cpp', line 61

Attachments (2)

byte.asc (1.7 KB ) - added by Even Rouault 12 years ago.
4011.patch (1.9 KB ) - added by Kyle Shannon 8 years ago.
Updated patch that handles external masks better

Download all attachments as: .zip

Change History (12)

by Even Rouault, 12 years ago

Attachment: byte.asc added

comment:1 by Kyle Shannon, 10 years ago

Cc: Kyle Shannon added

Even, In ticket #4539 I think I fixed this. The way I found it was by using the c api and then just changing the mask band:

#include "gdal_priv.h"
#include "gdal_alg.h"

int main()
{
    GDALAllRegister();
    GDALDatasetH hDS, hDstDS;
    GDALRasterBandH hBand, hMask, hDstBand;
    double adfGeoTransform[6];
    const char* pszProjection;
    GDALDriverH hDriver;
    int nXSize, nYSize;
    double dfNoDataValue;
    CPLErr e;

    int *panData;

    hDS = GDALOpen("warp.tif", GA_Update);
    GDALGetGeoTransform(hDS, adfGeoTransform);
    pszProjection = GDALGetProjectionRef(hDS);
    nXSize = GDALGetRasterXSize(hDS);
    nYSize = GDALGetRasterYSize(hDS);

    hBand = GDALGetRasterBand(hDS, 1);
    dfNoDataValue = GDALGetRasterNoDataValue(hBand, NULL);
    hMask = GDALGetMaskBand(hBand);

    hDriver = GDALGetDriverByName("GTiff");

    if(CPLCheckForFile("out.tif"))
        GDALDeleteDataset("out.tif");

    hDstDS = GDALCreate(hDriver, "out.tif", nXSize, nYSize, 1, GDT_Int16, NULL);
    GDALSetGeoTransform(hDstDS, adfGeoTransform);
    GDALSetProjection(hDstDS, pszProjection);
    hDstBand = GDALGetRasterBand(hDstDS, 1);
    GDALSetRasterNoDataValue(hDstBand, dfNoDataValue);
    /* MOVE THE MASK HANDLE TO THE DESTINATION BAND */
    hMask = GDALGetMaskBand(hDstBand);
    panData = (int*)CPLMalloc(nXSize*nYSize*sizeof(int));

    e = GDALRasterIO(hBand, GF_Read, 0, 0, nXSize, nYSize, panData, nXSize,
                     nYSize, GDT_Int16, 0, 0);
    if(e != CE_None)
    {
        fprintf(stderr, "Failed to read raster data\n");
        exit(1);
    }
    e = GDALRasterIO(hDstBand, GF_Write, 0, 0, nXSize, nYSize, panData,
                     nXSize, nYSize, GDT_Int16, 0, 0);
    if(e != CE_None)
    {
        fprintf(stderr, "Failed to read raster data\n");
        exit(1);
    }
    e = GDALFillNodata(hDstBand, hMask, 100, 0, 3, NULL, NULL, NULL);

    if(e != CE_None)
    {
        fprintf(stderr, "Error returned in GDALFillNodata\n");
        exit(1);
    }
    else
    {
        printf("Success\n");
    }
    CPLFree(panData);
    return 0;
}

It worked. I just made sure that the no data value on the hDstBand and PamDataset returns a No Data mask by default(?). I think that explains why it works. If the mask handle is null, I believe it runs.

kss

comment:2 by Kyle Shannon, 10 years ago

Also, if -nomask is passed to the script, it will run.

comment:3 by Even Rouault, 10 years ago

Just for completness, before the crash, Valgrind complains about use of uninitialized values.

0...10...20...30...40...50...60...70==11729== Conditional jump or move depends on uninitialised value(s)
==11729==    at 0x72F0027: GDALFillNodata (rasterfill.cpp:708)
==11729==    by 0x6667C9E: FillNodata(void*, void*, double, int, char**, int (*)(double, char const*, void*), void*) (gdal_wrap.cpp:4729)
==11729==    by 0x666C691: _wrap_FillNodata (gdal_wrap.cpp:18560)
==11729==    by 0x4A8783: PyEval_EvalFrameEx (in /usr/bin/python2.6)
==11729==    by 0x4A9670: PyEval_EvalCodeEx (in /usr/bin/python2.6)
==11729==    by 0x4A9741: PyEval_EvalCode (in /usr/bin/python2.6)
==11729==    by 0x4C9A0D: PyRun_FileExFlags (in /usr/bin/python2.6)
==11729==    by 0x4C9C23: PyRun_SimpleFileExFlags (in /usr/bin/python2.6)
==11729==    by 0x41A7FE: Py_Main (in /usr/bin/python2.6)
==11729==    by 0x5EEEC4C: (below main) (libc-start.c:226)

comment:4 by Kyle Shannon, 10 years ago

Fair enough, I new it was hacky. Possibly there should be better checking in gdal_fillnodata.py and set the maskband=None if necessary. I am looking a bit more, I will try to find the _real_ issue.

comment:5 by Jukka Rahkonen, 8 years ago

Did you find the _real_issue, kyle?

in reply to:  5 comment:6 by Kyle Shannon, 8 years ago

Replying to jratike80:

Did you find the _real_issue, kyle?

No, I haven't had time to look into it again.

comment:7 by Kyle Shannon, 8 years ago

Description: modified (diff)

It appears that the smoothing needs to use an updated mask with the filled data. This happens easily when a dataset is update in place, using GDALFlushRasterCache() on the mask band for the data. If the mask is external, though, the values are reverted back to the original no data mask. I *think* the original intention was to use the updated mask, which worked if filling in place. The assert would make more sense then, as the filtered data would have to have a new value, not no data.

To resolve this, before starting the smoothing, I replace the supplied mask band with the filled target mask band, recomputed. This works with the test data on this ticket, the data on #4539, and I tested all three mask options, default, nomask, and a mask file. So #4539 can be closed if this works,

patch attached.

comment:8 by Kyle Shannon, 8 years ago

Summary: GDALFillNoData(): Assertion triggered by -si option[PATCH]GDALFillNoData(): Assertion triggered by -si option

#3988 may close too with the patch. Valgrind didn't show any warnings, although there is a lot of python noise, I may have missed something.

by Kyle Shannon, 8 years ago

Attachment: 4011.patch added

Updated patch that handles external masks better

comment:9 by Kyle Shannon, 8 years ago

Description: modified (diff)

Fix erroneous description change.

comment:10 by Even Rouault, 4 years ago

Milestone: closed_because_of_github_migration
Resolution: wontfix
Status: newclosed

This ticket has been automatically closed because Trac is no longer used for GDAL bug tracking, since the project has migrated to GitHub. If you believe this ticket is still valid, you may file it to https://github.com/OSGeo/gdal/issues if it is not already reported there.

Note: See TracTickets for help on using tickets.