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

Clean Core module #25002

Open
vpisarev opened this issue Feb 12, 2024 · 2 comments
Open

Clean Core module #25002

vpisarev opened this issue Feb 12, 2024 · 2 comments
Labels
cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) feature
Milestone

Comments

@vpisarev
Copy link
Contributor

vpisarev commented Feb 12, 2024

Describe the feature and motivation

core module is the cornerstone of OpenCV. It implements basic functionality and so it's used in any OpenCV-based application. Therefore it makes sense to clean it up in OpenCV 5.0 in order to reduce footprint as much as possible. Sometimes this footprint is not just about the consumed space, but also about the compile time overhead.

Here are the suggested items (these are not all the changes proposed for core module in OpenCV 5, a part of them):

  • clean modules/core/include/opencv2/core directory as much as possible. Currently there are some obsolete items there and generally the stuff is quite poorly organized. Note that all the programs include many of those headers indirectly, and so it makes compile time longer. Besides, it creates a lot of confusion for people who explore OpenCV source code. These are some examples of headers that can be combined with other headers or removed or moved to other modules or to src directory:
    • neon_utils.cpp, sse_utils.cpp, vsx_utils.cpp. Clearly, they should be removed or duplicated in those .cpp files where the functionality is needed. No need to make this stuff public across modules.
    • quaternion and dualquaternion can be combined. It's just one little and rarely used feature in OpenCV.
    • parallel/ subdirectory - those details should not be exposed outside of the core module. All modules should use cv::parallel_for_().
    • llapi.h name is misleading, the content must be edited and probably revised, merged with some other headers.
    • private/* must be removed
    • the whole stuff in detail/* can be combined with other headers or moved to src.
    • cuda* and cuda/* contain way more stuff than needed. In a separate feature request (TBD) it's suggested to move CUDA completely to 3rdparty/cuda HAL module.
    • affine.hpp should probably be moved to 3d module.
    • etc.
  • Remove IPP, OpenVX and CUDA as described in separate tickets (Remove OpenVX #24995, Move IPP to opencv/3rdparty (just like Carotene) #24996, (TBD for CUDA))
  • Probably remove optim.hpp and all the functionality defined in it. Move it to some module in opencv_contrib (or maybe to 3d module if some of this functionality is needed there)
  • Remove old data structures (MemStorage etc.) We don't need them anymore (maybe except for Chessboard corner detector)
  • Remove SparseMat data structure that we don't need apparently.
  • Retain Algorithm, but remove ParamType<> trait.
  • Possibly move kmeans, PCA to the refreshed 'Features' module

Additional context

No response

@vpisarev vpisarev added feature cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) labels Feb 12, 2024
@vpisarev vpisarev added this to the 5.0 milestone Feb 12, 2024
@vpisarev vpisarev changed the title Clean core module Clean Core module Feb 14, 2024
@fengyuentau
Copy link
Member

parallel/ subdirectory - those details should not be exposed outside of the core module. All modules should use cv::parallel_for_().

Probably we should extend cv::parallel_for_() as we need a threading cost model analysis like Eigen offers so as to parallelize simple yet potentially large for loop (just like in nary_eltwise_layers.cpp).

@opencv-alalek
Copy link
Contributor

opencv-alalek commented Feb 17, 2024

parallel/ subdirectory - those details should not be exposed outside of the core module. All modules should use cv::parallel_for_().

Completely wrong assumption.
There is API for customized parallel backends. In addition there are samples of minimal parallel_for backends for TBB and OpenMP application-driven shims.
Refer to related PR and documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) feature
Projects
None yet
Development

No branches or pull requests

3 participants