Change Image (and other) messages to use byte[] rather than uint8[]

[ghost]

Change request

In the Image, CompressedImage, and PointCloud2 messages there are data fields that have type uint8[]. I suggest these are chaged to byte[].

Change description

According to the wiki, uint8 is interpreted as a python int, whereas byte is interpreted as a python byte. For the messages where arrays of uint8 are used, it's normally because the data is some arbitrary binary format. Currently, it seems that rclpy takes this binary data, copies it out into a list[int], which client code then normally converts back to some collection of byte types for use. This is much more processing and copying than is necessary.

This should also somewhat mitigate this issue, as no range check will be necessary.

Implementation considerations

I can think of no cons of this change

connects to ros2/rosidl#190

[mikaelarguedas]

Implementation consideration: evaluate how this plays with current ROS1 serialization of these messages and detail potential additional work to be able to keep bridging such messages over the ros1_bridge

[sloretz]

Related to ros2/rosidl#190

[kervel]

Hello,
i think this would not be sufficient. even for byte[] the generated convert_to_py still creates a python List, see:
https://github.com/ros2/rosidl_python/blob/master/rosidl_generator_py/resource/_msg_support.c.em#L339

i tried to make a ByteImage (by copying Image and changing uint8 to byte) to get faster python IO but i still get a List in python..

i think bytes --> byte[] is only for conversion from python, not conversion to python..;

See also ros2/rosidl#276

Frank

[ghost]

@kervel As I understand it, when you say "still creates a python list" this is presumably a python list of byte types, rather than a python list of int types, which is still a semantic and performance win in my book.

There are two options I might suggest:

• Just change uint8[] to byte[] in relevant messages. This is (a little) semantically clearer and is a performance win in python since byte is interpreted as python byte, while uint8 is interpreted as python int. Currently, the manual type checking therefore has to iterate through the entire list of ints checking they're all in range 0-255. This wouldn't be necessary if they were python bytes.
• Do the above AND have the IDL interpret byte[] as bytes in python (or maybe bytearray, I'm not very familiar with the difference). This may be better, assuming that most python end-users want the data in this type, but that's an assumption.

This issue is only for the first.

Personally I'd be in favour of both, but I thought I'd create the issue for the much easier and less controversial first option. It's more likely to get implemented, it's still a win (as far as I can tell), and it doesn't preclude also going for the second option at a later date.

[kervel]

@johnmarkwayve indeed. i should check what exactly would be the gain, now i was under the assumption that most time would be spent in traversing linked lists (i'll try to find out).
I was using the python bindings now to feed an image to a keras model, and while the keras model inference only takes 7msec, but the whole conversion from buffer to python linkedlist to buffer takes almost 40msec..

Frank

[dirk-thomas]

In the scope of ros2/design#190 the mapping of char[] is defined as bytes in Python. Once that change has been rolled out the image message will be changed to use that type instead of uint8[].