Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating STAR detector and FREAK descriptor to work with large and/or 16-bit images #1654

Closed
wants to merge 27 commits into from

Conversation

seth-planet
Copy link

A project that I've been working with involves large, 16-bit, images. The current implementations of the STAR feature detector and the FREAK feature descriptor don't support either large or 16-bit images. I've written a set of changes to support both. Much of the work goes into adding more bits to the integral image when appropriate. I'll discuss what needed doing:

  • In STAR, the method of detecting if the matrix was color didn't make sense. I'm using channels() instead of type() now.
  • In STAR, he integral image data type needed to be changed for two reasons. If each cell can have the value of 256, the integer type could overflow when the image is greater than 8 megapixels. If you are working with 16-bit imagery imagery instead of 8-bit, this will happen at much smaller image sizes. So, I've used templates to use a 32-bit integer under the normal case, and 64-bit double for large images and/or higher bit depth.
  • In STAR, SSE2 is only enabled if the data type is the original 32-bit integer. It was my aim to support the new data types (shorts & doubles) without removing the existing SSE usage.
  • In integral(), in sumpixels.cpp, I added templated support for working with 'short' & 'ushort' precision input. As mentioned before, it's appropriate to use double precision when working with 16-bit images (so that's what I did).
  • In FREAK, I broke most of computeImpl() into a new function, computeDescriptors(). I did this so I could use templates and thus keep all the data type permutations sane.
  • In FREAK, SSE2 is only used if the average value is expected to be an 8-bit number. This is the original usage, and it should be just as fast as before. 16-bit data falls back to the scalar code.
  • In FREAK, meanIntensity() now uses templates.
  • In FREAK, the return value of meanIntensity() is 'int' instead of 'uchar' for compatibility between possible data types. It should make no speed difference.

Vladislav Vinogradov and others added 15 commits August 30, 2013 14:46
Because apparently I love running performance tests for debug builds.
…lining.

Java inlines static finals if they're defined with a constant expression. In
case of version constants we don't want that to happen, since they obviously
change from version to version. If the user substitutes a different OpenCV
jar without recompiling, we want user code to still have relevant values for
the version constants.

This arranges that by turning constant values into function calls, which no
longer count as a constant expression.
Conflicts:
	modules/features2d/include/opencv2/features2d/features2d.hpp
	modules/features2d/src/stardetector.cpp
	modules/java/generator/gen_java.py
	samples/gpu/CMakeLists.txt
@SpecLad
Copy link

SpecLad commented Oct 21, 2013

There are a lot of commits here that don't belong.

@seth-planet
Copy link
Author

How do I get the branch synced with the current master? I'm a git noob, I'm not sure what attempt number we're on now, and this was the best I could come up with.

Original bugfix:
http://code.opencv.org/issues/3322

On Oct 21, 2013, at 7:30 AM, Роман Донченко notifications@github.com wrote:

There are a lot of commits here that don't belong.


Reply to this email directly or view it on GitHub.

@apavlenko
Copy link
Contributor

The windows build fails due to level 4 warnings in freak.cpp

@seth-planet
Copy link
Author

Hm. How can I get this only on Windows?? I have all of the compile warnings & errors fixed.
"
Features2d_DescriptorExtractor_FREAK.regression (108 ms) :
Average time of computing one descriptor = 3.09155e-007 ms.
Max distance between valid and computed descriptors 348>12 - bad accuracy!
"

@SpecLad
Copy link

SpecLad commented Oct 22, 2013

@seth-planet Looks like you submitted the same branch for this PR as for the old one (#1639). You need to rebase this branch locally on top of master, and then push -f it to your fork.

@apavlenko
Copy link
Contributor

now it fails FREAK test (http://build.opencv.org/builders/precommit_windows/builds/6528)

@seth-planet
Copy link
Author

Can I run the FREAK test on Mac (or Linux)? For my own test cases, I'm getting identical results between master and my branch. I'd rather not have to install Windows just to replicate this bug.

Vladislav Vinogradov and others added 3 commits October 22, 2013 11:15
…lining.

Java inlines static finals if they're defined with a constant expression. In
case of version constants we don't want that to happen, since they obviously
change from version to version. If the user substitutes a different OpenCV
jar without recompiling, we want user code to still have relevant values for
the version constants.

This arranges that by turning constant values into function calls, which no
longer count as a constant expression.
@seth-planet
Copy link
Author

@SpecLad So, I tried to follow your rebasing instructions. Hopefully it helped things.

@SpecLad
Copy link

SpecLad commented Oct 23, 2013

You have even more commits now, not less. Try interactive rebase or perhaps the --onto option. You'll know you got it if git log --oneline origin/master..lg_16_bit_img_features only shows commits you made.

Can I run the FREAK test on Mac (or Linux)?

Not sure what you mean here. Every test can be run on any platform.

@seth-planet
Copy link
Author

When i try to run the tests on my Mac, they all fail due to missing an image. I can't find any documentation on this.

But the test reportedly only fails on windows anyway.

Sent from my iPhone

On Oct 23, 2013, at 1:13 AM, Роман Донченко notifications@github.com wrote:

You have even more commits now, not less. Try interactive rebase or perhaps the --onto option. You'll know you got it if git log --oneline origin/master..lg_16_bit_img_features only shows commits you made.

Can I run the FREAK test on Mac (or Linux)?

Not sure what you mean here. Every test can be run on any platform.


Reply to this email directly or view it on GitHub.

@SpecLad
Copy link

SpecLad commented Oct 24, 2013

Sounds like you're missing test data. You need to set the OPENCV_TEST_DATA_PATH environment variable to point to the testdata directory inside the opencv_extra repository.

@seth-planet
Copy link
Author

Ok, I worked my way through all the code, and figured out what the 'bug' is:

    if( image.channels() > 1 )
        cvtColor( image, grayImage, COLOR_BGR2GRAY );

I'm automatically changing color images to grayscale. If I comment that out, the test passes. However, I would think that conversion to grayscale would be preferred. (That's how it's done in the STAR detector.) What is the preferred behavior?

@SpecLad
Copy link

SpecLad commented Nov 5, 2013

@vpisarev I believe that's a question for you.
@seth-planet You still need to get rid of all those unrelated commits, by the way.

@seth-planet
Copy link
Author

I'll wait for @vpisarev and create a clean pull request.

@seth-planet
Copy link
Author

hello?

@ghost ghost assigned vpisarev Nov 13, 2013
@apavlenko
Copy link
Contributor

@vpisarev, could you please look at it?

@seth-planet
Copy link
Author

ping

@SpecLad
Copy link

SpecLad commented Nov 25, 2013

@seth-planet Actually, you might want to get rid of excess commits before the review - otherwise the diff is cluttered and it's not clear at all which changes are yours. Plus, a green build instills more confidence in your code. 😉

@seth-planet
Copy link
Author

Clean pull request:
#1932

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants