search

ros-naoqi / naoqi_driver

c++ bridge based on libqi
Apache License 2.0
52 stars 93 forks source link

kRawDepthColorSpace for depth image #74

Closed kochigami closed 8 years ago

kochigami commented 8 years ago

This pull request is to use kRawDepthColorSpace instead of kDepthColorSpace for depth image.

The point cloud from kRawDepthColorSpace is more clear. kDepthColorSpace + with black lenses: pepper_with_lens_kdepthimagecolorspace

kRawDepthColorSpace + with black lenses: pepper_with_lens_krawdepthimagecolorspace

kDepthColorSpace + without black lenses: pepper_without_lens_kdepthimagecolorspace

kRawDepthColorSpace + with black lenses: pepper_without_lens_krawdepthimagecolorspace

reference https://github.com/jsk-ros-pkg/jsk_robot/issues/619 (The last comment says that kRawDepthColorSpace has a strong effect when eye covers are removed.) https://github.com/ros-naoqi/pepper_robot/pull/27 (The last comment says that kRawDepthColorSpace will be required for SLAM.)

k-okada commented 8 years ago

please take rviz images without camera lens covers too.

◉ Kei Okada

On Wed, Sep 14, 2016 at 10:36 AM, Kanae Kochigami notifications@github.com wrote:

This pull request is to use kRawDepthColorSpace instead of kDepthColorSpace for depth image.

The point cloud from kRawDepthColorSpace is more clear. kDepthColorSpace: [image: 16feb5f8-2e4e-11e6-886a-7d8c3992348a] https://cloud.githubusercontent.com/assets/7259671/18497026/ff7e8a0c-7a65-11e6-8bf6-db2eec7dea57.png

kRawDepthColorSpace: [image: 2adc0288-2e4e-11e6-81f8-37c63b13dd8d] https://cloud.githubusercontent.com/assets/7259671/18497030/0490723a-7a66-11e6-8fc8-509204baeaf2.png

reference jsk-ros-pkg/jsk_robot#619 https://github.com/jsk-ros-pkg/jsk_robot/issues/619 (The last comment says that kRawDepthColorSpace has a strong effect when eye covers are removed.) ros-naoqi/pepper_robot#27 https://github.com/ros-naoqi/pepper_robot/pull/27 (The last comment says that kRawDepthColorSpace will be required for

SLAM.)

You can view, comment on, or merge this pull request online at:

https://github.com/ros-naoqi/naoqi_driver/pull/74 Commit Summary

  • kRawDepthColorSpace for depth image

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ros-naoqi/naoqi_driver/pull/74, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeG3FyN4HN8f5bwJJV9n5me5grIJH2Fks5qp0-AgaJpZM4J8UMA .

nlyubova commented 8 years ago

+1 it is known that this color space is better for any Pepper with and without lenses (tested on many Peppers), let's merge it

kochigami commented 8 years ago

I updated pictures. (The place Pepper is located is different in the condition with black lenses and without them though.)

Thank you very much for your comments.

k-okada commented 8 years ago

BTW, Is there any difference other than color space and dpeth quality between kDepthColorSpace vs kRawDepthColorSpace ? for example how

about the publishing frequency?

◉ Kei Okada

On Wed, Sep 14, 2016 at 5:22 PM, Kanae Kochigami notifications@github.com wrote:

I updated pictures. (The place Pepper is located is different in the condition with black lenses and without them.)

Thank you very much for your comments.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

k-okada commented 8 years ago

FYI: https://github.com/jsk-ros-pkg/jsk_robot/issues/619#issuecomment-246947372 has better images for depth quality between kDepthColorSpace vs kRawDepthColorSpace

furushchev commented 8 years ago

@k-okada @kochigami As seen in current implementation, IMO it is "best effort". Currently all data are enqueued to one global queue and published as ROS messages in one while loop. So the change from kDepthColorSpace to kRawDepthColorSpace can actually affects publish frequency of all topics. https://github.com/ros-naoqi/naoqi_driver/blob/master/src/naoqi_driver.cpp#L173 If possible, I think it's better to split to two lines one for publishing messages required to be real-time (joint_states, tf,...), and one for messages which is enough on best effort (image, sonar,...).

mikaelarguedas commented 8 years ago

agreed with @furushchev comment, it would make a lot of sense to separate lightweight reliable ROS messages from the heavy best effort ones.

kochigami commented 8 years ago

Hi, I looked into frequency of the topics (/pepper_robot/camera/depth/camera_info and /pepper_robot/camera/depth/image_raw). What I did is roslaunch pepper_bringup pepper_full.launch network_interface:=<> and rostopic hz <topic>.

I also checked other topics. I don't think the frequency of all topics changes significantly between kDepthColorSpace and kRawDepthColorSpace, but I didn't follow the current implementation of naoqi_driver.cpp, so I may have a misunderstanding.

(depth topics) Average frequency of ten topics: /pepper_robot/camera/depth/camera_info kDepthColorSpace (master branch): 8.779 kRawDepthColorSpace (try-depth-raw branch): 10.1229

/pepper_robot/camera/depth/image_raw kDepthColorSpace (master branch): 9.9972 kRawDepthColorSpace (try-depth-raw branch): 9.4303

(other topics) high frequency: (about 50) /joint_states, /tf

middile frequency: (about 7) /pepper_robot/camera/ir/camera_info (about 8) /pepper_robot/camera/ir/image_raw (about 10) /pepper_robot/imu/base, /pepper_robot/imu/torso, /pepper_robot/laser, /pepper_robot/sonar/front (about 11) /pepper_robot/audio, /pepper_robot/camera/bottom/camera_info, /pepper_robot/camera/bottom/image_raw, /pepper_robot/sonar/back

low frequency: (about 1) /diagnostics (about 2) /pepper_robot/camera/front/image_raw (about 2.7) /pepper_robot/camera/front/camera_info (about 5) /pepper_robot/pose/body_pose_naoqi/status, /pepper_robot/pose/joint_stiness_trajectory/status (about 5.5) /pepper_robot/pose/joint_trajectory/status (about 6) /pepper_robot/pose/body_pose/status

(reference) kDepthColorSpace (master branch)

rostopic hz /pepper_robot/camera/depth/camera_info 
subscribed to [/pepper_robot/camera/depth/camera_info]
average rate: 10.049
    min: 0.044s max: 0.301s std dev: 0.07459s window: 10
average rate: 9.418
    min: 0.041s max: 0.303s std dev: 0.08040s window: 17
average rate: 7.210
    min: 0.041s max: 0.784s std dev: 0.16110s window: 22
average rate: 7.923
    min: 0.041s max: 0.784s std dev: 0.13486s window: 32
average rate: 8.339
    min: 0.041s max: 0.784s std dev: 0.11883s window: 42
average rate: 8.662
    min: 0.041s max: 0.784s std dev: 0.10770s window: 52
average rate: 8.830
    min: 0.041s max: 0.784s std dev: 0.09922s window: 62
average rate: 9.028
    min: 0.041s max: 0.784s std dev: 0.09293s window: 72
average rate: 9.122
    min: 0.041s max: 0.784s std dev: 0.08776s window: 82
average rate: 9.209
    min: 0.041s max: 0.784s std dev: 0.08333s window: 92

rostopic hz /pepper_robot/camera/depth/image_raw
subscribed to [/pepper_robot/camera/depth/image_raw]
average rate: 10.760
    min: 0.045s max: 0.244s std dev: 0.06690s window: 9
average rate: 10.253
    min: 0.042s max: 0.244s std dev: 0.05615s window: 20
average rate: 9.702
    min: 0.040s max: 0.260s std dev: 0.06067s window: 28
average rate: 9.296
    min: 0.040s max: 0.367s std dev: 0.07257s window: 35
average rate: 9.677
    min: 0.040s max: 0.367s std dev: 0.06933s window: 47
average rate: 10.113
    min: 0.040s max: 0.367s std dev: 0.06455s window: 60
average rate: 10.012
    min: 0.040s max: 0.367s std dev: 0.06254s window: 69
average rate: 10.082
    min: 0.039s max: 0.367s std dev: 0.06080s window: 79
average rate: 10.007
    min: 0.039s max: 0.367s std dev: 0.05960s window: 89
average rate: 10.070
    min: 0.039s max: 0.367s std dev: 0.05816s window: 99

kRawDepthColorSpace (try-depth-raw branch) 

rostopic hz /pepper_robot/camera/depth/camera_info 
subscribed to [/pepper_robot/camera/depth/camera_info]
average rate: 11.706
    min: 0.045s max: 0.107s std dev: 0.02449s window: 8
average rate: 10.604
    min: 0.041s max: 0.199s std dev: 0.03417s window: 19
average rate: 9.644
    min: 0.040s max: 0.397s std dev: 0.07149s window: 27
average rate: 10.035
    min: 0.040s max: 0.397s std dev: 0.06804s window: 38
average rate: 9.583
    min: 0.040s max: 0.397s std dev: 0.07461s window: 46
average rate: 10.191
    min: 0.040s max: 0.397s std dev: 0.07040s window: 59
average rate: 10.091
    min: 0.040s max: 0.397s std dev: 0.07208s window: 69
average rate: 9.881
    min: 0.040s max: 0.397s std dev: 0.07463s window: 77
average rate: 9.778
    min: 0.040s max: 0.397s std dev: 0.07459s window: 85
average rate: 9.716
    min: 0.040s max: 0.397s std dev: 0.07393s window: 95

rostopic hz /pepper_robot/camera/depth/image_raw
subscribed to [/pepper_robot/camera/depth/image_raw]
average rate: 8.587
    min: 0.047s max: 0.202s std dev: 0.06842s window: 7
average rate: 9.408
    min: 0.042s max: 0.202s std dev: 0.05892s window: 17
average rate: 9.180
    min: 0.041s max: 0.265s std dev: 0.06764s window: 25
average rate: 8.417
    min: 0.041s max: 0.395s std dev: 0.08317s window: 31
average rate: 8.969
    min: 0.039s max: 0.395s std dev: 0.07960s window: 43
average rate: 9.658
    min: 0.039s max: 0.395s std dev: 0.07529s window: 56
average rate: 10.011
    min: 0.039s max: 0.395s std dev: 0.07036s window: 69
average rate: 10.089
    min: 0.039s max: 0.395s std dev: 0.06783s window: 79
average rate: 10.076
    min: 0.039s max: 0.395s std dev: 0.06959s window: 89
average rate: 9.908
    min: 0.039s max: 0.395s std dev: 0.06895s window: 98
k-okada commented 8 years ago

ok, so shell we merge this PR ? or need to publish both kRawDepthColorSpace and kDepthColorSpace ? or allow to switch this with json.conf ?

mikaelarguedas commented 8 years ago

lgtm the impact on performance doesnt seem to be too significant. What do the maintainers think about this @Karsten1987 @vrabaud @suryaambrose ?

Karsten1987 commented 8 years ago

:+1: lgtm

suryaambrose commented 8 years ago

Seems good to me as well

kochigami commented 8 years ago

Hi, if there is anything I can do for this pull request, please let me know. Sorry for my trouble...