dyne / frei0r

A large collection of free and portable video plugins
https://frei0r.dyne.org/
GNU General Public License v2.0
420 stars 90 forks source link

Compilation problem with OpenCV #70

Closed rrrapha closed 5 years ago

rrrapha commented 5 years ago

On OpenBSD, the following error occurs when compiling facedetect/facebl0r:

$ cmake .
$ make VERBOSE=1
[  1%] Building CXX object src/filter/facebl0r/CMakeFiles/facebl0r.dir/facebl0r.cpp.o
cd /tmp/frei0r/src/filter/facebl0r && /usr/bin/c++  -Dfacebl0r_EXPORTS -I/tmp/frei0r/include -I/usr/local/include/opencv  -g -DNDEBUG -fPIC   -o CMakeFiles
/facebl0r.dir/facebl0r.cpp.o -c /tmp/frei0r/src/filter/facebl0r/facebl0r.cpp
/tmp/frei0r/src/filter/facebl0r/facebl0r.cpp:21:10: fatal error: 'opencv2/opencv.hpp' file not found
#include <opencv2/opencv.hpp>
         ^~~~~~~~~~~~~~~~~~~~

Workaround: $ cmake . -DCMAKE_CXX_FLAGS=-I/usr/local/include

ddennedy commented 5 years ago

See #58 I am very tired of dealing with the OpenCV includes. I do not believe there is an approach that can work everywhere without someone setting up a comprehensive collection of build targets and tests them all. We have something fairly comprehensive with MLT and Shotcut where things are working now (mix of CMake and autotools depending on target). Have you done that? I doubt it. Why are we using OpenCV_INCLUDE_DIR instead of DIRS? Referencing one version of OpenCV docs is not enough as we have seen OpenCV heavily changing things in the area of includes and macros from one version to the next. Until I get more explanation on this, I do not approve of this change.

jaromil commented 5 years ago

I share your frustration with opencv...

rrrapha commented 5 years ago

I share your frustration too,.. but this issue is not (directly) related to #58.

After reading FindOpenCV.cmake again, I still think using OpenCV_INCLUDE_DIRS is more correct and portable. The value in OpenCV_INCLUDE_DIR is not the correct path for 'include_directories'. I am almost sure that using OpenCV_INCLUDE_DIRS instead of OpenCV_INCLUDE_DIR cannot break anything.

FindOpenCV.cmake is a bit confusing, it basically does the following:

  1. Search for an OpenCVConfig.cmake file and include it if found.
  2. If no OpenCVConfig.cmake is found, try pkg-config. Both methods set the correct value for 'include_directories' in OpenCV_INCLUDE_DIRS.

(There is also code in FindOpenCV.cmake that tries to find the OpenCV version by looking at cvver.h. If I'm not mistaken, this cannot work for OpenCV >= 2.2)

ddennedy commented 5 years ago

I appreciate your explanation, and I am comfortable now to give it a try. After all, we can easily revert it if it becomes a problem, right? What do you say @jaromil and @d-j-a-y?

d-j-a-y commented 5 years ago

remplacing

option (WITHOUT_OPENCV "Disable plugins dependent upon OpenCV" OFF)
if (NOT WITHOUT_OPENCV)
  find_package (OpenCV)
endif ()

by

option (WITHOUT_OPENCV "Disable plugins dependent upon OpenCV" OFF)
if (NOT WITHOUT_OPENCV)
  find_package( OpenCV REQUIRED )
  include_directories( ${OpenCV_INCLUDE_DIRS} )
endif ()

? from https://docs.opencv.org/master/db/df5/tutorial_linux_gcc_cmake.html nota: i can build/run out of the box this example (debian 9 / opencv 2.4.9). can you @rrrapha ?

rrrapha commented 5 years ago

@d-j-a-y Yes, this would work too, but it adds the opencv include flags to all plugins, not only the ones that need them.

rrrapha commented 5 years ago

I came to the conclusion that FindOpenCVConfig.cmake should be removed completely. This means that cmake relies on the OpenCVConfig.cmake file provided by OpenCV.

It is likely that this works on all systems (if OpenCV is installed correctly). (If OpenCV is installed in a non-standard location, the path can be set via environment variable OpenCV_DIR.)

Using the config mode of findpackage makes sure that all OpenCV* variables are set correctly. On my system:

-- OpenCV_CONFIG=/usr/local/share/OpenCV/OpenCVConfig.cmake
-- OpenCV_CONFIG_PATH=/usr/local/share/OpenCV
-- OpenCV_DIR=/usr/local/share/OpenCV
-- OpenCV_FOUND=1
-- OpenCV_INCLUDE_DIRS=/usr/local/include;/usr/local/include/opencv
-- OpenCV_INSTALL_PATH=/usr/local
-- OpenCV_LIBRARIES=opencv_calib3d;opencv_core;opencv_dnn;opencv_features2d;opencv_flann;[...]
-- OpenCV_LIBS=opencv_calib3d;opencv_core;opencv_dnn;opencv_features2d;opencv_flann;[...]
-- OpenCV_VERSION=3.4.5
-- OpenCV_VERSION_COUNT=3
-- OpenCV_VERSION_MAJOR=3
-- OpenCV_VERSION_MINOR=4
-- OpenCV_VERSION_PATCH=5
-- OpenCV_opencv_aruco_FOUND=TRUE
-- OpenCV_opencv_bgsegm_FOUND=TRUE
-- OpenCV_opencv_bioinspired_FOUND=TRUE
-- OpenCV_opencv_calib3d_FOUND=TRUE
-- OpenCV_opencv_ccalib_FOUND=TRUE
-- OpenCV_opencv_core_FOUND=TRUE
[...]

For example, OpenCV_DIR could then be used to provide a better default for the cassifier path (currently hardcoded) here: https://github.com/dyne/frei0r/blob/e6057edba7820c51f15e10875d3e580fc40adbb1/src/filter/facedetect/facedetect.cpp#L66.