Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Support for the latest OpenCV version #54

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

kbinias
Copy link
Contributor

@kbinias kbinias commented Jan 30, 2020

Added support for the latest OpenCV version (tested with 2.4.x, 3.4.x, 4.2.x)

@kbinias kbinias requested a review from sfraczek January 30, 2020 15:26
Copy link
Collaborator

@sfraczek sfraczek left a comment

Choose a reason for hiding this comment

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

Just some questions and one suggestion.

@@ -141,6 +141,10 @@ find_package(PkgConfig REQUIRED)

find_package(OpenCV REQUIRED)

if(NOT (${OpenCV_VERSION} VERSION_LESS 3))
Copy link
Collaborator

Choose a reason for hiding this comment

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

what do you think about that?

Suggested change
if(NOT (${OpenCV_VERSION} VERSION_LESS 3))
if${OpenCV_VERSION} VERSION_GREATER_EQUAL 3)

@@ -77,6 +77,10 @@ image::extractor::extractor(const image::config& cfg)
{
_pixel_type = CV_MAKETYPE(CV_8U, cfg.channels);
_color_mode = cfg.channels == 1 ? CV_LOAD_IMAGE_GRAYSCALE : CV_LOAD_IMAGE_COLOR;
#ifdef OPENCV_LEGACY_MODE
// Do not run ApplyExifOrientation as this causes Segmentation faults
_color_mode = _color_mode | CV_LOAD_IMAGE_IGNORE_ORIENTATION;
Copy link
Collaborator

Choose a reason for hiding this comment

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

So with OpenCV>=3 we have to add CV_LOAD_IMAGE_IGNORE_ORIENTATION to prevent the unexpected orientation change?

@@ -86,7 +90,11 @@ shared_ptr<image::decoded> image::extractor::extract(const void* inbuf, size_t i

// It is bad to cast away const, but opencv does not support a const Mat
// The Mat is only used for imdecode on the next line so it is OK here
#ifdef OPENCV_LEGACY_MODE
cv::Mat input_img(insize * CV_MAT_CN(_pixel_type), 1, CV_8UC1, (char*)inbuf);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this constructor do?

libs = ""
include_dirs = ""
include_dirs += " -I${CMAKE_PREFIX_PATH}/include "
lib_dirs = "${CMAKE_PREFIX_PATH}/lib"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened here to this file?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants