search

nasa / astrobee

NASA Astrobee Robot Software
https://www.nasa.gov/astrobee
Apache License 2.0
1.01k stars 307 forks source link

Add draft bad pixel detection and correction #776

Closed trey0 closed 3 months ago

trey0 commented 3 months ago

This PR introduces code for bad pixel detection and correction.

After merge, the main integration users would immediately start to see is that rosbag_debayer.py would do bad pixel correction by default before applying debayering. (It can be disabled with --no-correct.)

Incidentally, it should also fix the double-debayer bug I noticed for rosbag_debayer.py grayscale output.

bcoltin commented 3 months ago

I think it may make more sense to put all of this in tools/bag_processing since it is all post-facto analysis on bags, and this isn't all that related to the current camera package which is more about camera geometry. What do you think @rsoussan ?

rsoussan commented 3 months ago

I think it may make more sense to put all of this in tools/bag_processing since it is all post-facto analysis on bags, and this isn't all that related to the current camera package which is more about camera geometry. What do you think @rsoussan ?

That seems like a good idea to me

trey0 commented 3 months ago

I think it may make more sense to put all of this in tools/bag_processing since it is all post-facto analysis on bags, and this isn't all that related to the current camera package which is more about camera geometry. What do you think @rsoussan ?

I don't have a super-strong opinion on this but thought I should explain the logic... if this change appears to be worthwhile in terms of potential benefits to localization, a likely next step would be to port one of the BadPixelCorrector implementations to C++ so the correction could be applied onboard prior to debayering. I selected localization/camera as a logical place to put that C++ implementation, as it could be used either onboard or in post-processing, having nothing directly to do with the hardware, and camera was already a dependency of hardware/is_camera, which is what would call this code in onboard use. Having worked that out, I thought it would be good to keep the Python code in the same package as the yet-to-be-written C++.

Do you guys find that logic convincing or still want to move it to tools/bag_processing?

bcoltin commented 3 months ago

I think it may make more sense to put all of this in tools/bag_processing since it is all post-facto analysis on bags, and this isn't all that related to the current camera package which is more about camera geometry. What do you think @rsoussan ?

I don't have a super-strong opinion on this but thought I should explain the logic... if this change appears to be worthwhile in terms of potential benefits to localization, a likely next step would be to port one of the BadPixelCorrector implementations to C++ so the correction could be applied onboard prior to debayering. I selected localization/camera as a logical place to put that C++ implementation, as it could be used either onboard or in post-processing, having nothing directly to do with the hardware, and camera was already a dependency of hardware/is_camera, which is what would call this code in onboard use. Having worked that out, I thought it would be good to keep the Python code in the same package as the yet-to-be-written C++.

Do you guys find that logic convincing or still want to move it to tools/bag_processing?

I would still suggest we move it to tools/bag_processing. I think we are still a long way from on-board correction on the robot, and it would end up being entirely different C++ code. The camera node is currently a very simple library about camera geometry and this would make it much more complicated. Depending on the details, I would actually consider making a separate library for debayering, since this would only be used in the hardware node, whereas camera is used in many places.

trey0 commented 3 months ago

I would still suggest we move it to tools/bag_processing.

Ok, fixed in 6f329f96f619d3aa353f22daafdd5a5294f30081