bugGNU Octave - Bugs: bug #42115, image package: imadjust does not...

 
 

bug #42115: image package: imadjust does not support integers

Submitter:  None
Submitted:  Mon 14 Apr 2014 11:34:18 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  None Assigned to:  None
Originator Name:  Anonymous Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * other
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 20 Nov 2015 08:16:19 PM UTC, comment #11: 

(I haven't managed to "make" the whole image package. But the error message from the make command showed me what parameters for mkoctfile were necessary... So here I am...)

I have used the new version of imadjust.m (also using intlut.cc, is_image.m and stretchlim.m) a bit with uint8 images. And I have NOT come across any obvious errors. So, thank you for expanding the integer support in the image package by a significant bit!


Hartmut <hardy>
Fri 20 Nov 2015 11:11:23 AM UTC, comment #10: 


> I tried to do it the lazy way, and wanted to simply compile the new file intlut.cc from inside a normal Octave 4.0 (release) installation (in Ubuntu 14.04, compiling oct files normally works).


That will work if you set the right flags. The image package will handle the flags for you. Just run make install (or make run if you want to just try the package without installing).

Carnë Draug <carandraug>
Group Member
Thu 19 Nov 2015 07:41:03 PM UTC, comment #9: 

(I am NOT sure if this is a bug at all...)

I tried to do it the lazy way, and wanted to simply compile the new file intlut.cc from inside a normal Octave 4.0 (release) installation  (in Ubuntu 14.04, compiling oct files normally works).

But when I call "mkoctfile intlut.cc", then I get the following lengthy error message:


intlut.cc: In function ‘T intlut(const T&, const T&)’:
intlut.cc:45:15: error: ISO C++ forbids declaration of ‘A_vec’ with no type [-fpermissive]
   const auto* A_vec = A.fortran_vec ();
               ^
intlut.cc:46:15: error: ISO C++ forbids declaration of ‘lut_vec’ with no type [-fpermissive]
   const auto* lut_vec = lut.fortran_vec ();
               ^
intlut.cc:49:9: error: ISO C++ forbids declaration of ‘B_vec’ with no type [-fpermissive]
   auto* B_vec = B.fortran_vec ();
         ^
intlut.cc: In instantiation of ‘T intlut(const T&, const T&) [with T = intNDArray<octave_int<unsigned char> >]’:
intlut.cc:123:3:   required from here
intlut.cc:45:38: error: cannot convert ‘const octave_int<unsigned char>*’ to ‘const int*’ in initialization
   const auto* A_vec = A.fortran_vec ();
                                      ^
intlut.cc:46:42: error: cannot convert ‘const octave_int<unsigned char>*’ to ‘const int*’ in initialization
   const auto* lut_vec = lut.fortran_vec ();
                                          ^
intlut.cc:49:32: error: cannot convert ‘octave_int<unsigned char>*’ to ‘int*’ in initialization
   auto* B_vec = B.fortran_vec ();
                                ^
intlut.cc:55:69: error: no matching function for call to ‘intlut_index(P_val_type, const int*&)’
     *B_vec = intlut_index (static_cast<P_val_type> (*A_vec), lut_vec);
                                                                     ^
intlut.cc:55:69: note: candidate is:
intlut.cc:28:1: note: template<class P> P intlut_index(typename P::val_type, const P*)
 intlut_index (const typename P::val_type A, const P lut_vec[])
 ^
intlut.cc:28:1: note:   template argument deduction/substitution failed:
intlut.cc: In substitution of ‘template<class P> P intlut_index(typename P::val_type, const P*) [with P = int]’:
intlut.cc:55:69:   required from ‘T intlut(const T&, const T&) [with T = intNDArray<octave_int<unsigned char> >]’
intlut.cc:123:3:   required from here
intlut.cc:28:1: error: ‘int’ is not a class, struct, or union type
intlut.cc: In instantiation of ‘T intlut(const T&, const T&) [with T = intNDArray<octave_int<short unsigned int> >]’:
intlut.cc:124:8:   required from here
intlut.cc:45:38: error: cannot convert ‘const octave_int<short unsigned int>*’ to ‘const int*’ in initialization
   const auto* A_vec = A.fortran_vec ();
                                      ^
intlut.cc:46:42: error: cannot convert ‘const octave_int<short unsigned int>*’ to ‘const int*’ in initialization
   const auto* lut_vec = lut.fortran_vec ();
                                          ^
intlut.cc:49:32: error: cannot convert ‘octave_int<short unsigned int>*’ to ‘int*’ in initialization
   auto* B_vec = B.fortran_vec ();
                                ^
intlut.cc:55:69: error: no matching function for call to ‘intlut_index(P_val_type, const int*&)’
     *B_vec = intlut_index (static_cast<P_val_type> (*A_vec), lut_vec);
                                                                     ^
intlut.cc:55:69: note: candidate is:
intlut.cc:28:1: note: template<class P> P intlut_index(typename P::val_type, const P*)
 intlut_index (const typename P::val_type A, const P lut_vec[])
 ^
intlut.cc:28:1: note:   template argument deduction/substitution failed:
intlut.cc: In substitution of ‘template<class P> P intlut_index(typename P::val_type, const P*) [with P = int]’:
intlut.cc:55:69:   required from ‘T intlut(const T&, const T&) [with T = intNDArray<octave_int<short unsigned int> >]’
intlut.cc:124:8:   required from here
intlut.cc:28:1: error: ‘int’ is not a class, struct, or union type
intlut.cc: In instantiation of ‘T intlut(const T&, const T&) [with T = intNDArray<octave_int<short int> >]’:
intlut.cc:125:8:   required from here
intlut.cc:45:38: error: cannot convert ‘const octave_int<short int>*’ to ‘const int*’ in initialization
   const auto* A_vec = A.fortran_vec ();
                                      ^
intlut.cc:46:42: error: cannot convert ‘const octave_int<short int>*’ to ‘const int*’ in initialization
   const auto* lut_vec = lut.fortran_vec ();
                                          ^
intlut.cc:49:32: error: cannot convert ‘octave_int<short int>*’ to ‘int*’ in initialization
   auto* B_vec = B.fortran_vec ();
                                ^
intlut.cc:55:69: error: no matching function for call to ‘intlut_index(P_val_type, const int*&)’
     *B_vec = intlut_index (static_cast<P_val_type> (*A_vec), lut_vec);
                                                                     ^
intlut.cc:55:69: note: candidate is:
intlut.cc:28:1: note: template<class P> P intlut_index(typename P::val_type, const P*)
 intlut_index (const typename P::val_type A, const P lut_vec[])
 ^
intlut.cc:28:1: note:   template argument deduction/substitution failed:
intlut.cc: In substitution of ‘template<class P> P intlut_index(typename P::val_type, const P*) [with P = int]’:
intlut.cc:55:69:   required from ‘T intlut(const T&, const T&) [with T = intNDArray<octave_int<short int> >]’
intlut.cc:125:8:   required from here
intlut.cc:28:1: error: ‘int’ is not a class, struct, or union type
warning: mkoctfile exited with failure status
warning: called from
    mkoctfile at line 171 column 5


Is this a problem? Or would I need to compile the whole image repository in order to test this single new file intlut.cc?

Hartmut <hardy>
Wed 18 Nov 2015 09:04:01 PM UTC, comment #8: 

As I mentioned before, I pushed some more changes to cover special cases, only for sake of performance

http://hg.code.sf.net/p/octave/image/rev/8dd432a8ad92

I'll consider it all done now but would welcome any feedback and testing.

Carnë Draug <carandraug>
Group Member
Wed 18 Nov 2015 02:51:01 PM UTC, comment #7: 

I have pushed http://hg.code.sf.net/p/octave/image/rev/b9120f602e44 which adds support for images of class integer. It uses the approach suggested by Hartmut. It still needs some performance improvements but I already have some ideas how to do it. Testing would be welcome.

This will be part of the next minor release, 2.6.0 or later.

Carnë Draug <carandraug>
Group Member
Tue 17 Nov 2015 01:50:06 PM UTC, comment #6: 

I like the LUT alternative.

Unfortunately, as you noticed, intlut also converted to double. I rewrote it in C++ and now performs 5-10x faster and without the bump in memory usage (I'm testing with 500MB uint8 images). I have also been making a bunch of other changes to imadjust, fixing this step by step.

Carnë Draug <carandraug>
Group Member
Sun 01 Nov 2015 08:26:45 PM UTC, comment #5: 

How about this alternative approach to deal with integer inputs to imadjust?

  • First, create a lookup table (LUT) where you specify the intended (integer) output value for every possible (integer) input value (there might be for example 256 possibilities, or 65536 ones, depending on the input type). For this creation of a LUT, one could use a conversion to double and then back to integers, because this wouldn't significantly increase the memory consumption. (There are much less possible integer values than pixels in a typical image.)


  • Second, use the existing function intlut.m to convert the integer input image into the integer output image. Since intlut (assumingly) works without an intermediate conversion to double, we wouldn't need significantly more memory for this step either.


Sorry, I've just realised that intlut.m DOES convert the input image to double in an intermediate step. So this approach would also consume much much more memory :(

We could fix this by

  • fixing intlut to NOT convert to double as an intermediate step.  (But I am not sure why intlut does this conversion anyways, so I've probably missed something there.)
  • doing something similar to intlut, but as own code inside of the function imadjust itself. Just WITHOUT the step of intermediate conversion to double type.



Hartmut <hardy>
Mon 19 Oct 2015 09:10:16 PM UTC, comment #4: 

If you have a better solution, please push it. Otherwise,
the attached cs solves the problem, and should also solve
https://savannah.gnu.org/bugs/?40109.

Avinoam Kalma <avinoam>
Group Member
Mon 24 Aug 2015 05:48:41 PM UTC, comment #3: 

I would rather have a nicer fix for this, within imadjust. One that ideally won't cause the image to suddenly take 8x more memory. While the attached changeset is a good temporary workaround, I'd rather have the user do it themselves than apply a quick fix that will most likely become permanent.

Carnë Draug <carandraug>
Group Member
Sun 16 Aug 2015 10:09:20 PM UTC, comment #2: 


Please review the attached cs.
It transform the image to double and then back to the original class.


(file #34661)

Avinoam Kalma <avinoam>
Group Member
Mon 14 Apr 2014 02:31:42 PM UTC, comment #1: 

This is a known bug. Unfortunately, we haven't come around fixing it. The very first problem is that imadjust expects image of class double, i.e., values in the range [0 1]. If you can provide a patch that would be perfect.

In the mean time, assuming your original image was uint8, you can work around this using:


im2uint8 (imadjust (im2double (img), [0; 0.8], [0; 0.7]))


Carnë Draug <carandraug>
Group Member
Mon 14 Apr 2014 11:34:18 AM UTC, original submission:  

# image *|   2.2.1


    octave:184> img=imread('img.bmp');
    octave:185> n = imadjust(img, [0; 0.8], [0; 0.7]);
    octave:186> sum(sum(n))
    ans =
    ans(:,:,1) =               307200
    ans(:,:,2) =               307200
    ans(:,:,3) =               307199
    >> img=imread('img.bmp');
    >> n = imadjust(img, [0; 0.8], [0; 0.7]);
    >> sum(sum(n))
    ans(:,:,1) =
        37132769
    ans(:,:,2) =
        36824936
    ans(:,:,3) =
        37609049


Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #34661:  bug42115.cs added by avinoam (1KiB - text/plain)

 

Carbon-Copy List
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by avinoam (Updated the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-11-18 carandraug Originator Namenotagain Anonymous
        Open/ClosedOpen Closed
        Release3.8.1 other
        Summaryimage package: imadjust output differs from Matlab image package: imadjust does not support integers
    2015-11-10 carandraug Dependencies- bugs #40109 is dependent
    2015-08-16 avinoam Attached File- Added bug42115.cs, #34661
    2014-09-28 carandraug Dependencies- bugs #43315 is dependent
    2014-04-14 carandraug Summaryimadjust output differs from Matlab image package: imadjust output differs from Matlab

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code