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

Enabled the choice between OpenCV 2.4.x or OpenCV 3.x #1714

Closed
wants to merge 3 commits into from
Closed

Enabled the choice between OpenCV 2.4.x or OpenCV 3.x #1714

wants to merge 3 commits into from

Conversation

madduci
Copy link

@madduci madduci commented Jan 13, 2015

Selecting OpenCV version during CMake configuration, defining variable OPENCV_3 for newer OpenCV 3.x and OPENCV_2 for older API (2.4.6+).
Added the fix for imread to solve the compiling error

@Nerei
Copy link

Nerei commented Jan 13, 2015

@blackibiza I think this PR should be canceled. Your changes are not actually required, CV_LOAD_IMAGE_COLOR exits in 3.0

Offtopic, but fyi: #1667 (at least your linking error is fixed)

@Nerei
Copy link

Nerei commented Jan 13, 2015

But according to our plans API will diverge extremely in future. Imagine that we will remove STL from OpenCV interface in 3.x :-)

@Nerei
Copy link

Nerei commented Jan 13, 2015

From other side, It seems now #inckude <opencv2/imgcodes/imgcodes_c.h> causes conditional compilation anyway and the picture in my mind is obsolete. (I pefer 2.4.x only). And umbrella header opencv2/opencv.hpp doesn't help anymore. So might worth merging if very needed.

IMHO, better to create caffe::imread() function, call cv::imread() with default parameter and convert to gray-scale manually to define the constants in caffe code. But this (that is better) is just my speculations.

@madduci
Copy link
Author

madduci commented Jan 13, 2015

But how long will be OpenCV 2.x around? I am of the opinion that OpenCV 3.x will break somehow the API in future. But Travis still uses 2.3.1 version of OpenCV. I believe there's still a need to have both the possibilities implemented, so in this way it's possible to keep them.

@sanchom
Copy link

sanchom commented Jan 13, 2015

@Nerei Where is CV_LOAD_IMAGE_COLOR in OpenCV 3.0? As far as I can tell, it isn't part of the C++ code anymore.

@Nerei
Copy link

Nerei commented Jan 13, 2015

@sanchom #inckude <opencv2/imgcodes/imgcodes_c.h>

@Nerei
Copy link

Nerei commented Jan 13, 2015

@blackball

Don't know. For stable commercial products we stick to 2.x. Some of us advocate 3.x

Update: Just looked more precisely, it seems Android Manager will be released for 3.0 and and that means even binary compatibility must be supported. It seems removing STL is canseled or postponed till 4.x

@sanchom
Copy link

sanchom commented Jan 13, 2015

In my opinion the 2.3.x branch of OpenCV is outdated and is not necessary to support, other than for its use by Travis CI.

From 2.4.0 and onward into 3.x, the cv::IMREAD_COLOR, etc. constants were available.

Including opencv2/imgcodecs/imgcodecs_c.h is non-standard, and isn't automatically included by opencv2/imgcodecs.h. If we're using C++, we should avoid introducing global C constants and functions.

@Nerei
Copy link

Nerei commented Jan 16, 2015

Should to remove OPENCV_3/OPENCV_2 definitions and replace with CV_MAJOR_VERSION from <opencv2/opencv.hpp>, and link errors are fixed in #1667.

#include <opencv2/core/version.hpp>

#if CV_MAJOR_VERSION >= 3
  ... (use 3.x API here) ...
#else
  ... (use 2.x API here) ...
#endif

@Nerei
Copy link

Nerei commented Jan 18, 2015

All required changes are done in #1667. It compiles with OpenCV 3.x (master) update: and with 2.x. This PR to be closed.

@madduci
Copy link
Author

madduci commented Jan 18, 2015

But this change enables both OpenCV 2.x and OpenCV 3.0 to be compiled. 2.3
for Travis CI, OpenCV > 2.4 for everyone else.
It's a different solution. To me, out of the box, caffe didn't compile
using OpenCV 3.0
Am 18.01.2015 20:42 schrieb "Anatoly Baksheev" notifications@github.com:

All required changes are done in #1667
#1667. It compiles with OpenCV 3.0.
This PR to be closed.


Reply to this email directly or view it on GitHub
#1714 (comment).

@Nerei
Copy link

Nerei commented Jan 18, 2015

yes. both opencv branches should compile after merging #1667

shelhamer pushed a commit that referenced this pull request Feb 17, 2015
@Nerei
Copy link

Nerei commented Feb 18, 2015

@blackibiza I believe this can be closed. Just take the latest dev branch.

@madduci madduci closed this Feb 18, 2015
@madduci
Copy link
Author

madduci commented Feb 18, 2015

no problem!

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.

3 participants