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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support for android #12

Merged
merged 2 commits into from
Dec 21, 2023
Merged

Conversation

jaroslawkrol
Copy link
Contributor

Pull Request Comment

馃殌 Feature: Enable the usage of the react-native-fast-tflite library on Android.

馃攧 Changes:

  1. Downgraded the org.tensorflow:tensorflow-lite library from version 2.13.0 to 2.12.0 to resolve header extraction issues.
  2. Modified the C++ JNI code implementation to ensure the thread is attached to the JVM. Callback handling was problematic, and the current changes aim to address this issue.
  3. There is a TODO marked for checking thread detachment after function completion. This is crucial to investigate potential memory leaks.

馃攳 To-Do:

  • Verify thread detachment after the function execution to prevent potential memory leaks.

馃摑 Note:
This Pull Request aims to enhance compatibility and resolve issues related to the use of react-native-fast-tflite on Android. Please review the changes and provide feedback.

馃檹 Thank you!

@mrousavy
Copy link
Owner

hey - thanks for tackling this issue man, this is amazing!

Looking at the implementation, was the only thing I missed just to downgrade the library? The newest version cannot be linked in C++? lol

@mrousavy mrousavy merged commit 13db933 into mrousavy:main Dec 21, 2023
0 of 3 checks passed
@mrousavy
Copy link
Owner

lgtm! thanks!

@mrousavy
Copy link
Owner

We can merge this for now, but we should really switch to using the tensorflow/ source we already have in the git repo. Right now, we build tensorflow from source for iOS from the tensorflow/ folder, and on Android we use an older version from Gradle.

To fix any potential incompatibilities or something and also potentially a lighter package size, we should really try to figure out how we can use the tensorflow/ sources to build the Android library as well.

@jaroslawkrol
Copy link
Contributor Author

@mrousavy Hey mate! 馃憢 Regarding linking tensorflow/ that was my first thought and I think it is not much more difficult. However, I didn't want to change much in the approach assumed by you.

Downgrade of the library is not everything. The most difficult problem was solving the callback function from Tflite.cpp, which was throwing an exception: Unable to retrieve jni environment. is the thread attached? It succeeded by attaching the thread, but still dettaching the thread crashes the app. (i marked it in code as TODO) I'm sure someone with more experience in C++/JNI can handle this better.

Another solution is to throw away the callback from the install method and use conditional usage (depending on the platform) at the TflitePlugin level.
Btw. have you considered using the curl library to retrieve the bytecode of the model file?

Cheers

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.

None yet

2 participants