search

ros-perception / vision_msgs

Algorithm-agnostic computer vision message types for ROS.
Apache License 2.0
155 stars 74 forks source link

add compressed image messages #21

Closed jveitchmichaelis closed 3 years ago

jveitchmichaelis commented 5 years ago

Addresses https://github.com/Kukanani/vision_msgs/issues/20

mintar commented 5 years ago

The messages also need to be added to the CMakeLists.txt . Otherwise looks good. :)

gavanderhoorn commented 5 years ago

Not sure something can be done about it, but to me (non-native speaker) it sounds like the detections are compressed, not just the source image data.

jveitchmichaelis commented 5 years ago

As a native speaker I agree. The alternative is to roll it into the existing message, as the image is optional in any case? Then there's the issue of not knowing which image message to use. I don't have a strong opinion either way though.

Kukanani commented 5 years ago

Thanks for the PR!

I'd prefer multiple message types for clarity. As far as naming goes, I agree that CompressedDetection is slightly ambiguous. The only suggestion I have is something like Detection2DWithCompression, which is perhaps more explicit, but certainly more clunky. Instead of renaming the message, can you simply make a note in the header comments for the new message that the difference between this and the standard Detection2D is that the source image is compressed?

Also note #19, which I hope to merge tomorrow, would mean that we should include tracking information in this new message type as well.

Kukanani commented 5 years ago

I renamed the compression messages and added a comment in their headers about how they differ from the non-compression versions. Let me know if you feel the names are too clunky, otherwise I'll merge in a week or so.

mintar commented 5 years ago

Still missing the tracking fields.

Kukanani commented 5 years ago

Rebased and added tracking fields

SteveMacenski commented 3 years ago

Closing - we've removed the data sources entirely so this is no longer required