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

Optionally generate black background when crop area is outside of source image #428

Open
lucasw opened this issue Jul 11, 2019 · 5 comments

Comments

@lucasw
Copy link
Contributor

lucasw commented Jul 11, 2019

if for example the image were 1000 x 1000 and the configuration specified x y offset 900 and width and height 200 x 200, instead of shrinking down the width and height of the output cropped image, instead have a black background filling the pixels where there is no source image to copy in. Or if the x and y offset are outside the source image entirely then output a black image of the requested width and height.

CameraInfo x_offset and y_offset would have to be made signed instead of uint32 to really follow this through, which would impact lots of nodes, for now don't pursue that and just allow the extra blank image in the positive directions.

Possibly some downstream nodes will not properly handle getting a camera info that has the roi outside of the width and height of the source image, and could crash.

@SteveMacenski
Copy link
Member

I would personally put this in the category of "masking real issues" that I'm not a fan of. Things should be configured to work and I think lots of warnings to correct mis-configuration or mis-use is a better answer, especially when it has ramifications for other nodelets downstream of it. An example that I would be concerned by is the crop is for processing a depth image, and someone thinks their robot is well configured but actually a chunk of the FOV of their sensors are flipped and creating a huge virtual blind spot.

But I'm open to other people's opinions on it @JWhitleyAStuff what are your thoughts?

@JWhitleyWork
Copy link
Collaborator

I tend to agree with @SteveMacenski. I think there are two options to get around this:

  1. As soon as the first image is received, calculate the bounds of the ROI relative to the image. If roi_x + width > image_width or roi_y + height > image_height, produce an error and don't publish the ROI. This will both provide the user with useful information and keep downstream nodes from using invalid or misleading input.
  2. Do the same calculation as for 1 but output a warning and produce an image where the ROI size = image_width - roi_x by image_height - roi_y. This lets also provides the user with useful information but allows the downstream nodes to have something to process which isn't quite as misleading.

I would still lean more toward option 1, though. 2 is still somewhat misleading but at least it's possible for the user to know why.

@lucasw
Copy link
Contributor Author

lucasw commented Jul 11, 2019

Is there anything wrong with providing the capability, if by default it is disabled?

Further on there could be a valid pixel latched image topic that only updates when needed (maybe others have used alpha channels in the same way at greater bandwidth cost).

@JWhitleyWork
Copy link
Collaborator

Just so we can get some context, what is your use case for this feature?

@lucasw
Copy link
Contributor Author

lucasw commented Jul 11, 2019

I have a camera that I plan to have publish a window/roi most of the time, but want an interactive way for users (maybe at the level of trained technicians) to define that roi, so they'll switch over to a full sensor mode. Downstream there will be crop node/s further cutting down the image size for various processing tasks which also will occasionally be interactively adjusted. I'd like to reduce visual artifacts and oddities that result from cropped images shifting, shrinking and changing aspect ratio or ceasing publishing entirely- the black pixels seem preferable. I don't expect them to be looking at log output. There is the potential for someone to get lost where they lose overlap between regions applied in sequence but some custom visualization of rois can address that.

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

No branches or pull requests

4 participants