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

Added camera_info publishing synced with image publishing. #18

Open
wants to merge 1 commit into
base: indigo-devel
Choose a base branch
from

Conversation

basti35
Copy link

@basti35 basti35 commented Apr 20, 2017

I added also a default calibration file (for a Basler acA1600-20uc with a 8mm lens from computar).
The camera_info msg is very cheap, but sometimes is necessary.

@basti35 basti35 closed this Apr 20, 2017
@basti35 basti35 reopened this Apr 20, 2017
@jonbinney
Copy link

It looked like this was already being done for the image_raw topic and you're adding it for the image_rect topic, right? If there are subscribers to both image_raw and image_rect, won't this publish camera info too often?

Actually, why does pylon_camera do its own image rectification anyway? Wouldn't it be simpler for pylon_camera to just publish image_raw and camera_info, and to add image_proc to do the launchfiles to do the rectification?

Also, since the calibration is going to be different for every camera and every lens, I think it probably doesn't make sense to include a default here. If the camera hasn't been calibrated, there shouldn't be any rectified images published.

@marc-up
Copy link
Contributor

marc-up commented Apr 24, 2017

Thanks @basti35 for your contribution, but I think we won't merge you're PR.
As @jonbinney pointed out, the calibration of a camera is highly device and lens specific, so the calibration you added should not be part of this pkg. It will only generate valid rectificated images for your individual case but not in general. In case no calibration is given, only raw_images should be provided, as it is done up to now.
Furthermore, I didn't get the point, why you want to add a second camera_info publisher. The CameraInfo message is always sent in case someone listens to the topic. So what do you mean by 'camera_info publishing synced with image publishing'? Can you describe the situation when they are not synced?
img_raw_pub_.getNumSubscribers() > 0 is true in case s.o. listens to either camera_info or image_raw as you can see here, so you should always receive the CameraInfo message, in case you want it.

@basti35
Copy link
Author

basti35 commented Apr 24, 2017

Sorry for the delay of my reply.
The reason of my PR is that sometimes nodes use image_transport for subscribing. It waits for both the messages to spin the callback.
Because the publisher of img_raw is called only if there are subscribers, it could happen that the camera_info is not published concurrently with the image_rect.
e.g. I am using apriltag_ros , and it use a image_transport subscriber. Thus, it requires camera_info. It use this msg for obtaining the center of the image.

@marc-up
Copy link
Contributor

marc-up commented Apr 27, 2017

Ok, I got the point. But is that the standard way this is done? Publishing the camera_info msg that is static in most situations with every new acquired image does not sound like the perfect solution for me.
The size of the CameraInfo-msg is on the other hand not that big...
Does anyone have another idea how this issue could be resolved?

@jonbinney
Copy link

Publishing exactly one CameraInfo for every image, although it seems a bit odd, actually works quite well (in my experience). ROS nodes that use calibrated images know to read both the camera_info topic and the images topic, and look for exact timestamp matches. Using this approach works fine for static calibrations, but also handles the case where the camera is periodically recalibrated, or even calibrated on the fly. Most importantly, it is a common practice and so other nodes/users expect it and know how to deal with it. For example, there is a message_filters package with a time synchronizer filter that will subscribe to two topics and call your callback with matched pairs of image+info: http://wiki.ros.org/message_filters

@mgrrx
Copy link
Member

mgrrx commented Oct 13, 2017

@basti35 thank you again for providing this PR. In order to get this merged can you please:

If you are no longer interested in this PR please consider to give the maintainers access to your branch.

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.

4 participants