[Insight-developers] openjpg include dir not exported

[Bradley Lowekamp]

On Aug 28, 2010, at 3:49 PM, Luis Ibanez wrote:

> Hi Brad,
> 
> The changes look good for the most part.
> 
> 
> There are only two issues:
> 
> A) In http://review.source.kitware.com/#change,5
> 
> The new code is incorrect:
> http://review.source.kitware.com/#patch,sidebyside,5,1,Code/Review/itkJPEG2000ImageIO.cxx
> 
> The following code in lines 86 to 91 is incorrect
> 
> if( extension == ".j2k" |
> extension == ".j2k" |
> extension == ".j2k" )
> {
> return true;
> }
> 
> The operator "|" is a boolean OR. The code should use the logical "OR" operator "||".
> 
> Also the three extensions are the same. (copy pasting ?)
> 
> The code probably should have been
> 
> if( extension == ".j2k" ||
> extension == ".jp2" ||
> extension == ".jpt" )
> {
> return true;
> }
> 
> This made fail the test   itkJPEG2000Test02, since the Cevennes2.jp2 
> image was not recognized as a valid extension for JPEG2000.

I rebased, amended and pushed to stage and github... updating Gerrit will be a little more complicated.

I blame the poor conflict resolution for this. I don't know how I could have possible done such a silly mistake on my own :)

> 
> 
> B) The change:
> http://review.source.kitware.com/#change,7
> 
> Has some logic issues, but they are actually more rooted
> on the fact that ITK doesn't have an explicit notion of 
> "Color" image. This requires a larger discussion.
> 
> In its current form we assume that an image of 3 components
> is RGB, and that more than 3 must be a vector. This is incorrect
> for both RGBA, and for deformation fields in 3D.

There are some details of the file format that I don't understand. However, my tools don't work when a ImageIO, reports a multi-component scalar, as it is a logic error in many ways.

I based this logic on the same logic that was in the regular jpeg io.

It is my understanding that many of the lossy compression algorithms available for jpeg2000 are based on human perception. That is they will give more accuracy to the luminance over say the blue channel by perform compression of variable bits in opponent color space. We really don't want to be using such a space for vectors either. Additionally, with only supporting 8bit and 16bit unsigned values, its much closer to multi-channel color then a vector space.

Likely we will need to look closer into the color space field of the image format to make a better determination of the pixel type, and see how the compression algorithm interacts with this.

The ImageIO does only supports 1 channel and 3 channels currently for input and output.

As for the style of a trinary operator vs seven lines of code, I just found seven to be too many.


> 
> 
> -----
> 
> How do you want to proceed from here ?
> 
> Would you like to fix the bug in the stage first ?
> 
> or should I merge with the openjpegpimpl branch 
> and fix the bug in the process ?
> 
> 
>    Please let me know,
> 
> 
>       Thanks
> 
> 
>           Luis
> 
> 
> 
> -----------------------------------------------------------------------
> On Sat, Aug 28, 2010 at 2:02 PM, Bradley Lowekamp <blowekamp at mail.nih.gov> wrote:
> Luis, 
> 
> That is a great idea to use a private implementation. Should help with compile time too (every little bit helps)!
> 
> Unfortunately, your patch is going to conflict with mine. This is interesting in Gerrit because I would expect something to come up to showing that there is a conflict between these two topic. But I don't see anything.
> 
> Luis you started to review my topic. Do you think it's good to go? So I can hurry up to stage and merge my topic, before yours :P ( I already had the burden of rebasing and merging after the uncrustification ) 
> 
> Brad
> 
> On Aug 28, 2010, at 12:17 PM, Luis Ibanez wrote:
> 
>> BTW, 
>> 
>> We probably could use PIMPL in the itkJPEG2000ImageIO
>> class, so that the headers and data structures of openjpeg
>> are not exposed to ITK developers...
>> 
>> I'll give it a try...
>> 
>> 
>>      Luis
>> 
>> 
>> --------------------------------------------------------------------------
>> On Sat, Aug 28, 2010 at 12:15 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>> 
>> Hi Gaetan,
>> 
>> 
>> Thanks for the clarification.
>> 
>> 
>> Can you please try this patch:
>> 
>> diff --git a/itkIncludeDirectories.cmake b/itkIncludeDirectories.cmake
>> index 36b2ab5..38584fb 100644
>> --- a/itkIncludeDirectories.cmake
>> +++ b/itkIncludeDirectories.cmake
>> @@ -37,6 +37,7 @@ SET(ITK_INCLUDE_DIRS_BUILD_TREE ${ITK_INCLUDE_DIRS_BUILD_TREE}
>>    ${ITK_SOURCE_DIR}/Utilities/nifti/niftilib
>>    ${ITK_SOURCE_DIR}/Utilities/nifti/znzlib
>>    ${ITK_SOURCE_DIR}/Utilities/itkExtHdrs
>> +  ${ITK_SOURCE_DIR}/Utilities/openjpeg
>>    ${ITK_BINARY_DIR}/Utilities
>>    ${ITK_SOURCE_DIR}/Utilities
>>  )
>> @@ -149,6 +150,7 @@ SET(ITK_INCLUDE_RELATIVE_DIRS ${ITK_INCLUDE_RELATIVE_DIRS}
>>    Utilities/nifti/niftilib
>>    Utilities/nifti/znzlib
>>    Utilities/itkExtHdrs
>> +  Utilities/openjpeg
>>    Utilities
>>  )
>> 
>> 
>> and if it works, 
>> please go ahead and push the change.
>> 
>> 
>>      Thanks
>> 
>> 
>>              Luis
>> 
>> 
>> -----------------------------------------------------------
>> 2010/8/28 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>
>> 
>> 
>> Le 28 août 10 à 00:53, Luis Ibanez a écrit :
>> 
>> 
>> Hi Gaetan,
>> 
>> Hi Luis,
>> 
>> 
>> 
>> Are you doing this with an installed version of ITK ?
>> 
>> No, I'm using it in the build tree.
>> 
>> 
>> 
>> I just verified that I can use JPEG2000ImageIO
>> from a small example outside of the ITK build tree.
>> 
>> (Note that you must have configured ITK with
>>  ITK_USE_REVIEW ON).
>> 
>> Yes, that's on.
>> 
>> 
>> 
>> Please let me know how I can reproduce the problem
>> that you are observing.
>> 
>> I'm trying to add JPEG2000ImageIO to wrapitk, but openjpeg.h can't be found.
>> I have to add the path to this file in ITK_INCLUDE_DIRS in ITKConfig.cmake
>> 
>> Without this manual fix, I get the following error message:
>> 
>> [ 80%] Generating wrap_ITKIOBase.xml
>> In file included from /home/glehmann/src/tests/wrapitk/build/Libraries/IO/wrap_ITKIOBase.cxx:77:
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:35:24: error: openjpeg.h: No such file or directory
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:36:19: error: j2k.h: No such file or directory
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:37:19: error: jp2.h: No such file or directory
>> In file included from /home/glehmann/src/tests/wrapitk/build/Libraries/IO/wrap_ITKIOBase.cxx:77:
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:111: error: 'opj_dparameters_t' does not name a type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:130: error: ISO C++ forbids declaration of 'opj_codec_t' with no type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:130: error: expected ';' before '*' token
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:132: error: 'OPJ_UINT32' does not name a type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:133: error: 'OPJ_UINT32' does not name a type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:135: error: 'OPJ_UINT32' does not name a type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:136: error: 'OPJ_UINT32' does not name a type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:138: error: 'OPJ_UINT32' does not name a type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:139: error: 'OPJ_UINT32' does not name a type
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h: In member function 'void itk::JPEG2000ImageIO::SetTileSize(int, int)':
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:102: error: 'm_TileWidth' was not declared in this scope
>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:103: error: 'm_TileHeight' was not declared in this scope
>> make[3]: *** [Libraries/IO/wrap_ITKIOBase.xml] Erreur 1
>> make[2]: *** [Libraries/IO/CMakeFiles/IOIdx.dir/all] Erreur 2
>> make[1]: *** [Libraries/IO/CMakeFiles/IOPython.dir/rule] Erreur 2
>> make: *** [IOPython] Erreur 2
>> 
>> Gaëtan
>> 
>> 
>> 
>> 
>> 
>>     Thanks
>> 
>> 
>>         Luis
>> 
>> 
>> -------------------
>> 2010/8/27 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>
>> 
>> Hi,
>> 
>> I found a small problem with openjpg: the include dir is not exported, and thus openjpeg.h can't be found while using JPEG2000ImageIO outside of the ITK build tree.
>> I tried to fix that but I'm not sure how to do it right.
>> 
>> It would be nice if someone used to how the utilities are managed in ITK can fix that!
>> 
>> Thanks,
>> 
>> Gaëtan
>> 
>> 
>> -- 
>> Gaëtan Lehmann
>> Biologie du Développement et de la Reproduction
>> INRA de Jouy-en-Josas (France)
>> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>> http://voxel.jouy.inra.fr  http://www.itk.org
>> http://www.mandriva.org  http://www.bepo.fr
>> 
>> 
>> _______________________________________________
>> Powered by www.kitware.com
>> 
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>> 
>> Kitware offers ITK Training Courses, for more information visit:
>> http://kitware.com/products/protraining.html
>> 
>> Please keep messages on-topic and check the ITK FAQ at:
>> http://www.itk.org/Wiki/ITK_FAQ
>> 
>> Follow this link to subscribe/unsubscribe:
>> http://www.itk.org/mailman/listinfo/insight-developers
>> 
>> 
>> 
>> -- 
>> Gaëtan Lehmann
>> Biologie du Développement et de la Reproduction
>> INRA de Jouy-en-Josas (France)
>> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>> http://voxel.jouy.inra.fr  http://www.itk.org
>> http://www.mandriva.org  http://www.bepo.fr
>> 
>> 
>> 
>> <ATT00001..txt>
> 
> 
> _______________________________________________
> Powered by www.kitware.com
> 
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
> 
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
> 
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
> 
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100828/31813fe5/attachment-0001.htm>