fix(cloud_point): validation, thread safety, and test cleanup

- Parse port inside try block for proper error reporting instead of unhandled exception
- Make frame_counter atomic to eliminate data race under TcpServer per-client threads
- Validate num_disparities is positive multiple of 16 in StereoMatcherFactory
- Validate stereo pair dimensions match in ScaredDatasetLoader
- Silence nodiscard warnings via std::ignore in tests

TG-3 #ready-for-test
TG-2 #ready-for-test
This commit is contained in:
Artur Mukhamadiev 2026-07-12 22:15:17 +03:00
parent e139a55143
commit f8177d8926
7 changed files with 49 additions and 6 deletions

View File

@ -18,7 +18,8 @@ namespace score {
class ScaredDatasetLoader { class ScaredDatasetLoader {
public: public:
/// @brief Load calibration and images from @p keyframe_dir. /// @brief Load calibration and images from @p keyframe_dir.
/// @throws std::runtime_error if any file cannot be opened or parsed. /// @throws std::runtime_error if any file cannot be opened or parsed,
/// or if the left and right images have different dimensions.
explicit ScaredDatasetLoader(const std::string &keyframe_dir); explicit ScaredDatasetLoader(const std::string &keyframe_dir);
/// @brief Return the stereo calibration DTO (translation in metres). /// @brief Return the stereo calibration DTO (translation in metres).

View File

@ -13,6 +13,9 @@ class StereoMatcherFactory {
/// @brief Create a stereo matcher of the requested type. /// @brief Create a stereo matcher of the requested type.
/// If GPU is requested but unavailable, falls back to CPU. /// If GPU is requested but unavailable, falls back to CPU.
/// @param num_disparities Number of disparity levels for SGBM (default 128). /// @param num_disparities Number of disparity levels for SGBM (default 128).
/// Must be a positive multiple of 16.
/// @throws std::invalid_argument if @p num_disparities is not a positive
/// multiple of 16.
[[nodiscard]] static std::unique_ptr<IStereoMatcher> [[nodiscard]] static std::unique_ptr<IStereoMatcher>
create(StereoAlgorithmType type, int num_disparities = 128); create(StereoAlgorithmType type, int num_disparities = 128);
}; };

View File

@ -75,6 +75,16 @@ ScaredDatasetLoader::ScaredDatasetLoader(const std::string &keyframe_dir) {
left_image_ = load_bgr_image(left_path); left_image_ = load_bgr_image(left_path);
right_image_ = load_bgr_image(right_path); right_image_ = load_bgr_image(right_path);
if (left_image_.width != right_image_.width ||
left_image_.height != right_image_.height) {
throw std::runtime_error(
"ScaredDatasetLoader: stereo pair dimension mismatch: left " +
std::to_string(left_image_.width) + "x" +
std::to_string(left_image_.height) + " vs right " +
std::to_string(right_image_.width) + "x" +
std::to_string(right_image_.height));
}
calib_.width = left_image_.width; calib_.width = left_image_.width;
calib_.height = left_image_.height; calib_.height = left_image_.height;

View File

@ -11,6 +11,7 @@
#include "cloud_point_rpc/rpc_dto.hpp" #include "cloud_point_rpc/rpc_dto.hpp"
#include "cloud_point_rpc/rpc_server.hpp" #include "cloud_point_rpc/rpc_server.hpp"
#include "cloud_point_rpc/tcp_server.hpp" #include "cloud_point_rpc/tcp_server.hpp"
#include <atomic>
#include <glog/logging.h> #include <glog/logging.h>
#include <nlohmann/json.hpp> #include <nlohmann/json.hpp>
#include <string> #include <string>
@ -28,16 +29,17 @@ int main(int argc, char *argv[]) {
} }
const std::string keyframe_dir = argv[1]; const std::string keyframe_dir = argv[1];
const int port = (argc >= 3) ? std::stoi(argv[2]) : 8080;
LOG(INFO) << "SCARED dataset server starting"; LOG(INFO) << "SCARED dataset server starting";
LOG(INFO) << " keyframe_dir = " << keyframe_dir; LOG(INFO) << " keyframe_dir = " << keyframe_dir;
LOG(INFO) << " port = " << port;
try { try {
const int port = (argc >= 3) ? std::stoi(argv[2]) : 8080;
LOG(INFO) << " port = " << port;
score::ScaredDatasetLoader loader(keyframe_dir); score::ScaredDatasetLoader loader(keyframe_dir);
uint64_t frame_counter = 0; std::atomic<uint64_t> frame_counter{0};
score::RpcServer rpc_server; score::RpcServer rpc_server;
rpc_server.register_method( rpc_server.register_method(

View File

@ -2,11 +2,19 @@
#include "cloud_point/cpu_stereo_matcher.hpp" #include "cloud_point/cpu_stereo_matcher.hpp"
#include "cloud_point/gpu_stereo_matcher.hpp" #include "cloud_point/gpu_stereo_matcher.hpp"
#include <glog/logging.h> #include <glog/logging.h>
#include <stdexcept>
#include <string>
namespace score { namespace score {
std::unique_ptr<IStereoMatcher> std::unique_ptr<IStereoMatcher>
StereoMatcherFactory::create(StereoAlgorithmType type, int num_disparities) { StereoMatcherFactory::create(StereoAlgorithmType type, int num_disparities) {
if (num_disparities <= 0 || num_disparities % 16 != 0) {
throw std::invalid_argument(
"StereoMatcherFactory: num_disparities must be a positive "
"multiple of 16, got " +
std::to_string(num_disparities));
}
switch (type) { switch (type) {
case StereoAlgorithmType::CPU: case StereoAlgorithmType::CPU:
return std::make_unique<CpuStereoMatcher>(0, num_disparities); return std::make_unique<CpuStereoMatcher>(0, num_disparities);

View File

@ -6,6 +6,7 @@
/// SCARED_KEYFRAME_DIR=/path/to/test_dataset_8/keyframe_0 \ /// SCARED_KEYFRAME_DIR=/path/to/test_dataset_8/keyframe_0 \
/// ./build/tests/unit_tests --gtest_filter=ScaredDataset* /// ./build/tests/unit_tests --gtest_filter=ScaredDataset*
#include <algorithm> #include <algorithm>
#include <atomic>
#include <chrono> #include <chrono>
#include <cmath> #include <cmath>
#include <cstdlib> #include <cstdlib>
@ -83,7 +84,7 @@ TEST_F(ScaredDatasetTest, ComputeCloudFromRealData) {
ScaredDatasetLoader loader(keyframe_dir_); ScaredDatasetLoader loader(keyframe_dir_);
uint64_t frame_counter = 0; std::atomic<uint64_t> frame_counter{0};
auto rpc = std::make_unique<RpcServer>(); auto rpc = std::make_unique<RpcServer>();
rpc->register_method( rpc->register_method(

View File

@ -1,5 +1,6 @@
#include <gtest/gtest.h> #include <gtest/gtest.h>
#include <opencv2/core.hpp> #include <opencv2/core.hpp>
#include <tuple>
#include "cloud_point/cpu_stereo_matcher.hpp" #include "cloud_point/cpu_stereo_matcher.hpp"
#include "cloud_point/gpu_stereo_matcher.hpp" #include "cloud_point/gpu_stereo_matcher.hpp"
@ -46,13 +47,30 @@ TEST(StereoMatcherTest, FactoryCpuCreatesNonNull) {
EXPECT_FALSE(disparity.empty()); EXPECT_FALSE(disparity.empty());
} }
TEST(StereoMatcherTest, FactoryRejectsInvalidNumDisparities) {
EXPECT_THROW(std::ignore = StereoMatcherFactory::create(
StereoAlgorithmType::CPU, 0),
std::invalid_argument);
EXPECT_THROW(std::ignore = StereoMatcherFactory::create(
StereoAlgorithmType::CPU, -16),
std::invalid_argument);
EXPECT_THROW(std::ignore = StereoMatcherFactory::create(
StereoAlgorithmType::CPU, 150),
std::invalid_argument);
}
TEST(StereoMatcherTest, FactoryAcceptsValidNumDisparities) {
auto matcher = StereoMatcherFactory::create(StereoAlgorithmType::CPU, 160);
ASSERT_NE(matcher, nullptr);
}
TEST(StereoMatcherTest, FactoryGpuFallsBackToCpuWhenUnavailable) { TEST(StereoMatcherTest, FactoryGpuFallsBackToCpuWhenUnavailable) {
// On this machine CUDA is absent; factory should fall back to CPU. // On this machine CUDA is absent; factory should fall back to CPU.
auto matcher = StereoMatcherFactory::create(StereoAlgorithmType::GPU); auto matcher = StereoMatcherFactory::create(StereoAlgorithmType::GPU);
ASSERT_NE(matcher, nullptr); ASSERT_NE(matcher, nullptr);
auto [left, right] = make_synthetic_stereo(); auto [left, right] = make_synthetic_stereo();
EXPECT_NO_THROW(matcher->compute(left, right)); EXPECT_NO_THROW(std::ignore = matcher->compute(left, right));
} }
TEST(StereoMatcherTest, GpuMatcherThrowsOnThisMachine) { TEST(StereoMatcherTest, GpuMatcherThrowsOnThisMachine) {