Fix integer overflow in cv::Luv2RGBinteger::process.#21112
Fix integer overflow in cv::Luv2RGBinteger::process.#21112alalek merged 2 commits intoopencv:3.4from
Conversation
4adb648 to
7735216
Compare
| long long int xv = ((int)up)*(long long)vp; | ||
| int x = (int)(xv/BASE); | ||
| x = y*x/BASE; | ||
| x = ((long long int)y)*x/BASE; |
There was a problem hiding this comment.
Could you please add test for the case with overflow?
Existed Imgproc_ColorLuv_Full.bitExactness doesn't cover this code at all.
@savuor Which test should execute this "bit-exact" code (Luv2RGBinteger)?
Update: reproduced with -DCV_DISABLE_OPTIMIZATION=ON
There was a problem hiding this comment.
I tried to create a test but it is tough: a Luv(49,205,23) gives RGB(0,103,255) without my patch and (255,0,255) with my patch. Then again, converting back to Luv we respectively get (122,80,15) and (153,157,31). We hit the white point limitation as referenced in the doc and here https://answers.opencv.org/question/205330/converting-luv-rgb-luv-results-in-different-values/
If we use https://www.easyrgb.com/en/convert.php, the results are very sensitive at this point and due to rounding, I cannot tell which value should be returned.
I guess we hit the limits of the space and there is no real answer: https://upload.wikimedia.org/wikipedia/commons/8/8d/SRGB_gamut_within_CIELCHuv_color_space_mesh.webm
There was a problem hiding this comment.
Ok, I added a test to ensure it gives the same result as the float version.
There was a problem hiding this comment.
@alalek The test you've mentioned should check this code for bit-exactness. If it's not true, then why?
There was a problem hiding this comment.
@savuor : the bit exactness code has the same overflow bug
opencv/modules/imgproc/test/test_color.cpp
Line 2572 in d58b5ef
For LL=49, uu=205, vv=23, we end up with x=7373056 and y=458 which overflows y*x.
7735216 to
607cddf
Compare
|
I just need to merge those two commits before it can merged right ? |
|
Not needed - I will squash commits during the merge. |
modules/imgproc/test/test_color.cpp
Outdated
|
|
||
| TEST(Imgproc_ColorLuv, Overflow_21112) | ||
| { | ||
| const Size sz(107, 16); // unaligned size to run both SIMD and generil code |
There was a problem hiding this comment.
BTW, generil -> generic
For LL=49, uu=205, vv=23, we end up with x=7373056 and y=458
which overflows y*x.
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request