Skip to content

Commit

Permalink
Changes to support OpenCV 3.x
Browse files Browse the repository at this point in the history
  • Loading branch information
Kevin George committed Jan 20, 2018
1 parent 7f040ed commit c0a0581
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lsd_slam_core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ set(CMAKE_BUILD_TYPE Release)

find_package(catkin REQUIRED COMPONENTS
cv_bridge
OpenCV
dynamic_reconfigure
sensor_msgs
image_transport
Expand Down Expand Up @@ -94,6 +95,7 @@ include_directories(
${PROJECT_SOURCE_DIR}/thirdparty/Sophus
${CSPARSE_INCLUDE_DIR} #Has been set by SuiteParse
${CHOLMOD_INCLUDE_DIR} #Has been set by SuiteParse
${OpenCV_INCLUDE_DIRS}
${catkin_INCLUDE_DIRS}
)

Expand All @@ -114,5 +116,6 @@ add_dependencies(lsdslam lsd_slam_viewer_generate_messages_cpp)
add_dependencies(live_slam lsd_slam_viewer_generate_messages_cpp)
add_dependencies(dataset lsd_slam_viewer_generate_messages_cpp)
target_link_libraries(dataset lsdslam ${catkin_LIBRARIES} ${G2O_LIBRARIES})
target_link_libraries(live_slam lsdslam ${OpenCV_LIBRARIES})

# TODO add INSTALL

7 comments on commit c0a0581

@mcschwartzman
Copy link

Choose a reason for hiding this comment

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

Does this require opencv3 or will opencv2 work. Also, does this version work with Catkin

@bespoke-code
Copy link

Choose a reason for hiding this comment

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

This version surely works with catkin and OpenCV 3.3.1. I haven't tested it with OpenCV2 though.

@kevin-george
Copy link
Owner

Choose a reason for hiding this comment

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

ROS has moved to OpenCV 3.x, which is why I made these changes.
If using with ROS Kinetic and above, I recommend using this AS IS.
If not, I would say avoid this commit.

@OAkyildiz
Copy link

Choose a reason for hiding this comment

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

@mcschwartzman, I could possibly ask this in person but it is better to keep things logged. Can you attach the dump, please? you can apt-get it. This gives you a hadnful of options:
http://valgrind.org/docs/manual/manual-core.html (you will probably only need --leak-check=full)
valgrind -v YourExecutable

On a side note: opencv (or python module cv) stands for the C API of OpenCV whereas opencv2 or cv2 is the C++ extension and does not specify the version. It is frustrating, but when in doubt do cv2.__version__

@kevin-george
Copy link
Owner

Choose a reason for hiding this comment

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

Hey guys, I would appreciate it if we could continue this conversation on a issue so other people can benefit from the debugging. Else this would just be valuable info on a "hard-to-find" commit.
Thank you! :)

@OAkyildiz
Copy link

Choose a reason for hiding this comment

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

My apologies I assumed only issues were dropping to my inbox. @mcschwartzman So can you dump the core dump (:D) to a new issue and refer this commit, please?

@kevin-george
Copy link
Owner

Choose a reason for hiding this comment

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

#3

Please sign in to comment.