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 <[email protected]> > 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 <[email protected]> > > > > > 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. >
_______________________________________________ gdal-dev mailing list [email protected] http://lists.osgeo.org/mailman/listinfo/gdal-dev
