ECW Version 3 file support (ECW SDK 5.0).

[jcztery]

Please find attached patch for ECW version 3 support:

1. Statistics/histograms read/write support for version 3 files.
   
2. UInt16 type support.
   
3. Fixed crash when writing JP2 boxes to JP2 files.
   
4. Improved color space detection (if source dataset is RGB(A) then it will be sRGBA, otherwise Multiband with proper descriptions).
   
5. Fixed bug where Grayscale image with opacity band did not report opacity.
   

[jcztery]

Modified files.

[warmerdam]

Jacek,

Would you be willing to try and reproduce this as a patch, and without changing text format, or other secondary whitespace changes?

I'm pleased to see you seem to have made 5.0 support conditional.

[jcztery]

Frank,
I can try. The change of ecw.py to UTF8 is necessary however, because there has been a fix for opening ecw files using utf8-encoded file name. So there is a corresponding test for that case.
Please let me know if You want to keep the test.

[warmerdam]

Jacek,

Hmm, I'm not entirely clear on what the implications are of a .py script being utf8, but feel free to include that in the patch. I hadn't got that far in my very cursory review.

[jcztery]

Frank,
I reworked the test to reference the file with unicode name from python using javascript/C unicode escapes. And data file with unicode name will be created by the script so it doesn't need to be in the repository. Attached patch.

[Even Rouault]

The removal of CPLFree(sCachedMultiBandIO.pabyData); (line 582 of current ecwdataset.cpp) seems unwanted. As well as if( eRWFlag == GF_Write ) return CE_Failure; at lines 407 and 1076.

Regarding the new tests, I guess there should be a condition on the gdaltest.ecw_drv.major_version to skip them if it's < 5.

The test "if( eType != GDT_Byte && (bECWV3 && eType!=GDT_UInt16 ) && !bIsJPEG2000 ) " at line 959 (after patch) is likely wrong. I think it should rather be : if( eType != GDT_Byte && !( bECWV3 && eType==GDT_UInt16 ) && !bIsJPEG2000 )

poDriver->SetMetadataItem( GDAL_DMD_CREATIONDATATYPES, "Byte UInt16" ); could be conditionalized by #if ECWSDK_VERSION >= 50 and just advertize Byte for older values.

In the doc, the "ECW Version 3 Files" section should rather mention GDAL 1.10 (unless a backport is intended to 1.9)

[jcztery]

Fixed version of patch.

[jcztery]

1. Added CPLFree(sCachedMultiBandIO.pabyData); back.
   
2. Added testing for SDK version in new tests.
   
3. Refactored "if( eType != GDT_Byte && (bECWV3 && eType!=GDT_UInt16 ) && !bIsJPEG2000 ) " so it is easier to understand and correct
   
4. Conditionalized Creationdatatypes by SDK version
   
5. Yeah. The backport is intended. Do i need to create another patch for different branch?
   

[jcztery]

Added user agent setting for ECWP.

[jcztery]

Forgot to check make previous attachment with the same name obsolete. GDAL_ECW_5.0.patch is obsoleted by GDAL_ECW_5.0.2.patch

GDAL_ECW_5.0.2.patch contains in addition :

1. When opening ECWP http user agent will be set
   
2. Put checking for write flag back
   

[jcztery]

Cumulative patch containing everything above + progress notification for SDK 5.0

[jcztery]

GDAL_ECW_5.0.3.patch contains in addition:

1. Compression progress notification for SDK 5.0.

[Even Rouault]

r25784 "ECW: integrate GDAL_ECW_5.0.3.patch with a few adjustments ( compilation fix for NCSecwSetConfig(NCSCFG_PROJECTION_FORMAT, NCS_PROJECTION_ERMAPPER_FORMAT) not available in SDK 4.x; logic to detect RGB or RGBA source input dataset broken; make a copy of strings when filling the papszBandDescriptions array; adjustment in NITF driver to correctly set color interpretation) (#5029)"

I've also change the doc to indicate that ECW v5 support is in GDAL 1.10 or later. I don't think the backport in 1.9 is reasonable. There are too many changes.

Note: the data/UInt16_big.tif wasn't included in the patch, so it should be attached to this ticket.

[Even Rouault]

Ah, as I've not access to ECW SDK v5, please test that the trunk after r25784 is fine.

[jcztery]

Missing UInt16_big.tif

[jcztery]

Additional patch to be run on top of previous (5.0.3)

[jcztery]

Added GDAL_ECW_NON_UNICODE_AND_WARNINGS.patch contains fixes for warnings and fixes issues with compilation without UNICODE being set (always).

[Even Rouault]

r25804 "ECW: Additional patch to be run on top of previous (5.0.3) (GDAL_ECW_NON_UNICODE_AND_WARNINGS.patch, #5029)"

UInt16_big.tif isn't very convincing for a unit test for int16 data type: the statistics of the file show that values don't go over 255 ... and it is a bit big to be included in the autotest suite. Perhaps a more artificial file with full 16bit dynamics could be better compressed and test better int16 compression support.

[jcztery]

Metadata, Linux, and a few other things.

[jcztery]

GDAL_ECW_NON_UNICODE_AND_WARNINGS.patch (to be run on top of previous) contains:

1. Improved error messages from SDK 5.0.
   
2. Fixed issue with JP2/ECW with 4 bands where fourth band's color interpretation was always returned as Alpha for sRGB.
   
3. Set description on band.
   
4. When bForce = TRUE GetStatistics/DefaultHistogram will compute and save histogram/statistics.
   
5. Statistics won't be reported/saved for Alpha bands.
   
6. ECW v2 with opacity are not supported as output. Please use v3 files as output if source dataset has opacity.
   
7. Added metadata reading and preserving/updating during create copy.
   
8. NSSFileInfo memory is alocated on stack or on ECW SDK heap.
   

Linux changes:

9. SDK 5.0 does not require tbb any more. Removed references to it.
   
10. SDK 5.0 does not require DUNICODE and D_UNICODE any more.
    

I was not sure if i should/how to regenerate configure script using autoconf so i did not update it. Configure.in is updated however.
Please contact me at jacek.tomaka at intergraph.com if You would like to obtain prerelease SDK 5.0 version for GDAL driver testing purposes.

[Even Rouault]

r25849 "ECW: integrate GDAL_METADATA_AND_LINUX.patch with a few fixes to make it still work with SDK 3.3, changes in GetCompressionSoftwareName() and changes to how some metadata items are not copied from source ECW to target ECW (#5029)"

Would you have a sample ECW v3 file that could be used to test at least read support ? Ideally it should be freely redistribuable in the GDAL autotest suite for regression test purposes. For that, you could for example generate one with "gdal_translate autotest/gcore/data/stefan_full_rgba.tif stefan_full_rgba_ecwv3.ecw -of ecw -co ECW_FORMAT_VERSION=3". And possibly with some variations like some of the FILE_METADATA_ metadata items set in it and/or statistics etc..

It would also be good if you could ammend frmt_ecw.html to document the FILE_METADATA_* stuff.

[Even Rouault]

With ECW SDK v5, there's a remaining compiler warning. Not sure how it can be fixed :

ecwdataset.cpp: In static member function ‘static GDALDataset* ECWDataset::Open(GDALOpenInfo*, int)’:
ecwdataset.cpp:2173: warning: enumeration value ‘NCSCT_NUMVALUES’ not handled in switch

[Even Rouault]

Note: I get a segfault in the ECW SDK v5 when opening the following ecwp://

$ LD_LIBRARY_PATH=/home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release:$LD_LIBRARY_PATH gdb --args gdalinfo ecwp://iws.erdas.com/images/geodetic/world/landsat742.ecw
GNU gdb (GDB) 7.1-ubuntu
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/even/gdal/svn/trunk/gdal/apps/gdalinfo...done.
(gdb) r
Starting program: /home/even/gdal/svn/trunk/gdal/apps/gdalinfo ecwp://iws.erdas.com/images/geodetic/world/landsat742.ecw
[Thread debugging using libthread_db enabled]
[New Thread 0x7fffe5a9c700 (LWP 14685)]
[New Thread 0x7fffe529b700 (LWP 14686)]
[New Thread 0x7fffe4498700 (LWP 14687)]
[New Thread 0x7fffe3c97700 (LWP 14688)]
[New Thread 0x7fffe3496700 (LWP 14689)]
[New Thread 0x7fffe2c95700 (LWP 14690)]
[New Thread 0x7fffe2494700 (LWP 14691)]
terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_S_construct NULL not valid

Program received signal SIGABRT, Aborted.
0x00007fffec6a0b25 in *__GI_raise (sig=<value optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64	../nptl/sysdeps/unix/sysv/linux/raise.c: Aucun fichier ou dossier de ce type.
	in ../nptl/sysdeps/unix/sysv/linux/raise.c
(gdb) bt
#0  0x00007fffec6a0b25 in *__GI_raise (sig=<value optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007fffec6a4670 in *__GI_abort () at abort.c:92
#2  0x00007fffeccd68c5 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/libstdc++.so.6
#3  0x00007fffeccd4cf6 in ?? () from /usr/lib/libstdc++.so.6
#4  0x00007fffeccd4d23 in std::terminate() () from /usr/lib/libstdc++.so.6
#5  0x00007fffeccd4e1e in __cxa_throw () from /usr/lib/libstdc++.so.6
#6  0x00007fffecc70987 in std::__throw_logic_error(char const*) () from /usr/lib/libstdc++.so.6
#7  0x00007fffeccb2911 in ?? () from /usr/lib/libstdc++.so.6
#8  0x00007fffeccb2a33 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) ()
   from /usr/lib/libstdc++.so.6
#9  0x00007fffeefa0bb8 in NCScnetPostURLExtW () from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#10 0x00007fffeefa1489 in NCScnetPostURLW () from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#11 0x00007fffef2b37fe in NCS::SDK::CECWP3Client::Open(NCS::CString) () from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#12 0x00007fffef29ea0c in NCS::SDK::CECWP3IOStream::Open(NCS::CString const&, bool) () from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#13 0x00007fffef2af6eb in NCS::SDK::CFileBase::CFileFactory::Open(NCS::SDK::CFileClient*, NCS::CString const&, bool, NCS::CString const&) ()
   from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#14 0x00007fffef2996fd in NCS::SDK::CFileBase::sOpen(NCS::SDK::CFileClient*, NCS::CString const&, bool, NCS::CString const&) ()
   from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#15 0x00007fffef127362 in NCS::CView::OpenForRead(NCS::CString const&, NCS::CIOStream*, bool) () from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#16 0x00007fffef11e3fb in NCS::CView::Open(NCS::CString const&, bool, bool) () from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#17 0x00007fffef1203b1 in NCS::CView::Open(char*, bool, bool) () from /home/even/gdal/svn/trunk/gdal/ecwjp2_sdk/lib/x64/release/libNCSEcw.so
#18 0x00007ffff6cbae3d in ECWDataset::OpenFileView (pszDatasetName=0x657508 "ecwp://iws.erdas.com/images/geodetic/world/landsat742.ecw", bProgressive=false, 
    bUsingCustomStream=@0x7fffffff38f8, bWrite=false) at ecwdataset.cpp:1996
#19 0x00007ffff6cbb35c in ECWDataset::Open (poOpenInfo=0x7fffffff3a00, bIsJPEG2000=0) at ecwdataset.cpp:2122
#20 0x00007ffff6cbad99 in ECWDataset::OpenECW (poOpenInfo=0x7fffffff3a00) at ecwdataset.cpp:1972
#21 0x00007ffff70b55ef in GDALOpenInternal (oOpenInfo=..., papszAllowedDrivers=0x0) at gdaldataset.cpp:2282
#22 0x00007ffff70b540f in GDALOpenInternal (pszFilename=0x656910 "ecwp://iws.erdas.com/images/geodetic/world/landsat742.ecw", eAccess=GA_ReadOnly, papszAllowedDrivers=0x0)
    at gdaldataset.cpp:2231
#23 0x00007ffff70b53c9 in GDALOpen (pszFilename=0x656910 "ecwp://iws.erdas.com/images/geodetic/world/landsat742.ecw", eAccess=GA_ReadOnly) at gdaldataset.cpp:2222
#24 0x0000000000403c52 in main (argc=2, argv=0x6568d0) at gdalinfo.c:178
(gdb)

[jcztery]

As for crash - there is a new version of linux installer available to You.

[jcztery]

Regarding NCSCT_NUMVALUES - this is basically last value used if You need to iterate over this enum. We could either define default or define case. In either case it is an error.

[jcztery]

A bunch of V3 files.

[jcztery]

Files without metadata:
stefan_full_rgba_ecwv3.ecw
Generated from gdalinfo -stats stefan_full_rgba_ecwv3.ecw
stefan_full_rgba_ecwv3_stats.ecw
Generated from gdalinfo -hist stefan_full_rgba_ecwv3.ecw
stefan_full_rgba_ecwv3_stats_hist.ecw

Files with metadata
stefan_full_rgba_ecwv3_meta.ecw
Generated from gdalinfo -stats stefan_full_rgba_ecwv3_meta.ecw
stefan_full_rgba_ecwv3_meta_stats.ecw
Generated from gdalinfo -hist stefan_full_rgba_ecwv3_meta.ecw
stefan_full_rgba_ecwv3_meta_stats_hist.ecw

[jcztery]

Metadata documentation patch.

[Even Rouault]

ecw_documentation.patch committed in r25864

[jcztery]

FIXED_JP2_WRITE_SUPPORT_AND_MESSAGE_IS_MORE_EXPLICIT_ABT_ECW3

[jcztery]

1. Fixed problem with JP2 write (it failed because it tried to write FILE_METADATA_).
2. Changed error/warning message when ECW 3 format version is not specified.
3. Added comment about histograms. GDAL ECW driver shouldn't adjust histogram ranges when reading to the file. This will screw reading histogram in GDAL created elsewhere.

[Even Rouault]

FIXED_JP2_WRITE_SUPPORT_AND_MESSAGE_IS_MORE_EXPLICIT_ABT_ECW3.patch committed in trunk (r25923) and branches/1.10 (r25924)

As far as histograms are concerned, I believe that my change was correct (it should be balanced with the change in both GetDefaultHistogram() and SetDefaultHistogram()). GDAL convention is to report the min and max bounds with a half bucket width offset. For example if you compute 256 buckets for a range of values in [0;255], GDAL will report statistics for -0.5 --> 0.5; 0.5 --> 1.5; ... 254.5 --> 255.5. The change was necessary because I noticed that there was an inconsistency between statistics reported with PAM only (ECW v2) and with embedded statistics of ECW v3. From the user point of view, this didn't make sense.

Ah, please fix your editor to use spaces and not tabulations.