Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements around C++ code + better error handling + two new OpenCV functions + clearBuffers support for skipping some objects (by ids) #44

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

experiment322
Copy link
Contributor

@experiment322 experiment322 commented Dec 12, 2024

This is a big diff that I've accumulated while integrating this project into one of my projects. I'm not sure about the changes in build.gradle, especially the one pinning the react-native version, but that helped me get gradle and Android Studio to work and to help with C++ code while working on this.

Trying to resume the changes:

  • added calcOpticalFlowPyrLK and phaseCorrelate OpenCV functions
  • fixed checks for optional arguments inside the invoke C++ function (check argument count before asserting the type, because otherwise we're accessing out of bounds indexes in the arguments array -> bad memory access?)
  • also implemented the index check in FOCV_FunctionArguments and throw some errors if the index is out of bounds + also implemented a type check before trying to cast (if the index check passed) + implemented some new OpenCV types (I think TermCriteria is the only one?)
  • fixed FOCV_Object Mat creation when given an array with existing data (it didn't respect the number of channels specified in the requested type, always reshaping to 1) + improved array value extraction performance (reserved the required length ahead of time) + again added some index checks for vectors - other small fixes?
  • implemented a whitelist of ids to keep when clearing buffers in FOCV_Storage::clear so you can exclude some objects from the cleanup because you might need them later (I used this lib in combination with react-native-vision-camera, and inside the frame processor - a function that runs for each frame from the camera - I did some opencv stuff and I needed to retain an image for the next loop while clearing the other stuff to avoid filling the memory and to improve performance) - the implementation is questionable as it accepts an array of ids due to simplicity, so in js you'd have to pass [opencvObject.id] instead of [opencvObject]
  • maybe other stuff?

I opened the PR because I considered it's a nice improvement that would benefit everybody, but it's not very refined, well tested and very well documented yet, so any feedback is welcome. I might revert some changes, especially from the build.gradle, but they don't seem to break anything yet. Also, these changes shouldn't introduce any breaking change. Also, I didn't test very much on iOS, but will do in the near future.

PS: Thank you for this library!

PS PS: calcOpticalFlowPyrLK works pretty well, that's the one I'm mainly using, but phaseCorrelate I'm not so sure of, I tested it a little and it was outputting some values, but they seemed strange to me, but maybe I didn't fully understand it. I was wondering if I'm doing some bad casting, but not sure.

@experiment322 experiment322 changed the title Improvements around C++ code + better error handling + two new OpenCV functions + clearBuffers support for skipping some ids Improvements around C++ code + better error handling + two new OpenCV functions + clearBuffers support for skipping some objects (by ids) Dec 12, 2024
@lukaszkurantdev
Copy link
Owner

@experiment322 Wow, thanks for your work! Sorry for the delay, but I need some more time to process it. I'll add it to the next version, but I need to do some tests and improvements in different parts.

@lukaszkurantdev lukaszkurantdev merged commit fce23fa into lukaszkurantdev:main Jan 9, 2025
3 checks passed
@lukaszkurantdev
Copy link
Owner

@experiment322 Thanks one more time for your improvements. I've merged them to current lib repo.

Unfortunately I need to remove calcOpticalFlowPyrLK support – it's function from object detection module of OpenCV and currently it's not supported on iOS (library currently uses a slimmed-down customised build of OpenCV without this module).

Other changes remain and will be adapted for a future version :)

@experiment322
Copy link
Contributor Author

Ok, thanks for getting this in! But I have two things to mention:

  • I was and I'm still not sure if the build.gradle changes are good, but if it doesn't break anything I suppose it's fine
  • I didn't document the storage selective clear functionality, you might want to do that when you got time so people are aware of it

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.

2 participants