Opened 12 years ago
Closed 10 years ago
#4575 closed defect (fixed)
Kakadu 7 requires changes to jp2kakdataset
Reported by: | Ari Jolma | Owned by: | warmerdam |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | ConfigBuild | Version: | unspecified |
Severity: | normal | Keywords: | jp2kak |
Cc: | rprinceley, bledoux |
Description
jp2kakdataset.cpp:2037: lines[c].pre_create() requires two new arguments extend_left and extend_right (see kdu_sample_processing.h:433)
jp2kakdataset.cpp:2150: engines[c].push() does not accept boolean argument after line (see kdu_sample_processing.h:901)
I tried setting extend_left and extend_right to 0 and removing the boolean argument and the driver works but there may be subtle issues.
Change History (18)
comment:1 by , 12 years ago
Cc: | added |
---|
comment:2 by , 11 years ago
comment:3 by , 11 years ago
Cc: | added |
---|---|
Component: | default → ConfigBuild |
Keywords: | jp2kak added |
Status: | new → assigned |
I am not seeing this in trunk on 64bit linux with Kakadu 7.2.2:
warmerdam@gdal:~/gdal/frmts/jp2kak$ make g++ -fPIC -g -DDEBUG -DHAVE_SSE_AT_COMPILE_TIME -Wall -I/usr/local/google/home/warmerdam/gdal/port -I/usr/local/google/home/warmerdam/gdal/gcore -I/usr/local/google/home/warmerdam/gdal/alg -I/usr/local/google/home/warmerdam/gdal/ogr -I/usr/local/google/home/warmerdam/gdal/ogr/ogrsf_frmts -I/usr/local/google/pkg/kakadu-7.2.2/coresys/common -I/usr/local/google/pkg/kakadu-7.2.2/apps/compressed_io -I/usr/local/google/pkg/kakadu-7.2.2/apps/jp2 -I/usr/local/google/pkg/kakadu-7.2.2/apps/image -I/usr/local/google/pkg/kakadu-7.2.2/apps/args -I/usr/local/google/pkg/kakadu-7.2.2/apps/support -I/usr/local/google/pkg/kakadu-7.2.2/apps/kdu_compress -I. -DOGR_ENABLED -D_REENTRANT -I/usr/local/google/home/warmerdam/gdal/port -I/usr/include -I/usr/local/google/bld/include -c -o jp2kakdataset.o jp2kakdataset.cpp warmerdam@gdal:~/gdal/frmts/jp2kak$
The line number quoted does not appear to be current trunk. Could someone encountering the problem quote their exact compile line and error with trunk?
Note that the pre_create line looks like this in trunk:
#if KAKADU_VERSION >= 700 lines[c].pre_create(&allocator,nXSize,bReversible,bReversible,0,0); #else lines[c].pre_create(&allocator,nXSize,bReversible,bReversible); #endif
Near the top of the file version is determined like this:
// Generally Kakadu does not advertise its version well, so we look for a // clue to V6 vs V7, the main difference in api. #ifndef KAKADU_VERSION # ifdef KDU_TARGET_CAP_SEQUENTIAL # define KAKADU_VERSION 700 # else # define KAKADU_VERSION 600 # endif #endif
comment:4 by , 11 years ago
Hello Frank,
Here is the complete compilation error we get with gdal 1.9.2 :
g++ -m32 -fPIC -Wall -I./gdal-1.9.2/port -I./gdal-1.9.2/gcore -I./60_gdal-1.9.2/gdal-1.9.2/alg -I./gdal-1.9.2/ogr -I./gdal-1.9.2/ogr/ogrsf_frmts -I./kakadu/coresys/common -I./kakadu/apps/compressed_io -I./kakadu/apps/jp2 -I./kakadu/apps/image -I.kakadu/apps/args -I./kakadu/apps/support -I.kakadu/apps/kdu_compress -I. -DOGR_ENABLED -m32 -I./gdal-1.9.2/port -c -o ../o/jp2kakdataset.o jp2kakdataset.cpp [2013-06-24 12:44:22] jp2kakdataset.cpp: In function Ôint JP2KAKCreateCopy_WriteTile(GDALDataset*, kdu_tile&, kdu_roi_image*, int, int, int, int, bool, int, GDALDataType, kdu_codestream&, int, kdu_long*, int, GDALProgressFunc, void*, bool)Ô: [2013-06-24 12:44:22] jp2kakdataset.cpp:2016:70: error: no matching function for call to Ôkdu_line_buf::pre_create(kdu_sample_allocator*, int&, bool&, bool&)Ô [2013-06-24 12:44:22] jp2kakdataset.cpp:2016:70: note: candidate is: [2013-06-24 12:44:22] ./kakadu/coresys/common/kdu_sample_processing.h:430:10: note: void kdu_line_buf::pre_create(kdu_sample_allocator*, int, bool, bool, int, int) [2013-06-24 12:44:22] ./kakadu/coresys/common/kdu_sample_processing.h:430:10: note: candidate expects 6 arguments, 4 provided
Best Regards,
Bruno
comment:5 by , 11 years ago
Bruno, Frank mentionned trunk and not GDAL 1.9.2. Actually I can see that the support for Kakadu 7 was done in r25950, which was even after GDAL 1.10 release, so this is expected.
comment:6 by , 10 years ago
Building against v7.1, KDU_TARGET_CAP_SEQUENTIAL doesn't appear to be defined in the Kakadu headers, but manually adding CXXFLAGS := $(CXXFLAGS) -DKAKADU_VERSION=710
to frmts/jp2kak/GNUMakefile
works fine.
Will re-check with 7.2.2 in a few days.
comment:8 by , 10 years ago
r27264 handles the v7's allocator.finalize(oCodeStream). Tested with v7_3_3.
comment:10 by , 10 years ago
@goatbear shouldn't it be #if KAKADU_VERSION >= 730
? Not that there's a way of specifying it at compile time except via Makefile hacks. Maybe I should fix that :)
I might give lobbying Kakadu to at least add a version identifier a go... these API changes are getting kind of silly now, and it feels like we'll (easily) be breaking older versions without knowing if it keeps up.
comment:11 by , 10 years ago
r27264 breaks compilation with Kakadu 7.3 (.0?)
jp2kakdataset.cpp: In function ‘int JP2KAKCreateCopy_WriteTile(GDALDataset*, kdu_tile&, kdu_roi_image*, int, int, int, int, bool, int, GDALDataType, kdu_codestream&, int, kdu_long*, int, int (*)(double, const char*, void*), void*, bool)’: jp2kakdataset.cpp:1900: error: no matching function for call to ‘kdu_sample_allocator::finalize(kdu_codestream&)’ /home/foo/kakadu/coresys/common/kdu_sample_processing.h:305: note: candidates are: void kdu_sample_allocator::finalize()
comment:12 by , 10 years ago
Reverted in r27265. I only have kakadu v7_3_2, v7_3_3, and v6_4_1. We have upstreamed almost all of our local fixes of kakadu, but have not got them to do a matching version number.
I do see this in coresys/common/kdu_compressed.h, but it's not in the most useful format:
#define KDU_CORE_VERSION "v7.3.3"
comment:13 by , 10 years ago
fwiw, we first ran into the finalize()
issue with a 7.3.2 build.
AFAIK splitting the KDU_CORE_VERSION string back into number(s) is Really Hard(TM) at compile time (I looked into it), would be magnitudes easier for them to add a KDU_MAJOR_VERSION, KDU_MINOR_VERSION KDU_BUILD_VERSION etc
Another option is for the gdal jp2kak Makefile to figure out the version by parsing that header file/string, then define KAKADU_VERSION from it?
comment:14 by , 10 years ago
One idea would indeed to have a KAKADU_VERSION macro. Unix configure could detect the version with by compiling tiny .cpp that would try to use a function (see how it is done for poppler that has the same issue). And for Windows, people would have to define it manually in nmake.local.
comment:15 by , 10 years ago
Is there a way in the configure step to set KAKADU_VERSION in cpl_config.h? I can think of two approaches off hand:
- use the preprocessor to get the KDU_CORE_VERSION from kdu_compress.h
- use a small c or c++ program to get an/or write the version info
I've checked back to v5_1-00418C:
#define KDU_CORE_VERSION "v5.1" #define KDU_CORE_VERSION "v6.4.1" #define KDU_CORE_VERSION "v7.3.3"
Probably not the best ever code, but here is some quick c++ that could output what would need to be added to config.h. I'm not sure how to do this within autoconf and the I am not attached to the names of the defines. And... The KAKADU_VERSION will fail to work is the MINOR or MINOR_MINOR is ever more than a single digit.
#include <iostream> #include <sstream> #include <string> #include <vector> #include <kdu_compressed.h> using namespace std; int main(int argc, char **argv) { stringstream stream(string(KDU_CORE_VERSION)); vector<string> parts; string part; while (getline(stream, part, '.')) { parts.push_back(part); } if (parts.size()==2) parts.push_back(string("0")); parts[0].erase(0,1); cout << "#define KDU_MAJOR_VERSION " << parts[0] << endl; cout << "#define KDU_MINOR_VERSION " << parts[1] << endl; cout << "#define KDU_MINOR_MINOR_VERSION " << parts[2] << endl; cout << "#define KAKADU_VERSION " << parts[0] << parts[1] << parts[2] << endl; return 0; }
comment:16 by , 10 years ago
Here is an example of using the preprocessor to get a version number:
http://git.savannah.gnu.org/cgit/autoconf-archive.git/tree/m4/ax_path_bdb.m4#n181
HEADER_VERSION=`eval "$ac_cpp conftest.$ac_ext" 2> /dev/null \ | grep AX_PATH_BDB_STUFF | sed 's/[[^0-9,]]//g;s/,/./g;1q'` ],[])
And there is also ax_absolute_header.m4:
http://git.savannah.gnu.org/cgit/autoconf-archive.git/tree/m4/ax_absolute_header.m4
Basically, I was thinking something along the lines of
grep KDU_CORE_VERSION kdu_compressed.h | grep define | cut -dv -f2 | cut -d. -f1 grep KDU_CORE_VERSION kdu_compressed.h | grep define | cut -dv -f2 | cut -d. -f2 grep KDU_CORE_VERSION kdu_compressed.h | grep define | cut -dv -f2 | cut -d. -f3 | cut -d\" -f1
comment:17 by , 10 years ago
Progress upstream!
https://groups.yahoo.com/neo/groups/kakadu_jpeg2000/conversations/messages/6928
We will add such #defines, exactly as suggested, to all future releases.
#define KDU_CORE_VERSION="v7.3.3" // existing #define KDU_MAJOR_VERSION=7 #define KDU_MINOR_VERSION=3 #define KDU_PATCH_VERSION=3
comment:18 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
r27787 (trunk) starts using the new official Kakadu version headers available from v7.5
Update the JP2KAK and JPIPKAK drivers for compatibility with Kakadu v7.5.
This essentially is just the new kdu_core
and kdu_supp
C++ namespaces.
Consistently use the new KDU_{MAJOR,MINOR,PATCH}_VERSION defines instead of the compile-time KAKADU_VERSION define that we unofficially supported and tried badly to autodetect. For older Kakadu releases, require that the user specify the new-style defines at compile-time.
For older v7.x versions the autodetection was so badly broken, I think it's safer and better to go back to specifying it at compile time:
CXXFLAGS="KDU_MAJOR_VERSION=7 KDU_MINOR_VERSION=3 KDU_PATCH_VERSION=0" ./configure
If someone feels particularly strongly I guess we could add something to the configure script to specify a version and define those variables? ala ./configure --with-kakadu=/path/to/kakadu --kakadu-version=7.3.0
I've updated all the #ifdef logic to use the new defines as well.
We ran into the same issue as ajolma while upgrading to Kakadu 7.2 and had to step back to version 6.4. Are there any plans to provide a fix soon ?
Best Regards.