Repository navigation
UCAL enhancements and refactoring - #9336
Conversation
3c7738c to
9c5d9d5
Compare
2. Taget based focal length calibration. 3. UVMapping calibration.
…perators, floating point conversion
…Minor inconsistencies
…fication adjustments
Change-Id: I441f7208edd6de25f433b53fc3f216d3fe8fd2b0
Change-Id: Iaa92be84b7b77e9dbac126535dd2e320f534a7c0
Change-Id: I35043b064960fb8509085898329f6c9e8d15d2be
Change-Id: I6d75b823d3a58b49c832cc25601e130453853337
…gly. Verify that the cali table is valid before calling FW flash API.Remove comments Change-Id: Iefb5b37a871af9ce4cdd8185b7c8d36e9d814879
Change-Id: I1750b5e1f2e23fed537be228147610b7de2af33e
Change-Id: Ibb4f45c86a88d693214dc9d46b08da8edc9efb62
Change-Id: I5d5a0bdca32e1be13452e81c6d3a7c7aa692fb9f
Change-Id: I9ba2ae9fbe1a6f268cb79d8651e39049b5f25dc0
Change-Id: Ic0ddf6b1a0935367ab0237e971820d089c982cbd
Change-Id: I7cf8a2bc2335f9f543c17a217be59365b7a78c98
Change-Id: Idc41413272ae5236976fc65b68302f702613823f
remibettan
left a comment
There was a problem hiding this comment.
minor changes - Impressive work!
| namespace helpers | ||
| { | ||
| // Calculate CRC code for arbitrary characters buffer | ||
| uint32_t calc_crc32(const uint8_t* buf, size_t bufsize); |
There was a problem hiding this comment.
Where is the implementation of this method?
There was a problem hiding this comment.
calibration-model.cpp
| : process_manager("On-Chip Calibration"), _model(model), _dev(dev), _sub(sub), _viewer(viewer), _sub_color(sub_color), py_px_only(!uvmapping_calib_full) | ||
| { | ||
| auto dev_name = dev.get_info(RS2_CAMERA_INFO_NAME); | ||
| if (!strcmp(dev_name, "Intel RealSense D415")) |
There was a problem hiding this comment.
Can you make it characteristics oriented instead of specific camera name?
There was a problem hiding this comment.
The spec is SKU-specific. I will change the code for robustness
| { | ||
| if (_sub) | ||
| { | ||
| _sub->show_algo_roi = true; |
There was a problem hiding this comment.
seems like same code instead of the pointer, both for on, and off - please consider to refactor with some helper function.
There was a problem hiding this comment.
Agree, but lower priority
| { | ||
| // Stop viewer UI | ||
| _sub->stop(_viewer.not_model); | ||
| if (_sub_color.get()) |
There was a problem hiding this comment.
Is there a reason why there is a need to check _sub_color pointer and not _sub?
There was a problem hiding this comment.
_sub stands for depth, and _sub_color - RGB
| { | ||
| auto profiles = _sub->get_selected_profiles(); | ||
| _sub->stop(_viewer.not_model); | ||
| if (_sub_color.get()) |
There was a problem hiding this comment.
same question as before
| for (int i = 0; i < _sub_color->res_values.size(); i++) | ||
| { | ||
| auto kvp = _sub_color->res_values[i]; | ||
| if (kvp.first == 1280 && kvp.second == 720) |
| int counter = 0; | ||
| int frm_idx = 0; | ||
| int limit = 50; // input frames required to calculate the target | ||
| float step = 50.f / limit; // frames gathering is 50% of the process, the rest is the internal data extraction and algo processing |
There was a problem hiding this comment.
It is used by GUI marker, don't think that it is necessary
| ImGui::SetCursorScreenPos({ float(x + 135), tmp_y }); | ||
| if (ImGui::RadioButton("OCC Extended", (int *)&(get_manager().action), 0)) | ||
| get_manager().action = on_chip_calib_manager::RS2_CALIB_ACTION_ON_CHIP_OB_CALIB; | ||
| // Deprecase OCC-Extended |
There was a problem hiding this comment.
comments - consider to remove
There was a problem hiding this comment.
I think it is better to keep it till the version is stabilized.
I'm not sure that this decision will hold
| auto table = (librealsense::ds::coefficients_table*)calib_table.data(); | ||
|
|
||
| float ar[2] = { 0 }; | ||
| float tmp = left_rect_sides[2] + left_rect_sides[3]; |
There was a problem hiding this comment.
consider to use helper function in order to remove repetition of code for left and right
There was a problem hiding this comment.
The algorithmic part implementation is according to a reference design. I'd rather not refactor it to keep the Matlab/C++ parity
| if (ar[0] > 0.0f) | ||
| align = ar[1] / ar[0] - 1.0f; | ||
|
|
||
| float ta[2] = { 0 }; |
There was a problem hiding this comment.
please comment these "ta" and "gt" so that their meaning would be clear (or change the names)
There was a problem hiding this comment.
I wish I knew it :) - it is the algo domain
|
Approved |
Change-Id: Ib28fc35ebbd9fc783d25b8b992fedc5879e614d9
Change-Id: Ia86a5a76f2b0a11ae56deba3d0b426a909e84191
Fix python compilation for new calibration values. add comments Fix some warnings move calculation of sum of pixels in depth image to function: get_depth_frame_sum fix merge. retry on set_pu SEMAPHORE_TIMEOUT error CR updates add ticket number to comment changed inheritence to using added comment fix callback ptr from PR realsenseai#9336
Tracked on LRS-62