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

Add BuildFromBuffer init variant for Java binding #3152

Open
vikramambrose opened this issue Jul 13, 2020 · 10 comments
Open

Add BuildFromBuffer init variant for Java binding #3152

vikramambrose opened this issue Jul 13, 2020 · 10 comments
Labels
waiting-on-reporter Waitiing on more informations from reporter

Comments

@vikramambrose
Copy link

Currently the libdeepspeech java binding only offers DeepSpeech(String modelPath) for initialization, This doesn't work in a production environment where the Android app may not be able to provide a "path". Current TFLite java bindings allow for init from java.io.File and java.nio.ByteBuffer. Please add one of these options.

@lissyx
Copy link
Collaborator

lissyx commented Jul 13, 2020

This doesn't work in a production environment where the Android app may not be able to provide a "path".

Please document this usecase, this is not one we got any report from.

Please add one of these options.

Adding those will impair the ability to perform mmap operation and it does have a very negative impact on the performances.

@reuben
Copy link
Contributor

reuben commented Jul 13, 2020

There's also a constructor that takes a java.nio.MappedByteBuffer, maybe that is enough?

@reuben
Copy link
Contributor

reuben commented Jul 13, 2020

(And doesn't impair performance)

@vikramambrose
Copy link
Author

@lissyx Apps delivered through the app store can only access assets through the AssetManager. These files are not kept individually on the filesystem. They can only be accessed through limited means. See https://developer.android.com/reference/kotlin/android/content/res/AssetManager The "filename" in the docs refers to a virtual, relative path into the "assets" blob. Its not something one can fopen in C.

@reuben I think MappedByteBuffer should be fine too. It looks like a MappedByteBuffer can be obtained through an AssetFileDescriptor. In fact I am already doing this for other TFL models.

    public MappedByteBuffer mapAssetFD(AssetFileDescriptor afd) throws IOException {
        FileInputStream inputStream = afd.createInputStream();
        long startOffset = afd.getStartOffset();
        long declaredLength = afd.getDeclaredLength();

        FileChannel fileChannel = inputStream.getChannel();
        return fileChannel.map(FileChannel.MapMode.READ_ONLY, startOffset, declaredLength);
    }

Note that whatever mechanism is added, it needs to work for everything that currently relies on a system file path, so Scorer and Alphabet would also need to be enhanced.

I would recommend looking into what NDK api is available to make this easier; https://developer.android.com/ndk/reference/group/asset

@lissyx
Copy link
Collaborator

lissyx commented Jul 14, 2020

Apps delivered through the app store can only access assets through the AssetManager. These files are not kept individually on the filesystem. They can only be accessed through limited means.

Thanks, i know that, but you can easily download and place those files in your app storage directory, it works very well.

Note that whatever mechanism is added, it needs to work for everything that currently relies on a system file path, so Scorer and Alphabet would also need to be enhanced.

Yes, you are starting to grasp the issue here, its not that trivial.

External scorer we ship is 900MB so we need to be careful there...

@lissyx
Copy link
Collaborator

lissyx commented Jul 14, 2020

If you are able to assemble a patch set that improves behavior, you are welcome, though.

@vikramambrose
Copy link
Author

vikramambrose commented Jul 14, 2020

External scorer we ship is 900MB so we need to be careful there...

Exactly! Making copies is not an option.

You too are starting to grasp the issue here, its not that trivial :)

@lissyx
Copy link
Collaborator

lissyx commented Jul 15, 2020

External scorer we ship is 900MB so we need to be careful there...

Exactly! Making copies is not an option.

You too are starting to grasp the issue here, its not that trivial :)

Don't package it as an asset in the application ? I know it's not the most convenient way to distribute, but for the moment, alternatives, as you said, are much worse.

@lissyx
Copy link
Collaborator

lissyx commented Jul 21, 2020

@reuben I think MappedByteBuffer should be fine too. It looks like a MappedByteBuffer can be obtained through an AssetFileDescriptor. In fact I am already doing this for other TFL models.

@vikramambrose So, do you have experience on that? We're kind of overloaded so it's complicated for us, as of now, to investigate that kind of change, but if you have feedback / patches to share, that's welcome.

@lissyx lissyx added the waiting-on-reporter Waitiing on more informations from reporter label Jul 21, 2020
@lissyx
Copy link
Collaborator

lissyx commented Jul 27, 2020

@vikramambrose Can you share feedback on that matter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-reporter Waitiing on more informations from reporter
Projects
None yet
Development

No branches or pull requests

3 participants