search

dyne / frei0r

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

Include opencv2/imgproc.hpp for CV_RGB() #27

Closed cwilling closed 6 years ago

cwilling commented 6 years ago

Signed-off-by: Christoph Willing chris.willing@linux.com

This addresses issue #26

jaromil commented 6 years ago

Hi! thanks for the fix. Is there a way to detect OpenCV's version, to wrap the include into ifdefs? else we'll provoke a regression on all systems with older library versions (as for instance the error on the CI test build here https://travis-ci.org/dyne/frei0r/builds/403881174#L1938

 filter/facebl0r/facebl0r.cpp:22:31: fatal error: opencv2/imgproc.hpp: No such file or directory
 #include <opencv2/imgproc.hpp>
jaromil commented 6 years ago

Had a quick look and found CV_VERSION: https://docs.opencv.org/3.4.2/d7/dad/version_8hpp.html#a4f050efda432e041621b3af3df140adc Would you mind extending your PR to do this check?

jaromil commented 6 years ago

Thanks! the check looks good, yet the CI test fails (travis uses ubuntu 14 as base). In my experience, travis is not really reliable (nor Ubuntu). I reproduced the build of your PR on my own machine (running Devuan Beowulf) with stock openCV 3.2 libraries and there is no problem when linking facebl0r. So if there are no further objections / insights I'll proceed merging this patch and setting up another CI build system.

cwilling commented 6 years ago

Not being sure of what versions of anything were being used by travis, I have been working back through earlier versions of opencv trying elicit the same failure here - no luck back to opencv-3.2.0. I'm definitely happy to stop looking ...

r41d commented 5 years ago

The last release is from May and hence doesn't include this fix, is it possible to release a new version so that this fix is actually rolled out?

jaromil commented 5 years ago

makes sense to have a release yes, let's perhaps solve #71 and then I'll get to it.

r41d commented 5 years ago

Looks like #71 is merged. Could a new release now be drafted?