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

File descriptor support for Android/Java/JNI #3507

Open
danielzgtg opened this issue Jan 20, 2021 · 7 comments
Open

File descriptor support for Android/Java/JNI #3507

danielzgtg opened this issue Jan 20, 2021 · 7 comments

Comments

@danielzgtg
Copy link

I would like the Android/Java/JNI bindings to support file descriptors. Currently, they only support paths.

I am using Android's SAF (the OS's graphical file picker) to open the model and scorer files. It avoids asking for invasive permissions but returns a file descriptor instead of a path. On Android 10, I could just convert the file descriptor into a path using the expression "/proc/self/fd/" + fd. After updating to Android 11, this fails with a permission denied error. A workaround might be to request for access to the entire internal storage, but this is not ideal for privacy reasons.

I heard that dup works while fopen doesn't. This leads to two possible solutions:

  • Add another constructor that accepts an int to use as a file descriptor, and pass that all the way into native code
  • Have the existing constructor detect whether the path starts with /proc/self/fd/, and if it does, use the existing file descriptor instead of opening a file

If you've found a bug, or have a feature request, then please create an issue with the following information:

  • Have I written custom code (as opposed to running examples on an unmodified clone of the repository): WIP Overhaul Android app DeepSpeech-examples#101
  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Android 11 Beta 1 on OnePlus 7 Pro
  • TensorFlow installed from (our builds, or upstream TensorFlow): N/A
  • TensorFlow version (use command below): N/A
  • Python version: N/A
  • Bazel version (if compiling from source): N/A
  • GCC/Compiler version (if compiling from source): N/A
  • CUDA/cuDNN version: N/A
  • GPU model and memory: N/A
  • Exact command to reproduce: N/A
  • Gradle dependency: implementation 'org.mozilla.deepspeech:libdeepspeech:0.9.2'
@lissyx lissyx closed this as completed Jan 21, 2021
@lissyx
Copy link
Collaborator

lissyx commented Jan 21, 2021

See #3331

@danielzgtg
Copy link
Author

See

That PR adds the option of using an array or string instead of the path. I have an int file descriptor, not an array or string. If I wanted to read the file descriptor myself, I would have used a temporary file. Reading it to an array is not ideal because RAM is at a premium on Anhroid devices. This is why I want to pass in the file descriptor and have it mmaped

@lissyx
Copy link
Collaborator

lissyx commented Jan 21, 2021

See

That PR adds the option of using an array or string instead of the path. I have an int file descriptor, not an array or string. If I wanted to read the file descriptor myself, I would have used a temporary file. Reading it to an array is not ideal because RAM is at a premium on Anhroid devices. This is why I want to pass in the file descriptor and have it mmaped

Yes, please have a look at the discussions about that: long story short, as much as I can remember, TensorFlow does not allows us to do this.

Current implementation already relies on mmap() on all platforms.

Putting the model into the app data directory should avoid requesting extra permissions.

@danielzgtg
Copy link
Author

already relies on mmap() on

This is what I would like to keep doing

Putting the model into the app data directory

There are two data directories on Android:

  • /data/data/org.deepspeechdemo: To get the file here, we would need to copy it from internal storage. This would lead to duplicating 1 GiB of data, and there isn't that much storage on a phone. There is a potential alternative of using internet instead, but that is even worse
  • /sdcard/Android/data/org.deepspeechdemo: This is what we were using before my PR. Android 11 has removed the ability for users and apps to access that directory. A USB cable has become required, and that is not convenient for the user

TensorFlow does not allows us to do this

I looked into the TensorFlow code. It seems that, sadly, only file paths not file descriptors are supported. I will have to find another solution then

@lissyx
Copy link
Collaborator

lissyx commented Jan 21, 2021

I looked into the TensorFlow code. It seems that, sadly, only file paths not file descriptors are supported. I will have to find another solution then

Maybe try to open the issue upstream ? Tensorflow folks might be interested. They are usually open.

@danielzgtg
Copy link
Author

I would like this issue to be reopened. This issue was closed because we deferred the action to upstream. Today upstream just implemented their part to support the functionality (tensorflow/tensorflow@555034b). I would appreciate it if this request was reconsidered, now that there are no more technical limitations

@lissyx
Copy link
Collaborator

lissyx commented Feb 10, 2021

I would appreciate it if this request was reconsidered, now that there are no more technical limitations

That's a bit of a long shot, it's still on current master, and we would need to update to the release branch containing this. If you want this feature, now that upstream allows, you might need to send PR though.

@lissyx lissyx reopened this Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants