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

image_view_node: support bayer images #1046

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

rursprung
Copy link

so far bayer images always failed with an error:

[ERROR] [..] [image_view_node]: Unable to convert 'bayer_rggb8' image for display: 'cv_bridge.cvtColorForDisplay() does not have an output encoding                that is color or mono, and has is bit in depth'

the stereo_view_node on the other hand already supports bayer images, however it always forcibly converts them to monochrome, even if they are colour images.

for now, this adds the same logic for the single-image viewer and thus only partially resolves #1045.

so far bayer images always failed with an error:
```
[ERROR] [..] [image_view_node]: Unable to convert 'bayer_rggb8' image for display: 'cv_bridge.cvtColorForDisplay() does not have an output encoding                that is color or mono, and has is bit in depth'
```

the `stereo_view_node` on the other hand already supports bayer images,
however it always forcibly converts them to monochrome, even if they are
colour images.

for now, this adds the same logic for the single-image viewer and thus
only partially resolves ros-perception#1045.
Copy link
Member

@mikeferguson mikeferguson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable given what we do in stereo node

@rursprung
Copy link
Author

i just realised that i was a bit tired when i thought yesterday that it's better to go with mono for now and then figure out colour later on... bayer is of course always colour, so i guess it'd be fine to just always use bgr (both here & in stereo)? if you concur, i can add a commit to this PR which changes it in both places

@mikeferguson
Copy link
Member

bayer is of course always colour, so i guess it'd be fine to just always use bgr (both here & in stereo)

I'm pretty sure this won't show the expected colors you want. I think the trick here is that by casting to 'mono8' we treat the 4 pixels (2 green, 1 red, 1 blue) as a black and white image that is basically 2x the resolution of the actual image and so it looks roughly correct.

If you do test out 'bgr' and it looks OK - please post some screen shots here of the 'mono' and 'bgr' for review (I don't have any bayer cameras around to test with).

@rursprung
Copy link
Author

ok, i'll do that either tonight or in the coming days.
it might be cleanest to merge this PR for now and then i'll post the information on the issue and raise a new PR?

it did look ok when i tried it (though the colours were off, but that's because i'd need to set white balance & co. in the camera for it to look correct)

@rursprung
Copy link
Author

If you do test out 'bgr' and it looks OK - please post some screen shots here of the 'mono' and 'bgr' for review (I don't have any bayer cameras around to test with).

i've posted it on the issue: #1045 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

image_view: limited bayer support
2 participants