Skip to content

Improved Mat::release()#2901

Closed
nisargthakkar wants to merge 3 commits intoopencv:2.4from
nisargthakkar:bug_3757
Closed

Improved Mat::release()#2901
nisargthakkar wants to merge 3 commits intoopencv:2.4from
nisargthakkar:bug_3757

Conversation

@nisargthakkar
Copy link
Copy Markdown
Contributor

@SpecLad, @ilya-lavrenov I tried to understand what was going on in the .create() of the Mat class. From what I understood, if the current object has the same number of rows and cols [or same size[i] for each i] as the one specified in the function call, we need not reallocate space. But, if atleast one is different, it calls .release() first and then allocates the required memory spaces. So, considering this, if an object is released, we want it to be reset to initial conditions.
Now, rows and cols are also made zero because if dims > 2, they are both equal to -1 and are not reset by the previously changed code.

This is a continuation of #2891

@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented Jul 1, 2014

dims should not be cleared, it may lead to memory leak or memory access violation. please, fix it. Also, for some reason one of the tests fails with your patch: http://pullrequest.opencv.org. Please, take a look at it.

@nisargthakkar
Copy link
Copy Markdown
Contributor Author

And what about the flags? Is it ok to clear it?

@nisargthakkar
Copy link
Copy Markdown
Contributor Author

And if we are keeping dims as it is, it is better to leave rows and cols as they are. Because if dims == 2, then rows and cols are cleared. But if dims > 2, rows and cols should be -1. And I'm not too sure about flags. So, If you agree, I think we should close this PR.

@nisargthakkar
Copy link
Copy Markdown
Contributor Author

We can also free size[i] for all dims and then make dims = 0. And the default value of flags has been set to MAGIC_VAL.

@vpisarev
Copy link
Copy Markdown
Contributor

the code still does not pass our tests; such changes when you try to modify behavior of very basic methods, used everywhere in the library, should be thought out and done very, very carefully. Let's just use m.empty() to check whether matrix is empty or not. And let me close this PR.

@vpisarev vpisarev closed this Jul 14, 2014
@nisargthakkar nisargthakkar deleted the bug_3757 branch January 8, 2015 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants