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

fix: read local files on Android for 'file:///...' #82

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

davidgaag
Copy link
Contributor

@davidgaag davidgaag commented Aug 14, 2024

When loading the model with a file:/// URI on Android, a network request was attempted, causing Android to throw the error Expected URL scheme 'http' or 'https' but was 'file'.

This pull request adds a check for the URI scheme, and if it's file, the app will now load from the local file system.

I also included a check for the .tflite extension when loading the file to ensure other files aren't read/misused.

Please note that I tried to run the Jest tests, but was stopped by an error in a react-native file/dependency. I tried to figure out how to exclude that file and run the tests anyways, but I had no luck.

(This is my first PR/open source contribution, so I hope I did this right 😅)

fixes #80 fixes #63

@davidgaag davidgaag marked this pull request as draft August 14, 2024 15:49
@davidgaag
Copy link
Contributor Author

Drafted because of an out of memory error I discovered with this approach. Will re-open if/when I discover a fix.

@davidgaag davidgaag marked this pull request as ready for review August 14, 2024 19:46
@phuvinhng
Copy link

How can you fix on Android? iOS working ok

@phuvinhng
Copy link

Hey @davidgaag, I tried your updated code, and surprisingly it worked, you can check some more reasonable cases if needed, thanks

@mrousavy
Copy link
Owner

thanks for your PR! We'll test this internally

@lucksp
Copy link

lucksp commented Sep 2, 2024

Should the loading-models section of README be updated to show the difference between File & URL? I think the current URL scheme is documented incorrectly. The reason is, if you don't use {url: string} the error says:

[Error: TFLite: Invalid source passed! Source should be either a React Native require(..) or a { url: string } object!]

The example should be:

// Asset from React Native Bundle
loadTensorflowModel(require('assets/my-model.tflite'))
// File on the local filesystem
loadTensorflowModel('file:///var/mobile/.../my-model.tflite')
// Remote URL
loadTensorflowModel({ url: 'https://tfhub.dev/google/lite-model/object_detection_v1.tflite' })

@Ajith-JSN
Copy link

Hi @mrousavy , could you please let us know when this PR might be merged?

@mrousavy
Copy link
Owner

@Ajith-JSN did you test this PR? Does it work for you?

@Ajith-JSN
Copy link

@Ajith-JSN did you test this PR? Does it work for you?

Yes, I have tested it, and it works fine

@mrousavy mrousavy merged commit 478f124 into mrousavy:main Sep 25, 2024
1 check passed
@mrousavy
Copy link
Owner

Then LGTM!

image

@mrousavy
Copy link
Owner

thanks for your PR! :)

@xinjietang
Copy link

xinjietang commented Oct 24, 2024

Hi everyone, has this commit been included in the 1.4.0 release?

@lucksp
Copy link

lucksp commented Oct 30, 2024

Has this been tested on iOS also? I am seeing the same issue there when trying to load a file:// URI:

Error: TFLite: Invalid source passed! Source should be either a React Native require(..) or a { url: string } object!

@xinjietang
Copy link

Has this been tested on iOS also? I am seeing the same issue there when trying to load a file:// URI:

Error: TFLite: Invalid source passed! Source should be either a React Native require(..) or a { url: string } object!

I have created a pull request at #102. Please try out if it fixed the issue.

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.

[Android] model import of file path causes crash Unable to load model from local directory
6 participants