mirror of
https://github.com/opencv/opencv.git
synced 2026-07-21 19:33:03 +04:00
Merge pull request #28267 from akretz:absdiff-int
Fix absdiff with int arguments #28267 I believe the fix to the undefined behavior described in #27080 is simply casting to unsigned before subtraction, because 1. casting int to unsigned is well-defined; negative values get represented modulo $2^{32}$ 2. overflow in unsigned subtraction is well-defined and the results are modulo $2^{32}$ Since we are computing everything modulo $2^{32}$ and the result must always be a non-negative number below $2^{32}$, this computation should be well-defined and correct. I have verified this on ARM Apple Clang and on x64 Linux gcc with `-O3` and both produce the correct values in the reproducer of #27080. The test I have added fails with the integer overflow on both platforms I have tested. Perhaps @fengyuentau could verify if this also fixes the issue on the platforms he has tested? I have added a fix and removed the workarounds. I recommended this approach in #28229, but he decided to revert it. ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake
This commit is contained in:
@@ -316,7 +316,7 @@ inline int cv_absdiff(uchar x, uchar y) { return (int)std::abs((int)x - (int)y);
|
||||
inline int cv_absdiff(schar x, schar y) { return (int)std::abs((int)x - (int)y); }
|
||||
inline int cv_absdiff(ushort x, ushort y) { return (int)std::abs((int)x - (int)y); }
|
||||
inline int cv_absdiff(short x, short y) { return (int)std::abs((int)x - (int)y); }
|
||||
inline unsigned cv_absdiff(int x, int y) { return (unsigned)(std::max(x, y) - std::min(x, y)); }
|
||||
inline unsigned cv_absdiff(int x, int y) { return (unsigned)std::max(x, y) - (unsigned)std::min(x, y); }
|
||||
inline unsigned cv_absdiff(unsigned x, unsigned y) { return std::max(x, y) - std::min(x, y); }
|
||||
inline uint64 cv_absdiff(uint64 x, uint64 y) { return std::max(x, y) - std::min(x, y); }
|
||||
inline float cv_absdiff(hfloat x, hfloat y) { return std::abs((float)x - (float)y); }
|
||||
|
||||
@@ -77,32 +77,6 @@ struct NormDiffL1_SIMD {
|
||||
}
|
||||
};
|
||||
|
||||
// This specialization is needed because https://github.com/opencv/opencv/issues/27080
|
||||
template <>
|
||||
struct NormDiffL1_SIMD<int, double> {
|
||||
inline double operator() (const int* src1, const int* src2, int n) const {
|
||||
double s = 0;
|
||||
int j = 0;
|
||||
#if (CV_SIMD_64F || CV_SIMD_SCALABLE_64F)
|
||||
v_float64 r0 = vx_setzero_f64(), r1 = vx_setzero_f64();
|
||||
for (; j <= n - VTraits<v_int32>::vlanes(); j += VTraits<v_int32>::vlanes()) {
|
||||
v_int32 v01 = vx_load(src1 + j), v02 = vx_load(src2 + j);
|
||||
v_uint32 v0 = v_absdiff(v01, v02);
|
||||
v_uint64 ev0, ev1;
|
||||
v_expand(v0, ev0, ev1);
|
||||
r0 = v_add(r0, v_cvt_f64(v_reinterpret_as_s64(ev0)));
|
||||
r1 = v_add(r1, v_cvt_f64(v_reinterpret_as_s64(ev1)));
|
||||
}
|
||||
s += v_reduce_sum(v_add(r0, r1));
|
||||
#endif
|
||||
for (; j < n; j++) {
|
||||
double d1 = (double)src1[j], d2 = (double)src2[j];
|
||||
s += (double)std::abs(d1 - d2);
|
||||
}
|
||||
return s;
|
||||
}
|
||||
};
|
||||
|
||||
template <typename T, typename ST>
|
||||
struct NormDiffL2_SIMD {
|
||||
inline ST operator() (const T* src1, const T* src2, int n) const {
|
||||
@@ -1348,27 +1322,6 @@ normDiffL1_(const T* src1, const T* src2, const uchar* mask, ST* _result, int le
|
||||
return 0;
|
||||
}
|
||||
|
||||
// This specialization is needed because https://github.com/opencv/opencv/issues/27080
|
||||
template<> int
|
||||
normDiffL1_(const int* src1, const int* src2, const uchar* mask, double* _result, int len, int cn) {
|
||||
double result = *_result;
|
||||
if( !mask ) {
|
||||
NormDiffL1_SIMD<int, double> op;
|
||||
result += op(src1, src2, len*cn);
|
||||
} else {
|
||||
for( int i = 0; i < len; i++, src1 += cn, src2 += cn ) {
|
||||
if( mask[i] ) {
|
||||
for( int k = 0; k < cn; k++ ) {
|
||||
double d1 = (double)src1[k], d2 = (double)src2[k];
|
||||
result += (double)std::abs(d1 - d2);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
*_result = result;
|
||||
return 0;
|
||||
}
|
||||
|
||||
template<typename T, typename ST> int
|
||||
normDiffL2_(const T* src1, const T* src2, const uchar* mask, ST* _result, int len, int cn) {
|
||||
ST result = *_result;
|
||||
|
||||
@@ -2931,4 +2931,20 @@ TEST(Mat, copyAt_regression27298)
|
||||
}
|
||||
}
|
||||
|
||||
TEST(Mat, issue_27080)
|
||||
{
|
||||
const std::vector<int> src1 = {
|
||||
605100952,281728648,-950765003,-270340785,-799079584,-1622966965,
|
||||
-1717860046,-203390322,-1624721112,881300643,-1841646762,247438602,
|
||||
-1871047900,2119496294,-868027786,1408019766,-2070408066,1782683247};
|
||||
const std::vector<int> src2 = {
|
||||
1746441532,892162071,-61301557,-1851870915,-1183525195,693709865,
|
||||
1669181466,746688189,1883246439,-436799897,1428311842,-531665722,
|
||||
1634935494,-1926207121,360363535,701351235,-1113337985,40823006};
|
||||
const std::vector<int> src3 = {INT_MAX};
|
||||
const std::vector<int> src4 = {INT_MIN};
|
||||
EXPECT_EQ(cv::norm(src1, src2, NORM_L1), 32321818045.);
|
||||
EXPECT_EQ(cv::norm(src3, src4, NORM_L1), UINT_MAX);
|
||||
}
|
||||
|
||||
}} // namespace
|
||||
|
||||
Reference in New Issue
Block a user