-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add support for bayer images to Ogre and Ogre2 #838
Add support for bayer images to Ogre and Ogre2 #838
Conversation
Signed-off-by: tejalbarnwal <tejalbarnwal@gmail.com>
Signed-off-by: tejalbarnwal <tejalbarnwal@gmail.com>
Since the title of the PR ends up in the changelog, mind changing it to maybe just "Add support for bayer images to Ogre and Ogre2". Feel free to reword, but "Pr" and "gzrendering7" should not be in the title. |
include/gz/rendering/Utils.hh
Outdated
|
||
// \brief convert a rgb image data into bayer image data | ||
GZ_RENDERING_VISIBLE | ||
void ConvertRGBToBayer(Image &_image); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for global functions, we use a lowercase first letter for the function name. Can you also make this return a copy of the image instead of modifying the source image, i.e. Image convertRGBToBayer(const Image &_image);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will return the destination image aka the variable destImageData
and use memcpy()
to actually modify the source image inside OgreRenderTarget.cc
. That seems good, right?
Signed-off-by: tejalbarnwal <tejalbarnwal@gmail.com>
Signed-off-by: tejalbarnwal <tejalbarnwal@gmail.com>
/// < Bayer GBGR, 1-byte per channel | ||
PF_BAYER_GBGR8 = 6, | ||
/// < Bayer GRGB, 1-byte per channel | ||
PF_BAYER_GRGB8 = 7, | ||
/// < Bayer GBRG, 1-byte per channel | ||
PF_BAYER_GBRG8 = 6, | ||
/// < Bayer GRBG, 1-byte per channel | ||
PF_BAYER_GRBG8 = 7, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @iche033, I have made the changes to the enum elements corresponding to bayer formats so that they are consistent with gazebo-classic. As you suggested, we could now review the outcomes of the ABI build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: The ABI compatibility failed :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes but looking closely at the report, the compatibility is 100%. It did catch the behavior change, which I'm leaning towards letting it slide since Bayer format was not working before.
/// < Bayer GBGR, 1-byte per channel | ||
PF_BAYER_GBGR8 = 6, | ||
/// < Bayer GRGB, 1-byte per channel | ||
PF_BAYER_GRGB8 = 7, | ||
/// < Bayer GBRG, 1-byte per channel | ||
PF_BAYER_GBRG8 = 6, | ||
/// < Bayer GRBG, 1-byte per channel | ||
PF_BAYER_GRBG8 = 7, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes but looking closely at the report, the compatibility is 100%. It did catch the behavior change, which I'm leaning towards letting it slide since Bayer format was not working before.
Signed-off-by: tejalbarnwal <tejalbarnwal@gmail.com>
Signed-off-by: tejalbarnwal <tejalbarnwal@gmail.com>
Signed-off-by: Ian Chen <ichen@openrobotics.org>
updated channel count of bayer image format from to 4 to 1 and fixed crash in 79a83c6 |
Signed-off-by: Ian Chen <ichen@openrobotics.org>
Codecov Report
@@ Coverage Diff @@
## gz-rendering7 #838 +/- ##
=================================================
- Coverage 77.37% 77.09% -0.28%
=================================================
Files 164 164
Lines 14459 14521 +62
=================================================
+ Hits 11187 11195 +8
- Misses 3272 3326 +54
|
🎉 New feature
Closes
gz-sensors
issue 299Sub-Tasks
BAYER_RGGB8
withOGRE
OGRE
OGRE2
Summary and Related PRs
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 toUtils.cc
insidegz-rendering
.gz-sensors
: Adds a switch case forRGGB bayer format
insideCameraSensor.cc
and passes theR8G8B8
format to render the image.gz-redering
: AddsConvertRGBToBayer()
toUtils.cc
, modifiesOgreRenderTarget.cc
to call the conversion function, and handles image format conversion functions withif-else
statements.gz-gui
: Adds switch cases to display Bayer images in Gazebo GUI inside. It treats them as single-channel 8-bit images.gz-common
: ModifiesImage::SetFromData
insideImage.cc
to support saving of Bayer images. In order to save it It treats them as single-channel 8-bit images.Test it
In order to test this, one can modify
camera_sensor.sdf
insidegz-sim
here. Would have just to replace the part with the following snippet.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.