-
Notifications
You must be signed in to change notification settings - Fork 70
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
Add line segment detector(lsd) #35
base: indigo
Are you sure you want to change the base?
Conversation
iory
commented
Sep 15, 2016
85a250b
to
c17a3e4
Compare
test/CMakeLists.txt
Outdated
@@ -35,6 +35,11 @@ add_rostest(test-goodfeature_track.test ARGS gui:=false) | |||
add_rostest(test-camshift.test ARGS gui:=false) | |||
add_rostest(test-fback_flow.test ARGS gui:=false) | |||
add_rostest(test-lk_flow.test ARGS gui:=false) | |||
if(OpenCV_VERSION VERSION_LESS "3.0") | |||
message(STATUS "======================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am testing, so this commit will delete.
std::string new_window_name; | ||
cv::Mat grad; | ||
|
||
cv::Ptr<cv::LineSegmentDetector> lsd = cv::createLineSegmentDetector(lsd_refine_, lsd_scale_, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have to create this instance every loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have to create it every loop.
re-create instance when rosparam changed.
https://github.com/ros-perception/opencv_apps/pull/35/files#diff-51d9a14bcb3567cf07467833b23d4bebR91
boost::shared_ptr<ReconfigureServer> reconfigure_server_; | ||
|
||
bool debug_view_; | ||
ros::Time prev_stamp_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we use this variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prev_stamp_?
opencv_apps's nodelet almost have prev_stamp_.
https://github.com/ros-perception/opencv_apps/blob/indigo/src/nodelet/camshift_nodelet.cpp#L75
https://github.com/ros-perception/opencv_apps/blob/indigo/src/nodelet/lk_flow_nodelet.cpp#L251
I think these don't have an efficient role.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this variable is not necessary.
launch/line_segment_detector.launch
Outdated
<arg name="use_camera_info" default="false" doc="Indicates that the camera_info topic should be subscribed to to get the default input_frame_id. Otherwise the frame from the image message will be used." /> | ||
<arg name="debug_view" default="true" doc="Specify whether the node displays a window to show edge image" /> | ||
|
||
<arg name="lsd_refine_type" default="2" doc="Line Segment Detector Modes"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add list of types : see https://github.com/ros-perception/opencv_apps/blob/indigo/launch/edge_detection.launch#L9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
cfa63e6
to
d99a758
Compare
Config config_; | ||
boost::shared_ptr<ReconfigureServer> reconfigure_server_; | ||
|
||
cv::Ptr<cv::LineSegmentDetector> lsd_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you do not have to define global variable, use flag to update the param see https://github.com/ros-perception/opencv_apps/blob/indigo/src/nodelet/camshift_nodelet.cpp#L163
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I could not understand well.
global variable is lsd_?
please put line_segment_detector.png so that we can put that imgae on http://wiki.ros.org/opencv_apps |
need rebase origin/indigo (see #45) |
69cfaef
to
b2ea66b
Compare
@iory failing on test -> https://travis-ci.org/ros-perception/opencv_apps/jobs/166045561
|
31df0c7
to
fafed04
Compare
Test passed. |
@@ -9,6 +9,9 @@ message(STATUS "OpenCV Components: ${OpenCV_LIB_COMPONENTS}") | |||
if(OpenCV_VERSION VERSION_LESS "3.0" OR TARGET opencv_optflow) | |||
set(OPENCV_HAVE_OPTFLOW TRUE) | |||
endif() | |||
if(TARGET opencv_line_descriptor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need if(TARGET opencv_line_descriptor)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is function of opencv3.
So we need target.
```
if(TARGET opencv_line_descriptor)
set(OPENCV_HAVE_LINE_DESCRIPTOR TRUE)
endif()
if(OPENCV_HAVE_LINE_DESCRIPTOR)
opencv_apps_add_nodelet(line_segment_detector
line_segment_detector/line_segment_detector
src/nodelet/line_segment_detector_nodelet.cpp) # ./lsd_lines.cpp
endif()
```
if target set, set OPENCV_HAVE.... and if OPENCV_HAVE.. is set, set target.
seems very strange....
why are you confident with this.
…--
◉ Kei Okada
2017-04-03 11:47 GMT+09:00 iory <[email protected]>:
***@***.**** commented on this pull request.
------------------------------
In CMakeLists.txt
<#35 (comment)>
:
> @@ -9,6 +9,9 @@ message(STATUS "OpenCV Components: ${OpenCV_LIB_COMPONENTS}")
if(OpenCV_VERSION VERSION_LESS "3.0" OR TARGET opencv_optflow)
set(OPENCV_HAVE_OPTFLOW TRUE)
endif()
+if(TARGET opencv_line_descriptor)
This is function of opencv3.
So we need target.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#35 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAeG3DbYZnV_MncJUsZRnm3zQSmb21__ks5rsF3cgaJpZM4J-LYQ>
.
|
I understand that "if(TARGET opencv_line_descriptor)" is true when opencv have line_segment_descriptor (in opencv3). Is this understanding wrong? |
I see, you're correct. that's why we need `find_packge(OpenCV)` and
sometime we have trouble when we install `ros-indgo-opencv3`, becuase
`ros-indigo-cv-bridge` is linked against opnecv2, but
`find_packaage(OpenCV)` look for `ros-indigo-opencv3`
…--
◉ Kei Okada
2017-04-04 18:46 GMT+09:00 iory <[email protected]>:
I understand that "if(TARGET opencv_line_descriptor)" is true when opencv
have line_segment_descriptor (in opencv3). Is this understanding wrong?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#35 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAeG3BGbkVFHuZLXHJkH0ImOQQ6jp6HRks5rshF3gaJpZM4J-LYQ>
.
|
@iory please rebase origin/master |