Marking this RFC, because it doesn't follow OpenCV guidelines to avoid using of raw native intrinsics in OpenCV modules.

Hi @alalek, thank you for looking into this. We (me and @fpetrogalli) submitted it like this because we weren't sure what correct approach was. Also, sorry if this is an silly question, but what does it mean to mark it as RFC?

One possible solution to the raw intrinsics is to keep the #if CV_NEON block in color_lab.cpp but rewrite it using the HAL intrinsics. Another solution would be to split it into a neon specific file, like in the case of resize.cpp, resize.avx2.cpp and resize.sse4_1.cpp for example. Are either of these acceptable or preferable? Or is there a another way which would achieve the same goal?

@jondea, thank you for the contribution!
as @alalek said, for the tiny OpenCV core team it's simply unfeasible to maintain separate code branches for the growing amount of code and the growing number of platforms that we support. With time we hope to port most of the remaining native branches to HAL/universal intrinsics. There will be some exceptions, like deep learning, where the amount of critical kernels is not that big and where we can afford separate branches, but overall universal intrinsics is the preferable (by far) option.

I'd start with the first option that you suggested - keep the separate branch under CV_NEON, but rewrite it using HAL intrinsics. I briefly looked at the current implementation and I found it too bulky for the equivalent C code that it accelerates. So, I'm 60-80% sure that the HAL code that you write will be faster than the existing implementation not just on ARM, but on the other platforms as well. And then we will just replace that code with yours, i.e. remove #if CV_NEON ... #endif around your code and remove the other branch.

The changes have been rewritten to use just HAL intrinsics, any feedback would be appreciated.

@vpisarev @alalek is there anything else which needs to be done before this can be merged?

Hi @vpisarev @alalek would you be able to take another look at this please and let me know if it can be merged?

@jondea, thank you very much! I tested the code both on Mac-Intel and Mac-ARM (M1), it works well, the claimed acceleration is achieved. On Intel it's no slower than the previous version, but, unfortunately, it's 128-bit only.

In any case, it can be merged as-is, and later we can modify this code to use some new variations of v_lut() intrinsic.

👍

@jondea, thank you very much! I tested the code both on Mac-Intel and Mac-ARM (M1), it works well, the claimed acceleration is achieved. On Intel it's no slower than the previous version, but, unfortunately, it's 128-bit only.

@vpisarev , @jondea is working on an equivalent version that uses SVE2 intrinsics. He is using the intrinsic svld1uh_gather_s32index_s32 for the variation of v_lut() that does a gather from the indexes. It is a Vector Length Agnositc (VLA) version, so it could be ported easily to HAL once we make the nlanes field to be a runtime value and we have the correspondent indexed lut intrinsic.