Heap corruption due to vpImageConvert::convert(cv::Mat, vpImage)

[rolalaro]

Hi,

A bug has been revealed thanks to the CI on Windows in the test catchColorConversion.
A call to the method void vpImageConvert::convert(const cv::Mat &src, vpImage<uint16_t> &dest, bool flip) leads to a heap corruption.

After some research, I narrowed down the problem to the following line:

        for (unsigned int i = 0; i < destRows; ++i) {
          memcpy(dest.bitmap + (i * destCols), src.data + (i * src.step1() * sizeof(uint16_t)), static_cast<size_t>(src.step));
        }

It corresponds to the copy of the cv::Mat data into the vpImage object when the cv::Mat is a view (i.e. not a matrix whose memory is not contiguous) of an actual cv::Mat object.

The problem comes from the fact that the step attribute corresponds to the step of the original matrix and thus too many data are copied into the vpImage object, which leads to heap corruption.

@fspindle I see 2 solutions to the problem:

1. we assume that the matrix is a submatrix of a bigger cv::Mat object and thus that we can copy destCols consecutive uint16_t from it.
   The line would then be:

for (unsigned int i = 0; i < destRows; ++i) {
          memcpy(dest.bitmap + (i * destCols), src.data + (i * src.step1() * sizeof(uint16_t)), static_cast<size_t>(sizeof(uint16_t) * destCols));
        }

2. We have no assumption on how the cv::Mat view is created and thus we copyy one item at the time.

What would you rather do ?

[fspindle]

@rolalaro What do you propose ?

• I would prefer option1.
• Can you identify a code example where option 1 doesn't work and we have to copy individual elements one after the other (option 2)? If not, modify with option 1

[rolalaro]

@rolalaro What do you propose ?

* I would prefer option1.

* Can you identify a code example where option 1 doesn't work and we have to copy individual elements one after the other (option 2)? If not, modify with option 1

OK I'm gonna check if a view can be completely discontinuous (e.g. if it can be a view of column 1 and 3 but not caontaining column 2)
If it doesn't seem possible, I'll implement option 1.