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

highgui: change behaviour of cv::waitKey #7190

Closed
wants to merge 2 commits into from
Closed

Conversation

alalek
Copy link
Member

@alalek alalek commented Aug 29, 2016

DO NOT MERGE Testing only.
Discussion is here: #7105

@StevenPuttemans
Copy link

@alalek even though ABI compliance is an issue there, I am still considering this as an issue. The compiler will not complain with this code, but in C++ at runtime all instances of waitKey will be replaced by their inline counterpart. This is possible in C++, however Python does not support this functionality as far as I am aware.

Did you create a python test sample trying to actually use this code?

@StevenPuttemans
Copy link

Also I guess the ABI issues will always stick, because due to the inline, the code will get a changed behaviour and thus not support backwards compatibility. Therefore I still guess the double functionality is the best way to go, to avoid ABI issues also.

@alalek
Copy link
Member Author

alalek commented Aug 30, 2016

C++ user code uses replaced version, which can be selected by specified define.
Python code can use only one version which is compiled into cv2 module (no selection is allowed). I tried to run interactive Python code like this and it works fine:

import cv2
cv2.imshow('key', 0)
print(cv.waitKey())

P.S. I believe this Linux/GCC ABI problem can be resolved. On Windows there is no ABI issue (inlined cv::waitKey is still exported from DLL)

@StevenPuttemans
Copy link

👍 okay seems valid then!

@@ -340,7 +347,21 @@ If there are several HighGUI windows, any of them can be active.

@param delay Delay in milliseconds. 0 is the special value that means "forever".
*/
#if defined __OPENCV_BUILD
Copy link
Contributor

Choose a reason for hiding this comment

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

can we hide these details in a .cpp file?

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

3 participants