-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Standalone Java bindings #3503
base: master
Are you sure you want to change the base?
Standalone Java bindings #3503
Conversation
native_client/java/Makefile
Outdated
@@ -1,31 +1,178 @@ | |||
.PHONY: clean apk-clean | |||
# CMAKE generated file: DO NOT EDIT! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You cant over write our makefile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct yeah, that one should not be overriden, because it's not used for the standalone java. Will fix.
native_client/java/Makefile
Outdated
|
||
bindings: clean ds-swig | ||
$(DS_SWIG_ENV) swig -c++ -java -package org.deepspeech.libdeepspeech -outdir libdeepspeech/src/main/java/org/deepspeech/libdeepspeech/ -o jni/deepspeech_wrap.cpp jni/deepspeech.i | ||
.PHONY: clean apk-clean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check your commit, this file seems to be removed / added, it's adding noise to the PR
native_client/java/README.md
Outdated
### Notes | ||
- At this moment, this will only run on x86-64 Linux. | ||
|
||
### Example usage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please put this into a file directly
@@ -1,6 +1,6 @@ | |||
/* ---------------------------------------------------------------------------- | |||
* This file was automatically generated by SWIG (http://www.swig.org). | |||
* Version 4.0.1 | |||
* Version 3.0.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this shows you have not used the swig version we provide, please do so
@@ -0,0 +1,26 @@ | |||
/* ---------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SWIG generated should not be committed
@@ -0,0 +1,26 @@ | |||
/* ---------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, no swig generated
@@ -0,0 +1,26 @@ | |||
/* ---------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, no swig generated
@@ -0,0 +1,26 @@ | |||
/* ---------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, no swig generated
@@ -1,16 +1,13 @@ | |||
/* ---------------------------------------------------------------------------- | |||
* This file was automatically generated by SWIG (http://www.swig.org). | |||
* Version 4.0.1 | |||
* Version 3.0.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong version
@@ -0,0 +1,157 @@ | |||
/* ---------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, no swig generated
@@ -0,0 +1,55 @@ | |||
/* ---------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, no swig generated
native_client/java/setup.sh
Outdated
@@ -0,0 +1,115 @@ | |||
#!/bin/sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should not require this kind of script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A makefile would be a better solution probably. But I don't know how to work with a Makefile, and have not figured out yet how to make CMake interact with Gradle without using Android
Let me finish my sport, i might have a better idea than gradle composition |
native_client/java/Makefile
Outdated
|
||
include ../definitions.mk | ||
|
||
ARCHS := $(shell grep 'ABI_FILTERS' libdeepspeech/gradle.properties | cut -d'=' -f2 | sed -e 's/;/ /g') | ||
GRADLE ?= ./gradlew | ||
|
||
all: apk | ||
all: apk jre | ||
android: apk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rather just change apk
to android
instead of adding another layer
all: apk | ||
all: apk jre | ||
android: apk | ||
standalone: jre |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
native_client/java/Makefile
Outdated
@@ -20,12 +22,50 @@ libdeepspeech/libs/%/libdeepspeech.so: | |||
-mkdir libdeepspeech/libs/$*/ | |||
cp ${TFDIR}/bazel-out/$*-*/bin/native_client/libdeepspeech.so libdeepspeech/libs/$*/ | |||
|
|||
apk: apk-clean bindings $(patsubst %,libdeepspeech/libs/%/libdeepspeech.so,$(ARCHS)) | |||
apk-prepare: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is weird and likely error prone, we should rather have libdeepspeech_android/build.gradle
and use that directly instead of doing this cp
dance
native_client/java/Makefile
Outdated
$(GRADLE) build | ||
|
||
jre: jre-prepare jre-collect jre-restore-makefile jre-clean | ||
jre-prepare: $(patsubst %,libdeepspeech/libs/%/libdeepspeech.so,$(ARCHS)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
$(GRADLE) build | ||
|
||
jre-collect: jre-gradle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this one?
native_client/java/Makefile
Outdated
cp libdeepspeech/libs/x86_64/libdeepspeech.so build/ | ||
cp libdeepspeech/build/libs/libdeepspeech.jar build/ | ||
|
||
jre-clean: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleaning step should be next to apk-clean
native_client/java/Makefile
Outdated
maven-bundle: apk | ||
$(GRADLE) uploadArchives | ||
$(GRADLE) zipMavenArtifacts | ||
|
||
bindings: clean ds-swig | ||
$(DS_SWIG_ENV) swig -c++ -java -package org.deepspeech.libdeepspeech -outdir libdeepspeech/src/main/java/org/deepspeech/libdeepspeech/ -o jni/deepspeech_wrap.cpp jni/deepspeech.i | ||
$(DS_SWIG_ENV) swig -c++ -java -package org.deepspeech.libdeepspeech -outdir libdeepspeech/src/main/java/org/deepspeech/libdeepspeech/ -o jni/deepspeech_wrap.cpp jni/deepspeech.i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't change line ending
|
||
``make android`` | ||
|
||
>Note: The current example app in `./App` is not up to date with the latest changes to the bindings! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this ?
@@ -1,5 +1,5 @@ | |||
distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.6-all.zip | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-6.5.1-bin.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we avoid updating gradle
wrapper? this adds noise and is not really needed
@@ -1,5 +1,21 @@ | |||
#!/usr/bin/env sh | |||
|
|||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, avoid updating gradlew
for the moment
@@ -1,3 +1,19 @@ | |||
@rem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, avoid updating for the moment
|
||
add_library( deepspeech-jni SHARED ../jni/deepspeech_wrap.cpp ) | ||
|
||
find_library(deepspeech-lib NAMES deepspeech PATHS ${CMAKE_SOURCE_DIR}/libs/x86_64/ REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hard coded arch is something we will want to fix later, I think
@@ -1 +1 @@ | |||
ABI_FILTERS = arm64-v8a;armeabi-v7a;x86_64 | |||
ABI_FILTERS = x86_64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't change this please (and avoid changing chmod
).
* Create a new DeepSpeechModel object and load the native libraries.<br> | ||
* Use on: Android | ||
*/ | ||
public DeepSpeechModel() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will change the API, what is wrong with static {}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static would always load the libraries that specific way. The way the libraries are loaded for Android don't play nice with standalone.
For end-developers nothing should change though, since the load libraries function is now called in the constructor, for if they dont specify the boolean nothing changes
* Use on: Android, Standalone | ||
* @param loadNativeLibrariesManually if true, the native libraries will not be loaded. The developer must do this manually then. | ||
*/ | ||
public DeepSpeechModel(boolean loadNativeLibrariesManually) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is something we want
native_client/java/standalone.md
Outdated
@@ -0,0 +1,193 @@ | |||
# DeepSpeech for standalone Java |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be a .md
sitting in the directory but rather a part of doc/
native_client/java/standalone.md
Outdated
|
||
### Adding DeepSpeech to your Java project | ||
>Note: You can do this on your own way too, but this works with the example usage code provided below. | ||
1. Copy `libdeepspeech.jar` from `./build/` into `{YOUR PROJECT ROOT}/libs/` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make sure we don't push people to re-building code themselves except when really required, .jar
should have everything bundled as well and people should pull from repos.
native_client/java/standalone.md
Outdated
//This class facilitates the loading of the native libraries. | ||
//The implementation in here works with the Gradle guide provided in the README.md document. | ||
//However you can modify this to whatever you wish. | ||
public class LibDeepSpeech extends DeepSpeechModel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the user should not have to do this, as I explained earlier, it needs to be taken care of in DeepSpeechModel
class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep correct. it's no longer up-to-date. Since the DeepSpeechModel class is no longer abstract that can go away. Though the library loading is better off in the hands of the user for non-Android, since it differs per project how it's done. Although if we figure out how to ship the .so files in the libdeepspeech jar, then we can 'standardize' it
…sible having 'vanilla' Java and the Android bits side by side
- Fixed up Android's Makefile, this got overriden by CMake for the standalone version - Seperate build.gradle and CMakeList.txt for Android and standalone - Changed DeepSpeechModel.java to give control of library loading back to the user - Updated README.md to reflect changes to DeepSpeechModel.java
…on for building for Android
- Split out README.md into standalone.md, for all the bits that are standalone-specific - Removed all automatically generated java files (generated by SWIG) - Updated .gitignore to reflect the change above - Updated DeepSpeechModel.java to be API-compatible with how it was, whilst also allowing the standalone to function (allowing you to load the libraries yourself, if wanted)
- Replaced setup.sh with target `standalone` in Makefile - Updated documentation accordingly - Updated CMakeLists_standalone to work with it now being used from Make (some path things for libdeepspeech.so) - Fixed some wrongly formatted javadoc in DeepSpeechModel.java
- Split out libdeepspeech into _android and _jre, for the respective versions - Moved settings.gradle to both versions, and modified them appropriately - Removed .so files, which were accidentally included in the previous commit, same goes for Makefile.original - Updated .gitignore's to work with the new structure - libdeepspeech_android is exactly as it is currently in mozilla:master - Updated Makefile to work with the new structure
|
||
repositories { | ||
google() | ||
jcenter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jcenter()
is dying :'(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what we can do about that. Will need to check out which dependencies are from jcenter, I dont think we have any since the Android stuff is pulled from Google
Hi TheDutchMC, |
Hey there, have you built DeepSpeech before trying to create the java bindings? See the documentation here |
Except for compiling DeepSpeech, which platform should be built? If you could upload libdeepspeech.jar and |
For now only x86-64 (linux I believe) works, though you could easily modify the Makefile for your platform. This is a WIP. Keep in mind the code for the standalone jdk is probably going to change a bit, I dont like the way library loading is done at the moment. I should have some time to look at it again tomorrow afternoon and Saturday. |
Thanks for your work! |
Solves #3502