perf: skip frames, roi, fixed aspect ratio #225
Conversation
copy calibration tools from private repository
merge main for galactic release
…g elements for eval mode
…and calibration parameters into a file, change ui elements in eval mode.
…luation_mode Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>
…rinsics_evaluation_mode Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>
Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>
…e' into feat/camera_profiles
|
just fixed it by removing some blank spaces |
| @@ -34,6 +36,12 @@ def __init__(self, **kwargs): | |||
| self.resized_max_resolution = Parameter(int, value=1000, min_value=500, max_value=3000) | |||
| self.sub_pixel_refinement = Parameter(bool, value=True, min_value=False, max_value=True) | |||
| self.set_parameters(**kwargs["cfg"]) | |||
| self.roi = None | |||
| self.lost_frames = 0 | |||
| self.max_lost_frames = 3 | |||
There was a problem hiding this comment.
Please use the Parameter class
There was a problem hiding this comment.
When you do, you need to change the restart_lost_frames_counter as well
There was a problem hiding this comment.
self.lost_frames = self.max_lost_frames.value
Access need to be protected by a mutex if I remember correctly
There was a problem hiding this comment.
applies to self.padding.value as well
| @@ -54,15 +62,42 @@ def detect(self, img: np.array, stamp: float): | |||
| resized_detection = self.resized_detection.value | |||
| resized_max_resolution = self.resized_max_resolution.value | |||
|
|
|||
| def get_roi(corners, frame_shape, padding=120): | |||
There was a problem hiding this comment.
Is it better to have the padding as an absolute value? relative to the size of the image?
Any way, please use the Parameter class
There was a problem hiding this comment.
yes, it might be better to make it relative to the image size, I will try to do it later but to solve this asap just made it a parameter.
6acce91
| if not ok: | ||
| self.detection_results_signal.emit(img, None, stamp) | ||
| return | ||
| if self.roi is None or self.lost_frames >= self.max_lost_frames: |
There was a problem hiding this comment.
This method became quite long and now has too many branches.
Can you either remove branches/indendation level via refactoring or split what can be split in smaller methods?
There was a problem hiding this comment.
I'm thinking about how to refactor, i see we have the function find chessboard in common I can try to do something about it, I am not sure how clean it is going to look, but I will try
| @@ -992,6 +1012,7 @@ def process_detection_results(self, img: np.array, detection: BoardDetection, im | |||
| alpha_indicators=self.indicators_alpha_spinbox.value(), | |||
| value=False, | |||
| ) | |||
| self.skip_next_img = 5 # skips the next images if there are no detections | |||
There was a problem hiding this comment.
Please make this into a parameter
| @@ -165,6 +167,12 @@ def make_mode_group(self): | |||
|
|
|||
| self.image_view_type_combobox.setEnabled(False) | |||
|
|
|||
| self.rectify_label = QLabel("Rectify option:") | |||
| self.rectify_type_combobox = QComboBox() | |||
| self.rectify_type_combobox.addItem("Opencv", 0) | |||
There was a problem hiding this comment.
It is not like we are likely to add more rectification options, but for consistency, please use the Enums that are used throughout this package
| @@ -220,10 +228,14 @@ def on_evaluation_sample_changed(index): | |||
| img = self.data_collector.get_evaluation_image(index) | |||
| self.process_db_data(img) | |||
|
|
|||
| def on_rectify_type_change(index): | |||
| self.calibrated_camera_model._cached_undistorted_model = None | |||
There was a problem hiding this comment.
Variables with _ as a prefix indicate that they should not be modified from the outside
There was a problem hiding this comment.
create a function that does the same, but inside the class
a2462e0
...mera_calibrator/intrinsic_camera_calibrator/intrinsic_camera_calibrator/camera_calibrator.py
Outdated
Show resolved
Hide resolved
| and self.skip_next_img > 1 | ||
| and self.data_source_type != DataSourceEnum.FILES | ||
| ): | ||
| self.detector.restart_lost_frames_counter() # to force next frame detection |
There was a problem hiding this comment.
The detector acts in another thread
There was a problem hiding this comment.
Adding a mutex inside the function to make it safe
9bc13bd
| return | ||
|
|
||
| if self.data_source_type == DataSourceEnum.FILES: | ||
| self.detector.restart_lost_frames_counter() # to force next frame detection |
There was a problem hiding this comment.
The detector acts in another thread
There was a problem hiding this comment.
Adding a mutex inside the function to make it safe
9bc13bd
...brator/intrinsic_camera_calibrator/intrinsic_camera_calibrator/camera_models/camera_model.py
Show resolved
Hide resolved
…rator/intrinsic_camera_calibrator/camera_calibrator.py Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/camera_models/camera_model.py Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
I was checking a book that has a good explanation of the rectification process, and the equations used on the undistortion map seem to be good, https://amroamroamro.github.io/mexopencv/matlab/cv.initUndistortRectifyMap.html https://books.google.ch/books?id=seAgiOfu2EIC&lpg=PA373&pg=PA373#v=onepage&q&f=false |
Description
This PR includes:
-ROI to keep track of the calibration board, making future detections faster (performance improvement).
-Will skip board detection on some future frames, if the board was not detected on the current frame (avoid long queues).
-Increase the decimal digits when saving intrinsic parameters, to avoid the bubble effect when rectifying the image.
-Fix the issue that suddenly was pausing the operation of the calibration tool.
-Implements a new method to fix the aspect ratio on the rectified image.
-Changes the distortion model according to the number of distortion coefficients.
Related links
Tests performed
Tested mainly on an x86 platform and using rosbags, but also non-extensive tests on anvil were performed.
Notes for reviewers
It might have some spelling errors, please let me know.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.