-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Use proper C++ types. #3703
Use proper C++ types. #3703
Conversation
This is necessary to get opencv/opencv#25248 working.
modules/videostab/src/precomp.hpp
Outdated
@@ -59,7 +59,7 @@ | |||
|
|||
inline float sqr(float x) { return x * x; } | |||
|
|||
inline float intensity(const cv::Point3_<uchar> &bgr) | |||
inline float intensity(const cv::Point3_<unsigned char> &bgr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not uint8_t
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am trying to stay close to the current OpenCV typedefs at https://github.com/opencv/opencv/blob/3aefd4862c5ee92918e15d96e9973cbfde48e5c2/modules/core/include/opencv2/core/hal/interface.h#L51
But indeed, we might as well use the proper types from <cstdint>
. Should I switch to that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this proposal for 5.x -opencv/opencv#24994 , so eventually, we will switch to the <cstdint>
types. Since changes in this PR do not affect public interfaces, I suggest using these types here. I'm not sure regarding opencv/opencv#25248 though, perhaps it can be partially converted to uint8_t
(internal usage, C++ code) and partially to unsigned char
(C-API).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: switched to uint_8_t.
For the other PR, let's not change the typedefs right now. All it does is encapsulate the OpenCV typedefs in the cv
namespace, that's it. As this header is public, it prevents conflicts with other libraries that also define it in the global namespace (like SWIG), but differently (int64 is a long long int
and not an int64_t
like OpenCV).
This is necessary to get opencv/opencv#25248 working.
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.