Skip to content
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

optical flow in opencv_apps #2

Open
vrabaud opened this issue May 7, 2016 · 3 comments
Open

optical flow in opencv_apps #2

vrabaud opened this issue May 7, 2016 · 3 comments

Comments

@vrabaud
Copy link
Contributor

vrabaud commented May 7, 2016

Hi! First of all I'm quite new to ROS so I'm not sure whether this is a silly question.
copied from ros-perception/vision_opencv#67

What role do the opencv_apps play in this repository? Are those meant just as code examples and not meant to be used within 'proper' applications?

In short: is it advisable to use opencv_apps/Flow.msg and fback_flow_nodelet in my application or should I roll my own? The fact that fback_flow_nodelet unconditionally draws the flow visualization suggests that it's really meant more as an example application, rather than a general nodelet for optical flow.


If opencv_apps are meant to be useful apart from serving as a code example I would suggest a few changes (and gladly provide pull requests) to the optical flow part.

In particular I think that the definition of the flow messages should be different. In Flow.msg the flow vector is called velocity, but what fback_flow_nodelet and lk_flow_nodelet really compute are the motions between two frames in pixels irrespective of the time passed between the two frames. So either the nodelets need to scale the movement by the time between the frames, so that the velocity field in Flow.msg get a proper unit (px/s e.g.), or velocity needs to be renamed to displacement and a time field needs to be added to Flow.msg.

PS: great repo! 👍 😄

wkentaro pushed a commit to wkentaro/opencv_apps that referenced this issue Mar 24, 2017
AddingImages: add arbitrary dtype images
@fujimo-t
Copy link
Contributor

I created small ROS packages for DIS Flow using OpenCV implementation and consider to create pull request to opencv_apps.

But FlowArray.msg is not suitable for dense flow as pointed in ros-perception/vision_opencv#67 (comment) .
Is it OK to add new msg for dense flow if I send pull request?

My current implementation is just wrapping sensor_msgs/Image like stereo_msgs/DisparityImage.
The reason of this is that Image has enough information to handle data array, and can be accessed easily by cv_bridge.

@k-okada
Copy link
Contributor

k-okada commented Mar 18, 2019

Patches are always welcome, so feel free to create new PR.

  • I prefer not to add new messages, especially just for one node, because we have to create viewer, test code and so on just for that node
  • I like the comparison of algorithms, so if your new node outputs the same message as other existing nodes, then we're easy to compare the outputs
    • so if you can implement multiple dense optical flow algorithm, that's would be good
    • or enable to output new message for all existing optical flow algorithm

@fujimo-t
Copy link
Contributor

I see.

Then I will create PR using existing message opencv_apps/Flow.msg.
It is not convinience for dense flow but also can be used.

Later I try to implement other dense algorithms or change to new message if I have time.

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

No branches or pull requests

3 participants