Add callback registration for TTS, VAD, and Voice Agent components#335
Add callback registration for TTS, VAD, and Voice Agent components#335siddhant-0707 wants to merge 3 commits intoRunanywhereAI:mainfrom
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to bb9beb1 in 12 seconds. Click for details.
- Reviewed
502lines of code in5files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_voxkN7tStwdZBivg
You can customize 
📝 WalkthroughWalkthroughAdds per-component JNI callback state and adapters for TTS, VAD, and VoiceAgent; implements racTts/Vad/VoiceAgentSetCallbacks and streaming/event adapters in the JNI layer; wires Kotlin bridge extensions to call the new RunAnywhereBridge racSetCallbacks bindings. Changes
Sequence DiagramsequenceDiagram
participant Kotlin as Kotlin Extension<br/>(CppBridge)
participant Bridge as RunAnywhereBridge
participant JNI as JNI Layer<br/>(runanywhere_commons_jni.cpp)
participant JVM as Java VM
Note over Kotlin,JVM: Callback Registration Flow
Kotlin->>Bridge: racTtsSetCallbacks(audioCb, progressCb)
Bridge->>JNI: native racTtsSetCallbacks call
rect rgba(100,150,200,0.5)
JNI->>JVM: GetJavaVM() → cache g_tts_callbacks.jvm
JNI->>JVM: NewGlobalRef(audioCb)
JNI->>JVM: NewGlobalRef(progressCb)
JNI->>JVM: GetMethodID(streamAudioCallback)
JNI->>JVM: GetMethodID(progressCallback)
end
JNI-->>Bridge: registration complete
Bridge-->>Kotlin: return
Note over Kotlin,JVM: Callback Unregistration Flow
Kotlin->>Bridge: racTtsSetCallbacks(null, null)
Bridge->>JNI: native racTtsSetCallbacks call
rect rgba(200,100,150,0.5)
JNI->>JVM: DeleteGlobalRef(audioCb)
JNI->>JVM: DeleteGlobalRef(progressCb)
JNI->>JNI: clear g_tts_callbacks state
end
JNI-->>Bridge: cleanup complete
Bridge-->>Kotlin: return
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| env->GetJavaVM(&g_tts_callbacks.jvm); | ||
|
|
||
| if (audioCallback != nullptr) { | ||
| g_tts_callbacks.audio_callback = env->NewGlobalRef(audioCallback); | ||
| jclass callbackClass = env->GetObjectClass(audioCallback); | ||
| g_tts_callbacks.audio_method = | ||
| env->GetMethodID(callbackClass, "streamAudioCallback", "([BZ)Z"); | ||
| env->DeleteLocalRef(callbackClass); |
There was a problem hiding this comment.
Unhandled JNI pending exceptions
In rac*Tts/Vad/VoiceAgent*SetCallbacks, GetMethodID failures can leave a pending Java exception (e.g., signature mismatch). Right now the code only checks for a null jmethodID and keeps going, which can cause subsequent JNI calls to fail/abort with the original exception still pending. After each GetMethodID (and GetObjectClass), check/clear env->ExceptionCheck() (or return early after logging) so registration can’t proceed with a pending exception.
Also appears in: sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp:1712-1716, :1823-1827, and the other GetMethodID sites in those functions.
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp
Line: 1506:1513
Comment:
**Unhandled JNI pending exceptions**
In `rac*Tts/Vad/VoiceAgent*SetCallbacks`, `GetMethodID` failures can leave a pending Java exception (e.g., signature mismatch). Right now the code only checks for a null `jmethodID` and keeps going, which can cause subsequent JNI calls to fail/abort with the original exception still pending. After each `GetMethodID` (and `GetObjectClass`), check/clear `env->ExceptionCheck()` (or return early after logging) so registration can’t proceed with a pending exception.
Also appears in: `sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp:1712-1716`, `:1823-1827`, and the other `GetMethodID` sites in those functions.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (3)
Prompt To Fix With AIThis is a comment left during a code review.
Path: sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp
Line: 147:159
Comment:
**Leaked global JNI refs**
`JNI_OnUnload` only deletes `g_platform_adapter` but never releases the new global callback refs (`g_tts_callbacks.*`, `g_vad_callbacks.*`, `g_voice_agent_callbacks.*`). If the library is unloaded/reloaded (tests, dynamic feature modules, etc.), those refs keep Java objects alive and leak memory. Clean up all non-null callback global refs and null their method IDs/state in `JNI_OnUnload` as well.
How can I resolve this? If you propose a fix, please make it concise.
This is not just naming: it changes what native sees and breaks telemetry/registry assumptions. Prompt To Fix With AIThis is a comment left during a code review.
Path: sdk/runanywhere-kotlin/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/foundation/bridge/extensions/CppBridgeVAD.kt
Line: 606:610
Comment:
**Wrong arg passed to JNI**
`loadModel(modelPath, modelId, ...)` calls `RunAnywhereBridge.racVadComponentLoadModel(handle, modelPath, config.toJson())`, but the JNI signature is `racVadComponentLoadModel(handle: Long, modelId: String?, configJson: String?)` (`RunAnywhereBridge.kt:299`). This means you’re passing the *path* as the *modelId*, and the real `modelId` is ignored entirely. Fix the call site to pass `modelId` (and adjust JNI side if it actually needs a path).
This is not just naming: it changes what native sees and breaks telemetry/registry assumptions.
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: sdk/runanywhere-kotlin/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/foundation/bridge/extensions/CppBridgeVoiceAgent.kt
Line: 741:753
Comment:
**Missing native library guard**
`create()` calls `nativeCreate()` directly. Unlike TTS/VAD, there’s no check that the JNI library is actually loaded/available, so apps that don’t bundle the native libs will get an `UnsatisfiedLinkError` at runtime instead of a controlled `SDKError.notInitialized(...)`. Add the same native-library availability check/try-catch pattern used in `CppBridgeTTS.create`/`CppBridgeVAD.create` before calling `nativeCreate()`.
How can I resolve this? If you propose a fix, please make it concise. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp`:
- Around line 95-135: The global callback state structs (VadCallbackState,
TtsCallbackState, VoiceAgentCallbackState) and their instances (g_vad_callbacks,
g_tts_callbacks, g_voice_agent_callbacks) are accessed concurrently by native
callback threads while registration/unregistration mutates and deletes Java
global refs; add synchronization to prevent races/use-after-free by embedding a
std::mutex (or std::shared_mutex) in each struct, acquire the mutex in all
setter/unregister functions that modify jobjects or jmethodIDs and also acquire
it (or take an atomic snapshot under the lock) at the start of every callback
invocation path before using
frame_callback/speech_start_callback/audio_callback/state_callback/etc. so that
DeleteGlobalRef and method calls cannot race. Ensure you hold the lock while
copying jobject/method IDs out (or incrementing ref counts) and release it
before making JNI calls if desired, or keep it held if needed to guarantee
safety, and update all functions that touch these symbols (e.g., where
registration/unregistration and callback dispatch reference g_vad_callbacks.*,
g_tts_callbacks.*, g_voice_agent_callbacks.*) to use the new mutex.
- Around line 1495-1560: The JNI currently populates g_tts_callbacks in
Java_com_runanywhere_sdk_native_bridge_RunAnywhereBridge_racTtsSetCallbacks but
never wires them into the C API; create C-side adapter functions (e.g.,
tts_stream_adapter and tts_progress_adapter) that attach to the stored
g_tts_callbacks.jvm, convert parameters and call the stored Java methods via
CallBooleanMethod/CallVoidMethod using g_tts_callbacks.audio_method and
g_tts_callbacks.progress_method, then pass those adapters into the
rac_tts_synthesize_stream (or synthesize_stream) call instead of the blocking
synthesize(), and ensure global refs are deleted in JNI_OnUnload; apply the same
adapter + synthesize_stream + cleanup pattern to g_vad_callbacks and
g_voice_agent_callbacks (create VAD and VoiceAgent adapters, use the
streaming/event APIs) so callbacks are actually invoked and cleaned up on
unload.
- Introduced TTS stream adapter to handle audio data and invoke callbacks. - Implemented VAD activity adapter to manage speech start and end events. - Added mutexes to callback states for thread safety. - Included forward declarations for helper functions to improve code organization.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp`:
- Around line 181-187: NewByteArray can return nullptr on OOM; add a null check
after jbyteArray jAudio = env->NewByteArray(...) to avoid calling
SetByteArrayRegion on a null pointer: if jAudio == nullptr, call
clearPendingException(env, "NewByteArray"), delete the callback local ref
(env->DeleteLocalRef(callback)), and return early from tts_stream_adapter;
otherwise continue with
SetByteArrayRegion/CallBooleanMethod/DeleteLocalRef(jAudio). Apply the same
pattern in voice_agent_event_adapter for its NewByteArray usage.
- Around line 1710-1725: The code is invoking Java (env->CallBooleanMethod)
while holding g_tts_callbacks.mutex which can deadlock if the Java callback
re-enters racTtsSetCallbacks; fix by copying the needed references under the
lock and performing the JNI call and local-ref deletions after releasing the
lock: inside the scope guarded by g_tts_callbacks.mutex, read
g_tts_callbacks.audio_callback and g_tts_callbacks.audio_method into local
variables (creating a NewLocalRef for audio_callback if non-null) and then drop
the lock; after the lock scope, if the local callback and method are set, create
the empty jbyteArray, call env->CallBooleanMethod, call
clearPendingException(env, "racTtsComponentSynthesizeStream final callback"),
and finally delete the local refs (env->DeleteLocalRef for the jbyteArray and
callback).
🧹 Nitpick comments (4)
sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp (4)
1849-1860: Inconsistent indentation in theelseblocks.Throughout
racTtsSetCallbacks,racVadSetCallbacks, andracVoiceAgentSetCallbacks, the code inside theelseblocks is under-indented relative to the braces (e.g., lines 1850–1859 vs. the enclosingelse {at line 1849). The same pattern repeats in every callback registration block. This makes the logic flow harder to follow during review.
442-455:getJNIEnv()attaches native threads without ever detaching them.
AttachCurrentThreadis called at line 450, but there's no correspondingDetachCurrentThread. Every native callback thread that invokestts_stream_adapter,vad_activity_adapter, orvoice_agent_event_adapterthrough this helper gets permanently attached. If the C API spawns short-lived threads for callbacks, these will leak JVM resources when the native thread exits without detaching.Consider either:
- Tracking whether the thread was attached by this call and detaching after the callback completes (as done in
llm_stream_callback_tokenat line 1042–1067), or- Using a thread-local destructor (via
pthread_key_create) to auto-detach on thread exit.
1816-1883: ~400 lines of copy-paste callback registration boilerplate.Each
racTtsSetCallbacks,racVadSetCallbacks, andracVoiceAgentSetCallbacksrepeats the identical per-callback pattern: delete old global ref →NewGlobalRef→GetObjectClass→GetMethodID→ error cleanup. A small helper would collapse this dramatically:static bool registerJniCallback(JNIEnv* env, jobject javaCallback, const char* methodName, const char* methodSig, jobject& outGlobalRef, jmethodID& outMethod) { outGlobalRef = env->NewGlobalRef(javaCallback); jclass cls = env->GetObjectClass(javaCallback); if (clearPendingException(env, "registerJniCallback GetObjectClass") || !cls) { env->DeleteGlobalRef(outGlobalRef); outGlobalRef = nullptr; return false; } outMethod = env->GetMethodID(cls, methodName, methodSig); env->DeleteLocalRef(cls); if (clearPendingException(env, "registerJniCallback GetMethodID") || !outMethod) { env->DeleteGlobalRef(outGlobalRef); outGlobalRef = nullptr; outMethod = nullptr; return false; } return true; }Also applies to: 2032-2156, 2158-2370
103-147:jvmfield in each callback state struct is set but never read.
g_tts_callbacks.jvm,g_vad_callbacks.jvm, andg_voice_agent_callbacks.jvmare populated during registration (lines 1839, 2067, 2212) but all three adapter functions use the globalgetJNIEnv()(which readsg_jvm) instead. These fields are dead state. Consider removing them to avoid confusion, or switching the adapters to use them for consistency.
| jbyteArray jAudio = env->NewByteArray(static_cast<jsize>(audio_size)); | ||
| env->SetByteArrayRegion(jAudio, 0, static_cast<jsize>(audio_size), | ||
| reinterpret_cast<const jbyte*>(audio_data)); | ||
| env->CallBooleanMethod(callback, method, jAudio, JNI_FALSE); | ||
| env->DeleteLocalRef(jAudio); | ||
| env->DeleteLocalRef(callback); | ||
| clearPendingException(env, "tts_stream_adapter CallBooleanMethod"); |
There was a problem hiding this comment.
Missing null check on NewByteArray return — potential JVM crash on OOM.
env->NewByteArray() can return nullptr on allocation failure (e.g., large audio chunks). Calling SetByteArrayRegion on a null array will crash the JVM. The same pattern also appears in voice_agent_event_adapter (line 328).
Proposed fix
jbyteArray jAudio = env->NewByteArray(static_cast<jsize>(audio_size));
+ if (!jAudio) {
+ env->DeleteLocalRef(callback);
+ clearPendingException(env, "tts_stream_adapter NewByteArray OOM");
+ return;
+ }
env->SetByteArrayRegion(jAudio, 0, static_cast<jsize>(audio_size),
reinterpret_cast<const jbyte*>(audio_data));🤖 Prompt for AI Agents
In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp` around lines 181
- 187, NewByteArray can return nullptr on OOM; add a null check after jbyteArray
jAudio = env->NewByteArray(...) to avoid calling SetByteArrayRegion on a null
pointer: if jAudio == nullptr, call clearPendingException(env, "NewByteArray"),
delete the callback local ref (env->DeleteLocalRef(callback)), and return early
from tts_stream_adapter; otherwise continue with
SetByteArrayRegion/CallBooleanMethod/DeleteLocalRef(jAudio). Apply the same
pattern in voice_agent_event_adapter for its NewByteArray usage.
| static void voice_agent_event_adapter(const rac_voice_agent_event_t* event, void* user_data) { | ||
| (void)user_data; | ||
| if (!event) { | ||
| return; | ||
| } | ||
|
|
||
| JNIEnv* env = getJNIEnv(); | ||
| if (!env) { | ||
| return; | ||
| } | ||
|
|
||
| jobject partial_cb = nullptr; | ||
| jobject response_cb = nullptr; | ||
| jobject audio_cb = nullptr; | ||
| jmethodID partial_method = nullptr; | ||
| jmethodID response_method = nullptr; | ||
| jmethodID audio_method = nullptr; | ||
|
|
||
| { | ||
| std::lock_guard<std::mutex> lock(g_voice_agent_callbacks.mutex); | ||
| if (g_voice_agent_callbacks.partial_transcription_callback && | ||
| g_voice_agent_callbacks.partial_transcription_method) { | ||
| partial_cb = env->NewLocalRef(g_voice_agent_callbacks.partial_transcription_callback); | ||
| partial_method = g_voice_agent_callbacks.partial_transcription_method; | ||
| } | ||
| if (g_voice_agent_callbacks.response_token_callback && | ||
| g_voice_agent_callbacks.response_token_method) { | ||
| response_cb = env->NewLocalRef(g_voice_agent_callbacks.response_token_callback); | ||
| response_method = g_voice_agent_callbacks.response_token_method; | ||
| } | ||
| if (g_voice_agent_callbacks.audio_chunk_callback && | ||
| g_voice_agent_callbacks.audio_chunk_method) { | ||
| audio_cb = env->NewLocalRef(g_voice_agent_callbacks.audio_chunk_callback); | ||
| audio_method = g_voice_agent_callbacks.audio_chunk_method; | ||
| } | ||
| } | ||
|
|
||
| switch (event->type) { | ||
| case RAC_VOICE_AGENT_EVENT_TRANSCRIPTION: { | ||
| if (partial_cb && partial_method && event->data.transcription) { | ||
| jstring jText = env->NewStringUTF(event->data.transcription); | ||
| env->CallVoidMethod(partial_cb, partial_method, jText); | ||
| env->DeleteLocalRef(jText); | ||
| clearPendingException(env, "voice_agent_event_adapter partialTranscriptionCallback"); | ||
| } | ||
| break; | ||
| } | ||
| case RAC_VOICE_AGENT_EVENT_RESPONSE: { | ||
| if (response_cb && response_method && event->data.response) { | ||
| jstring jToken = env->NewStringUTF(event->data.response); | ||
| env->CallBooleanMethod(response_cb, response_method, jToken, JNI_TRUE); | ||
| env->DeleteLocalRef(jToken); | ||
| clearPendingException(env, "voice_agent_event_adapter responseTokenCallback"); | ||
| } | ||
| break; | ||
| } | ||
| case RAC_VOICE_AGENT_EVENT_AUDIO_SYNTHESIZED: { | ||
| if (audio_cb && audio_method && event->data.audio.audio_data && | ||
| event->data.audio.audio_size > 0) { | ||
| jbyteArray jAudio = env->NewByteArray(static_cast<jsize>(event->data.audio.audio_size)); | ||
| env->SetByteArrayRegion(jAudio, 0, static_cast<jsize>(event->data.audio.audio_size), | ||
| reinterpret_cast<const jbyte*>(event->data.audio.audio_data)); | ||
| env->CallBooleanMethod(audio_cb, audio_method, jAudio, JNI_TRUE); | ||
| env->DeleteLocalRef(jAudio); | ||
| clearPendingException(env, "voice_agent_event_adapter audioChunkCallback"); | ||
| } | ||
| break; | ||
| } | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| if (partial_cb) | ||
| env->DeleteLocalRef(partial_cb); | ||
| if (response_cb) | ||
| env->DeleteLocalRef(response_cb); | ||
| if (audio_cb) | ||
| env->DeleteLocalRef(audio_cb); | ||
| } |
There was a problem hiding this comment.
voice_agent_event_adapter only routes 3 of 7 registered callback types.
The adapter handles TRANSCRIPTION, RESPONSE, and AUDIO_SYNTHESIZED events, but stateCallback, turnPhaseCallback, userInterruptCallback, and progressCallback are registered and stored (global refs created, method IDs cached) yet never invoked. Their global refs and local-ref snapshots are acquired but the corresponding event types are silently dropped by the default: break;. Either add cases for the remaining event types or avoid registering/caching those callbacks to prevent leaking attention and global refs for dead code.
Also, JNI_TRUE is hardcoded as the isFinal/isLast flag on lines 319 and 331 for every event. If the C API delivers incremental tokens/chunks, the Kotlin side will treat each one as the final piece.
| { | ||
| jobject callback = nullptr; | ||
| jmethodID method = nullptr; | ||
| std::lock_guard<std::mutex> lock(g_tts_callbacks.mutex); | ||
| if (g_tts_callbacks.audio_callback && g_tts_callbacks.audio_method) { | ||
| callback = env->NewLocalRef(g_tts_callbacks.audio_callback); | ||
| method = g_tts_callbacks.audio_method; | ||
| } | ||
| if (callback && method) { | ||
| jbyteArray empty = env->NewByteArray(0); | ||
| env->CallBooleanMethod(callback, method, empty, JNI_TRUE); | ||
| env->DeleteLocalRef(empty); | ||
| env->DeleteLocalRef(callback); | ||
| clearPendingException(env, "racTtsComponentSynthesizeStream final callback"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Deadlock risk: JNI call made while holding g_tts_callbacks.mutex.
The lock_guard at line 1713 holds the mutex until line 1725. CallBooleanMethod at line 1720 invokes Java while the lock is held. If the Java callback synchronously calls racTtsSetCallbacks (which also acquires this mutex), the thread will deadlock on the non-recursive std::mutex.
The streaming adapters (tts_stream_adapter, vad_activity_adapter) correctly copy refs under the lock and then call Java outside the lock. Apply the same pattern here.
Proposed fix
{
jobject callback = nullptr;
jmethodID method = nullptr;
- std::lock_guard<std::mutex> lock(g_tts_callbacks.mutex);
- if (g_tts_callbacks.audio_callback && g_tts_callbacks.audio_method) {
- callback = env->NewLocalRef(g_tts_callbacks.audio_callback);
- method = g_tts_callbacks.audio_method;
- }
- if (callback && method) {
- jbyteArray empty = env->NewByteArray(0);
- env->CallBooleanMethod(callback, method, empty, JNI_TRUE);
- env->DeleteLocalRef(empty);
- env->DeleteLocalRef(callback);
- clearPendingException(env, "racTtsComponentSynthesizeStream final callback");
+ {
+ std::lock_guard<std::mutex> lock(g_tts_callbacks.mutex);
+ if (g_tts_callbacks.audio_callback && g_tts_callbacks.audio_method) {
+ callback = env->NewLocalRef(g_tts_callbacks.audio_callback);
+ method = g_tts_callbacks.audio_method;
+ }
+ }
+ if (callback && method) {
+ jbyteArray empty = env->NewByteArray(0);
+ env->CallBooleanMethod(callback, method, empty, JNI_TRUE);
+ env->DeleteLocalRef(empty);
+ env->DeleteLocalRef(callback);
+ clearPendingException(env, "racTtsComponentSynthesizeStream final callback");
}
}🤖 Prompt for AI Agents
In `@sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp` around lines
1710 - 1725, The code is invoking Java (env->CallBooleanMethod) while holding
g_tts_callbacks.mutex which can deadlock if the Java callback re-enters
racTtsSetCallbacks; fix by copying the needed references under the lock and
performing the JNI call and local-ref deletions after releasing the lock: inside
the scope guarded by g_tts_callbacks.mutex, read g_tts_callbacks.audio_callback
and g_tts_callbacks.audio_method into local variables (creating a NewLocalRef
for audio_callback if non-null) and then drop the lock; after the lock scope, if
the local callback and method are set, create the empty jbyteArray, call
env->CallBooleanMethod, call clearPendingException(env,
"racTtsComponentSynthesizeStream final callback"), and finally delete the local
refs (env->DeleteLocalRef for the jbyteArray and callback).
|
@coderabbitai please do a detailed review. Thanks! |
|
@greptile please do a detailed review. Thanks! |
🧩 Analysis chain🏁 Script executed: Length of output: 170887 🏁 Script executed: Length of output: 47873 🏁 Script executed: Length of output: 45214 🏁 Script executed: Length of output: 57348 🏁 Script executed: Length of output: 29535
What’s working well
Must-fix before merge
C++ patch sketch: // helper
static int parse_audio_format(const char* json) {
if (!json) return 0; // default PCM_16
const char* key = "\"audio_format\":";
const char* p = strstr(json, key);
if (!p) return 0;
p += strlen(key);
while (*p == ' ') ++p;
return atoi(p); // 0 or 1
}
JNIEXPORT jstring JNICALL
Java_com_runanywhere_sdk_native_bridge_RunAnywhereBridge_racVadComponentProcess(
JNIEnv* env, jclass, jlong handle, jbyteArray audioData, jstring configJson) {
if (handle == 0 || audioData == nullptr) return nullptr;
jsize len = env->GetArrayLength(audioData);
jbyte* data = env->GetByteArrayElements(audioData, nullptr);
// Read audio_format from configJson
const char* json = nullptr;
if (configJson) json = env->GetStringUTFChars(configJson, nullptr);
int audio_format = parse_audio_format(json); // 0=PCM_16, 1=PCM_FLOAT
rac_bool_t out_is_speech = RAC_FALSE;
rac_result_t status = RAC_ERROR_INVALID_ARGUMENT;
if (audio_format == 1) {
// Float32 path: len must be multiple of 4
size_t count = static_cast<size_t>(len) / sizeof(float);
status = rac_vad_component_process(
reinterpret_cast<rac_handle_t>(handle),
reinterpret_cast<const float*>(data),
count, &out_is_speech);
} else {
// Int16 -> Float32 conversion
size_t count = static_cast<size_t>(len) / sizeof(int16_t);
std::vector<float> f(count);
const int16_t* s = reinterpret_cast<const int16_t*>(data);
for (size_t i = 0; i < count; ++i) f[i] = static_cast<float>(s[i]) / 32768.0f;
status = rac_vad_component_process(
reinterpret_cast<rac_handle_t>(handle),
f.data(), f.size(), &out_is_speech);
}
if (configJson && json) env->ReleaseStringUTFChars(configJson, json);
env->ReleaseByteArrayElements(audioData, data, JNI_ABORT);
if (status != RAC_SUCCESS) return nullptr;
char jsonBuf[256];
snprintf(jsonBuf, sizeof(jsonBuf), "{\"is_speech\":%s,\"probability\":%.4f}",
out_is_speech ? "true" : "false", out_is_speech ? 1.0f : 0.0f);
return env->NewStringUTF(jsonBuf);
}
C++ patch sketch: struct JniEnvScope {
JNIEnv* env = nullptr;
bool attached = false;
JniEnvScope() {
if (!g_jvm) return;
jint st = g_jvm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
if (st == JNI_EDETACHED) {
if (g_jvm->AttachCurrentThread(&env, nullptr) == JNI_OK) attached = true;
else env = nullptr;
}
}
~JniEnvScope() { if (attached && g_jvm) g_jvm->DetachCurrentThread(); }
JNIEnv* get() const { return env; }
};
// usage in adapters:
static void tts_stream_adapter(const void* audio_data, size_t audio_size, void* user_data) {
JniEnvScope es; JNIEnv* env = es.get(); if (!env) return;
...
}
fun create(): Int {
synchronized(lock) {
if (handle != 0L) return 0
// Match TTS/VAD pattern
if (!CppBridge.isNativeLibraryLoaded) {
CppBridgePlatformAdapter.logCallback(
CppBridgePlatformAdapter.LogLevel.ERROR, TAG,
"Native library not loaded. VoiceAgent requires native libs.")
throw SDKError.notInitialized("Native library not available. Bundle the native libs.")
}
val result = try { nativeCreate() } catch (e: UnsatisfiedLinkError) {
CppBridgePlatformAdapter.logCallback(
CppBridgePlatformAdapter.LogLevel.ERROR, TAG,
"Voice Agent creation failed: ${e.message}")
throw SDKError.notInitialized("Voice Agent native lib not available.")
}
if (result == 0L) return -1
handle = result
setState(VoiceAgentState.CREATED)
return 0
}
}
C++ patch sketch: `#define` GET_MID_OR_FAIL(var, cls, name, sig) \
var = env->GetMethodID(cls, name, sig); \
if (!var || env->ExceptionCheck()) { \
env->ExceptionDescribe(); env->ExceptionClear(); \
env->DeleteLocalRef(cls); \
env->DeleteGlobalRef(g_platform_adapter); g_platform_adapter = nullptr; \
return RAC_ERROR_INVALID_ARGUMENT; \
}
jclass adapterClass = env->GetObjectClass(adapter);
GET_MID_OR_FAIL(g_method_log, adapterClass, "log", "(ILjava/lang/String;Ljava/lang/String;)V")
GET_MID_OR_FAIL(g_method_file_exists, adapterClass, "fileExists", "(Ljava/lang/String;)Z")
// ... repeat for all cached methods
env->DeleteLocalRef(adapterClass);
JNIEXPORT jobject JNICALL
Java_com_runanywhere_sdk_native_bridge_RunAnywhereBridge_racGetPlatformAdapter(JNIEnv* env, jclass) {
std::lock_guard<std::mutex> lock(g_adapter_mutex);
return g_platform_adapter ? env->NewLocalRef(g_platform_adapter) : nullptr;
}
C++ patch sketch (assuming a flag exists): case RAC_VOICE_AGENT_EVENT_RESPONSE: {
if (response_cb && response_method && event->data.response) {
jstring jToken = env->NewStringUTF(event->data.response);
jboolean jIsFinal = event->is_final ? JNI_TRUE : JNI_FALSE; // or event->data.response_is_final
env->CallBooleanMethod(response_cb, response_method, jToken, jIsFinal);
env->DeleteLocalRef(jToken);
clearPendingException(env, "... responseTokenCallback");
}
break;
}
switch (activity) {
case RAC_SPEECH_STARTED: /* existing */ break;
case RAC_SPEECH_ENDED: /* existing */ break;
case RAC_SPEECH_ONGOING:
event_type = 2; is_speech = true; probability = 1.0f; break;
case RAC_SPEECH_NONE:
default:
event_type = 0; is_speech = false; probability = 0.0f; break;
}
Nice-to-have improvements
Tests and verification
Overall take Would you like me to prepare a small follow-up plan to land the must-fix patches above as a targeted commit? I can break it into:
🧠 Learnings used✅ Actions performedReview triggered.
|
| Java_com_runanywhere_sdk_native_bridge_RunAnywhereBridge_racVadComponentProcessStream( | ||
| JNIEnv* env, jclass clazz, jlong handle, jbyteArray audioData, jstring configJson) { | ||
| { | ||
| std::lock_guard<std::mutex> lock(g_vad_callbacks.mutex); | ||
| if (g_vad_callbacks.frame_callback || g_vad_callbacks.speech_start_callback || | ||
| g_vad_callbacks.speech_end_callback) { | ||
| rac_vad_component_set_activity_callback(reinterpret_cast<rac_handle_t>(handle), | ||
| vad_activity_adapter, nullptr); |
There was a problem hiding this comment.
Missing handle validation
racVadComponentProcessStream unconditionally calls rac_vad_component_set_activity_callback(reinterpret_cast<rac_handle_t>(handle), ...) before validating handle. If handle == 0 (or otherwise invalid), this introduces a new crash/UB path that wasn’t present before (the underlying racVadComponentProcess checks handle, but this call happens earlier). Add the same handle guard here before setting the callback.
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-commons/src/jni/runanywhere_commons_jni.cpp
Line: 1959:1966
Comment:
**Missing handle validation**
`racVadComponentProcessStream` unconditionally calls `rac_vad_component_set_activity_callback(reinterpret_cast<rac_handle_t>(handle), ...)` before validating `handle`. If `handle == 0` (or otherwise invalid), this introduces a new crash/UB path that wasn’t present before (the underlying `racVadComponentProcess` checks `handle`, but this call happens earlier). Add the same handle guard here before setting the callback.
How can I resolve this? If you propose a fix, please make it concise.| // VOICE AGENT CALLBACKS (rac_voice_agent.h) | ||
| // ======================================================================== | ||
|
|
||
| @JvmStatic | ||
| external fun racVoiceAgentSetCallbacks( | ||
| stateCallback: Any?, | ||
| turnPhaseCallback: Any?, | ||
| partialTranscriptionCallback: Any?, | ||
| responseTokenCallback: Any?, | ||
| audioChunkCallback: Any?, | ||
| userInterruptCallback: Any?, | ||
| progressCallback: Any?, | ||
| ) |
There was a problem hiding this comment.
JNI symbol not declared
The native side adds Java_com_runanywhere_sdk_native_bridge_RunAnywhereBridge_racVoiceAgentProcessStream(...) in runanywhere_commons_jni.cpp, but there is no corresponding external fun racVoiceAgentProcessStream(...) declaration in RunAnywhereBridge.kt. This makes the new JNI entrypoint unreachable from Kotlin and can also indicate API mismatch between layers. Add the missing external declaration (signature should match the JNI function params/return).
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt
Line: 340:352
Comment:
**JNI symbol not declared**
The native side adds `Java_com_runanywhere_sdk_native_bridge_RunAnywhereBridge_racVoiceAgentProcessStream(...)` in `runanywhere_commons_jni.cpp`, but there is no corresponding `external fun racVoiceAgentProcessStream(...)` declaration in `RunAnywhereBridge.kt`. This makes the new JNI entrypoint unreachable from Kotlin and can also indicate API mismatch between layers. Add the missing external declaration (signature should match the JNI function params/return).
How can I resolve this? If you propose a fix, please make it concise.
|
Hey @siddhant-0707 |
There is no reply should this PR be closed ? |
3 participants
Description
Implement JNI callback registration for VAD/TTS/VoiceAgent and wire Kotlin bridge registration/unregistration. Add VoiceAgent JNI declaration in
RunAnywhereBridge.Type of Change
Testing
Platform-Specific Testing (check all that apply)
Kotlin SDK / Android Sample:
Labels
Please add the appropriate label(s):
SDKs:
Swift SDK- Changes to Swift SDK (sdk/runanywhere-swift)Kotlin SDK- Changes to Kotlin SDK (sdk/runanywhere-kotlin)Flutter SDK- Changes to Flutter SDK (sdk/runanywhere-flutter)React Native SDK- Changes to React Native SDK (sdk/runanywhere-react-native)Commons- Changes to shared native code (sdk/runanywhere-commons)Checklist
Important
Implement callback registration for TTS, VAD, and Voice Agent components in JNI and Kotlin bridge.
runanywhere_commons_jni.cpp.racTtsSetCallbacks,racVadSetCallbacks, andracVoiceAgentSetCallbacksfunctions for managing callbacks.CppBridgeTTS.kt,CppBridgeVAD.kt, andCppBridgeVoiceAgent.ktto call respective JNI functions for registration and unregistration.nullduring unregistration to prevent memory leaks.racVoiceAgentSetCallbacksdeclaration inRunAnywhereBridge.kt.This description was created by
for bb9beb1. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Greptile Overview
Greptile Summary
runanywhere_commons_jni.cpp.RunAnywhereBridge.*SetCallbacksand clear them to avoid leaks.RunAnywhereBridge.kt.Confidence Score: 3/5
Important Files Changed
racTtsSetCallbacksby passingthiscallbacks and clearing them with nulls; no additional must-fix issues found in this diff.racVadSetCallbacks; no additional must-fix issues found in this diff.RunAnywhereBridgeand wires VoiceAgent callback registration/unregistration toracVoiceAgentSetCallbacks; no additional must-fix issues found in this diff.racVoiceAgentSetCallbacksexternal JNI declaration; found the new nativeracVoiceAgentProcessStreamJNI entrypoint is not declared here, leaving that functionality inaccessible from Kotlin and risking API mismatch.Sequence Diagram
sequenceDiagram participant App as Kotlin bridge (CppBridge*) participant JBridge as RunAnywhereBridge (JNI externals) participant JNI as runanywhere_commons_jni.cpp participant Core as RAC C/C++ core App->>JBridge: racTtsSetCallbacks(audioCb, progressCb) JBridge->>JNI: Java_*_racTtsSetCallbacks(...) JNI->>JNI: Store GlobalRefs + methodIDs App->>JBridge: racVadSetCallbacks(frameCb, startCb, endCb, progressCb) JBridge->>JNI: Java_*_racVadSetCallbacks(...) JNI->>JNI: Store GlobalRefs + methodIDs App->>JBridge: racVoiceAgentSetCallbacks(stateCb, phaseCb, ...) JBridge->>JNI: Java_*_racVoiceAgentSetCallbacks(...) JNI->>JNI: Store GlobalRefs + methodIDs App->>JBridge: racTtsComponentSynthesizeStream(handle, text, options) JBridge->>JNI: Java_*_racTtsComponentSynthesizeStream(...) JNI->>Core: rac_tts_component_synthesize_stream(..., tts_stream_adapter) Core-->>JNI: tts_stream_adapter(audioChunk) JNI-->>App: streamAudioCallback(byte[], isFinal=false) JNI-->>App: streamAudioCallback(byte[0], isFinal=true) App->>JBridge: racVadComponentProcessStream(handle, samples, options) JBridge->>JNI: Java_*_racVadComponentProcessStream(...) JNI->>Core: rac_vad_component_set_activity_callback(..., vad_activity_adapter) JNI->>Core: rac_vad_component_process(...) Core-->>JNI: vad_activity_adapter(activity) JNI-->>App: speechStartCallback/speechEndCallback + streamFrameCallback Note over App,JBridge: VoiceAgent processing JNI exists (racVoiceAgentProcessStream) Note over App,JBridge: but Kotlin external decl is missing in this PR