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

ASUS Xtion Pro support with OpenNI2 #757

Closed
mpp opened this issue Jun 20, 2014 · 3 comments · Fixed by #4141
Closed

ASUS Xtion Pro support with OpenNI2 #757

mpp opened this issue Jun 20, 2014 · 3 comments · Fixed by #4141
Labels
needs: pr merge Specify why not closed/merged yet

Comments

@mpp
Copy link

mpp commented Jun 20, 2014

I've just compiled from source enabling BUILD_OPENNI2 and WITH_OPENNI2 CMake variables.
I'm using the ASUS Xtion Pro with no RGB camera. When I create a pcl::io::openni2::OpenNI2DeviceManager object and I use its getAnyDevice() method my application crash because it is trying to perform this line of the file openni2_device.cpp: setColorVideoMode (getDefaultColorMode ())
I think that here there should be a check if the device support color or not.
Am I right?
here is a code sample:

#include <pcl\io\openni2_grabber.h>
#include <pcl\visualization\pcl_visualizer.h>
#include <pcl\visualization\boost.h>
#include <pcl\visualization\image_viewer.h>

#include <iostream>

int main (int argc, char** argv)
{
    std::cout << "hi" << std::endl;


    boost::shared_ptr<pcl::io::openni2::OpenNI2DeviceManager> 
    deviceManager = pcl::io::openni2::OpenNI2DeviceManager::getInstance ();

    if (deviceManager->getNumOfConnectedDevices () > 0)
    {
        boost::shared_ptr<pcl::io::openni2::OpenNI2Device> device = deviceManager->getAnyDevice ();
        cout << "Device ID not set, using default device: " << device->getStringID () << endl;
    }

    return 0;
}
@akshmakov
Copy link

I did some digging. Let me give the punchline first

pcl::io::openni2::OpenNI2Device::getDefaultColorMode () const
{
  // Search for and return VGA@30 Hz mode
  vector<OpenNI2VideoMode> modeList = getSupportedColorVideoModes ();
  for (vector<OpenNI2VideoMode>::iterator modeItr = modeList.begin (); modeItr != modeList.end (); modeItr++)
  {
    OpenNI2VideoMode mode = *modeItr;
    if ( (mode.x_resolution_ == 640) && (mode.y_resolution_ == 480) && (mode.frame_rate_ == 30.0) )
      return mode;
  }
  return (modeList.at (0)); // <<<<-----BADUM-TSCH
}

Now for the details

getDefaultColorMode() calls getSupportedColorVideoModes() . Which is the relevant function where the problem starts.

pcl::io::openni2::OpenNI2Device::getSupportedColorVideoModes () const
{
  boost::shared_ptr<openni::VideoStream> stream = getColorVideoStream ();

  color_video_modes_.clear ();

  if (stream)
  {
    const openni::SensorInfo& sensor_info = stream->getSensorInfo ();

    color_video_modes_ = openniModeToGrabberMode (sensor_info.getSupportedVideoModes ());
  }

  return (color_video_modes_);
}

getColorVideoStream() will actually check if the camera has a color stream and return color_video_stream_ . The color_video_modes_ is a vector and .clear() will clear the vector. Since color_video_stream_ is unset it nothing will ever be added to color_video_modes_ and so it is an empty list.

Going back up to getSupportedColorVideoModes() it will by default return the first element in the array (return (modeList.at (0))), but the array is empty, hence you have the segfault.

setColorVideoMode() isn't important here because it does nothing if it doesn't have a stream anyway. The problem is the empty vector dereference.

This is a bug, there needs to be some array size checking when dereferencing or an out_of_range exception catch, or potentially a change to the way getColorVideoStream() deals with non-existing streams. I'll wait for a dev to comment but I can volunteer to make a pull request.

@jspricke
Copy link
Member

Hi @akshmakov I think this is really old code, so it would be great if you can send a pull request with a fix, thanks for digging into it :).

@stale
Copy link

stale bot commented May 19, 2020

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: pr merge Specify why not closed/merged yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants