From 916e20bd36f3f27e79e7679090b73df445472f85 Mon Sep 17 00:00:00 2001 From: Denes Tarjan Date: Mon, 13 Jan 2025 15:56:59 +0000 Subject: [PATCH] Limit Remap source dimensions --- doc/opencv.md | 5 +- kleidicv/include/kleidicv/kleidicv.h | 17 ++-- kleidicv/include/kleidicv/transform/remap.h | 24 +++--- kleidicv/src/transform/remap_neon.cpp | 8 +- kleidicv/src/transform/remap_sc.h | 14 ++-- kleidicv_thread/src/kleidicv_thread.cpp | 6 +- test/api/test_remap.cpp | 91 ++++++++++++++++++--- 7 files changed, 119 insertions(+), 46 deletions(-) diff --git a/doc/opencv.md b/doc/opencv.md index bbc886fff..020406e17 100644 --- a/doc/opencv.md +++ b/doc/opencv.md @@ -209,9 +209,10 @@ Notes on parameters: Geometrically transforms the `src` image by taking the pixels specified by the coordinates from the `map` image. Notes on parameters: -* `src.step` - must be less than 2^16 +* `src.step` - must be less than 2^16 * `element size` +* `src.width`, `src_height` - must not be bigger than 2^15 * `src.depth()` - only supports `CV_8U` depth and 1 channel. -* `borderMode` - only supports `BORDER_REPLICATE` +* `borderMode` - only supports `BORDER_REPLICATE`. Supported map configurations: * `map1` is 16SC2: channel #1 is x coordinate (column) and channel #2 is y (row) * supported `interpolation`: `INTER_NEAREST` only diff --git a/kleidicv/include/kleidicv/kleidicv.h b/kleidicv/include/kleidicv/kleidicv.h index 3c95ef4c3..454fa7b00 100644 --- a/kleidicv/include/kleidicv/kleidicv.h +++ b/kleidicv/include/kleidicv/kleidicv.h @@ -1760,20 +1760,19 @@ KLEIDICV_API_DECLARATION(kleidicv_in_range_f32, const float *src, /// from the `mapxy` image. /// /// Width and height must be the same for `mapxy` and for `dst`. `src` -/// dimensions may be different, but due to the 16-bit signed format, elements -/// with any coordinates outside of [0, 2^16) cannot be used. Coordinates -/// outside of `src` dimensions are considered border. In case of @ref -/// KLEIDICV_BORDER_TYPE_REPLICATE, that means that negative coordinates map to -/// the first row/column (zero), and those bigger than height/width - 1 map to -/// the last row/column. +/// dimensions may be different. Coordinates outside of `src` dimensions are +/// considered border. /// /// @param src Pointer to the source data. Must be non-null. /// @param src_stride Distance in bytes from the start of one row to the /// start of the next row for the source data. Must /// not be less than `width * sizeof(type)`, except for -/// single-row images. Must be less than 2^16. -/// @param src_width Number of elements in the source row. -/// @param src_height Number of rows in the source data. +/// single-row images. Must be less than `2^16 * +/// sizeof(type)`. +/// @param src_width Number of elements in the source row. Must not be bigger +/// than 2^15. +/// @param src_height Number of rows in the source data. Must not be bigger +/// than 2^15. /// @param dst Pointer to the destination data. Must be non-null. /// @param dst_stride Distance in bytes from the start of one row to the /// start of the next row for the destination data. diff --git a/kleidicv/include/kleidicv/transform/remap.h b/kleidicv/include/kleidicv/transform/remap.h index 2332a8234..97f3f0d55 100644 --- a/kleidicv/include/kleidicv/transform/remap.h +++ b/kleidicv/include/kleidicv/transform/remap.h @@ -14,13 +14,15 @@ namespace kleidicv { template inline bool remap_s16_is_implemented( - size_t src_stride, size_t dst_width, kleidicv_border_type_t border_type, + size_t src_stride, size_t src_width, size_t src_height, size_t dst_width, + kleidicv_border_type_t border_type, size_t channels) KLEIDICV_STREAMING_COMPATIBLE { if constexpr (std::is_same::value) { - return ( - src_stride <= std::numeric_limits::max() && dst_width >= 8 && - border_type == kleidicv_border_type_t::KLEIDICV_BORDER_TYPE_REPLICATE && - channels == 1); + return (src_stride <= std::numeric_limits::max() && + src_width <= std::numeric_limits::max() + 1 && + src_height <= std::numeric_limits::max() + 1 && + dst_width >= 8 && border_type == KLEIDICV_BORDER_TYPE_REPLICATE && + channels == 1); } else { return false; } @@ -28,13 +30,15 @@ inline bool remap_s16_is_implemented( template inline bool remap_s16point5_is_implemented( - size_t src_stride, size_t dst_width, kleidicv_border_type_t border_type, + size_t src_stride, size_t src_width, size_t src_height, size_t dst_width, + kleidicv_border_type_t border_type, size_t channels) KLEIDICV_STREAMING_COMPATIBLE { if constexpr (std::is_same::value) { - return ( - src_stride <= std::numeric_limits::max() && dst_width >= 8 && - border_type == kleidicv_border_type_t::KLEIDICV_BORDER_TYPE_REPLICATE && - channels == 1); + return (src_stride <= std::numeric_limits::max() && + dst_width >= 8 && + src_width <= std::numeric_limits::max() + 1 && + src_height <= std::numeric_limits::max() + 1 && + border_type == KLEIDICV_BORDER_TYPE_REPLICATE && channels == 1); } else { return false; } diff --git a/kleidicv/src/transform/remap_neon.cpp b/kleidicv/src/transform/remap_neon.cpp index 716388943..43de2d561 100644 --- a/kleidicv/src/transform/remap_neon.cpp +++ b/kleidicv/src/transform/remap_neon.cpp @@ -84,8 +84,8 @@ kleidicv_error_t remap_s16(const T *src, size_t src_stride, size_t src_width, CHECK_IMAGE_SIZE(src_width, src_height); CHECK_IMAGE_SIZE(dst_width, dst_height); - if (!remap_s16_is_implemented(src_stride, dst_width, border_type, - channels)) { + if (!remap_s16_is_implemented(src_stride, src_width, src_height, dst_width, + border_type, channels)) { return KLEIDICV_ERROR_NOT_IMPLEMENTED; } @@ -238,8 +238,8 @@ kleidicv_error_t remap_s16point5( CHECK_IMAGE_SIZE(src_width, src_height); CHECK_IMAGE_SIZE(dst_width, dst_height); - if (!remap_s16point5_is_implemented(src_stride, dst_width, border_type, - channels)) { + if (!remap_s16point5_is_implemented(src_stride, src_width, src_height, + dst_width, border_type, channels)) { return KLEIDICV_ERROR_NOT_IMPLEMENTED; } diff --git a/kleidicv/src/transform/remap_sc.h b/kleidicv/src/transform/remap_sc.h index c0b965ed5..58e18c905 100644 --- a/kleidicv/src/transform/remap_sc.h +++ b/kleidicv/src/transform/remap_sc.h @@ -38,10 +38,8 @@ class RemapS16 { v_xmax_{v_x_max}, v_ymax_{v_y_max} { v_src_stride_ = svdup_u16(src_rows.stride()); - v_xmax_ = svdup_s16(static_cast( - std::min(std::numeric_limits::max(), src_width - 1))); - v_ymax_ = svdup_s16(static_cast( - std::min(std::numeric_limits::max(), src_height - 1))); + v_xmax_ = svdup_s16(static_cast(src_width - 1)); + v_ymax_ = svdup_s16(static_cast(src_height - 1)); } void process_row(size_t width, Columns mapxy, @@ -124,8 +122,8 @@ kleidicv_error_t remap_s16_sc(const T* src, size_t src_stride, size_t src_width, CHECK_IMAGE_SIZE(src_width, src_height); CHECK_IMAGE_SIZE(dst_width, dst_height); - if (!remap_s16_is_implemented(src_stride, dst_width, border_type, - channels)) { + if (!remap_s16_is_implemented(src_stride, src_width, src_height, dst_width, + border_type, channels)) { return KLEIDICV_ERROR_NOT_IMPLEMENTED; } @@ -403,8 +401,8 @@ kleidicv_error_t remap_s16point5_sc( CHECK_IMAGE_SIZE(src_width, src_height); CHECK_IMAGE_SIZE(dst_width, dst_height); - if (!remap_s16point5_is_implemented(src_stride, dst_width, border_type, - channels)) { + if (!remap_s16point5_is_implemented(src_stride, src_width, src_height, + dst_width, border_type, channels)) { return KLEIDICV_ERROR_NOT_IMPLEMENTED; } diff --git a/kleidicv_thread/src/kleidicv_thread.cpp b/kleidicv_thread/src/kleidicv_thread.cpp index 166da5d28..d4ce0b8c6 100644 --- a/kleidicv_thread/src/kleidicv_thread.cpp +++ b/kleidicv_thread/src/kleidicv_thread.cpp @@ -662,7 +662,8 @@ kleidicv_error_t kleidicv_thread_remap_s16_u8( size_t channels, const int16_t *mapxy, size_t mapxy_stride, kleidicv_border_type_t border_type, const uint8_t *border_value, kleidicv_thread_multithreading mt) { - if (!kleidicv::remap_s16_is_implemented(src_stride, dst_width, + if (!kleidicv::remap_s16_is_implemented(src_stride, src_width, + src_height, dst_width, border_type, channels)) { return KLEIDICV_ERROR_NOT_IMPLEMENTED; } @@ -684,7 +685,8 @@ kleidicv_error_t kleidicv_thread_remap_s16point5_u8( kleidicv_border_type_t border_type, const uint8_t *border_value, kleidicv_thread_multithreading mt) { if (!kleidicv::remap_s16point5_is_implemented( - src_stride, dst_width, border_type, channels)) { + src_stride, src_width, src_height, dst_width, border_type, + channels)) { return KLEIDICV_ERROR_NOT_IMPLEMENTED; } auto callback = [=](unsigned begin, unsigned end) { diff --git a/test/api/test_remap.cpp b/test/api/test_remap.cpp index e386e37fb..af585df00 100644 --- a/test/api/test_remap.cpp +++ b/test/api/test_remap.cpp @@ -7,6 +7,7 @@ #include "framework/array.h" #include "framework/generator.h" #include "framework/utils.h" +#include "kleidicv/ctypes.h" #include "kleidicv/kleidicv.h" template @@ -78,7 +79,30 @@ class RemapS16 : public testing::Test { *mapxy.at(row, column * 2 + 1) = corner_y_values[counter % ny]; } } - execute_test(mapxy, src_w, src_h, dst_w, dst_h, channels, padding); + + // This part is the same as execute_test() but without initializing source. + // Corner Cases use the biggest possible source. + size_t src_total_width = channels * src_w; + size_t dst_total_width = channels * dst_w; + + test::Array2D source{src_total_width, src_h, padding, channels}; + test::Array2D actual{dst_total_width, dst_h, padding, channels}; + test::Array2D expected{dst_total_width, dst_h, padding, + channels}; + + test::PseudoRandomNumberGenerator generator; + actual.fill(42); + + calculate_expected(source, mapxy, expected); + + ASSERT_EQ(KLEIDICV_OK, + kleidicv_remap_s16_u8( + source.data(), source.stride(), source.width(), + source.height(), actual.data(), actual.stride(), + actual.width(), actual.height(), channels, mapxy.data(), + mapxy.stride(), KLEIDICV_BORDER_TYPE_REPLICATE, {})); + + EXPECT_EQ_ARRAY2D(actual, expected); } private: @@ -165,10 +189,10 @@ TYPED_TEST(RemapS16, BlendBigStride) { } TYPED_TEST(RemapS16, CornerCases) { - size_t src_w = 3 * test::Options::vector_lanes() - 1; - size_t src_h = 4; - size_t dst_w = src_w; - size_t dst_h = src_h; + size_t src_w = std::numeric_limits::max() + 1; + size_t src_h = std::numeric_limits::max() + 1; + size_t dst_w = 3 * test::Options::vector_lanes() - 1; + size_t dst_h = 4; TestFixture::test_corner_cases(src_w, src_h, dst_w, dst_h, 1, 17); } @@ -198,11 +222,20 @@ TYPED_TEST(RemapS16, InvalidImageSize) { TypeParam dst[8]; int16_t mapxy[16] = {}; + EXPECT_EQ(KLEIDICV_ERROR_NOT_IMPLEMENTED, + kleidicv_remap_s16_u8( + src, 1, std::numeric_limits::max() + 2, 1, dst, 8, 8, + 1, 1, mapxy, 4, KLEIDICV_BORDER_TYPE_REPLICATE, nullptr)); + + EXPECT_EQ(KLEIDICV_ERROR_NOT_IMPLEMENTED, + kleidicv_remap_s16_u8( + src, 1, 1, std::numeric_limits::max() + 2, dst, 8, 8, + 1, 1, mapxy, 4, KLEIDICV_BORDER_TYPE_REPLICATE, nullptr)); + EXPECT_EQ(KLEIDICV_ERROR_RANGE, kleidicv_remap_s16_u8(src, 1, KLEIDICV_MAX_IMAGE_PIXELS + 1, 1, dst, 8, 8, 1, 1, mapxy, 4, KLEIDICV_BORDER_TYPE_REPLICATE, nullptr)); - EXPECT_EQ( KLEIDICV_ERROR_RANGE, kleidicv_remap_s16_u8(src, 1, KLEIDICV_MAX_IMAGE_PIXELS, @@ -354,7 +387,31 @@ class RemapS16Point5 : public testing::Test { ++counter; } } - execute_test(mapxy, mapfrac, src_w, src_h, dst_w, dst_h, channels, padding); + + // This part is the same as execute_test() but without initializing source. + // Corner Cases use the biggest possible source. + size_t src_total_width = channels * src_w; + size_t dst_total_width = channels * dst_w; + + test::Array2D source{src_total_width, src_h, padding, channels}; + test::Array2D actual{dst_total_width, dst_h, padding, channels}; + test::Array2D expected{dst_total_width, dst_h, padding, + channels}; + + test::PseudoRandomNumberGenerator generator; + actual.fill(42); + + calculate_expected(source, mapxy, mapfrac, expected); + + ASSERT_EQ( + KLEIDICV_OK, + kleidicv_remap_s16point5_u8( + source.data(), source.stride(), source.width(), source.height(), + actual.data(), actual.stride(), actual.width(), actual.height(), + channels, mapxy.data(), mapxy.stride(), mapfrac.data(), + mapfrac.stride(), KLEIDICV_BORDER_TYPE_REPLICATE, {})); + + EXPECT_EQ_ARRAY2D(actual, expected); } private: @@ -472,10 +529,10 @@ TYPED_TEST(RemapS16Point5, BlendBigStride) { } TYPED_TEST(RemapS16Point5, CornerCases) { - size_t src_w = 3 * test::Options::vector_lanes() - 1; - size_t src_h = 4; - size_t dst_w = src_w; - size_t dst_h = src_h; + size_t src_w = std::numeric_limits::max() + 1; + size_t src_h = std::numeric_limits::max() + 1; + size_t dst_w = 3 * test::Options::vector_lanes() - 1; + size_t dst_h = 4; TestFixture::test_corner_cases(src_w, src_h, dst_w, dst_h, 1, 17); } @@ -511,6 +568,18 @@ TYPED_TEST(RemapS16Point5, InvalidImageSize) { int16_t mapxy[2] = {}; uint16_t mapfrac[1] = {}; + EXPECT_EQ( + KLEIDICV_ERROR_NOT_IMPLEMENTED, + kleidicv_remap_s16point5_u8( + src, 1, std::numeric_limits::max() + 2, 1, dst, 1, 1, 1, 1, + mapxy, 4, mapfrac, 2, KLEIDICV_BORDER_TYPE_REPLICATE, nullptr)); + + EXPECT_EQ( + KLEIDICV_ERROR_NOT_IMPLEMENTED, + kleidicv_remap_s16point5_u8( + src, 1, 1, std::numeric_limits::max() + 2, dst, 1, 1, 1, 1, + mapxy, 4, mapfrac, 2, KLEIDICV_BORDER_TYPE_REPLICATE, nullptr)); + EXPECT_EQ(KLEIDICV_ERROR_RANGE, kleidicv_remap_s16point5_u8( src, 1, KLEIDICV_MAX_IMAGE_PIXELS + 1, 1, dst, 1, 1, 1, 1, -- GitLab