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

Fix a few minor issues in image_helper #68

Merged
merged 3 commits into from
Oct 29, 2020

Conversation

mjeronimo
Copy link
Contributor

  • Used ByteIO rather than StringIO is now required for PIL's Image.open
  • Remove unused variable, 'alpha'
  • Added support for '8UC1' and '32FC1' image types
  • Only do bgr2rgb if it's definitely a BGR type

Fixes #60

Signed-off-by: Michael Jeronimo michael.jeronimo@openrobotics.org

Copy link

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

Dependent on #67.

Is there an easy way to test this?

If I right-click on an image thumbnail > View > Raw, then I still see some errors that sound related to the fix here. Maybe that can be fixed at the same time?

Traceback (most recent call last):
  File "/home/developer/viper/catkin_ws/install_isolated/lib/python3/dist-packages/rqt_bag/plugins/message_view.py", line 93, in event
    self.message_viewed(bag, msg_data)
  File "/home/developer/viper/catkin_ws/install_isolated/lib/python3/dist-packages/rqt_bag/plugins/raw_view.py", line 73, in message_viewed
    self.message_tree.set_message(msg)
  File "/home/developer/viper/catkin_ws/install_isolated/lib/python3/dist-packages/rqt_bag/plugins/raw_view.py", line 115, in set_message
    self._add_msg_object(None, '', '', msg, msg._type)
  File "/home/developer/viper/catkin_ws/install_isolated/lib/python3/dist-packages/rqt_bag/plugins/raw_view.py", line 239, in _add_msg_object
    self._add_msg_object(item, subpath, subobj_name, subobj, subobj_type)
  File "/home/developer/viper/catkin_ws/install_isolated/lib/python3/dist-packages/rqt_bag/plugins/raw_view.py", line 239, in _add_msg_object
    self._add_msg_object(item, subpath, subobj_name, subobj, subobj_type)
  File "/home/developer/viper/catkin_ws/install_isolated/lib/python3/dist-packages/rqt_bag/plugins/raw_view.py", line 207, in _add_msg_object
    obj_repr = codecs.utf_8_decode(str(obj), 'ignore')[0]
TypeError: a bytes-like object is required, not 'str'

@mjeronimo
Copy link
Contributor Author

mjeronimo commented Oct 26, 2020

Sure, I found and fixed the sources of the "TypeError: a bytes-like object is required, not 'str'" It was missing a call to decode and is now like the code found in the plot view plugin.

obj_repr = codecs.utf_8_decode(str(obj).encode(), 'ignore')[0]

@mabelzhang
Copy link

I believe I still need a way to test this. If I interpret the changes correctly, 8UC1 and 32FC1 were having problems before? And maybe an image that's a RGB type got converted when it shouldn't be? Do you happen to have data at hand that expose these problems, or was this just a "oh this is obviously wrong code" kind of fix?

@mjeronimo
Copy link
Contributor Author

mjeronimo commented Oct 28, 2020

@mabelzhang These types showed up when running rqt_bag on ROS2 and using a rosbag from realsense. The same camera resulted in different data types with ROS1. FYI, it seems from here: IntelRealSense/realsense-ros#253, that 'mono8' is preferred, but similar images can come in as '8UC1'.

Copy link

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

The mono8 and 8UC1 link looks reasonable.
I was able to test the L and BGR change - it does affect Melodic where CompressedImage was showing up blue where it should have been red. In Noetic CompressedImages are currently not shown, so it's not seen.
I did not have 32FC1 data to test those additions, but they look reasonable enough.

* Used ByteIO rather than StringIO is now required for PIL's Image.open
* Remove unused variable, 'alpha'
* Added support for '8UC1' and '32FC1' image types
* Only do bgr2rgb if it's definitely a BGR type

Fixes #60

Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
@mjeronimo mjeronimo merged commit 9648cff into master Oct 29, 2020
@mjeronimo mjeronimo deleted the mjeronimo/image-helper-fixes branch October 29, 2020 02:53
@VictorLamoine
Copy link
Contributor

VictorLamoine commented Nov 6, 2020

This PR creates a bug. When trying to display thumbnails/images of compressed images (JPEG) the following error is displayed:

Can't convert image: local variable 'pil_mode' referenced before assignment

Reverting this merge request solves the issue. I'm using ROS melodic

@VictorLamoine
Copy link
Contributor

Example bag file (4.1 Mio)

path:        images_compressed.bag
version:     2.0
duration:    2.0s
start:       Sep 22 2020 14:03:24.01 (1600776204.01)
end:         Sep 22 2020 14:03:25.98 (1600776205.98)
size:        4.1 MB
messages:    60
compression: none [6/6 chunks]
types:       sensor_msgs/CompressedImage [8f7a12909da2c9d3332d540a0977563f]
topics:      /nit_gige_1/image/compressed   60 msgs    : sensor_msgs/CompressedImage

@mjeronimo
Copy link
Contributor Author

@VictorLamoine Ok, thanks. I'll investigate and fix.

@VictorLamoine
Copy link
Contributor

@mjeronimo did you find what was wrong?

@mjeronimo
Copy link
Contributor Author

Commented over on the issue, #74, thinking you'd see it. Apologies for not notifying you directly on this thread.

@peci1 peci1 mentioned this pull request Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential bug in image_helper.py
3 participants