Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#3925 closed enhancement (fixed)

Allow addition of pixel functions to a band by name during band addition

Reported by: klaricmn Owned by: warmerdam
Priority: normal Milestone: 1.9.0
Component: default Version: svn-trunk
Severity: normal Keywords: vrt
Cc: bclaywell, antonio

Description

The attached patch allows a pixel function to be added to a band by name (not function pointer) during programmatic creation of a VRT file.

See the following example:

#include <gdal_priv.h>
#include <gdal_vrt.h>

int main()
{
    GDALAllRegister();

    GDALDataset* old_dataset = static_cast<GDALDataset*>(GDALOpenShared("test.tif", GA_ReadOnly));

    int width = old_dataset->GetRasterXSize();
    int height = old_dataset->GetRasterYSize();
    
    GDALRasterBand* old_band = old_dataset->GetRasterBand(1);

    GDALDriver* vrt_driver = static_cast<GDALDriver*>(GDALGetDriverByName("VRT"));
    GDALDataset* vrt_dataset = vrt_driver->Create("test.vrt", width, height, 0, GDT_Byte, NULL);

    char** options = NULL;
    options = CSLAddNameValue(options, "subclass", "VRTDerivedRasterBand");
    options = CSLAddNameValue(options, "PixelFunctionType", "NameOfSomePixelFunction");
    /* NOTE: The above line is what's allow by this patch */

    vrt_dataset->AddBand(GDT_Byte, options);

    GDALRasterBand* new_band = vrt_dataset->GetRasterBand(vrt_dataset->GetRasterCount());
    
    VRTAddSimpleSource(static_cast<VRTSourcedRasterBandH>(new_band), band,
		       0, 0, width, height,
		       0, 0, width, height,
		       "near", VRT_NODATA_UNSET);

    GDALClose(vrt_dataset);
    GDALClose(rgb_dataset);
    
    return 0;
}

Attachments (2)

vrt_pixelfunc.patch (1.1 KB) - added by klaricmn 7 years ago.
vrt_pixelfunc_with_autotest.patch (8.5 KB) - added by antonio 6 years ago.
Extended patch with autotest

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by klaricmn

Attachment: vrt_pixelfunc.patch added

comment:1 Changed 7 years ago by Even Rouault

I'm not sure of the interest of the patch.

Why not just doing :

char** options = NULL;
    options = CSLAddNameValue(options, "subclass", "VRTDerivedRasterBand");

vrt_dataset->AddBand(GDT_Byte, options);

GDALRasterBand* new_band = vrt_dataset->GetRasterBand(vrt_dataset->GetRasterCount());

VRTDerivedRasterBand* vrtderivedband = (VRTDerivedRasterBand*)band;
vrtderivedband->SetPixelFunctionName("NameOfSomePixelFunction");

comment:2 in reply to:  1 Changed 7 years ago by bclaywell

Replying to rouault:

I'm not sure of the interest of the patch.

Why not just doing :

...

VRTDerivedRasterBand* vrtderivedband = (VRTDerivedRasterBand*)band; vrtderivedband->SetPixelFunctionName?("NameOfSomePixelFunction?");

I thought the VRTDerivedRasterBand interface was unavailable outside the VRT driver itself, judging from gdal_vrt.h. Is it actually declared somewhere else that client code can use?

comment:3 Changed 7 years ago by Even Rouault

It is declared in vrtdataset.h. gdal_vrt.h is supposed to be the "public" C API of the VRT driver, but as you're using the GDAL CPP API, you can as well use vrtdataset.h

comment:4 in reply to:  3 Changed 7 years ago by bclaywell

Replying to rouault:

It is declared in vrtdataset.h. gdal_vrt.h is supposed to be the "public" C API of the VRT driver, but as you're using the GDAL CPP API, you can as well use vrtdataset.h

Oops. I embarrassingly didn't know vrtdataset.h existed, so in that respect we probably don't need this patch :) But doesn't that still mean that users of the C API have no way to add a derived band in this way?

comment:5 Changed 7 years ago by Even Rouault

Yes, users of C API cannot add derived band. But they could neither register the pixel function, so you definitely need C++ to use those advanced features of the VRT driver. I think we are not too keen on extending more the VRT C API, as it can be difficult to guarantee its stability when the VRT driver evolves.

comment:6 Changed 6 years ago by antonio

Cc: antonio added

comment:7 Changed 6 years ago by antonio

I think that the patch proposed by klaricmn would be the perfect solution to allow users to access the powerful feature represented by pixel functions even from python and other bindings.

See: http://lists.osgeo.org/pipermail/gdal-dev/2011-May/028737.html, #3367 and attachment:pixfun-plugin-20110513.tar.gz:ticket:3367.

In short: pixel functions can be supplied and registered using plugins (small C/C++ coding) and then, with the applied patch, they could be used from the python API etc.

comment:8 in reply to:  5 Changed 6 years ago by antonio

Replying to rouault:

Yes, users of C API cannot add derived band. But they could neither register the pixel function, so you definitely need C++ to use those advanced features of the VRT driver. I think we are not too keen on extending more the VRT C API, as it can be difficult to guarantee its stability when the VRT driver evolves.

Sorry Even, I don't understand the above comment.

If my understanding is correct, GDALAddDerivedBandPixelFunc is in the C public API so users can register pixel function from C.

What am I missing?

comment:9 Changed 6 years ago by Even Rouault

Yeah, forget the part about GDALAddDerivedBandPixelFunc(). It is indeed in gdal.h, so in the C API

Changed 6 years ago by antonio

Extended patch with autotest

comment:10 in reply to:  5 Changed 6 years ago by antonio

Replying to rouault:

[CUT]

I think we are not too keen on extending more the VRT C API, as it can be difficult to guarantee its stability when the VRT driver evolves.

Just in case you change your mind I've just attached a patch that extends the one written by klaricmn to handle also the SourceTransferType parameter.

The new patch also include autotests.

comment:11 Changed 6 years ago by Even Rouault

Milestone: 1.8.11.9.0
Resolution: fixed
Status: newclosed

OK, you finally convinced me by providing the tests ;-)

r22374 /trunk/ (2 files in 2 dirs): VRT: Recognize PixelFunctionType? and SourceTransferType? options in AddBand?() for VRTDerivedRasterBand (#3925, patch contributed by klaricmn and antonio)

comment:12 in reply to:  7 Changed 6 years ago by klaricmn

Replying to antonio:

I think that the patch proposed by klaricmn would be the perfect solution to allow users to access the powerful feature represented by pixel functions even from python and other bindings.

...and just to give credit where credit is due, the patched that I submitted was mostly/entirely written by bclaywell (my co-worker). I can't remember if I had my hands directly in this one at all or not. I submitted this and several other patches during a "submit all of our local patches" spree a few months back.

Note: See TracTickets for help on using tickets.