gazebosim / gz-sensors

Provides numerous sensor models designed to generate realistic data from simulation environments.
https://gazebosim.org
Apache License 2.0
57 stars 58 forks source link

Bayer images not supported #299

Closed Acwok closed 1 year ago

Acwok commented 1 year ago

Environment

Description

I am trying to simulate bayer images using the BAYER_BGGR8 format.

<camera>
  <image>
    <height>${camera_height}</height>
    <width>${camera_width}</width>
    <format>BAYER_BGGR8</format>
  </image>
</camera>

It seems the bayer format is missing from the switch case here: https://github.com/gazebosim/gz-sensors/blob/8e8dfa84371d865f54223aa9585e8b255c0b6605/src/CameraSensor.cc#L210-L226

This results in the following error:

[ruby $(which ign) gazebo-1] [Err] [CameraSensor.cc:221] Unsupported pixel format [16]
iche033 commented 1 year ago

not supported yet in gz-rendering. One option is to port it from gazebo-classic: https://github.com/gazebosim/gazebo-classic/blob/gazebo11/gazebo/rendering/Camera.cc#L1381

tejalbarnwal commented 1 year ago

Hey, I would like to work on this issue and have been working on implementing an example for a custom rendering sensor. I had quite a few clarifications to make about what I understood till now about the way rendering sensors present in gz-sensors use the gz-rendering library with ogre.

I apologize for asking such a silly question, but I am just getting started with understanding the implementation of rendering sensors and would like to confirm it so I do not make a mistake. To find where exactly in gz-rendering I need to make changes to add bayer conversion function, I explored the codebase. I started with the normal camera sensor to get an initial understanding of the implementation of rendering sensors. Here's what I understood from the exploration:

Here’s what I wanted to ask:

  1. Am I headed in the right direction to infer the above ideas behind the implementation?
  2. Further, I couldn't find where the OgreRenderTarget::Copy or OgreRenderTexture::Buffer methods are called and where it actually converts the image format to, say, RGB to grayscale when simulation starts in gazebo. Could you please help me with that?
  3. I also found some OgreSensors like OgreCamera and OgreDepthCamera. What are they used for?

Thank you for your patience as well. My comment is probably rather lengthy.:sweat_smile:

iche033 commented 1 year ago

thanks for working on this! Let me answer your questions below:

  1. yes your understanding is correct. One thing to note is that by default gazebo launches with the ogre2 render engine. Nonetheless, we can come up with a generic solution that can be re-used by different render engines.
  2. OgreRenderTarget::Copy is called by BaseCamera::Copy which is in turn called by CameraSensor here. As for converting from RGB to grayscale, the underlying render engine does that for us. As an example, in ogre render engine when we create a render texture, we give it an image format here (ogreFormat).
  3. BaseCamera as its name suggests is the base class for camera. You will find that its functions are virtual and overriden by derived classes such as OgreCamera and Ogre2Camera which is doing most of the work. OgreDepthCamera renders depth data stored in floating point image data.

So going back to Bayer conversion. I think one implementation option would be to create a generic function convertRGBToBayer, e.g. in Utils.cc and call that in OgreRenderTarget::Copy / Ogre2RenderTarget::Copy.

One thing to watch out for is that if user specifies a format like BAYER_RGGB8, currently gz-rendering does not know about this format, so the image format conversion functions like here will not work and we will need to catch that. We can just create RGB render texture in the underlying render engine and do Bayer conversions when we read back the values in Copy.

tejalbarnwal commented 1 year ago

Hey @iche033, I was able to implemented and simulated bayer images using the BAYER_RGGB8 format for ogre :partying_face:! image

I'll do the same for ogre2 and extend the functionality to include the other three Bayer formats as well!

I wanted to discuss a few things:

  1. Presently, to display the image with Image Display Plugin under gz-gui, I have added the following snippet inside a switch statement under ImageDisplay::ProcessImage() in ImageDisplay.cc.

    case msgs::PixelFormatType::BAYER_RGGB8:
      common::Image::ConvertToRGBImage<uint8_t>(
          this->dataPtr->imageMsg.data().c_str(), width, height, output);
      break;

    which passes the image as a single channel 8-bit image to display it.

    I see(referring to the implementation with gazebo classic) that bayer images were implemented as single channel 8 bit per pixel images as specified here.

    Further, I found that under PixelUtil::channelCounts[PF_COUNT], the no of channels assigned to the bayer formats are 4. I understand that each channel corresponds to one color among RGGB. Could you please help me understand if four 8-bit channels are required, or could we work with just a single 8-bit channel? In either case, I will modify the ImageDisplay::ProcessImage() function inside the Image Display Plugin accordingly.

  2. I couldn't find image.format() being used with the Optix engine. Does it use image format? Though from its OptixRenderTarget::Copy(Image &_image) function, it seems to create a 3-channel image(might be RGB).

tejalbarnwal commented 1 year ago

Also, shall I create a draft PR in each affecting repository with what I have implemented so far since there are files in different repositories that would require some small additions/ modifications?

iche033 commented 1 year ago

Nice!

Presently, to display the image with Image Display Plugin under gz-gui, I have added the following snippet inside a switch statement under ImageDisplay::ProcessImage() in ImageDisplay.cc.

That's sounds good

Further, I found that under PixelUtil::channelCounts[PF_COUNT], the no of channels assigned to the bayer formats are 4. I understand that each channel corresponds to one color among RGGB. Could you please help me understand if four 8-bit channels are required, or could we work with just a single 8-bit channel?

oh good catch, I think we should be consistent with gazebo-classic and change it to 8 bit.

I couldn't find image.format() being used with the Optix engine. Does it use image format? Though from its OptixRenderTarget::Copy(Image &_image) function, it seems to create a 3-channel image(might be RGB).

Let's skip optix implementation. The code is outdated.

Also, shall I create a draft PR in each affecting repository with what I have implemented so far since there are files in different repositories that would require some small additions/ modifications?

Sure, I can take a look at the PRS once they are up. Something to watch out for is that If you had to modify enums, it could break ABI and the changes would need to be targeted at main branch.

tejalbarnwal commented 1 year ago

Hey @iche033, I have opened PRs in four repositories. Here's a summary of the same. The functionality reads the user input and renders an RGB image, which is later converted into a single channel 8bit Bayer image using ConvertRGBToBayer() added to Utils.cc inside gz-rendering.

I also had a few points to discuss with you:

  1. I found that there is a little discrepancy in the 2 BAYER formats (GRBG and GBRG) specified at two different places:PixelFormat.hh in gz-rendering and Image.hh in gz-common. I have implemented it with GRBG and GBRG which were also the formats used in gazebo-classic(haven't pushed the changes though). These Bayer formats would require changing enum elements inside PixelFormat.hh. Could you please confirm if I should go ahead with GRBG and GBRG instead of GRGB and GBGR?
  2. I also tried converting the no of channels assigned to the bayer formats inside PixelUtil::channelCounts[PF_COUNT] from 4 to 1. Upon modifying, I received error double linked list corrupted. I am still working on debugging it.
iche033 commented 1 year ago

thanks for the PRs!

  1. These Bayer formats would require changing enum elements inside PixelFormat.hh. Could you please confirm if I should go ahead with GRBG and GBRG instead of GRGB and GBGR?

hmm that's a difficult one as we usually we don't change enum values in a released version. On the other hand, no code is using that particular enum (since it never worked). Can you try make the change so that it is consistent with gazebo-classic. That'll trigger a new ABI build, and if the build show that ABI compatibility fails, we would need to think of an workaround.

I also tried converting the no of channels assigned to the bayer formats inside PixelUtil::channelCounts[PF_COUNT] from 4 to 1. Upon modifying, I received error double linked list corrupted. I am still working on debugging it.

~I tried changing them from 4 to 1 in you branch but didn't run into the crash.~ Update: oh spoke too soon, yep I get the crash

If it helps, you can debug gz with gdb with instructions from this page:

https://gazebosim.org/api/gazebo/7.0/debugging.html

You'll just need to replace ign with gz.

tejalbarnwal commented 1 year ago

Hey, @iche033, I am done with adding support for bayer formats to ogre and ogre2, along with all the modifications required in visualization plugins. Sorry, I was busy with some of my semester exams last week and therefore couldn't complete it.

The only thing that needs to be catered is the no of channels assigned to the bayer formats inside PixelUtil::channelCounts[PF_COUNT]. I tried changing it to 1 or 2, but the program crashed. Interestingly it doesn't crash if I keep it 3.

iche033 commented 1 year ago

thanks for iterating! Were you get to get a backtrace or find out where the crash is coming from?

iche033 commented 1 year ago

I changed the channel count to 1 and fixed the crash in your gz-rendering PR 79a83c6. See if that works for you.

tejalbarnwal commented 1 year ago

@iche033, Yup it works perfectly! Thank you!

I wanted to request your help in understanding how you were able to debug the segmentation fault issue in Gazebo that I was struggling with. I could only reach the point where I found that segmentation fault when trying to update entities. image

I would be grateful if you could explain to me how you approached the debugging process and any techniques or tools that you used. Specifically, I am interested in understanding how you made breakpoints and used Valgrind (incase you used it) on the SDF file. As I am not very familiar with using a debugger, I would greatly appreciate any help you offer.

iche033 commented 1 year ago

I didn't have a quick and easy way for debugging this one. Backtrace wasn't that useful but I know it's a memory corruption issue since we're change the number of bytes. So I started looking at places that could be impacted by the change. The more obvious one is here when an Image object is created. A Bayer image now has a a much smaller but correct size. Then I started taking a look at the new Bayer conversion function and how it's used. That's when I noticed the bug. We're passing an image of size h*w to the conversion function that expects an RGB image. Hope that helps

tejalbarnwal commented 1 year ago

I didn't have a quick and easy way for debugging this one. Backtrace wasn't that useful but I know it's a memory corruption issue since we're change the number of bytes. So I started looking at places that could be impacted by the change. The more obvious one is here when an Image object is created. A Bayer image now has a a much smaller but correct size. Then I started taking a look at the new Bayer conversion function and how it's used. That's when I noticed the bug. We're passing an image of size h*w to the conversion function that expects an RGB image. Hope that helps

yup, got it! Please let me know if there is anything else I need to do to get the PRs merged. Once again, thank you for your help with this.