stevenlovegrove / Pangolin

Pangolin is a lightweight portable rapid development library for managing OpenGL display / interaction and abstracting video input.
MIT License
2.39k stars 855 forks source link

VideoViewer cannot stream realsense2 #461

Open Clocktown opened 5 years ago

Clocktown commented 5 years ago

Running VideoViewer realsense2:// prints the following:

Stream 0: 640 x 480 GRAY16LE (pitch: 1280 bytes)
Stream 1: 640 x 480 RGB24 (pitch: 1920 bytes)
terminate called after throwing an instance of 'rs2::wrong_api_call_sequence_error'
  what():  start() cannot be called before stop()
Aborted (core dumped)

The camera is working in realsense-viewer as well as through the API with my own C++ examples (that do not use Pangolin).

System Info

Ubuntu 16.04 64 Bit GCC 5.4.0 master branch

stevenlovegrove commented 5 years ago

If you're able to compare the Pangolin implementation with your own, that would be a good start. @nazcaspider provided Pangolins implementation - perhaps he has an idea?

Clocktown commented 5 years ago

@stevenlovegrove Unfortunately, all the code (as well as the hardware) is at work, and I will only get back there sometime into the new year.

However, I did manage to fix it in a local clone of the repo some time after posting the issue at work. Juts like the error message says, the start method from realsense2 is called before stop - specifically, start is called twice without stop in between, and realsense2 does not like that.

This happens because the driver in Pangolin calls start already in the constructor, and then the VideoViewer application (or something else) calls the start member function of the driver in addition to that, which calls realsense2's start again, causing the error.

I see two options:

  1. Add a kind of flag to the driver implementation that remembers if it is started or stopped and simply early out of the start and stop members if in the wrong state (I implemented this at work)
  2. Simply require from the users of the drivers to never call start or stop member functions several times in a row, except interleaved and fix it for VideoViewer

However, (2) might break behaviour with other drivers if they somehow require multiple calls. There is also the question whether start should be called in the constructor and I didn't check what the other drivers do. If some do not call it, driver-agnostic code would definetly have to call start after construction and then (2) would not make sense. I'd just go with (1).

stevenlovegrove commented 5 years ago

Yeah - I should add to the interface that start and stop should be idempotent and it should not be an error to call multiple times. I agree, 1) sounds like the best option. Thanks!

lericson commented 5 years ago

FWIW I fixed this issue

diff --git a/src/video/drivers/realsense2.cpp b/src/video/drivers/realsense2.cpp
index 591d3b7..9f61868 100644
--- a/src/video/drivers/realsense2.cpp
+++ b/src/video/drivers/realsense2.cpp
@@ -9,12 +9,12 @@ RealSense2Video::RealSense2Video(ImageDim dim, int fps)

     sizeBytes = 0;

-    // Create RealSense pipeline, encapsulating the actual device and sensors
-    pipe = new rs2::pipeline();
-
     //Configure the pipeline
     cfg = new rs2::config();

+    // Allocated when started
+    pipe = nullptr;
+
     {   //config depth
         cfg->enable_stream(RS2_STREAM_DEPTH, dim_.x, dim_.y, RS2_FORMAT_Z16, fps_);
         StreamInfo streamD(PixelFormatFromString("GRAY16LE"), dim_.x, dim_.y, dim_.x*2, 0);
@@ -29,31 +29,42 @@ RealSense2Video::RealSense2Video(ImageDim dim, int fps)
         sizeBytes += streamRGB.SizeBytes();
     }

-    // Start streaming with default recommended configuration
-    pipe->start(*cfg);
-    rs2::pipeline_profile profile = pipe->get_active_profile();
-    auto sensor = profile.get_device().first<rs2::depth_sensor>();
-    auto scale = sensor.get_depth_scale();
-    std::cout << "Depth scale is: " << scale << std::endl;
-
     total_frames = std::numeric_limits<int>::max();
 }

 RealSense2Video::~RealSense2Video() {
-    delete pipe;
-    pipe = nullptr;
-
     delete cfg;
     cfg = nullptr;
 }

 void RealSense2Video::Start() {
+    if (pipe) {
+        std::cerr << "Start() called when already started" << std::endl;
+        return;
+    }
+
+    // Create RealSense pipeline, encapsulating the actual device and sensors
+    pipe = new rs2::pipeline();
+
+    // Start streaming with default recommended configuration
     pipe->start(*cfg);
+
+    rs2::pipeline_profile profile = pipe->get_active_profile();
+    auto sensor = profile.get_device().first<rs2::depth_sensor>();
+    auto scale = sensor.get_depth_scale();
+    std::cout << "Depth scale is: " << scale << std::endl;
+
     current_frame_index = 0;
 }

 void RealSense2Video::Stop() {
+    if (pipe == nullptr) {
+        std::cerr << "Stop() called before Start()" << std::endl;
+        return;
+    }
     pipe->stop();
+    delete pipe;
+    pipe = nullptr;
 }

 size_t RealSense2Video::SizeBytes() const {
@@ -66,6 +77,11 @@ const std::vector<StreamInfo>& RealSense2Video::Streams() const {

 bool RealSense2Video::GrabNext(unsigned char* image, bool /*wait*/) {

+    if (pipe == nullptr) {
+        std::cerr << "GrabNext() called before Start()" << std::endl;
+        Start();
+    }
+
     unsigned char* out_img = image;

     rs2::frameset data = pipe->wait_for_frames(); // Wait for next set of frames from the camera

Update: Also made ::Stop() idempotent.

lericson commented 5 years ago

My patch co-opts the pipe pointer to signal that the driver has started.

ihpdep commented 5 years ago

Hi, I got an error like

libc++abi.dylib: terminating with uncaught exception of type pangolin::VideoExceptionNoKnownHandler: No known video handler for URI 'realsense2'

when I called follow:

video = pangolin::OpenVideo("realsense2://");

Do I need to do anything before I use the realsense2? The device is Okay since all example in the official realsense2 code works. Also, I pull the latest code on the master branch.

lericson commented 5 years ago

Please don’t be so bad mannered as to clobber somebody’s legitimate bug report with your own tech support questions, @ihpdep.

ihpdep commented 5 years ago

@lericson Sorry about posting the question. I thought it was convenient to put similar questions together. I will post my question separately.

stevenlovegrove commented 3 years ago

@ihpdep I'm not sure if you still use this diff, but if you're able to open a PR against master I can merge in. Thanks!