async object detect

[abdelaziz-mahdy]

can you check the code and let me know if there something i am doing wrong, i am getting many errors in dart file, i was going to fix those, but i want to be sure my impl of c++ is correct before moving on to dart fully

[rainyl]

Ok, I will take a look.

[abdelaziz-mahdy]

thank you for the mentioned fixes, i am now stuck in dart some function params i cant understand, and ffi size and the other size are confusing me. can you check and point me to the right direction?

after fixing dart file will make the test cases to make sure all is implemented and good to go

[rainyl]

ffi.Size actually represents sizt_t, i.e., platform specific integers, I remember that ony several functions use it. so just ignore it.

cv::Size was wrapped with a struct in c header, and in dart it used to be typedef Size=(int, int), now it is wrapped with a class, but in order to keep compatibility, most functions were kept using (int, int).

Hope the above informations can help you. Also, please let me know if you have any suggestions.😄

[abdelaziz-mahdy]

ffi.Size actually represents sizt_t, i.e., platform specific integers, I remember that ony several functions use it. so just ignore it.

cv::Size was wrapped with a struct in c header, and in dart it used to be typedef Size=(int, int), now it is wrapped with a class, but in order to keep compatibility, most functions were kept using (int, int).

Hope the above informations can help you. Also, please let me know if you have any suggestions.😄

well i think thats the right way to do it, so all good, if i found a better way will let you know even though i think you did the best way

will wait for you to check the current dart impl, i always get lost with pointers and ref so sometimes as you can see i fail to do it correctly

[rainyl]

I will check them later, but I am still considering how to organize async APIs, for functions they can be placed in a new file and it's fine, but for classes like HOGDescriptor, only one class name is allowed in dart, if sync and async are split to TWO files (like the objdetect_async.dart now), users have to import them separately.

So, the best solution I can think of is that, for functions, split to TWO files, but for classes, split every/several class to a single file and place sync and async in one class (one file), e.g., split HOGDescriptor to hogdescriptor.dart, and implement both sync and async inside it.

If you have better solutions, let's discuss it.

BTW, another solution is to split to different packages, like opencv_core, opencv_dnn, opencv_imgproc, etc., but it's laborious...

[abdelaziz-mahdy]

I will check them later, but I am still considering how to organize async APIs, for functions they can be placed in a new file and it's fine, but for classes like HOGDescriptor, only one class name is allowed in dart, if sync and async are split to TWO files (like the objdetect_async.dart now), users have to import them separately.

So, the best solution I can think of is that, for functions, split to TWO files, but for classes, split every/several class to a single file and place sync and async in one class (one file), e.g., split HOGDescriptor to hogdescriptor.dart, and implement both sync and async inside it.

If you have better solutions, let's discuss it.

BTW, another solution is to split to different packages, like opencv_core, opencv_dnn, opencv_imgproc, etc., but it's laborious...

different packages will be alot of work and i dont think its needed

lets think about which will be easier for the users do you want them to be same class name? cant we do it like this

      cascade_classifier.dart       // Sync version
      cascade_classifier_async.dart // Async version

with classes being

class CascadeClassifier
class CascadeClassifierAsync

i think the separation will make it easier to maintain and use, but the problem that sync and async cant be used in the same class, like if an instance got created async user cant use sync methods, is that a use case we should cover

also cant we use part imports? to have sync and async separated but using one class can use them both? i dont know if that good or not but its an option

      cascade_classifier.dart        // Main class
      cascade_classifier_sync.dart   // Sync methods
      cascade_classifier_async.dart  // Async methods

lib/src/objdetect/cascade_classifier.dart:

library cv;

import 'dart:ffi' as ffi;
import 'package:ffi/ffi.dart';
import '../core/base.dart';
import '../core/mat.dart';
import '../core/rect.dart';
import '../core/size.dart';
import '../opencv.g.dart' as cvg;

part 'cascade_classifier_sync.dart';
part 'cascade_classifier_async.dart';

class CascadeClassifier extends CvStruct<cvg.CascadeClassifier> {
  CascadeClassifier._(cvg.CascadeClassifierPtr ptr, [bool attach = true])
      : super.fromPointer(ptr) {
    if (attach) {
      finalizer.attach(this, ptr.cast(), detach: this);
    }
  }
part of 'cascade_classifier.dart';

extension CascadeClassifierSync on CascadeClassifier {
  bool load(String name) {
    final cname = name.toNativeUtf8().cast<ffi.Char>();
    final p = calloc<ffi.Int>();
    cvRun(() => CFFI.CascadeClassifier_Load(ref, cname, p));
    calloc.free(cname);
    return p.value != 0;
  }

part of 'cascade_classifier.dart';

extension CascadeClassifierAsync on CascadeClassifier {
  Future<bool> loadAsync(String name) async {
    final cname = name.toNativeUtf8().cast<ffi.Char>();
    final rval = cvRunAsync<bool>(
        (callback) => CFFI.CascadeClassifier_Load_Async(ref, cname, callback),
        (c, p) {

[rainyl]

i think the separation will make it easier to maintain and use, but the problem that sync and async cant be used in the same class, like if an instance got created async user cant use sync methods, is that a use case we should cover

I personally think we should, I will be inflexible if users have to import both sync and async version of the same API. so keep sync and async available in same class name is better.

cascade_classifier.dart        // Main class
cascade_classifier_sync.dart   // Sync methods
cascade_classifier_async.dart  // Async methods

Oh, this looks better, I will test it in-depth.

Thanks for your insightful suggestions!

[abdelaziz-mahdy]

thank you for the example, i was able to fix most of the errors only one remains since it returns 4 params and there is no run_async4

[abdelaziz-mahdy]

i added runasync4 and fixed the error, should i implement test cases or wait for you to have a look?

[abdelaziz-mahdy]

Should I free values coming from callbacks too?

[rainyl]

Should I free values coming from callbacks too?

Yes, they are pointers, but only for primitive types like int, float, double, etc., for structs like Mat, Size, VecPoint etc., corresponding dart classes will be created using .fromPointer()constructor, so the created instance will manage the memory in later operations.

[rainyl]

https://github.com/rainyl/opencv_dart/blob/b966c6eb0e9ee580a046bd9a99d312756c76b36f/lib/src/core/base.dart#L186-L202

You can refer to the impl in #117 , if only one value will be returned, we can simplify the onComplete by reusable functions like intCompleter, matCompleter, vecPointCompleter etc., I have moved matCompleter to mat.dart, vecPointCompleter to point.dart, etc., you can copy those files to use them.

[abdelaziz-mahdy]

Should I free values coming from callbacks too?

Yes, they are pointers, but only for primitive types like int, float, double, etc., for structs like Mat, Size, VecPoint etc., corresponding dart classes will be created using .fromPointer()constructor, so the created instance will manage the memory in later operations.

thank you i followed the mentioned points and fixed all issues i can see

[abdelaziz-mahdy]

https://github.com/rainyl/opencv_dart/blob/b966c6eb0e9ee580a046bd9a99d312756c76b36f/lib/src/core/base.dart#L186-L202

You can refer to the impl in #117 , if only one value will be returned, we can simplify the onComplete by reusable functions like intCompleter, matCompleter, vecPointCompleter etc., I have moved matCompleter to mat.dart, vecPointCompleter to point.dart, etc., you can copy those files to use them.

i didnt use those, i think we can migrate to it after this is working and test cases are implemented so when we migrate nothing breaks

[abdelaziz-mahdy]

for line length this is the only fix i found https://github.com/dart-lang/dart_style/issues/918#issuecomment-2041518288

[rainyl]

for line length this is the only fix i found dart-lang/dart_style#918 (comment)

I really do not want to commit settings.json, according to this comment,

https://github.com/dart-lang/dart_style/issues/918#issuecomment-2036687775

I have added format to pre-commit to format the line length to 110, now the format will run before git commit, you may need to add the formatted files and commit again, although kind of fussy, it's the best solution for now I think.

[abdelaziz-mahdy]

add precommit script

is the script ran automatically? since i dont think so, i had to run it manually

[rainyl]

add precommit script

is the script ran automatically? since i dont think so, i had to run it manually

I forgot that git won't commit hooks, maybe add a github action to format code is better, will be resolved later.

[abdelaziz-mahdy]

async constructors is done too

[abdelaziz-mahdy]

should tests be separated or the same files test sync and async?

[rainyl]

should tests be separated or the same files test sync and async?

The separated way is better for maintenance, but I am just lazy so placed them in one file, do as what you like.

[abdelaziz-mahdy]

I see you implemented the objdetect tests, so I should just implement the dnn one?

[rainyl]

Yes, I just have some free time so finished it, you can implement the tests for dnn

[abdelaziz-mahdy]

Yes, I just have some free time so finished it, you can implement the tests for dnn

Ok, will do it, just a question why is the callback API not blocking and the normal calls blocking, even though both run the same c code instead of returns it calls the callback?

Can you explain it more to me, I see that flutter does it the same for java platform interface for async tasks too, but why it's not blocking

Will appreciate if there is a reference

[rainyl]

Ok, will do it, just a question why is the callback API not blocking and the normal calls blocking, even though both run the same c code instead of returns it calls the callback?

Can you explain it more to me, I see that flutter does it the same for java platform interface for async tasks too, but why it's not blocking

It's basically the credits to NativeCallable.listener, this API provided by dart::ffi also uses SendPort and ReceivePort internally, and it ensures the callback will be called in the future at the same isolate where the callback was created, and the pointers will be passed to that isolate too, so we can consume the pointers in that isolate.

The normal sync functions are called in the main isolate, as we know, dart is single-thread, the synchronized functions will be called sequentially so once some functions are time-consuming, the main isolate will be blocked.

However, because the pointers are unsendable across isolates, so if a Mat is constructed inside an isolate, it still can't be sent to another isolate, e.g., the following is ERROR:

import 'dart:isolate';

import 'package:opencv_dart/opencv_dart.dart' as cv;

void main(List<String> args) async {
  final im = await Isolate.run<cv.Mat>(() async => cv.imreadAsync("test/images/circles.jpg"));

  print(im.shape);  // ERROR!
}

However, because the callback will be called in the future, so it won't block the current/main isolate, with async/await, main isolate can process other instructions, for Flutter, this will NOT block main/UI thread so the app is still interactive.

[abdelaziz-mahdy]

wow, interested to see if it can use multicores or not, but in most cases async is much better due to its simple usage vs isolates, but will need to further test it

i checked the api it launches isolate and runs the function then closes the isolate, i think we will need to mention there is a max number of isolates then ui will get blocked https://github.com/dart-lang/sdk/issues/51261 i dont know if that will happen using the NativeCallable.listener or not, but a check will make it known

i added dnn_test but mat operations needs to be migrated to async ones i need but not migrated

minMaxLoc

convertTo

i guess that should be the next step migrating core.h and core.cpp to async

[rainyl]

i added dnn_test but mat operations needs to be migrated to async ones i need but not migrated

minMaxLoc

convertTo

i guess that should be the next step migrating core.h and core.cpp to async

Yes, they are next steps, for now just kepp using sync version.

[abdelaziz-mahdy]

done, i am missing the models, so i cant test the code, dont you think a script to download all the needed models and running the tests is needed? also i workflow to make sure all tests are passing, since i cant find it (correct me if it does exist )

[rainyl]

The models are located at https://github.com/rainyl/opencv_dart/releases/tag/dnn_test_files, the coverage workflow will download them, so don't worry~

BTW, I have fixed some string conversion issues, now I think it's time to merge

[abdelaziz-mahdy]

Let me know what you plan to work on, so I work on something else, if you want to give me a module I don't mind

I learned alot of ffi doing this so I am enjoying the learning experience and your explanation is awesome so thank you very much.

[rainyl]

Let's open new issues to track and assign them.