dataObject m_osize gets corrupted

[Former user]

The dataObject::m_osize field gets corrupted under certain circumstances, which in turn can cause data access using the according step size to read beyond data memory.

Currently I verified this behavior using the following sequence of c++ code:

ito::DataObject d1, d2;
int sizes[2] = { 9, 11 };
d1.rand(99, 4, ito::tFloat64);
d1.at(ito::Range::all(), ito::Range(0, 1)).copyTo(d2, 1);
d2 = d2.reshape(2, sizes);
d2 = d2.at(ito::Range(0, 1), ito::Range::all());
d2 = d2.trans();

Now checking the osize of d2 results in {11, 9}, but data will be in the format of a vector, which will result in reading beyond memory when used on d2 (stepping from one value to the next, using step). Checking the resulting dataObject from the first .at (d1.at), shows, that the returned underlying cv::Mat is actually not a submat. Probably this behavior should be generally revised. Actually the bug is in the transpose function, making a deep copy, not altering osize

[M. Gronle]

Hi Chris,

in Python a similar behaviour can be caused by:

d2 = dataObject.randN([9,11],'float32')
d2 = d2[0:1,:]
d2 = d2.trans()

print(d2.shape, d2.locateROI())
d2.adjustROI([0,0,0,8])
d2.data() #-> will only print a 11x1 vector instead of a 11x9 one

Then, the final d2 matrix is a size of 11x9 again, but the data()-method only prints out a 11x1 vector (but pretends to have a 11x9 matrix).

Am I correct, that the t()-Method of cv::Mat does not return a deep copy but a shallow copy, where the step sizes are changed? If the step-size is unequal to the element-size in the last dimension, then I think we have to do a deep copy, since all our algorithms do not handle a step size different than the element size in the last dimension.

Hence, doing a deep copy will return a new dataObject where m_roi is {0,0,...} and m_osize is equal to m_size. If we want to support a step-size in the last-dimension (X) unequal to the element size, all for-loops that iterate over a matrix have to be changed in itom as well as all plugins, since usually we write

ito::float32 *rowPtr;
for (int  r = 0; r < obj.getSize(0); ++r)
{
    rowPtr = obj.rowPtr<ito::float32>(r); //here, the step size from one row to the other one is considered!
    for(int c = 0; c < obj.getSize(1); ++c)
    {
        rowPtr[c] = newValue; //here we assume that the float values are always adjacent in one row, therefore the last step size of the cv::Mat has to be equal to sizeof(float)
    }
}

If we have to change the transpose-operation to a deep copy always, the change in the getSize() method is not necessary any more. By-the-way, the u->pointer of OpenCV only exists since OpenCV 3.0, in OpenCV 2.x I think that size is a direct member of cv::Mat.

It would be great to have that dynamic step size of OpenCV, but the change would be big.

[M. Gronle]

In principle, we also have to check, that the given cv::Mat planes in ito::DataObject::CreateFuncWithCVPlanes have always correct step sizes and that the last value in the step sizes vector is equal to the element size. Currently, this is not properly checked. Only, when construction a DataObject from a numpy-array, such a check is executed. If the numpy-array has a last step size (x-axis) unequal to the element size, a new axis is appended with a dimension of one, such that the inproper step size is the axis before the last one, where step sizes that are bigger than the number of dimensions in this axis times the element size, are considered by a region of interest.

[ckohler]

This is a bug of the ugly kind, got me trapped some time yesterday ;-) Well, it seems that OpenCV actually does not make deep copies when transposing, resulting in none unitary step sizes for the last dimension. Having that in itom would be quite elegant, as it would save some memory copies, though this would require quite a lot of testing. Thus, for the time being we will probably stay with some more checks and copying, and putting it on the wish list. The main point would be now, to identify all places where we have to check for this behavior and what to do. In principle it should only happen for vector shaped roi's being manipulated in some way. Therefore adding an extra dimension and making 2D objects of 1D ones, should not break other things, e.g. 2D CV::Mats becoming 3D, causing conflicts with itom's handling of planes. Anyway I'd like to change the code for the getStep to read the last dimensions from the cv::Mat, just in case something went wrong beforehand.

Am 29.08.2017 um 12:51 schrieb Marc Gronle:

[M. Gronle]

I close this issue, since on 2021-09-28 I could not reproduce this error again. Maybe this bug has also been fixed by the commit 67e5798885b923075fe79da6d6b7daaaf26c5407