[PATCH] JP2KAK creates incorrect files on big-endian platforms

[jeepingben]

When creating a jp2 file using jp2kak in gdal on a big-endian platform, the box identifiers ('uuid', 'asoc', ' xml') are stored backwards in the file ('diuu', 'cosa', 'lmx '). This seems to be due to an unconditional byte-swap in GDALJP2Box::SetType()

[Even Rouault]

Actually the whole byte-swapping around szBoxType seem to be overly complicated, both in SetType() and CreateAsocBox(). It is not sane that SetType() / GetType() does not round-trip. It would be more appropriate that the user code of GetType() does the byte-swapping if it wants to use the string as a numeric value, so JP2KAKWriteBox() likely needs to do CPL_MSBPTR32() on the result to preserve correct behaviour on little-endian platforms.

Could you try the attached patch and report ? I don't have access to Kakadu, so it would be nice if you could test the patch on a little-endian platform too to see if it hasn't broken it.

[jeepingben]

Thanks rouault.

I applied the patch to gdal-1.7.2 and it worked correctly for solaris-10 sparc and solaris-10 x86.

[Even Rouault]

jeepingben, before applying and as I cannot test myself, I'd like to be sure of the extent of testing you did. I'd suggest doing the follwing :

• generate a jp2 file with JP2 boxes on the sparc and read it on the sparc and x86
• generate a jp2 file with JP2 boxes on the x86 and read it on the sparc and x86

That way we are sure to have full interoperability between both endianness. Could you report the result of those tests? Could you also attach the produced files (as small as possible) to the ticket ? I'd like to be sure I can also read them with other jpeg2000 (non kakadu based) drivers. You can use as a source a small image like http://trac.osgeo.org/gdal/export/23074/trunk/autotest/gcore/data/byte.tif

Thanks

[jeepingben]

Tif converted to JP2 on sparc

[jeepingben]

Tif converted to jp2 on solaris-x86

[jeepingben]

I've attached the 2 files. These were passed through the product I am working on which means they were converted to NITF, then to JPEG2000. Previously I tested by looking in an editor to be sure the box delimeters were written correctly. I also checked the autotests of the product I am working on and that showed that the jp2 images created with your patch were producing valid images on either endian.

Thanks again for the patch, Ben

[Even Rouault]

Thanks for the reports. I also add to fix the ECW driver similarly to JP2KAK. I've looked at other potential users of GDALJP2Box::GetType() in GDAL code and ECW and JP2KAK showed up as the only users, so that should be all for now. Anyway, this impact on drivers show clearly that it is definitely not material for a stable branch.

r23076 /trunk/gdal/ (3 files in 3 dirs): GDALJP2Box::SetType() : remove byte-swapping so that SetType()/GetType() correctly round-trips. Consequently, use CPL_MSBPTR32() on GetType() result in JP2KAK and ECW drivers. (#4239)