[gdal-dev] ENVI Rotation Bug Fix

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[gdal-dev] ENVI Rotation Bug Fix

Taylor Alexander Brown
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-hyperspectral-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
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Even Rouault-2

On lundi 27 février 2017 03:55:54 CET Taylor Alexander Brown 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.

 

Yes, please issue a P.R. against the github mirror. Preferably against trunk which has more complete regression testing.

 

Even

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com


_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Damian Dixon
In reply to this post by Taylor Alexander Brown
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 <[hidden email]> 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-hyperspectral-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
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev


_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Taylor Alexander Brown
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 <[hidden email]> 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 <[hidden email]> 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-hyperspectral-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
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev



_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Even Rouault-2

On lundi 27 février 2017 14:12:46 CET Brown, Taylor Alexander 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.

 

M_PI is #define'd in port/cpl_port.h if not already defined.

 

>

>

> Taylor

>

> On Mon, Feb 27, 2017 at 5:01 AM, Damian Dixon <[hidden email]>

>

> 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 <

> >

> > [hidden email]> 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

> >> [hidden email]

> >> https://lists.osgeo.org/mailman/listinfo/gdal-dev

 

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com


_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Taylor Alexander Brown
I have pushed the requested changes to a pull request:
https://github.com/OSGeo/gdal/pull/197


Taylor

On 02/27/2017 02:20 PM, Even Rouault wrote:

> On lundi 27 février 2017 14:12:46 CET Brown, Taylor Alexander 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.
>
>  
>
> M_PI is #define'd in port/cpl_port.h if not already defined.
>
>  
>
>>
>
>>
>
>> Taylor
>
>>
>
>> On Mon, Feb 27, 2017 at 5:01 AM, Damian Dixon <[hidden email]>
>
>>
>
>> 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 <
>
>> >
>
>> > [hidden email]> 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
>
>> >> [hidden email]
>
>> >> https://lists.osgeo.org/mailman/listinfo/gdal-dev
>
>  
>
>  
>
> --
>
> Spatialys - Geospatial professional services
>
> http://www.spatialys.com
>
_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Damian Dixon
In reply to this post by Taylor Alexander Brown
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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-hyperspectral-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
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev




_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Kurt Schwehr-2
Warning... I did a cleanup pass and split out the class definitions into a header to allow direct C++ testing.  


I haven't written that testing yet, but it will be similar to this:


-kurt

On Wed, Mar 1, 2017 at 2:56 AM, Damian Dixon <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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-hyperspectral-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
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev




_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev



--

_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Even Rouault-2

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 <[hidden email]> 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 <

> >

> > [hidden email]> 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 <[hidden email]>

> >>

> >> 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 <

> >>>

> >>> [hidden email]> 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

> >>>> [hidden email]

> >>>> https://lists.osgeo.org/mailman/listinfo/gdal-dev

> >

> > _______________________________________________

> > gdal-dev mailing list

> > [hidden email]

> > https://lists.osgeo.org/mailman/listinfo/gdal-dev

 

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com


_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Kurt Schwehr-2
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: 


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 <[hidden email]> 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 <[hidden email]> 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 <

> >

> > [hidden email]> 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 <[hidden email]>

> >>

> >> 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 <

> >>>

> >>> [hidden email]> 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

> >>>> [hidden email]

> >>>> https://lists.osgeo.org/mailman/listinfo/gdal-dev

> >

> > _______________________________________________

> > gdal-dev mailing list

> > [hidden email]

> > https://lists.osgeo.org/mailman/listinfo/gdal-dev

 

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com




--

_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ENVI Rotation Bug Fix

Guillaume Pasero

Hi,

I have tried to use GDAL 2.2 in OTB. I have some issues with output images in the ENVI format. Some of the images I use have :

  • A geotransform with an origin, for instance : geotransform = {10,   1,   0,   10,  0,   1}
  • No map projection (the image comes from a sensor image, or may come from a plain jpg file).
As you can see, the 2x2 linear part of the geotransform is an identity matrix. When I write such an image using the ENVI format, I get a 90° rotation :
     map info = {Arbitrary, 1, 1, 10, 10, 1, 1, 0, North, rotation=90}

Does ENVI support negative values for "Pixel Y size", in order to handle this Y-flipping ?

Regards,
Guillaume Pasero

On 03/01/2017 04:03 PM, Kurt Schwehr wrote:
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: 


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.



_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Loading...