ARM-software / CMSIS_5

CMSIS Version 5 Development Repository
http://arm-software.github.io/CMSIS_5/index.html
Apache License 2.0
1.33k stars 1.08k forks source link

Potential bug in the CMSIS DSP library #805

Closed skapelko closed 4 years ago

skapelko commented 4 years ago

In my program I wanted to use the function arm_bilinear_interp_q7 form arm_math.h After long tests, however, it seems to me that there is a mistake. For the interpolation test, I used such an array

const arm_bilinear_interp_instance_q7 tabtest = { .numRows = 2, .numCols = 5, .pData = { 10, 20, 30, 40, 50, 80, 90, 100, 110, 120, } };

The concept of columns and rows is not unique in the code. for example this line (are these rows or columns?) uint32_t nCols = S-> numCols; / num of rows /

For the interpolation work correctly, I had to modify the following line:

/ Care taken for table outside boundary / / Returns zero output when values ​​are outside table boundary / if (rI <0 || rI> (S-> numRows - 1) || cI <0 || cI> (S-> numCols - 1))

I changed it to the following: if (rI <0 || rI> (S-> numCols - 2) || cI <0 || cI> (S-> numRows - 2))

I have replaced rows with columns in this line. In addition, I modified the limitation not to exceed the array range when calculating x2 and y2.

These notes also apply to the functions arm_bilinear_interp_q15, arm_bilinear_interp_q31 and arm_bilinear_interp_f32

Can anyone confirm that this is a mistake or am I doing something wrong?

Regards

christophe0606 commented 4 years ago

It is a known issue : https://github.com/ARM-software/CMSIS_5/issues/695 There are inconsistencies between the comments, the preconditions and the implementation.

skapelko commented 4 years ago

Will this be corrected in the next version of the library?

christophe0606 commented 4 years ago

@skapelko I am working on it now.

christophe0606 commented 4 years ago

@skapelko It should be corrected in the latest commit https://github.com/ARM-software/CMSIS_5/commit/a65bf008dd8a7c0a64ce7d4639322326d04a7354