Skip to content

Replace static numpy allocator by function containing static.#25493

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
vrabaud:tiff
Apr 27, 2024
Merged

Replace static numpy allocator by function containing static.#25493
asmorkalov merged 1 commit intoopencv:4.xfrom
vrabaud:tiff

Conversation

@vrabaud
Copy link
Copy Markdown
Contributor

@vrabaud vrabaud commented Apr 26, 2024

There is no bug with the current implementation.

But this change enables the numpy code to be its own library, in case some users want to (e.g. CLIF library) and have the Python wrapper link to it. It would not be possible without this change as there would be a static initialization order fiasco otherwise.

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

That enables the numpy code to be its own library, in case
some users want to (e.g. CLIF library).
@vrabaud vrabaud marked this pull request as ready for review April 26, 2024 14:36
@asmorkalov asmorkalov requested a review from VadimLevin April 27, 2024 08:02
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@asmorkalov asmorkalov added this to the 4.10.0 milestone Apr 27, 2024
@asmorkalov asmorkalov merged commit 8af7669 into opencv:4.x Apr 27, 2024
};

extern NumpyAllocator g_numpyAllocator;
inline NumpyAllocator& GetNumpyAllocator() {static NumpyAllocator gNumpyAllocator;return gNumpyAllocator;}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is implementation here and not in .cpp file?
Not sure if we want multiple instances of this object.


GetNumpyAllocator

OpenCV function names start with lowercase.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There won't be multiple allocator instances, because function is not marked as static, so linker should strip its all internal static variables to 1

@mshabunin mshabunin mentioned this pull request Jun 14, 2024
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.

4 participants