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

Add draft bad pixel detection and correction #776

Merged
merged 12 commits into from
Apr 11, 2024
Merged

Conversation

trey0
Copy link
Contributor

@trey0 trey0 commented Mar 27, 2024

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.

@trey0 trey0 marked this pull request as ready for review April 10, 2024 18:34
@trey0 trey0 requested review from marinagmoreira and bcoltin April 10, 2024 18:36
@bcoltin
Copy link
Member

bcoltin commented Apr 10, 2024

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
Copy link
Member

rsoussan commented Apr 10, 2024

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
Copy link
Contributor Author

trey0 commented Apr 11, 2024

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
Copy link
Member

bcoltin commented Apr 11, 2024

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
Copy link
Contributor Author

trey0 commented Apr 11, 2024

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

Ok, fixed in 6f329f9

@marinagmoreira marinagmoreira linked an issue Apr 11, 2024 that may be closed by this pull request
3 tasks
@trey0 trey0 merged commit a34e180 into nasa:develop Apr 11, 2024
4 checks passed
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.

Investigate possible bad pixels on sensors of Astrobee flight units
4 participants