Support OpenNI2 devices without RGB

[metacomgd]

Adds basic support for the structure.io sensor (and presumably the Asus Xtion without RGB)

[VictorLamoine]

Please delete the extra line returns (there are a few across your pull request)

[VictorLamoine]

Have you tested it successfully ? I'm surprised no one tested the structure sensor with PCL before you did!

[metacomgd]

Think that's the spacing fixed.
Tested openni2_viewer; it works fine. I only have a structure sensor, so I don't know if the patch breaks anything with sensors with rgb, although I don't see how it would. There are a couple more areas that I've patched but still in a very hacky way; I'll rewrite them . I think this also fixes #1134 (at least the problem expressed there)

[VictorLamoine]

I'll test Monday with a Kinect and give the feedback here

[taketwo]

Thanks for cleaning up. Unfortunately there are still some extra empty lines here and there, would be great if you remove them.

Also, please do not extend existing pull request with unrelated commits. If you want to add a new app (openni2_pcd_recorder), please create a separate request. Speaking of the app though, maybe it's better to extend existing openni_pcd_recorder to handle both types of camera rathers than copy/paste code?

[mortlind]

What is the status for merging this fix, or parts of it, for using the Structure Sensor? I really want to use OpenNI2 + PCL for grabbing point clouds.

[mortlind]

By manual copying of the three files affected by metacomgd's commit 7dfc3ca to the current head (b60e20d) in the official branch, I confirm that I can use pcl_openni2_viewer. I did, however, over some time get segmentation fault. If interested, I can try to trace the cause.

[taketwo]

We can merge this fix as soon as we make sure that a) it reliably works with the Structure sensor; b) it does not affect streaming with color. If you are willing to work on this, please take over and create a new pull request with the fix. Once you have found and solved the segfault I can jump in and test whether streaming with color still works fine on my side.

[mortlind]

I have grabbed a fresh coppy of occipital/OpenNI2 development branch and compiled. Then I cloned the master from metacomgd/pcl, added and merged master from PoinCloudLibrary/pcl. Built a minimal cmake for grabbing and showing with openni2.

The minimal, and for me most relevant, test was running pcl_openni2_pcd_recorder. It ran for a good 20 minutes (@30 Hz) without problems before I ctrl-c'ed it. No thorough inspection of the dumped PCDs, but at visual inspection they seem fine.

However, after a few minutes of running pcl_openni2_viewer, I get this message with the segfault:

metacomgd-pcl/build/bin$ pcl_openni2_viewer
...
...
Average framerate (cloud callback): 30.0554 Hz
Average framerate (depth callback): 27.9984 Hz
ERROR: In /build/vtk-6WQee5/vtk-5.8.0/Common/vtkDataArrayTemplate.txx, line 215
vtkIdTypeArray (0xafc5468): Unable to allocate 512342 elements of size 4 bytes. 

terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
Aborted
metacomgd-pcl/build/bin$ 

Looks like a problem with vtk. I will try replacing libvtk5 with libvtk6.

[taketwo]

I can confirm that for me upstream pcl_openni2_viewer works reliably with Xtion camera, no crashes after about 15 minutes, both with and without -xyz option. I have vtk5. So I think there must be something in these commits that breaks otherwise stable system.

[mortlind]

I know this is not a final solution, but it brings the PCL master into usage for me: How about taking a patch from metacomgd which only addresses the grabber and the device files?

[taketwo]

I think we can do this. Please extract the corresponding changes and send a pull request. Also please make sure to format them according to our style guide (two spaces indentation, spaces between function names and braces, no spaces inside braces).

[mortlind]

The change to extract for basic support of the Structure Sensor, i.e. no application or tool changes, is displayed below. The changes to the two files are taken directly from metacomgd/pcl. To me the changes looks very simple, and follows the code formatting in the surrounding code. I will be making a minimal, external project utilizing the added support of grabbing from the Structure Sensor. This will be based on Cython, aspiring to the functionality of the python wrapper from libfreenect. Should be a 5 min. effort, if not for my lack of knowledge of PCL and only entry level skill with Cython. Until I have this application running stably, I can not really prove the below patch as stable. In the meantime, could someone with a build-environment and several sensors test if there is any other instabilities incurred by the patch?

diff --git a/io/src/openni2/openni2_device.cpp b/io/src/openni2/openni2_device.cpp
index 445f4a4..285f4df 100644
--- a/io/src/openni2/openni2_device.cpp
+++ b/io/src/openni2/openni2_device.cpp
@@ -89,7 +89,10 @@ pcl::io::openni2::OpenNI2Device::OpenNI2Device (const std::string& device_URI) :
   // Set default resolution if not reading a file
   if (!openni_device_->isFile ())
   {
-    setColorVideoMode (getDefaultColorMode ());
+    if (openni_device_->hasSensor (openni::SENSOR_COLOR))
+    {
+     setColorVideoMode (getDefaultColorMode ());
+    }
     setDepthVideoMode (getDefaultDepthMode ());
     setIRVideoMode (getDefaultIRMode ());
   }
diff --git a/io/src/openni2_grabber.cpp b/io/src/openni2_grabber.cpp
index 35c4335..8ed156e 100644
--- a/io/src/openni2_grabber.cpp
+++ b/io/src/openni2_grabber.cpp
@@ -219,7 +219,7 @@ pcl::io::OpenNI2Grabber::start ()
   try
   {
     // check if we need to start/stop any stream
-    if (image_required_ && !device_->isColorStreamStarted () )
+    if (image_required_ && !device_->isColorStreamStarted () && device_->hasColorSensor () )
     {
       block_signals ();
       device_->startColorStream ();
@@ -401,8 +401,8 @@ pcl::io::OpenNI2Grabber::startSynchronization ()
 {
   try
   {
-    if (device_->isSynchronizationSupported () && !device_->isSynchronized () && !device_->isFile () &&
-      device_->getColorVideoMode ().frame_rate_ == device_->getDepthVideoMode ().frame_rate_)
+    if (device_->hasColorSensor () && (device_->isSynchronizationSupported () && !device_->isSynchronized () && !device_->isFile () &&
+      device_->getColorVideoMode ().frame_rate_ == device_->getDepthVideoMode ().frame_rate_))
       device_->setSynchronization (true);
   }
   catch (const IOException& exception)

[taketwo]

I will test the patch with Xtion, however only in second next week, i.e. in June.

[jspricke]

As far as I can see this looks good to be merged. @taketwo did you have time to test it?

[taketwo]

Will test on Friday and submit a pull request if it's stable.

[taketwo]

The essential part of this PR that allows using the grabber with Structure.io sensor has been merged, so closing this. Please create a new pull request in case you want to modify the pcl_openni2_viewer app.