moetsi / Sensor-Stream-Pipe

Open Source Sensor Stream Server and Client for Digitizing Reality
MIT License
71 stars 11 forks source link

ZDepth decoder failure #9

Closed eidetic-av closed 2 years ago

eidetic-av commented 3 years ago

I am trying to implement a server+client that uses a pub/sub architecture instead of push/pull.

It is working fine for raw frames ("null" types), and for frames encoded using libav (x264 and x265), but I am getting a crash decoding ZDepth frames.

ZDepthDecoder::Decode(..) is failing, and I see there are some comments surrounding it but I don't understand what they mean.

https://github.com/moetsi/Sensor-Stream-Pipe/blob/ad751d07301da4d988a3295bcbc1b67c2187112e/decoders/zdepth_decoder.cc#L15-L27

result != zdepth::DepthResult::Success is always equal to true. So I assume I am running into the "input error" mentioned in the comments.

@amourao, could you provide some insight into what this input error means?

Does this mean that the frame.frame I am passing in is invalid for the decoder? -- maybe my server is sending them incorrectly somehow?

Thanks!

amourao commented 3 years ago

Yeah, you are correct, that has to do with the I and P frames. Basically, to save bandwidth, only the first frame is sent as a full I frame; the remaining frames are sent as P (incomplete/additive frames) that depend on a I frame.

If you are getting that error, it means that you are not receiving the first frame correctly.

As a temporary fix, you can change this line https://github.com/moetsi/Sensor-Stream-Pipe/blob/ad751d07301da4d988a3295bcbc1b67c2187112e/encoders/zdepth_encoder.cc#L116

to compressor_.Compress(width_, height_, data, compressed_buffer_, true);

and send all frames as I frames.

Could you recompile SSP with that fix and check if it is working?

eidetic-av commented 3 years ago

Ah great, that makes sense!

The change you suggested works.

Since my client is only subscribing to the latest frames, if the server has been running for a while, the client never receives the I frame when it connects.

So, I am thinking of two ways I could solve this for my implementation...

  1. I could keep it so that the server sends P Frames but keeps the initial I Frame stored, with every new subscriber reading this I Frame first before subscribing to the P Frame stream.

  2. I could try to implement the "send I Frame every X frames" ... this seems like it could be more useful.

Is there a way for the decoder to recognize whether it is receiving a keyframe?

If not, perhaps every x frames a 'keyframe' parameter could be set in the FrameStruct.

struct FrameStruct {
    // false for P frame, true for I frame
    bool keyframe;
}

Which can then be checked on the decoder side. The decoding would then not begin until FrameStruct.keyframe == true.

Does this sound viable?

amourao commented 3 years ago

Option 1 is not feasible, as the first I frame is only valid if sent before frame 2. Option 2 is closer to how video codecs work.

I've implemented option 2, you can add send_I_frame_interval: 30 to the yaml config, right after type: "zdepth" to send a I frame every 30 frames. Let me know it this solves your problem.

eidetic-av commented 3 years ago

This is pretty much the solution I am using, although the implementation you commited doesn't solve the OpenCV error.

The cv::Mat constructor expects decompressed_buffer_.data() to have a size() > 0. So currently, the application still crashes since decompressor_.Decompress() returns before filling the buffer if there is no valid keyframe to use.

This is my solution for now:

cv::Mat ZDepthDecoder::Decode(FrameStruct& frame) {

  zdepth::DepthResult result =
      decompressor_.Decompress(frame.frame, width_, height_, decompressed_buffer_);
  if (result != zdepth::DepthResult::Success) {
    spdlog::error("ZDepth decompression error: {}", DepthResultString(result));
    // Return a blank texture on an error
    return cv::Mat::zeros(height_, width_, CV_16UC1);
  }

  return cv::Mat(height_, width_, CV_16UC1, decompressed_buffer_.data(),
                 cv::Mat::AUTO_STEP);
}

This way, the application doesn't crash. Decode just returns blank frames until a keyframe is received.

It works in a pinch, but it's not ideal because there's no indication to the caller that Decode is returning a dud frame. I still think a keyframe parameter in FrameStruct would be more useful, because then when the frame is deserialised the caller can decide whether to even try to decode it or not.

catid commented 3 years ago

Whew glad it wasn't a bug in the codec ^_^;

adammpolak commented 3 years ago

This is pretty much the solution I am using, although the implementation you commited doesn't solve the OpenCV error.

The cv::Mat constructor expects decompressed_buffer_.data() to have a size() > 0. So currently, the application still crashes since decompressor_.Decompress() returns before filling the buffer if there is no valid keyframe to use.

This is my solution for now:

cv::Mat ZDepthDecoder::Decode(FrameStruct& frame) {

  zdepth::DepthResult result =
      decompressor_.Decompress(frame.frame, width_, height_, decompressed_buffer_);
  if (result != zdepth::DepthResult::Success) {
    spdlog::error("ZDepth decompression error: {}", DepthResultString(result));
    // Return a blank texture on an error
    return cv::Mat::zeros(height_, width_, CV_16UC1);
  }

  return cv::Mat(height_, width_, CV_16UC1, decompressed_buffer_.data(),
                 cv::Mat::AUTO_STEP);
}

This way, the application doesn't crash. Decode just returns blank frames until a keyframe is received.

It works in a pinch, but it's not ideal because there's no indication to the caller that Decode is returning a dud frame. I still think a keyframe parameter in FrameStruct would be more useful, because then when the frame is deserialised the caller can decide whether to even try to decode it or not.

This is a good point @eidetic-av , we will look into this.