search

rainyl / opencv_dart

OpenCV bindings for Dart language and Flutter. Support Asynchronous Now!
https://pub.dev/packages/opencv_dart
Apache License 2.0
136 stars 18 forks source link

logging interface #175

Closed abdelaziz-mahdy closed 4 months ago

abdelaziz-mahdy commented 4 months ago

implementing logging interface fixing https://github.com/rainyl/opencv_dart/issues/173

sync and async interfaces and test cases

abdelaziz-mahdy commented 4 months ago

i dont think getLogTagLevel or stuff related to tags is needed, but will leave it since it may be needed in specific use cases that i dont know

also its test case fails, and i cant figure out the problem sadly

rainyl commented 4 months ago

Actually I think it's not necessary to add async version, getting and setting log level are fast enough. In my test, allocate and free memories have extra cost that will damage the performance.

abdelaziz-mahdy commented 4 months ago

Actually I think it's not necessary to add async version, getting and setting log level are fast enough. In my test, allocate and free memories have extra cost that will damage the performance.

i thought that too, but i saw openCvVersionAsync so i followed it to match the sync async for all

rainyl commented 4 months ago

Actually I think it's not necessary to add async version, getting and setting log level are fast enough. In my test, allocate and free memories have extra cost that will damage the performance.

i thought that too, but i saw openCvVersionAsync so i followed it to match the sync async for all

😂It's my fault too, async was somehow abused when we worked on it.

abdelaziz-mahdy commented 4 months ago

well well here i go will delete the async for logging and the version stuff

abdelaziz-mahdy commented 4 months ago

done, but i think test will fail again.

rainyl commented 4 months ago

done, but i think test will fail again.

Ok, I will test and fix it.

rainyl commented 4 months ago

@abdelaziz-mahdy I don't know why too, maybe the tag is not actually registered or available between different calls.

https://github.com/opencv/opencv/blob/eab21b61067385515c77783608f8c374bfe131c6/modules/core/src/logger.cpp#L146-L167

I searched in the repo of opencv and it seems the two API are not actually used. So, I think it's fine to remove setLogTagLevel and getLogTagLevel.

abdelaziz-mahdy commented 4 months ago

done, i think we can merge after ci is successful

rainyl commented 4 months ago

@abdelaziz-mahdy Merged, thanks for your contribution!