bugGNU Octave - Bugs: bug #51769, [octave forge] (image) imresize...

 
 

bug #51769: [octave forge] (image) imresize bilinear interpolation inaccurate

Submitted by:  Christof Kaufmann <christofkaufmann>
Submitted on:  Wed 16 Aug 2017 09:01:37 PM UTC  
 
Category:  Octave Forge Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Inaccurate Result
Status:  Patch Submitted Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  other Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Sat 02 Sep 2017 03:39:24 PM UTC, comment #8:

OK, I made a new commit. So Carne can still see the previous behavior with the changing scale factors.

Regarding the tests: OK, good to know. I added now two more tests, which require rounding the output size and confirm Matlab compatibility. I removed the test with the Gimp output, since no compatibility is desired here. Also for Gimp approach 2 is fine, since it shows the user the modified scale factors before scaling is done.

Christof Kaufmann <christofkaufmann>
Sat 02 Sep 2017 08:46:53 AM UTC, comment #7:

@Christof:

  • Regarding your argumentation about rounding issues: I would also prefer to use the path (1.), i.e. keeping the scale factor untouched and slightly increase the output image pixel number if necessary. If you have checked (without looking at the Matlab source code) that this is also the Matlab behavior, then this path is definitly even better, because it is more Matlab compatible (and still correct in some sense). Image analysis (like done with Octave's image package) might differ here from image edition (like done with Gimp).
  • Regarding your second patch: I think it doesn't make a difference if you prepare a new patch based on your old patch, or based on the image package repo version. It is good that you are already presenting your patches in a separate (bitbucket) repository, which is the preferred way of Carne to review patches (if I understood him right).
  • The philosophy about tests in the image package is to have the test results Matlab compatible (if somehow possible). If there is a (intentional or also unwanted) Matlab incompatibility, then it's good to leave a failing (!) test in place, to document this behavior. So please do not change existing test, just because they are failing, please only change tests if those tests weren't Matlab compatible and your improved function now is. You can always add new tests, if you want to.

Eventually we need to wait for Carne's comments and review, here. He is not only the package maintainer of the image package, but he is also the latest author of the current imresize.m function and its current indexing approach.

Hartmut <hardy>
Thu 31 Aug 2017 09:55:52 AM UTC, comment #6:

Regarding the changed tests, there are several reasons.

I added some tests to make the suite more complete:

  • nearest neighbor with x-scale 1 and y-scale 2
  • bilinear scaling with gimp output for reference
  • checkerboard

I replaced the nearest neighbor check, that used a reference matrix  to a more general test. The new pixel centers are all lying in between 4 points, similarly as shown in comment #4. So all neighbors have the same distance and thus there is no unique nearest neighbor. Hence I only checked that one of the 4 neighbors is used in the output.

The tests for bilinear and bicubic interpolation with matlab reference outputs were all failing. This is currently still the case for bicubic interpolation (I like to look at that later). However, to avoid rounding of the output size, I changed the input matrices from 7 x 10 to 8 x 10. This gives a 4 x 5 or a 12 x 15 matrix with scale factors 0.5 or 1.5, respectively. Because of the changed input I had to redo the reference outputs (but I could also not confirm all of the old Matlab results). Now the new bilinear interpolation results in the same output as the one from Matlab (without anti-aliasing).

Let me explain the issue with the output size rounding with a tiny example:
Let in = [1 2 3];
When scaling with 0.5 and the output size should be 1 x 2, there are two options:

  1. Replace the input by in = [1 2 3 3]; which will result in the output out = [1.5 3];
  2. Replace the scaling by 0.666..., which will result in the output out = [1.25 2.75];

(Note, this does not run in octave currently, since interp2 needs at least a 2 x 2 matrix to work.)
Matlab goes for the first approach, while I chose the second. Also Gimp seems to use the second. However, this might need some further discussion.

  1. Replacing the input seemed odd to me at first glance. However, it preserves the scale factor and gives accurate results for all pixels, that are not at the right or bottom border. In the example above you can see this from the first two values, which have been scaled down exactly to 1.5, which is what you expect with a factor of 0.5.
  2. Replacing the scale factor seemed better to me, since the output is accurate for the new scaling. However, this might result in a different scaling for x and y. This does clearly not preserve shapes. Also, the user might not notice that  the scaling changed at all.

So, now I think the first approach is better, since it gives less surprises. What do you think? If you agreed, I would like to change it. Are there any restrictions here about how to do it? Should I make a new commit or rather try to change the current one?

Regarding your question for the bicubic interpolation: Yes, the changed interpolation points affect all methods. Hopefully in a good way.

Sorry for the length of this comment and thanks for the good discussion!

Christof Kaufmann <christofkaufmann>
Wed 30 Aug 2017 04:35:08 PM UTC, comment #5:

Here is the Matlab result on simple sample inputs:

in1 = checkerboard (1,2, 2);

out1 = imresize (in1, 0.5, 'bilinear')   % for Octave (has no antialiasing)
%out1 = imresize (in1, 0.5, 'bilinear', 'Antialiasing', false) % for Matlab
out1_Matlab = [0.5 0.35; 0.5 0.35]

in2 = checkerboard (1,3, 3);

out2 = imresize (in2, 0.5, 'bilinear')  % for Octave
%out2 = imresize (in, 0.5, 'bilinear', 'Antialiasing', false) % for Matlab
out2_Matlab = [0.5 0.425 0.35; 0.5 0.425 0.35; 0.5 0.425 0.35]

In general I think the approach in comment #1 and comment #4 sounds reasonable.

I have not understood yet, why you needed to change existing tests in the code? Where they never Matlab compatible? Did the Matlab result change over the last years?

Do the results of bicubic interpolation also change with your patch?

To finally review (and hopefully commit in some way) your propsed code change, we need to wait for Carne, the current image package maintainer.

Hartmut <hardy>
Mon 28 Aug 2017 08:47:55 PM UTC, comment #4:

This problem is not mainly handling Matlab incompatibility. The approach was just not correct. Raster graphics have pixels and pixels represent the mean color (or value) of an area. They are not samples of infinitesimal small points. This is what I changed.

The image should cover the same area before and after scaling. A pixel has a width and height of 1 unit. This means the first pixel starts at (0.5, 0.5), has its center at (1, 1) and ends at (1.5, 1.5). The last pixel starts at (end-0.5, end-0.5), has its center at (end, end) and ends at (end+0.5, end+0.5). So the whole image starts at (0.5, 0.5) and ends at (end+0.5, end+0.5). The scaled image has to cover the same area, but with a different resolution. In the example of my first comment I scaled the image down by using a factor of 0.5. This means in the system of the original resolution the pixels have now a size of 2 units. So the new first pixel will start at (0.5, 0.5), have its center at (1.5, 1.5) and end at (2.5, 2.5) and so on.

From this you can see that the checkerboard in the example should give the mean value, since the new pixel centers (marked with an X) are exactly in the middle of 4 pixel groups:

+-------+-------+
| 0   1 | 0  0.7|
|   X   |   X   |
| 1   0 |0.7  0 |
+-------+-------+
| 0   1 | 0  0.7|
|   X   |   X   |
| 1   0 |0.7  0 |
+-------+-------+

Matlab shows this behavior in this example, but when a rounding  occured in the new image size, it would not adjust the scale factor. I handled rounding errors in the commit. So I was not trying to be 100% compatible, but rather to do it correct.

To answer you question: With the old behavior all positions were a bit off, since the first pixel center has always been at 1 and the last at end. The pixel centers in between were linearly spread. So for the checkerboard it just selected the 4 corner pixels.

Christof Kaufmann <christofkaufmann>
Mon 28 Aug 2017 05:25:19 PM UTC, comment #3:

Is this problem (i.e. Matlab incompatible results) only about pixels on the edge of a resized image, or are the pixels "inside" also wrong sometimes?

Hartmut <hardy>
Thu 17 Aug 2017 04:04:33 AM UTC, comment #2:

added people to cc

Avinoam Kalma <avinoam>
Project Member
Wed 16 Aug 2017 09:15:33 PM UTC, comment #1:

So I made a patch for that:
https://bitbucket.org/ChristofKaufmann/octave-image/commits/branch/imresize-fix
(If you like to have a patch as attached file rather, please tell me.)

The problem in the original code were mainly the indices. These were quite wrong. I fixed the indices, which changes the behaviour of all methods; 'bilinear' works now, 'nearest' was working and is still working, 'bicubic' was not passing the test and is still not. So 'bicubic' might have to be fixed. Also, anti-aliasing is not implemented yet.

The patch also updates the matlab reference outputs and adds and modifies a few tests. It is split into two commits, since the first just makes a few coding style changes, but does not change any behaviour. The second commit has the real content.

Christof Kaufmann <christofkaufmann>
Wed 16 Aug 2017 09:01:37 PM UTC, original submission:

octave-image version 6.2.1 (default branch from repository)

Hi,

I noticed very inaccurate results when using the 'bilinear' method with imresize. I can give a small example:

in = checkerboard (1, [2 2]);
imresize (in, 0.5, 'bilinear')
## gives:
## ans =  0.0   0.7
##        1.0   0.0

The expected behaviour would be (which matlab would give for a larger example):

imresize (in, 0.5, 'bilinear')
## gives:
## ans = 0.50   0.35
##       0.50   0.35

Christof Kaufmann <christofkaufmann>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-17 avinoam StatusNone => Patch Submitted
        Release4.0.3 => other
        Carbon-Copy- => Added carandraug@octave.org
        Carbon-Copy- => Added hg_code@gmx.de
    2017-08-16 christofkaufmann Carbon-Copy- => Added christofkaufmann

    Back to the top


    Powered by Savane 3.4