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

Add L16 pixel format to Camera pixel format conversion function #487

Merged
merged 4 commits into from
Feb 18, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Feb 5, 2021

So we can convert <camera><image><format> string to PixelFormatType when parsing sdf.

Signed-off-by: Ian Chen ichen@osrfoundation.org

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Feb 5, 2021
@iche033 iche033 changed the title Add L16 pixel format to Camera DOM Add L16 pixel format to Camera pixel format conversion function Feb 5, 2021
@codecov-io
Copy link

codecov-io commented Feb 5, 2021

Codecov Report

Merging #487 (36ed570) into sdf10 (d33b4ae) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            sdf10     #487      +/-   ##
==========================================
- Coverage   87.78%   87.76%   -0.02%     
==========================================
  Files          62       62              
  Lines        9577     9579       +2     
==========================================
  Hits         8407     8407              
- Misses       1170     1172       +2     
Impacted Files Coverage Δ
src/Camera.cc 83.05% <0.00%> (-0.47%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d33b4ae...36ed570. Read the comment docs.

@adlarkin
Copy link
Contributor

adlarkin commented Feb 5, 2021

Do we need to update the SDFormat specification to include this addition? Looking at the website, it shows that the options for <camera><image><format> are L8|R8G8B8|B8G8R8|BAYER_RGGB8|BAYER_BGGR8|BAYER_GBRG8|BAYER_GRBG8 (I don't see L16 as an option).

@scpeters
Copy link
Member

scpeters commented Feb 5, 2021

Do we need to update the SDFormat specification to include this addition? Looking at the website, it shows that the options for <camera><image><format> are L8|R8G8B8|B8G8R8|BAYER_RGGB8|BAYER_BGGR8|BAYER_GBRG8|BAYER_GRBG8 (I don't see L16 as an option).

that documentation comes from: https://github.com/osrf/sdformat/blob/sdformat10_10.2.0/sdf/1.7/camera.sdf#L21

@scpeters
Copy link
Member

scpeters commented Feb 5, 2021

here's the full list in the enum for reference: https://github.com/osrf/sdformat/blob/sdformat10_10.2.0/include/sdf/Camera.hh#L39-L60

should we add FLOAT16 and FLOAT32?

@adlarkin
Copy link
Contributor

adlarkin commented Feb 5, 2021

should we add FLOAT16 and FLOAT32?

I think these would be good to add, unless anyone else has a reason against it (I'm not too familiar with the osrf/sdformat repository, so perhaps there are reasons for not adding these that I am unaware of).

Signed-off-by: Ashton Larkin <ashton@openrobotics.org>
@adlarkin
Copy link
Contributor

should we add FLOAT16 and FLOAT32?

@scpeters I went ahead and added L8, FLOAT16 and FLOAT32 as camera formats to the documentation: 067cf73

@scpeters
Copy link
Member

scpeters commented Feb 10, 2021

should we add FLOAT16 and FLOAT32?

@scpeters I went ahead and added L8, FLOAT16 and FLOAT32 as camera formats to the documentation: 067cf73

thanks for making the change for L8, but now that I look more closely, I don't think we should document FLOAT16 or FLOAT32 unless we add support for them to the conversion functions. it seems that R_FLOAT16 and R_FLOAT32 are more precise and used in ign-msgs, so I retract my suggestion that we support FLOAT16 and FLOAT32. I'll go ahead and revert those from your change add the R_ prefix to those values; thanks for your patience

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@adlarkin adlarkin mentioned this pull request Feb 18, 2021
@adlarkin adlarkin merged commit abcd0a7 into sdf10 Feb 18, 2021
@adlarkin adlarkin deleted the camera_l16 branch February 18, 2021 18:24
chapulina added a commit that referenced this pull request Mar 9, 2021
* Add L16 pixel format to Camera pixel format conversion function (#487)

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Ashton Larkin <ashton@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>

* 🎈 10.3.0 (#498)

Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>
Co-authored-by: Ashton Larkin <ashton@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
@peci1
Copy link
Contributor

peci1 commented Mar 15, 2021

@adlarkin The online docs at http://sdformat.org/spec?ver=1.7&elem=sensor#image_format still aren't updated. Is some manual intervention needed to update them?

@chapulina
Copy link
Contributor

Is some manual intervention needed to update them?

Yup, I think @azeey has been triggering those.

@azeey
Copy link
Collaborator

azeey commented May 10, 2021

@adlarkin The online docs at http://sdformat.org/spec?ver=1.7&elem=sensor#image_format still aren't updated. Is some manual intervention needed to update them?

The online docs have been updated.

scpeters added a commit to scpeters/sdformat that referenced this pull request May 17, 2021
* added to sdf 1.7 in gazebosim#487

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit that referenced this pull request May 19, 2021
* sdf 1.8: Add <double_sided> to material from #410
* sdf 1.8: Add lightmap to 1.8 spec from #429
* sdf 1.8: document Add L16 camera pixel format from #487

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* sdf 1.8: Decrease far clip lower bound from #435

Signed-off-by: Nate Koenig <nate@openrobotics.org>

* sdf 1.8: Added render_order to material from #446

Signed-off-by: ahcorde <ahcorde@gmail.com>

* sdf 1.8: Add camera type aliases to docs. from #514
* sdf 1.8: Improve docs of collision_bitmask from #521

Signed-off-by: Martin Pecka <peckama2@fel.cvut.cz>

* sdf 1.8: support nested models in @attached_to from #316

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit to scpeters/sdformat that referenced this pull request Apr 8, 2022
…bosim#487)

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Ashton Larkin <ashton@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit that referenced this pull request Apr 8, 2022
Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Ashton Larkin <ashton@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants