Skip to content

Added heuristic to allocate buffer for FAST features depending on input image resolution #27657

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

Merged
merged 1 commit into from
Aug 12, 2025

Conversation

asmorkalov
Copy link
Contributor

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@@ -429,8 +429,10 @@ void FAST(InputArray _img, std::vector<KeyPoint>& keypoints, int threshold, bool
{
CV_INSTRUMENT_REGION();

const size_t max_fast_features = _img.total() / 4; // Simple heuristic that depends on resoption. most propably the same may be safly reduced more
Copy link
Contributor

@vpisarev vpisarev Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are a few typos: 1) resoption => resolution, 2) propably => probably.

recommended comment: "Use a simple heuristic depending on the image resolution; can probably be reduced even further".

also, sizeof(KeyPoint) is 28 bytes. If, say, image has a modest (by modern standards) resolution of 24Mpix, the buffer will have size 168Mb, which is rather big and allocation of such a buffer may slowdown the function. I suggest to reduce max_fast_features to std::max(_img.total()/100, (size_t)1000); that will give at max 10K features for 1000x1000 image and 240K features for 24Mpix image — more than enough for any kind of registration/scene recognition/loop closure algorithm.

@asmorkalov asmorkalov force-pushed the as/fast_buffer_size branch from 5cadea1 to 0f4aad6 Compare August 12, 2025 07:41
@asmorkalov asmorkalov force-pushed the as/fast_buffer_size branch from 0f4aad6 to 84d391a Compare August 12, 2025 08:07
@vpisarev vpisarev self-requested a review August 12, 2025 09:29
@opencv-alalek opencv-alalek removed their request for review August 12, 2025 09:48
@asmorkalov asmorkalov merged commit 2ecc22f into opencv:4.x Aug 12, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants