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 some issues for compiling kaldi for Android #1502

Merged
merged 2 commits into from Mar 19, 2017

Conversation

jcsilva
Copy link
Contributor

@jcsilva jcsilva commented Mar 19, 2017

After running ./configure in src folder, kaldi.mk is created with some problems.

  1. The variable ANDROIDINCDIR is defined, but ANDROIDINC is used.
  2. It seems that execinfo.h, which is included at base/kaldi-error.cc is not supported.

This commit treats these issues.

P.S. If you want to repeat all the steps for cross-compiling to Android, please follow the instructions published at http://jcsilva.github.io/2017/03/18/compile-kaldi-android/

@danpovey
Copy link
Contributor

@dogancan, does this look OK to you? And do you think there is a way we can optimally make use of, or link to, his documentation?

@dogancan
Copy link
Contributor

I must have messed these up when updating the build scripts. I think it would be more consistent if we were to replace all occurrences of ANDROIDINCDIR in configure with ANDROIDINC, i.e. sed -i 's/ANDROIDINCDIR/ANDROIDINC/g' src/configure. @jcsilva Do you mind making that change and updating the PR?

Regarding the documentation, we have some platform specific notes at the end of the Kaldi README; maybe we can add a line there linking to the blog post?

@jcsilva
Copy link
Contributor Author

jcsilva commented Mar 19, 2017

  1. Pull request was updated.
  2. When it is approved, I'll update the documentation published in the blog post...Currently it's necessary to edit kaldi.mk manually.

@danpovey
Copy link
Contributor

danpovey commented Mar 19, 2017 via email

@dogancan
Copy link
Contributor

dogancan commented Mar 19, 2017 via email

@danpovey
Copy link
Contributor

danpovey commented Mar 19, 2017 via email

@danpovey danpovey merged commit 25ca8e4 into kaldi-asr:master Mar 19, 2017
@jcsilva jcsilva deleted the android branch March 19, 2017 19:29
@jcsilva
Copy link
Contributor Author

jcsilva commented Mar 19, 2017

Blog post was updated.

@dogancan
Copy link
Contributor

@jcsilva Thanks for the blog post and contributing to Kaldi. I'm adding a few lines to Kaldi README regarding Android cross compilation and planning to give a link to your blog post if you don't mind.

From what I understand you need to patch openfst so that you can compile it with arm-linux-androideabi-g++. I'm wondering if that patch is needed also when you compile openfst with clang++ provided by Android NDK. Did you by any chance try that?

One more thing. Do you mind commenting out the line in the blog post where you are checking out a specific version of Kaldi? In general we recommend people to build the latest Kaldi and people following your instructions blindly might end up with an old version.

@jcsilva
Copy link
Contributor Author

jcsilva commented Mar 20, 2017 via email

@dogancan
Copy link
Contributor

@jcsilva Thanks for looking into compiling OpenFst with Android clang++ and addressing my other comment regarding the specific git commit checkout in your blog post which by the way is great!

Regarding cross compiling OpenFst, one approach would be to change the Makefile in tools directory so that when HOST variable is defined in the environment, it adds --host=$HOST to the OpenFst configure call. After that change you should be able to cross-compile OpenFst for Android with the following call in the tools directory as long as Android toolchain is in the path.

CXX=clang++ HOST=arm-linux-androideabi make openfst

@danpovey What do you think?

@danpovey
Copy link
Contributor

danpovey commented Mar 20, 2017 via email

david-ryan-snyder pushed a commit to david-ryan-snyder/kaldi that referenced this pull request Apr 12, 2017
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
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

3 participants