Skip to content

Use NaN-safe clip function.#21271

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
vrabaud:3.4_clip
Dec 16, 2021
Merged

Use NaN-safe clip function.#21271
opencv-pushbot merged 1 commit intoopencv:3.4from
vrabaud:3.4_clip

Conversation

@vrabaud
Copy link
Copy Markdown
Contributor

@vrabaud vrabaud commented Dec 16, 2021

This is a follow-up on #21111 .

BTW, may I suggest to add a v_clamp to HAL that would be NaN safe ? _mm_min_ss does not seem to be according to https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#techs=SSE,SSE2,SSE3,SSSE3,SSE4_1,SSE4_2&text=min_ss&ig_expand=4786

Pull Request Readiness Checklist

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.

This is to prevent more NaN to int conversions like in opencv#21111.
@alalek
Copy link
Copy Markdown
Member

alalek commented Dec 16, 2021

NaN safe SIMD intrinsic would be nice to have, but I'm not sure if it is possible.

Main problem is undocumented NaN behavior, so we can't guarantee anything without performance drop:

dst does not follow the IEEE Standard for Floating-Point Arithmetic (IEEE 754) minimum value when inputs are NaN or signed-zero values.

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@opencv-pushbot opencv-pushbot merged commit cee3ec6 into opencv:3.4 Dec 16, 2021
@vrabaud vrabaud deleted the 3.4_clip branch December 16, 2021 11:55
@alalek alalek mentioned this pull request Dec 18, 2021
@alalek alalek mentioned this pull request Dec 30, 2021
@alalek alalek mentioned this pull request Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants