search

rainyl / opencv_dart

OpenCV bindings for Dart language and Flutter.
https://pub.dev/packages/opencv_dart
Apache License 2.0
78 stars 10 forks source link

clone required for feature call #95

Closed abdelaziz-mahdy closed 2 weeks ago

abdelaziz-mahdy commented 2 weeks ago

i think clone should be added to c++ wrapper https://github.com/abdelaziz-mahdy/face_grouping/commit/544bc95202805f9fbe4615c6f9cb8638db05583b since if not called in dart it will override the value of other mats i took alot of time to debug this problem

should be added in wrapper or not is the question?

rainyl commented 2 weeks ago

i think clone should be added to c++ wrapper

Mat.clone() actually calls the c++ wrapper. https://github.com/rainyl/opencv_dart/blob/19ab8628a41439048e8ecf53d9cce90b8d27cdcc/lib/src/core/mat.dart#L1166-L1171

https://github.com/rainyl/opencv_dart/blob/19ab8628a41439048e8ecf53d9cce90b8d27cdcc/src/core/core.cpp#L147-L152

since if not called in dart it will override the value of other mats i took alot of time to debug this problem

I am not clear about what exactly you mean "if not called in dart it will override the value of other mats" , does it means the current clone will corrupt the data of other mats?

abdelaziz-mahdy commented 2 weeks ago

i meant that for FaceRecognizerSF feature call any other call will override the prev mat data, so i need to call clone from dart side, should we make the wrapper auto call clone to avoid that bug?

rainyl commented 2 weeks ago

Okay, I will take a look at it, but theretically every time the feature() is called, a new Mat will be created... not sure why.

rainyl commented 2 weeks ago

confirmed, it seems to be a bug of wrong pointer, i will locate and fix it recently.

abdelaziz-mahdy commented 2 weeks ago

confirmed, it seems to be a bug of wrong pointer, i will locate and fix it recently.

An easy and sufficient fix is cloning the mat and return the cloned mat, don't know if it's safe and memory safe or not

rainyl commented 2 weeks ago

Oh... I just noticed that the tutorial of opencv also manually clones the features image I am not sure why they design it that way but I think keeping the same behavior as official opencv is better, I can add an optional parameter as following to allow deep copying, which will be passed to C++ wrappers and clone in C++ side.

Mat feature(Mat alignedImg, {bool clone = false})
rainyl commented 2 weeks ago

@abdelaziz-mahdy I have published v1.0.8, you can update and try whether it works, if the problem still exists, please reopen this issue. 😄

abdelaziz-mahdy commented 2 weeks ago

Will check and let you know if there is a problem thank you ❤️