spencer-project / spencer_people_tracking

Multi-modal ROS-based people detection and tracking framework for mobile robots developed within the context of the EU FP7 project SPENCER.
http://www.spencer.eu/
660 stars 327 forks source link

Documentation implies Kinect v1 should work straight out of the box #20

Closed kotaweav closed 7 years ago

kotaweav commented 7 years ago

In the documentation, it says:

This is the easiest way to get started using just a single RGB-D sensor connected locally to your computer. Place your Asus Xtion Pro Live or Kinect v1 sensor horizontally on a flat surface, and connect it to your computer (or play the example bagfile linked in a section further below). Then run the following launch file from your people tracking workspace (make sure that you have sourced it, e.g. source devel/setup.bash):

roslaunch spencer_people_tracking_launch tracking_single_rgbd_sensor.launch height_above_ground:=1.6

There are a number of documented cases where this hasn't worked for people when using a Kinect 1 as documented in https://github.com/spencer-project/spencer_people_tracking/issues/6, https://github.com/spencer-project/spencer_people_tracking/issues/7, and https://github.com/spencer-project/spencer_people_tracking/issues/17

One solution is to use the OpenNI2-FreenectDriver The problem here is that the ROS openni2 stack seems unstable and crashes quite often (at least when I tried it)

The other solution is to use the freenect stack instead. There are two steps that are necessary for this in tracking_single_rgbd_sensor.launch:

I should mention that I haven't been using HOG; this is purely for the upper body detector. the upper body detector requires the unregistered depth image instead of registered. If HOG requires the registered depth image, perhaps the upper body detector should be modified to use the registered depth image, as it seems the freenect stack can only publish one or the other. upon further inspection looks like groundHOG only uses image data (instead of depth) anyway and can run with unregistered image data so this shouldn't be an issue.

Note that this still doesn't fix for https://github.com/spencer-project/spencer_people_tracking/issues/10 or https://github.com/spencer-project/spencer_people_tracking/issues/19

I think the documentation should mention the two steps necessary to run spencer with the Kinect v1 as it's pretty unclear and a lot of people seem to be running into trouble with it.

tlind commented 7 years ago

Hi, thanks for your helpful comment! Indeed, in our sensor setup, we were always testing with the Asus Xtion Pro Live sensors. We were referencing to it as a 1st generation Kinect-Sensor to distinguish it from the Kinect v2 which we used in some of our later research.

I wasn't aware that OpenNi 2 does not support the original Kinect v1 anymore. What about using OpenNi 1 (ros-${DISTRO}-openni) instead? We only switched to OpenNi 2 because we were having USB 3 port-related issues. I remember that the switch involved remapping certain topics, though. So maybe we should provide two different launch files and update the info accordingly.

It is correct that groundHOG uses no depth information.

kotaweav commented 7 years ago

Ah, makes sense. I really wish Asus didn't discontinue those sensors. I wouldn't be running into any of these issues in that case.

Using OpenNI1 works as well but only with turning off depth registration. OpenNI 1 and 2 seem to publish most of the same topics so remapping shouldn't be necessary, though OpenNI2 will actually write to both the registered and unregistered topics if registration is enabled while OpenNI1 (and Freenect) will only write to one or the other.

For the usage in this project, it seems to me it shouldn't matter whether you're using OpenNI1 or Freenect since you're really only using the raw depth and image data. Maybe it would be nice to use OpenNI instead of Freenect for the sake of consistency however.

tlind commented 7 years ago

I just ran some tests and noticed that the upper-body detector does not output any detections if subscribed to the depth/image_raw instead of depth/image_rect topic published by OpenNi2. That's why we were subscribing it to the rectified image. This might be due to the former image being encoded as 16UC1 and the latter one as 32FC1 (millimeters vs. meters): In main.cpp l.193, the depth matrix is interpreted as an array of floats. Also note that using the registered depth image can be useful if enabling visualization by subscribing to the topic:

/spencer/perception_internal/people_detection/rgbd_front_top/upper_body_detector/image

(Therefore obviously, changing WORLD_SCALE in config_Asus.inp to 1000 or 0.001 also did not help, nor did scaling of the ground plane distance have any positive effect.)

I don't have a Kinect v1 here, so I cannot test it with OpenNi 1. But also there, the detector should be subscribed to a 32FC1 topic. It could be useful to output an error message if the input image is not encoded as such.

tlind commented 7 years ago

I have made some experimental changes to make the upper-body detector (and tracking_single_rgbd_sensor.launch) work with the Kinect v1 in the upper-body-det-fixes branch. The launch file needs to be run with the argument use_openni1:=true. If this works, it still needs to be documented in the README.

Still need to merge PR #23.

tlind commented 7 years ago

PR #23 now also merged into branch upper-body-det-fixes and README section updated.

kotaweav commented 7 years ago

@tlind so I tried your branch and ran into a few issues:

[ERROR] [1489501006.828057907]: PluginlibFactory: The plugin for class 'jsk_rviz_p
lugin/CameraInfo' failed to load.  Error: According to the loaded plugin descripti
ons the class jsk_rviz_plugin/CameraInfo with base class type rviz::Display does n
ot exist. Declared types are  rviz/Axes rviz/Camera rviz/DepthCloud rviz/Effort rv
iz/FluidPressure rviz/Grid rviz/GridCells rviz/Illuminance rviz/Image rviz/Interac
tiveMarkers rviz/LaserScan rviz/Map rviz/Marker rviz/MarkerArray rviz/Odometry rvi
z/Path rviz/PointCloud rviz/PointCloud2 rviz/PointStamped rviz/Polygon rviz/Pose r
viz/PoseArray rviz/Range rviz/RelativeHumidity rviz/RobotModel rviz/TF rviz/Temper
ature rviz/WrenchStamped rviz_plugin_tutorials/Imu spencer_tracking_rviz_plugin/De
tectedPersons spencer_tracking_rviz_plugin/HumanAttributes spencer_tracking_rviz_p
lugin/SocialActivities spencer_tracking_rviz_plugin/SocialRelations spencer_tracki
ng_rviz_plugin/TrackedGroups spencer_tracking_rviz_plugin/TrackedPersons

Looks like this is because of the remapping of the camera_info topic.

[ERROR] [1489501016.292542125]: Depth input image provided to upper-body detector 
has wrong encoding! 32FC1 is required (depth in meters), usually offered by the re
gistered/rectified depth image. Maybe you are remapping the input topic incorrectl
y to the unregistered, raw image of type 16UC1 (depth in millimeters)? 

This is the issue you mentioned earlier. openni1 will publish a rectified image which is of form 32FC1 so I think we can just keep using that. However, the key is to turn off depth registration, because otherwise the only topics published are under the depth_registered namespace instead of the depth namespace.

I've made the minimal changes I've found to make it run with a Kinect v1 and have submitted a pull request. This uses the same use_openni1:=true flag for Kinect v1.

tlind commented 7 years ago

The first issue appears because I added an additional Rviz display to the visualization, which visualizes the extents of the view frustum computed from the CameraInfo. You need to run sudo apt-get install ros-kinetic-jsk-rviz-plugins to get it, but it is not strictly required.

Regarding the latter error message, you mention a "rectified image of form 32FC1". On which topic is that one being published?

I'm a bit reluctant to set depth_registration to true in the one case, and to false in the other. That might confuse people. So maybe we could keep it enabled in both cases, and try to remap the detector input topics correctly into the depth_registered namespace for OpenNi1. That's what I actually tried to achieve in commit 8ed1fff8d, but it seems my remapping did not work (or OpenNi1 is doing something different from what I expected). Because if the remapping worked, the error message would imply that

/spencer/sensors/rgbd_front_top/depth_registered/image_raw

is not of type 32FC1 (maybe still 16UC1)!? It is sad that the documentation of the topics/image types published by OpenNi1/2 is very incomplete.

kotaweav commented 7 years ago

@tlind yeah, image_raw is 16UC1. Gotta stick with image_rect. You're using the software rectification with OpenNI2 already aren't you? You're not subscribed to the depth_registered topics anyway with OpenNI2. I think it's a lot more confusing to remap the registered depth image as unregistered than setting depth_registration to true in one case and false in the other. Does the OpenNI2 version work if depth_registration is set to false? Or alternatively the solution might be to set depth_registration to true and instead of remapping outputs of OpenNI just register to the correct topics from the detectors.

tlind commented 7 years ago

Ok, good point, we are indeed not subscribing to the depth_registered topics, I overlooked that! I'll take care of this tomorrow or so.

Edit: Finally found a list of all topics published by openni_launch (hopefully still up-to-date) at: http://wiki.ros.org/openni_launch?distro=groovy

tlind commented 7 years ago

New commit in the upper-body-det-fixes branch (40b3de83), which ensures we always subscribe to the 32FC1 topics image_rect and image_rect_color. For OpenNi 1, the detector input topics are remapped into the depth_registered namespace so we can keep depth registration enabled in all cases. This might be useful later on, when support is added for running the PCL detector instead of the upper-body detector from tracking_single_rgbd_sensor.launch.

tlind commented 7 years ago

I have merged this into master.