ndarilek / tts-rs

115 stars 25 forks source link

Drop `ndk-glue` dependency from the main crate #50

Closed MarijnS95 closed 9 months ago

MarijnS95 commented 1 year ago

Commit d42d201 ("Update Android dependencies and example.") correctly replaces ndk-glue with ndk-context as a more generic crate to hold on to a global JavaVM and Android jobject Context, but didn't drop the unused ndk-glue crate from the list of Android dependencies. This crate is only used in the example crate, and shouldn't clobber downstream crates.

Besides, ndk-glue has been deprecated for some time and should be replaced by android-activity in the example in a followup PR.

MarijnS95 commented 1 year ago

Besides, ndk-glue has been deprecated for some time and should be replaced by android-activity in the example in a followup PR.

In hindsight, while looking into this project, it seems to rely on some complicated interplay between Android's NativeActivity class that _automatically loads the library specified via android.app.lib_name meta-data and calls into its entrypoint provided via ndk-glue or android-activity_, and again manually loading the library via System.loadLibrary in order to invoke JNI_OnLoad. That needs some more clarification and cleanup.

In fact:

  1. The Java VM is also given when starting a native activity, which could be passed into the backend just like winit;
  2. The Bridge class could be a very simple single .class file (afaik) that is pre-bundled with this crate;
  3. In turn users should be able to fully use this crate out of the box, in a native application (cargo-apk/xbuild) without any extra Java setup, gradle nor Android Studio;
  4. Likewise, if they wish to use it in a wrapper library in an existing Java/Kotlin app, it would also work mostly OOTB (as long as the VM is passed) without extra classes having to be copy-pasted around.
ndarilek commented 1 year ago

Awesome, if you can pull all that off, I'd be very appreciative. I'd love to see Android support improved but I'm so far behind on maintaining this project that it hasn't been a priority.

FWIW I've made attempts at this in the past and gave up partway along the way, but if you can improve it and verify that it works, that'd be great. I'd love to try mobile game development with TTS support on Android.

MarijnS95 commented 1 year ago

Sounds good; perhaps we can already get this PR in and I'll think about taking a look.


In my mind there are essentially two ways to develop Android apps with Rust:

  1. Purely native Rust app. Rely on Android's builtin NativeActivity to load a shared object with C symbols (provided by ndk-glue/android-activity), which contains a full activity lifecycle and the right APIs to render to the screen, handle input events, and a few releated actions (see the Android NDK: https://developer.android.com/ndk and https://github.com/rust-mobile/ndk). Android's NativeActivity marshals some of these lifecycle events back and forth over an FFI boundary. No Java/Kotlin code is compiled/needed;
    • There is an interim state with GameActivity, which behaves much like NativeActivity but has a Java/C++ support layer (that needs to be compiled and bundled) to provide more (and more consistent) functionality to a native implementation;
  2. Write your app in Java/Kotlin with Android Studio / gradle and all, and load a library to call some C functions on, e.g. to offload parts of your application while still handling most of the lifecycle and UI in Java (or not, it's up to the caller how to define the FFI/JNI layer between their managed and native implementation). NDK functionality is still used here to interface with Java/Android objects from C(++)/Rust.

This example app, from what I understand, appears to try both. I think we'd address the elephant in the room by moving to an explicit .with_android_app() constructor like winit, instead of having a JN_OnLoad behind the users' back.

That'd clean up the support for purely native apps with a minimal example, if we figure out how to cleanly embed the Bridge class.

Likewise, for non-android-activity apps there are different means to pass the JavaVM to tts-rs, and we can keep + simplify the Android-Studi/gradle-based app.


For now I'm not even sure how the example app is intended to be built, since a hello_world library is referenced twice (in android.app.lib_name meta-data _and in System.loadLibrary()), while app/gradle.build requests the tts crate to be built as cdylib?

MarijnS95 commented 10 months ago

@ndarilek is this something you might still want to merge? I won't have any time in the coming months to look into improving Android support for this project, but the PR/fix is valid as proposed and solves a real "issue" out in the wild.

We can always reference one of these replies in a new issue to keep track of ideas for future improvements.