OpenCV 3.1 + MingW build fix #217
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…forms now work with the same OpenCV version (3.1.0) - Updated MSVC build script to clone the 3.1.0 branch. Also updated the cmake commandline (3.1.0 needs opencv_ml to be built as well) - Updated MSVC readme to use 3.1.0 - Updated osx build script to also clone 3.1.0 branch instead of whatever is currently at master - Due to a bug in OpenCV 3.1.0 it is not possible to call cvGetSize from C source files under MSVC (it will crash due to incorrect code generation). As a workaround for this (until the bug in OpenCV is fixed), the psmove_tracker.c file (which is the only code calling cvGetSize) is now force-compiled as C++. See opencv/opencv#6221 for more info. - In order to be able to build psmove_tracker.c as C++, some code needed to be updated (extern "C" added where relevant, tracker_default_settings can no longer be initialized using C-style designated initializer)
…all is apparently not compatible with C99. Now using create_tracker_default_settings directly.
- Changed is_valid_float to use std::isnan instead of isnan (isnan is not supported on mingw). std::isnan is the portable version, but is obviously C++ only - Moved is_valid_float and assert_valid_float macros from psmove_math.h into psmove_quaternion.cpp (this is the only place where it was used anyway) in order to prevent it from being included by .c files by accident (which would cause a compile error due to std:: prefix)
| @@ -382,7 +390,7 @@ psmove_tracker_remember_color(PSMoveTracker *tracker, struct PSMove_RGBValue rgb | |||
| void | |||
| psmove_tracker_settings_set_default(PSMoveTrackerSettings *settings) | |||
| { | |||
| *settings = tracker_default_settings; | |||
| *settings = create_tracker_default_settings(); | |||
There was a problem hiding this comment.
If this is the only place where the default settings are used, we could make it so that the function takes a pointer to the object that should be set to defaults. Avoids two object copies. Also the "const" on the return value isn't really useful if returned by value.
static void set_tracker_default_settings(PSMoveTrackerSettings *settings)
{
settings->camera_frame_width = 0;
settings->camera_frame_height = 0;
settings->camera_frame_rate = 0;
settings->camera_auto_gain = PSMove_False;
settings->camera_gain = 0;
// ...
}
Come to think of it, at this point you could just move the setting code directly to psmove_tracker_settings_set_default, getting rid of the extra function:
void psmove_tracker_settings_set_default(PSMoveTrackerSettings *settings)
{
settings->camera_frame_width = 0;
settings->camera_frame_height = 0;
settings->camera_frame_rate = 0;
settings->camera_auto_gain = PSMove_False;
settings->camera_gain = 0;
// ...
}
Please also take care of indentation, don't mix tabs with spaces, and use 4 spaces indentation.
|
+1 on the mingw fix, i made the same change as well to compile psmoveapi on msys2 / mingw64 on windows. |
|
@thp Thanks, I'll make the changes tonight. |
…ctly in psmove_tracker_settings_set_default
|
Cool, thanks - merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've updated the Windows build to also use OpenCV 3.1 instead of the older 2.4.11 so that all platforms now use the same OpenCV version. I tried doing this a while ago as part of my VS2013 support work (#190), but back then I was seeing a crash when
cvGetSizewas called duringpsmove_tracker_blinking_calibration.That crash is still there, but I've spent some time investigating the crash and it turns out it's caused by a bug in OpenCV that causes incorrect code to be generated when
cvGetSizeis called from a C source file under MSVC (it works fine when called from C++/other compilers).I've opened a bug on OpenCV's tracker (opencv/opencv#6221) detailing my findings, but in the meantime, since
psmove_tracker.cis the only file that usescvGetSize, I've added a workaround to the CMake scripts for MSVC to force that file to be compiled as C++; this fixes the crash. I had to make some minor modifications to get it to compile as both C and C++.Finally, I've also fixed a MingW build error introduced as part of #215; it should now build cleanly on all platforms again.