[gdal-dev] VRTComplexSource with a LUT, proposal

[Saâd HESSANE]

Hi,

I see that your patch changes the definition of a function in the C API
> (VRTAddComplexSource). Up to now, the GDAL C API has been set in stone.
> This
> might change in GDAL 2.0 though, but I'm going to start a specific thread
> on
> this to have global feedback from the GDAL community.

Yes, I understand. I hesitated a bit before changing the public interface
but I had no choice except to add another function with another name and
the same argument ...

But even if we are OK to change the C API, I begin to think that this
> VRTAddComplexSource() function becomes ugly with so many parameters.
> Perhaps
> we should add a char** papszOptions to provide additional options without
> breaking it each time. Or I'm wondering if the C API is really the best
> option
> for doing such complex things, and using the C++ API wouldn't be just
> better.

I agree to change the VRTAddComplexSource() function to make it more
simple. If you use C++ to do it, I think a decorator pattern can be useful
to add fonctionnality to a VRT dataset.

But as I say, the main goal is to build VRT with LUT and filter. Maybe you
can envisage it for the gdal 2.0 release?

Cheers.



2012/5/20 Even Rouault <even.rouault at mines-paris.org>

> Le dimanche 20 mai 2012 16:07:07, Saâd HESSANE a écrit :
> > Hi Even,
> >
> > The main goal of this proposal is to built using "gdalbuildvrt" VRT files
> > containing LUT or filters.
> >
> > The patch that I send allow build VRT with LUT by specifying the -lutin
> and
> > -lutout parameters.
> > Ex :
> >   gdalbuildvrt -lutin "0 1 2" -lutout "4 5 6" test.vrt test.tif
> >
> > We can discuss the fact that define two parameters (-lutin and -lutout)
> or
> > just one parameter (-lut "0,4 1,5 2,6"). This is not a problem.
>
> Ah, your patch is more extensive than what I thought (I imagined only the
> SetLUT() function to be in it).
>
> I see that your patch changes the definition of a function in the C API
> (VRTAddComplexSource). Up to now, the GDAL C API has been set in stone.
> This
> might change in GDAL 2.0 though, but I'm going to start a specific thread
> on
> this to have global feedback from the GDAL community.
>
> But even if we are OK to change the C API, I begin to think that this
> VRTAddComplexSource() function becomes ugly with so many parameters.
> Perhaps
> we should add a char** papszOptions to provide additional options without
> breaking it each time. Or I'm wondering if the C API is really the best
> option
> for doing such complex things, and using the C++ API wouldn't be just
> better.
>
> >
> >
> > 2012/5/19 Even Rouault <even.rouault at mines-paris.org>
> >
> > > Le samedi 19 mai 2012 18:58:05, Saâd HESSANE a écrit :
> > > > Hi Even and thank you for the quick response.
> > > >
> > > > You should even not consider the VRTComplexSource class to be in the
> > >
> > > public
> > >
> > > > > API, so the visibility of its members is not significant. And
> playing
> > > > > with them
> > > > > from the outside isn't recommanded at all.
> > > >
> > > > The fact that you don't consider the class in the public API does not
> > > > excuse the fact that this is a mistake encapsulation :)
> > >
> > > I agree the encapsulation isn't ideal, but unless I'm wrong, this class
> > > is *not* in the public API. It is not marked as CPL_DLL exported, so on
> > > Windows,
> > > you should not be able to access it from the outside of the GDAL lib.
> On
> > > Unix/Linux, as, by default (unless GDAL is configured with
> > > --hide-internal- symbols), all symbols are exported, you can
> technically
> > > use it however.
> > >
> > > > And apart from that, nothing prevents me to use a special driver to
> do
> > > > specific things that are not directly provided by the public API.
> > > > Currently I need to build a VRT, and the VRT driver are fine for
> that.
> > >
> > > It's
> > >
> > > > dirrectly use in the gdalbuildvrt utility for example. If the VRT
> > > > plugin
> > >
> > > is
> > >
> > > > not safe to use, the solution is to correct it.
> > >
> > > There's always a trade off between exposing API and increasing
> > > maintenance burden. The more API you expose, the more difficult it is
> to
> > > make changes afterwards.
> > >
> > > > To read a VRT file outside from gdal I need to parse an XML file, so
> I
> > >
> > > have
> > >
> > > > to use another dependency (like xerces) to do just a small think.
> > >
> > > You don't need another dependency. You can use the CPL minixml API (see
> > > cpl_minixml.h) that is used by the VRT driver itself for example.
> > >
> > > > Another argument is the VRTKernelFiltredSource. To set a kernel
> filtre
> > > > we don't have to set the attributes nKernelSize, padfKernelCoefs and
> > > > bNormalized, because we can't (the attributes are protected). But the
> > > > API offers a setKernel that do exactly the same think that I hope the
> > > > setLUT method do in the VRTComplexSource.
> > >
> > > If you can come with a patch, I'll consider including it, but I still
> > > believe
> > > you should not rely on that.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/gdal-dev/attachments/20120521/8f70f06d/attachment.html