Even, I already do direct tests with jp2kakdataset_test.cc with bazel and it works great. I can directly probe the methods without worrying about any other mechanism getting between the test and the class. e.g. I can directly mess with Identify, Open and any helpers. I can also do things like disable identify so that fuzzing can more quickly get into the guts and find trouble:
https://trac.osgeo.org/gdal/browser/trunk/gdal/frmts/jp2kak/jp2kakdataset.cpp#L812 GDALDataset *JP2KAKDataset::Open( GDALOpenInfo * poOpenInfo ) { #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION // During fuzzing, do not use Identify to reject crazy content. if( !Identify(poOpenInfo) ) return NULL; #endif We are putting resources behind this... we are running fuzzing on jp2kak and related. e.g. with >600K core hours of fuzzing this month (> 14e12 iterations). If I don't break things apart, fuzzing a component goes much slower and is far less likely to identify issues in the lower level components and libs. On Wed, Mar 1, 2017 at 6:15 AM, Even Rouault <even.roua...@spatialys.com> wrote: > Kurt, > > > > I don't think the ENVIDataset class is exported (and that's a good thing), > so direct testing of the class might only be possible if you build without > --with-hide-internal-symbols. > > > > Why can't you test through the public GDAL API instead of directly > depnding on implementation details ? Actually looking at the class > definitions of the ENVI driver, the only public methods are virtual methods > from the public API, so there's no advantage on directly depending on it. > > > > Even > > > > > Warning... I did a cleanup pass and split out the class definitions into > a > > > header to allow direct C++ testing. > > > > > > https://trac.osgeo.org/gdal/changeset/37530 > > > https://trac.osgeo.org/gdal/changeset/37514 > > > https://trac.osgeo.org/gdal/changeset/37510 > > > > > > I haven't written that testing yet, but it will be similar to this: > > > > > > https://github.com/schwehr/gdal-autotest2/blob/master/ > cpp/frmts/jp2kak/jp2ka > > > kdataset_test.cc > > > > > > -kurt > > > > > > On Wed, Mar 1, 2017 at 2:56 AM, Damian Dixon <damian.di...@gmail.com> > wrote: > > > > Hi Taylor, > > > > > > > > M_PI is defined pretty much everywhere. The only compiler that you > have to > > > > jump through hoops to get it is Visual Studio where you have to define > > > > '#define _USE_MATH_DEFINES'. > > > > > > > > As Even has mentioned it's also defined in the GDAL portability header. > > > > > > > > The main reason the use of atan jumped out is that I've run into > problems > > > > with atan on Intel CPUs and GPUs in the past. Also the use of trig' of > any > > > > sort is expensive so I tend to take a look to see if it can be removed > or > > > > moved. > > > > > > > > I spotted that this is a very old bug so I'm pleased that you've taken > the > > > > time to fix it. > > > > > > > > Regards > > > > Damian > > > > > > > > > > > > > > > > On 27 February 2017 at 22:12, Brown, Taylor Alexander < > > > > > > > > browt...@oregonstate.edu> wrote: > > > >> Damian, > > > >> > > > >> I agree that it would be preferable to use a predefined constant. > > > >> However, I was under the impression that M_PI is a compiler extension > and > > > >> not part of standard C++, so I wanted to make sure it was compatible. > If > > > >> using M_PI is acceptable, I would be happy to change it. I will be > > > >> submitting my pull request shortly. > > > >> > > > >> > > > >> Taylor > > > >> > > > >> On Mon, Feb 27, 2017 at 5:01 AM, Damian Dixon <damian.di...@gmail.com > > > > > >> > > > >> wrote: > > > >>> Hi Taylor, > > > >>> > > > >>> I'm just lurking on the mailing list myself. > > > >>> > > > >>> I had a look at the code changes, as this is one of the formats I'm > > > >>> interested in, and one point springs out at me: > > > >>> > > > >>> const double PI = atan(1.0) * 4; > > > >>> > > > >>> I find this line potentially a confusing way to define PI, > particularly > > > >>> when you combine it with (PI/180) later. > > > >>> > > > >>> Personally I would use M_PI. > > > >>> > > > >>> Regards > > > >>> Damian > > > >>> > > > >>> > > > >>> > > > >>> On 27 February 2017 at 11:55, Taylor Alexander Brown < > > > >>> > > > >>> browt...@oregonstate.edu> wrote: > > > >>>> Greetings GDAL Community, > > > >>>> > > > >>>> I am developing an application [0] to process georeferenced raster > > > >>>> hyperspectral imagery in ENVI format from NASA's Airborne > > > >>>> Visual/Infrared Imaging Spectrometer (AVIRIS) [1]. > > > >>>> > > > >>>> The `map info` section of the header files contain an undocumented > [2] > > > >>>> rotation field which is currently ignored by GDAL. As a result, the > > > >>>> image is rotated incorrectly when I process it with `gdalwarp` or > the > > > >>>> GDAL Python API. Furthermore, the image is scaled incorrectly when I > > > >>>> try > > > >>>> to override the geotransform. > > > >>>> > > > >>>> I documented my experiences on the GIS Stack Exchange [3]. It has > > > >>>> previously been described on the GDAL mailing list [4] and bug > tracker > > > >>>> [5]. I believe this issue also affects ASTER imagery [6][7]. > > > >>>> > > > >>>> I have implemented a fix based off of branch `tags/2.1.3` and > shared my > > > >>>> code on GitHub [8]. It reads the `units` and `rotation` parameter > from > > > >>>> the `map info` section and uses these to calculate the geotransform. > > > >>>> The > > > >>>> scaling was off because the code expected the `units` parameter to > be > > > >>>> the last element in the list, when in my case it was the second to > > > >>>> last. > > > >>>> You can verify this behavior with the single-band image I have been > > > >>>> using for testing [9]. > > > >>>> > > > >>>> You are welcome to evaluate my code and contribute it back to the > > > >>>> library. I am a novice at both GDAL and GIS, so there may well be > > > >>>> problems. I would be willing to submit a pull request against your > > > >>>> GitHub mirror if desired. > > > >>>> > > > >>>> > > > >>>> Peace, > > > >>>> > > > >>>> Taylor Alexander Brown > > > >>>> > > > >>>> > > > >>>> [0] https://github.com/capstone-coal/pycoal > > > >>>> [1] https://aviris.jpl.nasa.gov/ > > > >>>> [2] http://www.harrisgeospatial.com/docs/enviheaderfiles.html > > > >>>> [3] > > > >>>> http://gis.stackexchange.com/questions/229952/rotate-envi-hy > > > >>>> perspectral-imagery-with-gdal > > > >>>> [4] https://lists.osgeo.org/pipermail/gdal-dev/2013- > January/035146.html > > > >>>> [5] https://trac.osgeo.org/gdal/ticket/1778 > > > >>>> [6] http://yceo.yale.edu/opening-aster-files-envi > > > >>>> [7] http://yceo.yale.edu/faq-page#t2n504 > > > >>>> [8] > > > >>>> https://github.com/OSGeo/gdal/compare/tags/2.1.3...browtayl: > > > >>>> fix-envi-rotation?expand=1 > > > >>>> [9] https://drive.google.com/open?id=0BxysdOuBmaIGalY5dVhCa1Y5M0k > > > >>>> _______________________________________________ > > > >>>> gdal-dev mailing list > > > >>>> gdal-dev@lists.osgeo.org > > > >>>> https://lists.osgeo.org/mailman/listinfo/gdal-dev > > > > > > > > _______________________________________________ > > > > gdal-dev mailing list > > > > gdal-dev@lists.osgeo.org > > > > https://lists.osgeo.org/mailman/listinfo/gdal-dev > > > > > > -- > > Spatialys - Geospatial professional services > > http://www.spatialys.com > -- -- http://schwehr.org
_______________________________________________ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
