From 674402b35c02bf4299feb736e290ac38fbc1615d Mon Sep 17 00:00:00 2001 From: jtrantow Date: Tue, 28 Jan 2020 15:06:43 -0600 Subject: [PATCH 1/3] Fix what appears to be a copy/paste redundant code problem. Fix some casting issues. Looks like quite a bit of double vs float slop in this file. --- src/proc/zero-order.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/proc/zero-order.cpp b/src/proc/zero-order.cpp index 3b23841690..ad679c21bd 100644 --- a/src/proc/zero-order.cpp +++ b/src/proc/zero-order.cpp @@ -103,7 +103,7 @@ namespace librealsense return val == 0; }), values_ir.end()); - if (values_rtd.size() == 0 || values_rtd.size() == 0) + if ((values_rtd.size() == 0) || (values_ir.size() == 0)) return false; *rtd_zo_value = get_zo_point_value(values_rtd); @@ -125,11 +125,13 @@ namespace librealsense auto i_threshold_relative = (double)options.ir_threshold / res; for (auto i = 0; i < intrinsics.height*intrinsics.width; i++) { - auto rtd_val = rtd[i]; - auto ir_val = ir_data[i]; + double rtd_val = rtd[i]; + uint8_t ir_val = ir_data[i]; - auto zero = (depth_data_in[i] > 0) && (ir_val < i_threshold_relative) && - (rtd_val > (zo_value - options.rtd_low_threshold)) && (rtd_val < (zo_value + options.rtd_high_threshold)); + bool zero = (depth_data_in[i] > 0) && + (ir_val < i_threshold_relative) && + (rtd_val > double(zo_value - float(options.rtd_low_threshold))) && + (rtd_val < double(zo_value + float(options.rtd_high_threshold))); zero_pixel(i, zero); } @@ -142,15 +144,15 @@ namespace librealsense const zero_order_options& options, int zo_point_x, int zo_point_y) { std::vector rtd(intrinsics.height*intrinsics.width); - z2rtd(vertices, rtd.data(), intrinsics, options.baseline); - double rtd_zo_value; + z2rtd(vertices, rtd.data(), intrinsics, int(options.baseline)); + double rtd_zo_value; // \\todo Not sure why try_get_zo_rtd_ir_point_values() returns a double and detect_zero_order() uses a float. Which is best representation? uint8_t ir_zo_value; if (try_get_zo_rtd_ir_point_values(rtd.data(), depth_data_in, ir_data, intrinsics, options,zo_point_x, zo_point_y, &rtd_zo_value, &ir_zo_value)) { detect_zero_order(rtd.data(), depth_data_in, ir_data, zero_pixel, intrinsics, - options, rtd_zo_value, ir_zo_value); + options, float(rtd_zo_value), ir_zo_value); return true; } return false; From 271bf1e7f16b0a37cda6c078c7c200e37afeb502 Mon Sep 17 00:00:00 2001 From: jtrantow Date: Wed, 29 Jan 2020 13:48:53 -0600 Subject: [PATCH 2/3] Cleanup compiler warnings from zero-order. Use value_rtd.empty() suggestion instead of (values_rtd.size() == 0). Declare zo_value as double. Use doubles throughout detect_zero_order(). Added intialializers for resolutions_depth and read_baseline. --- src/proc/zero-order.cpp | 23 ++++++++++++----------- src/proc/zero-order.h | 3 ++- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/proc/zero-order.cpp b/src/proc/zero-order.cpp index ad679c21bd..f8ba1aec96 100644 --- a/src/proc/zero-order.cpp +++ b/src/proc/zero-order.cpp @@ -44,7 +44,7 @@ namespace librealsense std::vector get_zo_point_values(const T* frame_data_in, const rs2_intrinsics& intrinsics, int zo_point_x, int zo_point_y, int patch_r) { std::vector values; - values.reserve((patch_r + 2) *(patch_r + 2)); + values.reserve((patch_r + 2UL) *(patch_r + 2UL)); for (auto i = zo_point_y - 1 - patch_r; i <= (zo_point_y + patch_r) && i < intrinsics.height; i++) { @@ -103,7 +103,7 @@ namespace librealsense return val == 0; }), values_ir.end()); - if ((values_rtd.size() == 0) || (values_ir.size() == 0)) + if (values_rtd.empty() || values_ir.empty()) return false; *rtd_zo_value = get_zo_point_value(values_rtd); @@ -115,14 +115,14 @@ namespace librealsense template void detect_zero_order(const double * rtd, const uint16_t* depth_data_in, const uint8_t* ir_data, T zero_pixel, const rs2_intrinsics& intrinsics, const zero_order_options& options, - float zo_value, uint8_t iro_value) + double zo_value, uint8_t iro_value) { - const int ir_dynamic_range = 256; + const double ir_dynamic_range = 256.0; - auto r = (double)std::exp((ir_dynamic_range / 2.0 + options.threshold_offset - iro_value) / (double)options.threshold_scale); + double r = std::exp((ir_dynamic_range / 2.0 + options.threshold_offset - iro_value) / (double)options.threshold_scale); - auto res = (1 + r); - auto i_threshold_relative = (double)options.ir_threshold / res; + double res = (1.0 + r); + double i_threshold_relative = options.ir_threshold / res; for (auto i = 0; i < intrinsics.height*intrinsics.width; i++) { double rtd_val = rtd[i]; @@ -130,8 +130,8 @@ namespace librealsense bool zero = (depth_data_in[i] > 0) && (ir_val < i_threshold_relative) && - (rtd_val > double(zo_value - float(options.rtd_low_threshold))) && - (rtd_val < double(zo_value + float(options.rtd_high_threshold))); + (rtd_val > (zo_value - options.rtd_low_threshold)) && + (rtd_val < (zo_value + options.rtd_high_threshold)); zero_pixel(i, zero); } @@ -143,7 +143,7 @@ namespace librealsense rs2_intrinsics intrinsics, const zero_order_options& options, int zo_point_x, int zo_point_y) { - std::vector rtd(intrinsics.height*intrinsics.width); + std::vector rtd(size_t(intrinsics.height)*intrinsics.width); z2rtd(vertices, rtd.data(), intrinsics, int(options.baseline)); double rtd_zo_value; // \\todo Not sure why try_get_zo_rtd_ir_point_values() returns a double and detect_zero_order() uses a float. Which is best representation? uint8_t ir_zo_value; @@ -159,7 +159,8 @@ namespace librealsense } zero_order::zero_order(std::shared_ptr is_enabled_opt) - : generic_processing_block("Zero Order Fix"), _first_frame(true), _is_enabled_opt(is_enabled_opt) + : generic_processing_block("Zero Order Fix"), _first_frame(true), _is_enabled_opt(is_enabled_opt), + _resolutions_depth { 0 } { auto ir_threshold = std::make_shared>( 0, diff --git a/src/proc/zero-order.h b/src/proc/zero-order.h index 1b78a9e4af..71f3b2dc10 100644 --- a/src/proc/zero-order.h +++ b/src/proc/zero-order.h @@ -30,7 +30,8 @@ namespace librealsense z_max(Z_MAX_VALUE), ir_min(IR_MIN_VALUE), threshold_offset(THRESHOLD_OFFSET), - threshold_scale(THRESHOLD_SCALE) + threshold_scale(THRESHOLD_SCALE), + read_baseline(false) {} uint8_t ir_threshold; From 9532ef143e19982648d4d8aaac83e8456da04e28 Mon Sep 17 00:00:00 2001 From: jtrantow Date: Thu, 30 Jan 2020 08:29:53 -0600 Subject: [PATCH 3/3] Remove the \\todo comment. Just use doubles. Removed an unnecssary float cast. Use +2ULL in the values.reserve((patch_r + 2UL) *(patch_r + 2UL)) line. This promotes each operand to a unsigned long long which matches the size_t used by reserved() and avoids compiler warnings. --- src/proc/zero-order.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/proc/zero-order.cpp b/src/proc/zero-order.cpp index f8ba1aec96..05c2a86a8a 100644 --- a/src/proc/zero-order.cpp +++ b/src/proc/zero-order.cpp @@ -44,7 +44,7 @@ namespace librealsense std::vector get_zo_point_values(const T* frame_data_in, const rs2_intrinsics& intrinsics, int zo_point_x, int zo_point_y, int patch_r) { std::vector values; - values.reserve((patch_r + 2UL) *(patch_r + 2UL)); + values.reserve((patch_r + 2ULL) *(patch_r + 2ULL)); for (auto i = zo_point_y - 1 - patch_r; i <= (zo_point_y + patch_r) && i < intrinsics.height; i++) { @@ -145,14 +145,14 @@ namespace librealsense { std::vector rtd(size_t(intrinsics.height)*intrinsics.width); z2rtd(vertices, rtd.data(), intrinsics, int(options.baseline)); - double rtd_zo_value; // \\todo Not sure why try_get_zo_rtd_ir_point_values() returns a double and detect_zero_order() uses a float. Which is best representation? + double rtd_zo_value; uint8_t ir_zo_value; if (try_get_zo_rtd_ir_point_values(rtd.data(), depth_data_in, ir_data, intrinsics, options,zo_point_x, zo_point_y, &rtd_zo_value, &ir_zo_value)) { detect_zero_order(rtd.data(), depth_data_in, ir_data, zero_pixel, intrinsics, - options, float(rtd_zo_value), ir_zo_value); + options, rtd_zo_value, ir_zo_value); return true; } return false;