From 631b9080e81c47b24cebedce340b333c6545f078 Mon Sep 17 00:00:00 2001 From: Jeff Zhou Date: Tue, 28 Jul 2026 15:37:25 +0800 Subject: [PATCH 1/2] mtmd-inference: fix garbage output Assisted-by: Trae + MiniMax-M3 --- .../src/main/java/kantvai/ai/ggmljava.java | 18 + .../ui/fragment/AIResearchFragment.java | 108 ++++-- core/jni/ggml-jni-context.cpp | 330 +++++++++++++++++- core/jni/ggml-jni-context.h | 94 ++++- core/jni/ggml-jni.c | 21 ++ core/jni/ggml-jni.h | 17 + core/jni/llm-inference.cpp | 73 +++- core/jni/mtmd-inference.cpp | 192 +++++++--- core/jni/realtime-video-recognition.cpp | 80 ++++- 9 files changed, 823 insertions(+), 110 deletions(-) diff --git a/android/kantvplayer-lib/src/main/java/kantvai/ai/ggmljava.java b/android/kantvplayer-lib/src/main/java/kantvai/ai/ggmljava.java index 04b03e290..b45036e3e 100644 --- a/android/kantvplayer-lib/src/main/java/kantvai/ai/ggmljava.java +++ b/android/kantvplayer-lib/src/main/java/kantvai/ai/ggmljava.java @@ -112,6 +112,24 @@ public final class ggmljava { public static native void llm_finalize(); + /** + * Release the llama backend (DSP + quant tables) installed at JNI init. + * Pair of {@link #backendInit()}. Safe to call multiple times. The + * intended call site is AIResearchFragment.onDestroy() so the Hexagon + * DSP gets released on activity teardown instead of holding resources + * until the OS kills the process. + */ + public static native void backendCleanup(); + + /** + * Release the currently loaded model (and mtmd/vision context if any). + * Subsequent inference calls will trigger a fresh 4GB read from disk + * - use this to free RAM when the user switches models in + * LLMSettingFragment, or to drop the model on activity teardown + * alongside {@link #backendCleanup()}. Idempotent. + */ + public static native void unloadModel(); + // ============================================================================================ // LLM/MTMD benchmark singleton state queries (UI helpers). // diff --git a/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java b/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java index 2d390c09a..cb298d947 100644 --- a/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java +++ b/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java @@ -394,32 +394,43 @@ public void run() { ggmljava.ggml_set_benchmark_status(0); if (isLLMModel) { - if (isMTMDModel) { - //LLM multimodal inference - KANTVLog.g(TAG, "multimodal model, media path:" + pathSelectedMedia); - if (KANTVAIUtils.isImageFile(pathSelectedMedia)) { - strBenchmarkInfo = ggmljava.mtmd_inference( - KANTVUtils.getSDCardDataPath() + AIModelMgr.getModelName(selectModelIndex), - KANTVUtils.getSDCardDataPath() + AIModelMgr.getMMProjmodelName(selectModelIndex), - pathSelectedMedia, - strUserInput, - 1, - backendIndex); + if (isMTMDModel) { + //LLM multimodal inference + KANTVLog.g(TAG, "multimodal model, media path:" + pathSelectedMedia); + if (KANTVAIUtils.isImageFile(pathSelectedMedia)) { + strBenchmarkInfo = ggmljava.mtmd_inference( + KANTVUtils.getSDCardDataPath() + AIModelMgr.getModelName(selectModelIndex), + KANTVUtils.getSDCardDataPath() + AIModelMgr.getMMProjmodelName(selectModelIndex), + pathSelectedMedia, + strUserInput, + 1, + backendIndex); } else if (KANTVAIUtils.isAudioFile(pathSelectedMedia)) { strBenchmarkInfo = ggmljava.mtmd_inference( - KANTVUtils.getSDCardDataPath() + AIModelMgr.getModelName(selectModelIndex), - KANTVUtils.getSDCardDataPath() + AIModelMgr.getMMProjmodelName(selectModelIndex), - pathSelectedMedia, - strUserInput, - 2, - backendIndex); - } else { - endTime = System.currentTimeMillis(); - duration = (endTime - beginTime); - isBenchmarking.set(false); - KANTVUtils.showMsgBox(mActivity, "only support MTMD audio and image currently"); - return; - } + KANTVUtils.getSDCardDataPath() + AIModelMgr.getModelName(selectModelIndex), + KANTVUtils.getSDCardDataPath() + AIModelMgr.getMMProjmodelName(selectModelIndex), + pathSelectedMedia, + strUserInput, + 2, + backendIndex); + } else { + endTime = System.currentTimeMillis(); + duration = (endTime - beginTime); + isBenchmarking.set(false); + KANTVUtils.showMsgBox(mActivity, "only support MTMD audio and image currently"); + return; + } + // NOTE: pathSelectedMedia / + // bitmapSelectedImage are NOT reset + // here - the unified cleanup at the + // endTime block below resets them after + // the inference has read the image. + // Resetting them here too would + // double-clear, and clearing them too + // early broke the MTMD branch decision + // (line 1668) and the image-validity + // check (line 1699) which read these + // fields in the same `if` block. } else { //general LLM inference strBenchmarkInfo = ggmljava.llm_inference( @@ -455,6 +466,21 @@ public void run() { duration = (endTime - beginTime); isBenchmarking.set(false); + // Clear the attachment state after inference + // returns, regardless of which branch ran. We + // intentionally leave these populated for the + // duration of inference (clearAttachment() in + // handleSend no longer touches them) so the + // MTMD branch decision at line 1668 and the + // image-validity check at line 1699 see the + // right state. For non-MTMD paths (regular + // LLM, ASR, MNIST, TTS) the fields were never + // read, but we still clear them here so the + // next user turn doesn't carry over a stale + // path / bitmap. + pathSelectedMedia = ""; + bitmapSelectedImage = null; + mActivity.runOnUiThread(new Runnable() { @Override public void run() { @@ -534,6 +560,12 @@ public void initListener() { @Override public void onDestroy() { super.onDestroy(); + // Release the cached model first (4GB) then the llama backend + // (DSP + quant tables). Order matters: unload_model() must run + // before llama_backend_free() because freeing the DSP backend + // would leave the model with dangling backend references. + kantvai.ai.ggmljava.unloadModel(); + kantvai.ai.ggmljava.backendCleanup(); } @Override @@ -1423,12 +1455,32 @@ public void onNothingSelected(AdapterView parent) {} } /** - * Clear the pending attachment (image / audio) and hide the - * preview row above the input. + * Clear the pending attachment's UI state (preview row, thumbnail, + * cached bitmap) and reset the data-model reference. + * + *

Note: we intentionally do not null out + * {@code pathSelectedMedia} OR {@code bitmapSelectedImage} + * here, because this method is called from + * {@code handleSend()} BEFORE {@code runInference()}, and + * runInference() needs BOTH fields to: + *

+ * Both fields are reset to null/"" later, in runInference() + * right before the endTime block (so all inference branches + * - MTMD / plain LLM / ASR / MNIST / TTS - end up clean for + * the next user turn). */ private void clearAttachment() { - pathSelectedMedia = ""; - bitmapSelectedImage = null; if (attachmentPreview != null) { attachmentPreview.setVisibility(View.GONE); } diff --git a/core/jni/ggml-jni-context.cpp b/core/jni/ggml-jni-context.cpp index 0f3265e59..8d86c1f1f 100644 --- a/core/jni/ggml-jni-context.cpp +++ b/core/jni/ggml-jni-context.cpp @@ -7,6 +7,21 @@ #include "ggml-cpu.h" #include "llamacpp/ggml/include/ggml-hexagon.h" +// common.h provides common_init() which used to be called at the top of +// mtmd_inference_main / llm_inference_main; now it is called once from +// ggml_jni_context::init() and must be in scope here. +// We use the explicit relative path "llamacpp/common/common.h" instead +// of bare "common.h" because the build's include path also contains +// core/llamacpp/ggml/src/ggml-cpu/ which has its own unrelated +// "common.h" (ggml-cpu internal). Without the explicit path, the +// compiler picks the ggml-cpu one and common_init() ends up undeclared. +#include "llamacpp/common/common.h" + +// mtmd.h provides mtmd_log_set() which is called from ggml_jni_context::init() +// to route libmtmd internal errors to logcat. Transitive includes from the +// ggml-jni.h chain do not expose this declaration, so we include it directly. +#include "mtmd.h" + extern "C" { #include "libavutil/avstring.h" #include "libavutil/eval.h" @@ -32,6 +47,22 @@ extern "C" { #include "libavutil/cde_assert.h" } +// Forward declare the GGML abort callback signature +// (typedef void (*ggml_abort_callback_t)(const char * error_message)) +// ggml.h is already included via ggml-jni.h transitively. +static void kantv_ggml_abort_callback(const char * error_message) { + // Route GGML_ASSERT / GGML_ABORT failure messages to logcat so they are + // visible in adb logcat. Otherwise ggml_abort() prints to stderr which + // Android typically discards. + if (error_message != nullptr) { + // __android_log_print is already declared as extern in cde_log.c and + // is the canonical logcat entry point in this codebase. The priority + // value 6 corresponds to ANDROID_LOG_FATAL in the NDK enum. + __android_log_print(6 /* ANDROID_LOG_FATAL */, "KANTV_GGML", + "%s", error_message); + } +} + void ggml_jni_context::init() { if (initialized) { LOGGD("already initialize"); @@ -39,9 +70,69 @@ void ggml_jni_context::init() { } llm_temperature = 0.8; llm_top_p = 0.9; + // Install a custom GGML abort callback so that GGML_ASSERT / GGML_ABORT + // failures inside libllama/libmtmd (e.g. common_params_parse pre-condition + // checks) reach logcat *before* the process is killed by abort(). Without + // this, the abort message is printed to stderr which Android typically + // drops, leaving us with only the SIGABRT tombstone and no actionable + // error message. + ggml_set_abort_callback(kantv_ggml_abort_callback); +#if defined(__ANDROID__) + // Same rationale for libmtmd: route its internal errors to logcat under + // the KANTV_MTMD tag. Without this, mtmd errors go to stderr (which + // Android drops) and we only see a SIGABRT with no actionable message. + // Use the cde_log.h CDE_LOG_* priority constants (transitively included + // via ggml-jni.h) to stay consistent with the rest of the JNI layer + // rather than dragging in here. + mtmd_log_set([](ggml_log_level level, const char * text, void * /*user_data*/) { + int prio; + switch (level) { + case GGML_LOG_LEVEL_ERROR: prio = CDE_LOG_ERROR; break; + case GGML_LOG_LEVEL_WARN: prio = CDE_LOG_WARN; break; + case GGML_LOG_LEVEL_INFO: prio = CDE_LOG_INFO; break; + case GGML_LOG_LEVEL_DEBUG: prio = CDE_LOG_DEBUG; break; + default: prio = CDE_LOG_DEBUG; break; + } + __android_log_print(prio, "KANTV_MTMD", "%s", text); + }, nullptr); +#endif + // Install the llama backend (ggml backend registry + quant tables) once + // for the whole JNI lifetime. The previous design called + // llama_backend_init() at the top of every mtmd_inference_main / + // llm_inference_main invocation and llama_backend_free() at the end - + // on the Hexagon CDSP/NPU build each such cycle tears down the DSP + // microcode + ION/DMA + RPC setup, which is both heat-generating and + // the source of the "2 succeed, 2 crash" MTMD regression. Doing it + // here, paired with the matching cleanup() called from the Java + // onDestroy() path, keeps the DSP alive across all inference calls + // in a session. + common_init(); + llama_backend_init(); + m_backend_initialized = true; initialized = true; } +void ggml_jni_context::cleanup() { + LOGGD("cleanup"); + // Drop any model that is still held in the singleton before tearing + // down the DSP backend - if a model is still loaded and the backend + // goes away, the model is left with dangling backend references and + // any subsequent operation would crash. unload_model() is a no-op + // when nothing is loaded, so it is safe to call unconditionally. + unload_model(); + if (m_backend_initialized) { + // Symmetric to llama_backend_init() in init(). llama_backend_free() + // just releases the quant tables (ggml_quantize_free) - it is + // cheap and safe to call once on app exit. Do NOT call this from + // per-inference paths: the DSP stays up for the whole session. + llama_backend_free(); + m_backend_initialized = false; + } + if (initialized) { + initialized = false; + } +} + void ggml_jni_context::finalize() { LOGGD("finalize"); if (initialized) { @@ -54,15 +145,192 @@ void ggml_jni_context::finalize() { ggml_jni_context::ggml_jni_context(): llm_inference_is_running(0), realtimemtmd_inference_is_running(0), - initialized(false) { + initialized(false), + m_backend_initialized(false), + loaded_model_path(), + loaded_mmproj_path(), + loaded_ngl(-100), // sentinel; no valid request matches -100 + loaded_model(nullptr), + loaded_mctx(nullptr) { init(); } ggml_jni_context::~ggml_jni_context() { - finalize(); + // At process exit the static singleton is destroyed. Mirror the Java + // onDestroy() cleanup so the DSP / quant tables are released exactly + // once. Idempotent because cleanup() is guarded by m_backend_initialized. + cleanup(); } -static class ggml_jni_context & g_jni_ctx = ggml_jni_context::get_instance(); +// ===== Phase 2: model singleton implementation ===== +// +// Design intent (see also the matching documentation in the header): +// * The model and mtmd context (mmproj) are heavy to load (4GB+ I/O, +// ~hundreds of ms of init on the Hexagon DSP). We hold them in this +// singleton for the lifetime of the JNI process and only reload when +// the (model_path, mmproj_path, backend_type) key changes. +// * llama_context (lctx), common_sampler, and llama_batch are NOT +// held in the singleton - they own per-conversation state (KV cache, +// sampling state) and must be created/freed per inference call. +// * m_model_mutex is the single-flight gate that makes a model switch +// atomic with respect to an in-flight inference: either the inference +// finishes on the old model, or it sees the new model - never a +// mid-flight unload while a forward pass is running. +int ggml_jni_context::ensure_model_loaded(const char * model_path, + const char * mmproj_path, + int n_gpu_layers) { + std::lock_guard lock(m_model_mutex); + + // Normalize inputs: callers may pass nullptr for mmproj_path on the + // pure-LLM path. Treat nullptr and "" identically for the cache key. + const std::string req_model_path = model_path ? model_path : ""; + const std::string req_mmproj_path = mmproj_path ? mmproj_path : ""; + + // Fast path: cache hit on all three key fields. Avoids the 4GB read. + // + // The third key field is n_gpu_layers (not the user-facing backend + // type). Two callers with the same backend can request different + // ngl values (e.g. LLM with ngl=99 vs MTMD with ngl=0 even when + // both pick the CDSP backend); using backend_type alone as the + // cache key caused the 4GB model to be loaded with the wrong + // offload configuration, which led to the vision encoder running + // out of Hexagon ION pool memory and producing garbage output. + if (loaded_model != nullptr + && loaded_model_path == req_model_path + && loaded_mmproj_path == req_mmproj_path + && loaded_ngl == n_gpu_layers) { + LOGGD("ensure_model_loaded: cache hit, reusing model '%s'%s%s (ngl=%d)", + req_model_path.c_str(), + req_mmproj_path.empty() ? "" : " + mmproj '", + req_mmproj_path.empty() ? "" : req_mmproj_path.c_str(), + n_gpu_layers); + return 0; + } + + // Slow path: either nothing is loaded yet, or the key changed. + // Unload any old model first so the new load starts from a clean + // state. unload_model() is a no-op if loaded_model == nullptr. + if (loaded_model != nullptr) { + LOGGD("ensure_model_loaded: cache miss, unloading previous model '%s' (ngl=%d)", + loaded_model_path.c_str(), loaded_ngl); + unload_model(); + } + + // Actually load the model. The full loading sequence is identical to + // what mtmd_inference_main / llm_inference_main used to do, just + // moved here so it runs at most once per (path, ngl) pair. + LOGGD("ensure_model_loaded: loading model '%s'%s%s (ngl=%d)", + req_model_path.c_str(), + req_mmproj_path.empty() ? "" : " + mmproj '", + req_mmproj_path.empty() ? "" : req_mmproj_path.c_str(), + n_gpu_layers); + + // CRITICAL: the n_gpu_layers value MUST be propagated into the + // common_params used to convert to llama_model_params. Previously + // we built a minimal common_params with only params.model.path set, + // which left params.n_gpu_layers at its default (-1 = "auto"); the + // resulting llama_model_params told the model loader to use GPU + // offload automatically, which on a device with a Hexagon backend + // caused the entire 4GB of weights to be allocated from the + // 4GB Hexagon ION pool. Once that pool was full, the vision + // encoder's compute buffers could not be allocated and produced + // garbage output (the "[multimodal]" token string in the user + // screenshot). Callers that want CPU-only must pass ngl=0 here so + // the weights stay in system memory. + { + common_params params; + params.model.path = req_model_path; + params.n_gpu_layers = n_gpu_layers; + llama_model_params mparams = common_model_params_to_llama(params); + loaded_model = llama_model_load_from_file(req_model_path.c_str(), mparams); + if (loaded_model == nullptr) { + LOGGD("ensure_model_loaded: llama_model_load_from_file failed for '%s'", + req_model_path.c_str()); + return 1; + } + } + + // Optional mtmd context (multimodal / vision encoder). Pure LLM + // calls pass an empty mmproj_path, in which case we leave + // loaded_mctx as nullptr. + if (!req_mmproj_path.empty()) { + mtmd_context_params mparams = mtmd_context_params_default(); + mparams.use_gpu = false; // see ggml-jni-context.cpp mtmd_inference + mparams.print_timings = false; + mparams.n_threads = std::thread::hardware_concurrency(); + loaded_mctx = mtmd_init_from_file(req_mmproj_path.c_str(), + loaded_model, mparams); + if (loaded_mctx == nullptr) { + LOGGD("ensure_model_loaded: failed to load mmproj '%s'", + req_mmproj_path.c_str()); + // The model loaded successfully but mmproj failed. We + // intentionally do not roll back the model load - the + // caller may want to retry with a different mmproj. The + // next call to ensure_model_loaded() will hit the + // mmproj_path mismatch and retry. + return 3; + } + LOGGD("ensure_model_loaded: mmproj '%s' loaded", req_mmproj_path.c_str()); + } + + // Update the cache key only after both loads succeeded. + loaded_model_path = req_model_path; + loaded_mmproj_path = req_mmproj_path; + loaded_ngl = n_gpu_layers; + + LOGGD("ensure_model_loaded: success"); + return 0; +} + +void ggml_jni_context::unload_model() { + std::lock_guard lock(m_model_mutex); + if (loaded_mctx != nullptr) { + LOGGD("unload_model: freeing mmproj context '%s'", loaded_mmproj_path.c_str()); + mtmd_free(loaded_mctx); + loaded_mctx = nullptr; + } + if (loaded_model != nullptr) { + LOGGD("unload_model: freeing model '%s' (ngl=%d)", + loaded_model_path.c_str(), loaded_ngl); + llama_model_free(loaded_model); + loaded_model = nullptr; + } + // Reset the cache key so a subsequent load with the same path is + // not treated as a no-op cache hit. + loaded_model_path.clear(); + loaded_mmproj_path.clear(); + loaded_ngl = -100; // sentinel; valid ngl values are >= -1 so -100 + // can never collide with a real request +} + +// Free-standing C entry point for unload_model(). Pairs with +// ggml_jni_context_cleanup() and is intended to be called from a +// Java unloadModel() JNI method (e.g. on LLMSettingFragment model +// change). Idempotent. +extern "C" void ggml_jni_unload_model() { + g_jni_ctx.unload_model(); +} + +// The single global reference to the JNI context singleton. Bound to +// the Meyers-singleton inside ggml_jni_context::get_instance() so the +// underlying object is constructed on first use and destroyed at +// process exit (its destructor calls cleanup()). The reference itself +// is a file-scope definition (not `static`) so that other translation +// units (mtmd-inference.cpp, llm-inference.cpp, +// realtime-video-recognition.cpp) can see it via the matching +// `extern` declaration in ggml-jni-context.h. We still need the +// header extern - the bare global reference at file scope without +// `static` is not visible across translation units on its own. +ggml_jni_context & g_jni_ctx = ggml_jni_context::get_instance(); + +// Free-standing C entry point so the JNI bridge (ggml-jni.c) can invoke +// the C++ singleton's cleanup() without dragging in the C++ class type. +// Wrapped in extern "C" so the symbol has C linkage and is callable from +// the C translation unit that holds the JNIEXPORT functions. Must come +// after the g_jni_ctx static above because it references g_jni_ctx. +extern "C" void ggml_jni_context_cleanup() { + g_jni_ctx.cleanup(); +} /** *helper functions to check whether normal LLM(LLM or normal MTMD) inference is running @@ -264,7 +532,19 @@ int llama_inference(const char * sz_model_path, const char * sz_user_data, int l int argc = (int)argv_vec.size(); llm_init_running_state(); - ret = llama_inference_main(argc, const_cast(argv_vec.data()), n_backend_type); + // Same guard as mtmd_inference: catch uncaught C++ exceptions from the + // llama.cpp internals so they do not abort() the whole process. + try { + ret = llama_inference_main(argc, const_cast(argv_vec.data()), n_backend_type); + } catch (const std::exception & e) { + __android_log_print(CDE_LOG_ERROR, "KANTV", + "llama_inference_main threw std::exception: %s", e.what()); + ret = -1; + } catch (...) { + __android_log_print(CDE_LOG_ERROR, "KANTV", + "llama_inference_main threw unknown (non-std) exception"); + ret = -1; + } llm_reset_running_state(); return ret; @@ -347,15 +627,21 @@ int mtmd_inference(const char * sz_model_path, const char * sz_mmproj_model_path argv_vec.push_back(args_storage.back().c_str()); if (n_backend_type == HEXAGON_BACKEND_CDSP) { + // WORKAROUND: MTMD (multimodal) inference on the Hexagon CDSP/NPU backend + // is unstable and intermittently aborts (SIGABRT after a few calls). The + // vision encoder (mmproj) emits ggml op patterns (convolutions and + // attention with image-embedding shapes) that the Hexagon backend does + // not fully implement, which causes a hard crash inside + // ggml_backend_hexagon_graph_compute. Text-only LLM inference on the + // same Hexagon backend works fine, so we keep -ngl 99 there; for MTMD + // we force -ngl 0 (CPU) regardless of the user-selected backend. The + // CPU path uses the same model file and the same chat template, just + // without the DSP offload - it is several times slower but stable. + LOGGW("mtmd_inference: forcing CPU backend (forcing -ngl 0) regardless of selected backend %d;" + " Hexagon NPU does not fully support the MTMD vision encoder ops, so the CPU path is" + " the only stable option for multimodal inference on this device.\n", n_backend_type); argv_vec.push_back("-ngl"); - argv_vec.push_back("99"); - argv_vec.push_back("-fa"); - argv_vec.push_back("on"); - argv_vec.push_back("--ubatch-size"); - argv_vec.push_back("64"); - argv_vec.push_back("--poll"); - argv_vec.push_back("1000"); - argv_vec.push_back("--no-mmap"); + argv_vec.push_back("0"); } else { argv_vec.push_back("-ngl"); argv_vec.push_back("0"); @@ -363,7 +649,25 @@ int mtmd_inference(const char * sz_model_path, const char * sz_mmproj_model_path int argc = (int)argv_vec.size(); llm_init_running_state(); - ret = mtmd_inference_main(argc, const_cast(argv_vec.data()), n_backend_type); + // Wrap mtmd_inference_main in a top-level try-catch to prevent uncaught C++ + // exceptions (e.g. from mtmd_context constructor when mmproj/text-model + // embeddings mismatch, or any other deep call) from calling std::terminate + // -> abort() and silently killing the process without surfacing an error + // to the Java layer. + try { + ret = mtmd_inference_main(argc, const_cast(argv_vec.data()), n_backend_type); + } catch (const std::exception & e) { + // Reachable when the inner catch in common_params_parse (which calls exit(1) + // for std::exception) does not catch the exception -- e.g. when the throw + // originates from a noexcept function or a destructor along the way. + __android_log_print(CDE_LOG_ERROR, "KANTV", + "mtmd_inference_main threw std::exception: %s", e.what()); + ret = -1; + } catch (...) { + __android_log_print(CDE_LOG_ERROR, "KANTV", + "mtmd_inference_main threw unknown (non-std) exception"); + ret = -1; + } llm_reset_running_state(); LOGGD("mtmd_inference return %d", ret); diff --git a/core/jni/ggml-jni-context.h b/core/jni/ggml-jni-context.h index 988b821db..70cddbf65 100644 --- a/core/jni/ggml-jni-context.h +++ b/core/jni/ggml-jni-context.h @@ -43,6 +43,16 @@ //ggml-jni #include "ggml-jni.h" +// Forward declarations for the llama.cpp and libmtmd types we hold across +// calls. We keep these as pointers in the singleton; the full definitions +// live in llama.h / mtmd.h which are only included in the .cpp file +// (including them in the header would leak C++ types into every .cpp that +// includes ggml-jni-context.h, plus they'd want a chunk of / +// etc. that the existing code does not need). +struct llama_model; +struct llama_context; +struct mtmd_context; + class ggml_jni_context { public: static ggml_jni_context & get_instance() { @@ -52,8 +62,44 @@ class ggml_jni_context { void init(); + void cleanup(); + void finalize(); + // ===== Phase 2: model singleton ===== + // ensure_model_loaded() is the idempotent "load the model if not already + // loaded under the given (model_path, mmproj_path, backend) key" entry + // point. Replaces the per-call common_init_from_params + + // mtmd_init_from_file pair inside mtmd_inference_main / llm_inference_main. + // The model and mtmd context (mmproj) are heavy to load (4GB+ I/O and + // hundreds of ms of init on Hexagon DSP) so we keep them alive between + // inference calls and only reload when the user switches model file or + // backend. The returned pointers are owned by the singleton; callers + // MUST NOT free them. + // + // Returns 0 on success, non-zero on failure (matching the convention + // used by mtmd_inference_main / llm_inference_main). On failure, the + // singleton is left in a consistent state (either old model still + // loaded, or fully unloaded). + int ensure_model_loaded(const char * model_path, + const char * mmproj_path, + int n_gpu_layers); + + // Release the currently loaded model + mmproj. Safe to call when + // nothing is loaded (no-op). Called explicitly from the Java side on + // model switch (LLMSettingFragment) and from process exit (cleanup()). + void unload_model(); + + bool is_model_loaded() const { return loaded_model != nullptr; } + + // Accessors for the cached model + mtmd context. The returned pointers + // are owned by the singleton and MUST NOT be freed by the caller. + // Callers must have already called ensure_model_loaded() to guarantee + // these are non-null. Used by mtmd_inference_main / llm_inference_main + // to skip the per-call 4GB reload on cache hits. + llama_model * get_loaded_model() const { return loaded_model; } + mtmd_context * get_loaded_mctx() const { return loaded_mctx; } + void set_top_p(float value) { llm_temperature = value; } float get_top_p() { return llm_top_p; } void set_temperature(float value) { llm_temperature = value; } @@ -101,4 +147,50 @@ class ggml_jni_context { std::atomic realtimemtmd_inference_is_running; bool initialized; -}; \ No newline at end of file + // Set true after llama_backend_init() in init() and false again after + // llama_backend_free() in cleanup(). Guards against double-free when + // cleanup() is invoked both from the Java onDestroy path and from the + // static singleton destructor at process exit. + bool m_backend_initialized; + + // ===== Phase 2: model singleton state ===== + // The triple (loaded_model_path, loaded_mmproj_path, loaded_ngl) is + // the cache key. A mismatch on any of the three triggers a full reload + // (unload old, then load new) inside ensure_model_loaded(). Two paths + // can be empty simultaneously: LLM inference sets mmproj_path="", + // MTMD inference sets mmproj_path=. The same is true for + // a real LLM/MTMD switch. + // + // The cache key intentionally uses n_gpu_layers (not the user-facing + // backend_type enum) because two callers with the same backend type + // can request different ngl values (LLM with ngl=99 vs MTMD with ngl=0 + // even when both pick the CDSP backend). Caching by ngl is the only + // way to guarantee that the cached model has the correct backend + // offload configuration: caching by backend_type alone caused the + // "MTMD garbage output" regression where a 4GB LLM model already + // resident in the Hexagon ION pool was reused for a CPU MTMD + // inference, and the vision encoder compute subsequently failed + // with "ion-batch: mempool full" because all 4GB were already in + // ION and there was no room for the vision encoder buffers. + std::string loaded_model_path; + std::string loaded_mmproj_path; + int loaded_ngl; // -1 (auto) / 0 (CPU) / 99 (CDSP offload all) + llama_model * loaded_model; // owned; freed in unload_model() + mtmd_context * loaded_mctx; // owned; freed in unload_model(); nullptr for LLM-only + + // Single-flight gate around load/unload/infer. The UI thread may + // trigger a model switch while a previous inference is still + // running (e.g. user picks a different model mid-generation); the + // inference thread holds this mutex during ensure_model_loaded so + // the unload+reload pair is atomic with respect to the inference. + std::mutex m_model_mutex; +}; + +// The single global reference to the JNI context singleton. Defined in +// ggml-jni-context.cpp (where it is bound to ggml_jni_context::get_instance()); +// declared here with `extern` so that other translation units +// (mtmd-inference.cpp, llm-inference.cpp, realtime-video-recognition.cpp) +// can use it to drive the model singleton. The Meyers-singleton in +// get_instance() guarantees there is only one underlying object even +// though the reference is now visible from multiple translation units. +extern ggml_jni_context & g_jni_ctx; \ No newline at end of file diff --git a/core/jni/ggml-jni.c b/core/jni/ggml-jni.c index d5f90eb77..e8c86684a 100644 --- a/core/jni/ggml-jni.c +++ b/core/jni/ggml-jni.c @@ -448,6 +448,27 @@ Java_kantvai_ai_ggmljava_llm_1is_1running_1state(JNIEnv *env, jclass clazz) { return (0 == result) ? JNI_FALSE : JNI_TRUE; } +// Release the llama backend (DSP + quant tables) installed once in +// ggml_jni_context::init(). Called from AIResearchFragment.onDestroy() so +// the DSP gets released on activity teardown rather than living until +// process death. Idempotent; safe to call multiple times. +JNIEXPORT void JNICALL +Java_kantvai_ai_ggmljava_backendCleanup(JNIEnv *env, jclass clazz) { + ggml_jni_context_cleanup(); +} + +// Release the currently loaded model + mmproj. The next inference call +// will trigger a fresh load. Intended call sites: +// * LLMSettingFragment when the user picks a different model +// * AIResearchFragment.onDestroy() (paired with backendCleanup()) so +// the 4GB model is returned to the OS as soon as the page is left +// rather than sitting in memory until the process dies. +// Idempotent; safe to call when nothing is loaded. +JNIEXPORT void JNICALL +Java_kantvai_ai_ggmljava_unloadModel(JNIEnv *env, jclass clazz) { + ggml_jni_unload_model(); +} + JNIEXPORT void JNICALL Java_kantvai_ai_ggmljava_realtimemtmd_1init_1running_1state(JNIEnv *env, jclass clazz) { realtimemtmd_init_running_state(); diff --git a/core/jni/ggml-jni.h b/core/jni/ggml-jni.h index 9b39c4c8f..b636af5dc 100644 --- a/core/jni/ggml-jni.h +++ b/core/jni/ggml-jni.h @@ -23,6 +23,23 @@ extern "C" { #endif +/** + * Release the llama backend (DSP + quant tables) installed by + * ggml_jni_context::init(). Called from ggml-java's backendCleanup() + * JNI method, which in turn is invoked from AIResearchFragment.onDestroy(). + * Implementation lives in ggml-jni-context.cpp (extern "C" wrapper around + * the C++ singleton). Idempotent: safe to call multiple times. + */ +void ggml_jni_context_cleanup(void); + +/** + * Release the currently loaded model + mtmd context (mmproj) held by the + * ggml_jni_context singleton. Subsequent inference calls will trigger a + * fresh load from disk. Implementation in ggml-jni-context.cpp; the + * matching JNI method is Java_kantvai_ai_ggmljava_unloadModel. + */ +void ggml_jni_unload_model(void); + /** * AI inference in native layer was stopped/interrupted from Java layer */ diff --git a/core/jni/llm-inference.cpp b/core/jni/llm-inference.cpp index 68ff6a7a2..0f2ac9a39 100644 --- a/core/jni/llm-inference.cpp +++ b/core/jni/llm-inference.cpp @@ -38,6 +38,9 @@ extern "C" { #include "libavutil/cde_assert.h" } #include "ggml-jni.h" +// For Phase 2 model singleton: brings in g_jni_ctx extern declaration +// and the accessors used by ensure_model_loaded() below. +#include "ggml-jni-context.h" #include "llamacpp/ggml/include/ggml-hexagon.h" #endif @@ -153,7 +156,12 @@ int llama_inference_main(int argc, char ** argv, int backend_type) { params.sampling.top_p = llm_get_top_p(); LOGGD("top_p %.2f\n", params.sampling.top_p); } - common_init(); + // NOTE: common_init() and llama_backend_init() used to be called here. + // They are now installed exactly once in ggml_jni_context::init() and + // torn down in ggml_jni_context::cleanup() (called from + // AIResearchFragment.onDestroy() via ggmljava.backendCleanup()). + // The previous per-call cycle tore the Hexagon DSP down and back up on + // every LLM inference, which was the dominant heat source. auto & sparams = params.sampling; if (params.embedding) { @@ -193,7 +201,12 @@ int llama_inference_main(int argc, char ** argv, int backend_type) { } // hexagon backend is statically linked into libkantv-core.so, no need to // load separate .so via ggml_backend_load_all_from_path() - llama_backend_init(); + // NOTE: llama_backend_init() used to be called here. It is now installed + // once for the whole JNI lifetime in ggml_jni_context::init() and torn + // down in ggml_jni_context::cleanup() (called from + // AIResearchFragment.onDestroy() via ggmljava.backendCleanup()). + // The previous per-call cycle tore the Hexagon DSP down and back up on + // every LLM inference, which was the dominant heat source. llama_numa_init(params.numa); log_timing("after llama_backend_init"); @@ -207,18 +220,41 @@ int llama_inference_main(int argc, char ** argv, int backend_type) { std::vector chat_msgs; - // load the model and apply lora adapter, if any + // load the model via the singleton (Phase 2). On a cache hit this + // returns in microseconds and skips the multi-second 4GB read that + // common_init_from_params() used to do here. On a cache miss + // (first call, or model path / backend mismatch) it loads the + // model into the singleton and we borrow the pointer. LOG_INF("%s: load the model and apply lora adapter, if any\n", __func__); - common_init_result_ptr llama_init = common_init_from_params(params); - - model = llama_init->model(); - ctx = llama_init->context(); - log_timing("after model load (common_init)"); - - if (model == NULL) { + int load_rc = g_jni_ctx.ensure_model_loaded( + params.model.path.c_str(), + /* mmproj_path */ "", + /* n_gpu_layers */ params.n_gpu_layers); // LLM path; pass the actual ngl + // so a CPU LLM (-ngl 0) does NOT + // reuse a model previously loaded + // with -ngl 99 into Hexagon ION + if (load_rc != 0) { + LOG_ERR("%s: ensure_model_loaded failed (rc=%d) for '%s'\n", + __func__, load_rc, params.model.path.c_str()); + return 1; + } + model = g_jni_ctx.get_loaded_model(); + if (model == nullptr) { LOG_ERR("%s: error: unable to load model\n", __func__); return 1; } + // Per-call llama_context from the cached model. Fresh KV cache per + // inference, with n_ctx / n_threads / batch settings from the + // caller's params. + { + llama_context_params ctx_params = common_context_params_to_llama(params); + ctx = llama_init_from_model(model, ctx_params); + } + if (ctx == nullptr) { + LOG_ERR("%s: failed to create llama_context from cached model\n", __func__); + return 1; + } + log_timing("after model load (singleton cache)"); const llama_vocab * vocab = llama_model_get_vocab(model); auto chat_templates = common_chat_templates_init(model, params.chat_template); @@ -1084,7 +1120,22 @@ int llama_inference_main(int argc, char ** argv, int backend_type) { common_sampler_free(smpl); - llama_backend_free(); + // lctx is per-call: it owns the KV cache for this conversation, + // so we always free it on exit. The cached llama_model lives in + // the ggml_jni_context singleton and is NOT freed here - it will + // be reused on the next LLM inference call (assuming the same + // model path) and is finally released by + // ggml_jni_context::unload_model() / cleanup() on app exit or + // explicit model switch. + if (ctx != nullptr) { + llama_free(ctx); + ctx = nullptr; + } + + // llama_backend_free() used to be called here. The DSP backend is now + // owned by ggml_jni_context and released in cleanup(); calling free + // here would tear down the DSP that the next inference call (and the + // MTMD path) depends on. ggml_threadpool_free_fn(threadpool); ggml_threadpool_free_fn(threadpool_batch); diff --git a/core/jni/mtmd-inference.cpp b/core/jni/mtmd-inference.cpp index 2ff8b1cb1..a59ba3b4e 100644 --- a/core/jni/mtmd-inference.cpp +++ b/core/jni/mtmd-inference.cpp @@ -38,8 +38,15 @@ #include #include +#if defined(__ANDROID__) +#include +#endif + //ggml-jni #include "ggml-jni.h" +// For Phase 2 model singleton: brings in the g_jni_ctx extern +// declaration and the singleton's method declarations. +#include "ggml-jni-context.h" //libllama #include "llama.h" @@ -72,8 +79,11 @@ static std::string fnv_hash(const uint8_t * data, size_t len) { //ref:https://github.com/ggml-org/llama.cpp/blob/master/tools/mtmd/mtmd-cli.cpp int mtmd_inference_main(int argc, char ** argv, int backend_type) { common_params params; - common_init_result_ptr llama_init; + // model and mctx are owned by the ggml_jni_context singleton - we + // borrow them via accessors after ensure_model_loaded() succeeds and + // do NOT free them here (only the per-call llama_context, sampler + // and batch are freed). llama_model * model = nullptr; llama_context * lctx = nullptr; const llama_vocab * vocab = nullptr; @@ -106,26 +116,34 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { params.sampling.temp = 0.2; // lower temp by default for better quality params.cpuparams.n_threads = thread_counts; LOGGD("mtmd_inference_main backend_type %d", backend_type); - //runtime decision based on backend_type: - // HEXAGON_BACKEND_CDSP: offload all layers to DSP - // HEXAGON_BACKEND_GGML: CPU only, no offload - if (backend_type == HEXAGON_BACKEND_CDSP) { - LOGGD("using hexagon CDSP backend (runtime decision, -ngl 99)"); - params.main_gpu = 0; - params.n_gpu_layers = 99; - } else { - LOGGD("using default ggml CPU backend (runtime decision, -ngl 0)"); - params.main_gpu = 0; - params.n_gpu_layers = 0; - } + // NOTE: do NOT pre-set params.n_gpu_layers / params.main_gpu here. The + // upstream common_params_parse() has a hard GGML_ASSERT(params.n_gpu_layers < 0) + // (see common/arg.cpp:2621) that aborts the process if the value is >= 0. + // The default is -1 ("auto"), which passes the assert; the actual value is + // then applied by the -ngl/--gpu-layers option handler later in + // common_params_parse() based on the argv built by the JNI bridge: + // - HEXAGON_BACKEND_CDSP -> -ngl 99 (offload all layers to DSP) + // - HEXAGON_BACKEND_GGML -> -ngl 0 (CPU only) if (!common_params_parse(argc, const_cast(argv), params, LLAMA_EXAMPLE_MTMD)) { LOGGD("common params parse failure\n"); return 2; } - common_init(); - // hexagon backend is statically linked into libkantv-core.so, no need to - // load separate .so via ggml_backend_load_all_from_path() - llama_backend_init(); + // runtime decision log: report the value that common_params_parse settled on + if (params.n_gpu_layers > 0) { + LOGGD("using hexagon CDSP backend (runtime decision, n_gpu_layers=%d)\n", + params.n_gpu_layers); + } else { + LOGGD("using default ggml CPU backend (runtime decision, n_gpu_layers=0)\n"); + } + // NOTE: common_init() and llama_backend_init() used to be called here. + // They are now installed exactly once in ggml_jni_context::init() and + // torn down in ggml_jni_context::cleanup() (called from + // AIResearchFragment.onDestroy() via ggmljava.backendCleanup()). + // The previous per-call cycle tore the Hexagon DSP down and back up on + // every MTMD inference, which was the dominant heat source and + // contributed to the "2 succeed, 2 crash" intermittent abort on + // CDSP. We keep llama_numa_init() here because it depends on the + // n_threads value chosen by the user for *this* inference. llama_numa_init(params.numa); LOGGD("system info: n_threads = %d, n_threads_batch = %d, total_threads = %d\n", params.cpuparams.n_threads, params.cpuparams_batch.n_threads, @@ -134,14 +152,55 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { LOGGD("%s\n", common_params_get_system_info(params).c_str()); LOGGD("\n"); - //step-2: load LLM model + //step-2: ensure model is loaded (singleton - skips 4GB reload on cache hit) + // + // Previously this function called common_init_from_params() here to + // read the 4GB model file from disk on every inference. On a phone + // this is several seconds of full-bandwidth storage I/O and is the + // dominant heat source of the AI Research page. We now delegate the + // load to ggml_jni_context::ensure_model_loaded(), which only + // touches disk on the first call (or when the model file / + // mmproj path / backend triple changes). Subsequent inferences on + // the same model reuse the cached llama_model + mtmd_context. LOGGD("loading model '%s'\n", params.model.path.c_str()); - llama_init = common_init_from_params(params); - model = llama_init->model(); - lctx = llama_init->context(); + // Pass n_gpu_layers (not the user-facing backend_type) as the cache + // key. After common_params_parse() above we know the runtime ngl + // value (0 for MTMD on this device - the JNI bridge forces -ngl 0 + // regardless of the user-selected backend). Caching by ngl + // guarantees that if a previous LLM run had loaded the same model + // file with ngl=99 (CDSP offload) into the Hexagon ION pool, we + // will NOT reuse that pool-resident model for a CPU MTMD + // inference - we will unload it and re-load with ngl=0 into + // system memory. This is the fix for the "MTMD garbage output" + // regression. + int load_rc = g_jni_ctx.ensure_model_loaded( + params.model.path.c_str(), + params.mmproj.path.c_str(), + params.n_gpu_layers); + if (load_rc != 0) { + LOGGD("ensure_model_loaded failed (rc=%d) for '%s'\n", + load_rc, params.model.path.c_str()); + return 3; + } + model = g_jni_ctx.get_loaded_model(); + mctx = g_jni_ctx.get_loaded_mctx(); if (model == nullptr) { - LOGGD("failed to load model, '%s'\n", params.model.path.c_str()); - llama_backend_free(); + LOGGD("cached model is null after ensure_model_loaded\n"); + return 3; + } + // Create a per-call llama_context from the cached model. This + // gives every inference a fresh KV cache (no leakage of prior + // conversations) while the heavy 4GB model is reused. Uses + // common_context_params_to_llama() so the n_ctx / n_threads / + // flash-attention / batch settings from the caller's params are + // honored on the new context. llama_init_from_model is the modern + // (non-deprecated) replacement for llama_new_context_with_model. + { + llama_context_params ctx_params = common_context_params_to_llama(params); + lctx = llama_init_from_model(model, ctx_params); + } + if (lctx == nullptr) { + LOGGD("failed to create llama_context from cached model\n"); return 3; } vocab = llama_model_get_vocab(model); @@ -153,20 +212,21 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { struct common_sampler * smpl = common_sampler_init(model, params.sampling); n_predict = params.n_predict < 0 ? INT_MAX : params.n_predict; - //step-3: load multimodal model - std::string & mmproj_path = params.mmproj.path; - mparams = mtmd_context_params_default(); - mparams.use_gpu = false; - mparams.print_timings = false; - mparams.n_threads = thread_counts; - mctx = mtmd_init_from_file(mmproj_path.c_str(), model, mparams); + //step-3: multimodal model (vision encoder) is already loaded by + //ensure_model_loaded() above. The mtmd context pointer was retrieved + //from the singleton into the local `mctx` variable. We sanity-check + //it here before proceeding - the singleton can legitimately hold a + //null mctx only if the caller passed an empty mmproj_path, which is + //not the case for the MTMD path that always sets a non-empty + //mmproj. if (mctx == nullptr) { - LOGGD("failed to load multimodal model, '%s'\n", mmproj_path.c_str()); + LOGGD("multimodal context is null after ensure_model_loaded; this should not happen for MTMD\n"); common_sampler_free(smpl); - llama_backend_free(); + llama_batch_free(batch); + llama_free(lctx); return 4; } - LOGGD("loaded multimodal model, '%s'\n", mmproj_path.c_str()); + LOGGD("loaded multimodal model, '%s'\n", params.mmproj.path.c_str()); //step-4: load media(image / audio) for (const auto & image : params.image) { @@ -177,8 +237,10 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { LOGGD("failed to load media\n"); GGML_JNI_NOTIFY("failed to load media\n"); common_sampler_free(smpl); - mtmd_free(mctx); - llama_backend_free(); + llama_batch_free(batch); + // lctx is per-call and must be freed; mctx is in the + // singleton and is NOT freed here (see failure: label). + llama_free(lctx); return 5; } // calculate bitmap hash (for KV caching) @@ -208,7 +270,19 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { "This may cause the model to output suboptimal responses\n", __func__); chat_templates = common_chat_templates_init(model, "chatml"); } - LOGGD("%s: chat template example:\n%s\n", __func__, common_chat_format_example(chat_templates.get(), params.use_jinja, params.default_template_kwargs).c_str()); + // The second example-format call is also wrapped in try-catch. If the + // fallback chatml template still throws (e.g. on a model whose vocab + // does not declare the chatml special tokens), the previous version of + // this code would let std::exception escape into std::terminate -> abort() + // and silently kill the process between the chat-template example log + // and the "starting media encoding" notify. Catching here lets the + // inference continue with a minimal raw-prompt fallback. + try { + LOGGD("%s: chat template example:\n%s\n", __func__, common_chat_format_example(chat_templates.get(), params.use_jinja, params.default_template_kwargs).c_str()); + } catch (const std::exception &e) { + LOGGD("%s: chat template example generation failed (%s); continuing with empty example\n", + __func__, e.what()); + } //params.prompt = prompt_str; //ref:https://github.com/ggml-org/llama.cpp/discussions/13759#discussioncomment-13294811 //if (params.prompt.find("<__media__>") == std::string::npos) { @@ -228,11 +302,27 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { tmpl_inputs.add_generation_prompt = true; tmpl_inputs.use_jinja = false; // jinja is buggy here { - auto formatted_chat = common_chat_templates_apply(chat_templates.get(), tmpl_inputs); - LOGGD("formatted_chat.prompt: %s\n", formatted_chat.prompt.c_str()); + // common_chat_templates_apply() can throw std::runtime_error (e.g. + // "this custom template is not supported, try using --jinja") when the + // built-in chat template does not match the model's expected format + // (commonly seen with Gemma3 and similar newer models). Without this + // try-catch the exception escapes into std::terminate -> abort() and + // silently kills the native process without surfacing any error to + // the Java layer. On failure, fall back to the raw prompt so the user + // still gets a response instead of a crash. + std::string formatted_prompt; + try { + auto formatted_chat = common_chat_templates_apply(chat_templates.get(), tmpl_inputs); + formatted_prompt = formatted_chat.prompt; + } catch (const std::exception & e) { + LOGGD("%s: chat template apply failed (%s); using raw prompt as fallback\n", + __func__, e.what()); + formatted_prompt = msg.content; + } + LOGGD("formatted_chat.prompt: %s\n", formatted_prompt.c_str()); mtmd_input_text inp_txt = { - formatted_chat.prompt.c_str(), - /* text_len */ formatted_chat.prompt.size(), + formatted_prompt.c_str(), + /* text_len */ formatted_prompt.size(), /* add_special */ true, /* parse_special */ true, }; @@ -352,8 +442,26 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { failure: //step-7: cleanup common_sampler_free(smpl); - mtmd_free(mctx); - llama_backend_free(); + // lctx is per-call: it owns the KV cache for this conversation, so + // we always free it on exit. The cached llama_model and mtmd + // context (mctx) live in the ggml_jni_context singleton and are + // NOT freed here - they will be reused on the next MTMD inference + // call (assuming the same model / mmproj / backend) and are + // finally released by ggml_jni_context::unload_model() / cleanup() + // on app exit or explicit model switch. + if (lctx != nullptr) { + llama_free(lctx); + lctx = nullptr; + } + // llama_batch was created with llama_batch_init(params.n_batch, 0, 1) + // (see step-2). It owns three heap-allocated arrays (token / pos / + // logits) of size n_batch each. Without llama_batch_free() those + // allocations leak on every call. Over a few MTMD inference calls in + // the same process this can grow large enough to interact badly with + // the ggml-hexagon ION pool and trigger a crash that looks + // non-deterministic. The upstream mtmd-cli.cpp does this in + // ~mtmd_cli_context(); we mirror it here for the goto-failure path. + llama_batch_free(batch); LOGGD("return"); if (0 == llm_inference_interrupted) diff --git a/core/jni/realtime-video-recognition.cpp b/core/jni/realtime-video-recognition.cpp index cb733e331..cdda35f4a 100644 --- a/core/jni/realtime-video-recognition.cpp +++ b/core/jni/realtime-video-recognition.cpp @@ -40,6 +40,9 @@ //ggml-jni #include "ggml-jni.h" +// For Phase 2 model singleton: brings in g_jni_ctx extern declaration +// and the accessors used by ensure_model_loaded() below. +#include "ggml-jni-context.h" //libllama #include "llama.h" @@ -129,7 +132,10 @@ class multimodal_inference { private: common_params params; - common_init_result_ptr llama_init; + // llama_init was removed in Phase 2: model loading now goes through + // ggml_jni_context::ensure_model_loaded() which caches the + // llama_model in the singleton. The local `model` pointer is + // borrowed from the singleton and is NOT owned by this class. llama_model * model = nullptr; llama_context * lctx = nullptr; @@ -201,9 +207,15 @@ bool multimodal_inference::model_init(const char * llm_model_name, LOGGD("using default ggml CPU backend (compile-time decision)"); params.main_gpu = 0; #endif - common_init(); - - llama_backend_init(); + // NOTE: common_init() and llama_backend_init() used to be called here. + // They are now installed exactly once in ggml_jni_context::init() and + // torn down in ggml_jni_context::cleanup() (called from + // AIResearchFragment.onDestroy() via ggmljava.backendCleanup()). + // The previous per-call cycle tore the Hexagon DSP down and back up on + // every MTMD inference, which was the dominant heat source and + // contributed to the "2 succeed, 2 crash" intermittent abort on + // CDSP. We keep llama_numa_init() here because it depends on the + // n_threads value chosen by the user for *this* inference. llama_numa_init(params.numa); LOGGD("system info: n_threads = %d, n_threads_batch = %d, total_threads = %d\n", params.cpuparams.n_threads, params.cpuparams_batch.n_threads, @@ -212,14 +224,34 @@ bool multimodal_inference::model_init(const char * llm_model_name, LOGGD("%s\n", common_params_get_system_info(params).c_str()); LOGGD("\n"); - //step-2: load LLM model + //step-2: ensure model is loaded via the singleton (Phase 2). + //On a cache hit this skips the 4GB read. LOGGD("loading model '%s'\n", params.model.path.c_str()); - llama_init = common_init_from_params(params); - model = llama_init->model(); - lctx = llama_init->context(); + int load_rc = g_jni_ctx.ensure_model_loaded( + params.model.path.c_str(), + params.mmproj.path.c_str(), + params.n_gpu_layers); + if (load_rc != 0) { + LOGGD("ensure_model_loaded failed (rc=%d) for '%s'\n", + load_rc, params.model.path.c_str()); + return false; + } + model = g_jni_ctx.get_loaded_model(); + mctx = g_jni_ctx.get_loaded_mctx(); if (model == nullptr) { - LOGGD("failed to load model, '%s'\n", params.model.path.c_str()); - llama_backend_free(); + LOGGD("cached model is null after ensure_model_loaded\n"); + return false; + } + // Per-call llama_context from the cached model. Fresh KV cache per + // inference, with n_ctx / n_threads / batch settings from the + // caller's params. llama_init_from_model is the modern + // (non-deprecated) replacement for llama_new_context_with_model. + { + llama_context_params ctx_params = common_context_params_to_llama(params); + lctx = llama_init_from_model(model, ctx_params); + } + if (lctx == nullptr) { + LOGGD("failed to create llama_context from cached model\n"); return false; } vocab = llama_model_get_vocab(model); @@ -241,7 +273,7 @@ bool multimodal_inference::model_init(const char * llm_model_name, if (mctx == nullptr) { LOGGD("failed to load multimodal model, '%s'\n", mmproj_path.c_str()); common_sampler_free(smpl); - llama_backend_free(); + // llama_backend_free() removed: backend is owned by ggml_jni_context. return false; } LOGGD("loaded multimodal model, '%s'\n", mmproj_path.c_str()); @@ -445,10 +477,28 @@ void multimodal_inference::finalize() { if (nullptr != smpl) common_sampler_free(smpl); - if (nullptr != mctx) - mtmd_free(mctx); - - llama_backend_free(); + // lctx is per-call (in the realtime path each frame's inference + // reuses the same lctx for KV cache, so we free it here too). + if (nullptr != lctx) + llama_free(lctx); + + // batch is a llama_batch value (not a pointer), so a nullptr + // check would not type-check. llama_batch_free() is safe to + // call on a default-initialized llama_batch{} because the + // three owned pointers (token / pos / logits) are all null + // and free(NULL) is a no-op. We always free here so the + // n_batch-sized heap arrays do not leak across finalizes. + llama_batch_free(batch); + + // model and mctx are NOT freed: they are owned by the + // ggml_jni_context singleton and are reused across calls. + // They are released by ggml_jni_context::unload_model() / + // cleanup() on app exit or explicit model switch. + + // llama_backend_free() removed: backend is owned by ggml_jni_context + // and released in its cleanup() (called from Java onDestroy via + // ggmljava.backendCleanup()). Calling free here would tear down the + // DSP that subsequent inference calls depend on. initialized = false; } else { From ef109646b4f530061dc1d04640a47659700b46d9 Mon Sep 17 00:00:00 2001 From: Jeff Zhou Date: Tue, 28 Jul 2026 21:01:26 +0800 Subject: [PATCH 2/2] inference&UI: fix complex scenarios in AIResearch page Assisted-by: Trae + MiniMax-M3 --- .../main/java/kantvai/ai/KANTVAIUtils.java | 13 +- .../ui/fragment/AIResearchFragment.java | 140 +++++++++++++++++- .../fragment/settings/LLMSettingFragment.java | 35 +++++ core/jni/ggml-jni-context.cpp | 93 +++++++++++- core/jni/ggml-jni-context.h | 5 + core/jni/mtmd-inference.cpp | 7 +- 6 files changed, 280 insertions(+), 13 deletions(-) diff --git a/android/kantvplayer-lib/src/main/java/kantvai/ai/KANTVAIUtils.java b/android/kantvplayer-lib/src/main/java/kantvai/ai/KANTVAIUtils.java index f3124b6fd..b3e1f9324 100644 --- a/android/kantvplayer-lib/src/main/java/kantvai/ai/KANTVAIUtils.java +++ b/android/kantvplayer-lib/src/main/java/kantvai/ai/KANTVAIUtils.java @@ -316,6 +316,13 @@ public static boolean isMTMD_ImageModel(String name) { public static boolean isAudioFile(String filename) { //naive method + //IMPORTANT: must match what mtmd-helper actually supports. + //mtmd-helper.cpp uses miniaudio to decode audio and only accepts + //wav/mp3/flac. aac and ac3 (e.g. from m4a containers) are NOT + //decoded by miniaudio and will be rejected inside mtmd, so we + //should not even route them to the audio MTMD path. If a future + //build adds aac support, add it here and update the list of + //supported audio types accordingly. String suffix = filename.substring(filename.lastIndexOf(".") + 1); if (suffix.contains("wav")) { return true; @@ -325,11 +332,7 @@ public static boolean isAudioFile(String filename) { return true; } - if (suffix.contains("aac")) { - return true; - } - - if (suffix.contains("ac3")) { + if (suffix.contains("flac")) { return true; } diff --git a/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java b/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java index cb298d947..f13f7ae14 100644 --- a/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java +++ b/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/AIResearchFragment.java @@ -387,7 +387,29 @@ public void run() { backendIndex = mSettings.getLLMbackend(); KANTVLog.g(TAG, "backendIndex: " + backendIndex); - initKANTVMgr(); + // BUGFIX (regression from the Phase 2 refactor): + // The naive fix of skipping initKANTVMgr() entirely on + // non-ASR bench types breaks the GGML_JNI_NOTIFY event + // pipe: KANTVMgr's constructor (native_setup) and + // initASR() are what build the libkantv-media.so side + // of the JNI->Java dispatch. Without them, the "starting + // media encoding" hint, per-token streaming, and + // llama-timings perf string that C++ emits via + // GGML_JNI_NOTIFY() never reach Java's + // MyEventListener.onEvent(), and the chat RecyclerView + // stays empty ("..." with no progress) even though the + // native inference completes successfully. + // + // The actual deadlock source was startASR(), which + // spins up a permanent ASR listen thread that the + // following release() then waits for. Skipping that + // alone (while still doing new KANTVMgr + initASR) + // is the safe fix: event pipe is rebuilt every send, + // but no permanent listen thread is created, so + // release() returns immediately. + int benchType = nBenchmarkIndex; + boolean isAsrBench = (KANTVAIUtils.bench_type.GGML_BENCHMARK_ASR.ordinal() == benchType); + setupKANTVMgr(isAsrBench); while (isBenchmarking.get()) { beginTime = System.currentTimeMillis(); @@ -587,7 +609,24 @@ public void onResume() { backendIndex = storedBackend; setTextGGMLInfo(strModeName); } - } + + // NOTE: do NOT re-read the LLM model selection from Settings here. + // LLM Settings and AI Research are intentionally decoupled: + // * LLM Settings owns the download/preference for which model + // files are available on disk and which one is the "default" + // for the next cold start. + // * AI Research owns the inference-time model selection via + // its Bench/Model dialog, which writes back to Settings for + // cross-restart persistence but is the source of truth + // while the user is interacting with this page. + // A previous onResume() sync tried to pull the LLM index from + // Settings on every tab-switch, which caused regressions + // (MTMD audio silently became pure-text inference because the + // sync raced the Bench dialog). The send-time fallback in + // runInference() (comparing llm_get_loaded_model_path with + // strModeName) is the single source of truth for "is the + // loaded model the one the user wants to run with". + } @Override public void onStop() { @@ -836,6 +875,12 @@ private void handleEventOnUiThread(KANTVEventType eventType, int what, int arg1, // appended to the bubble so the user sees the // timing numbers, but it's also the cue to flip // the bubble into COMPLETE state. + // + // The native side emits "llama-timings:\n..." with + // no leading blank line, so it would visually run + // into the previous token. We prepend "\n\n" here + // (UI concern) rather than in the C++ perf_str + // format, so logcat / file output stays raw. KANTVLog.j(TAG, "LLM timings"); // Flush any still-buffered streaming tokens so the // last 80ms-worth of text lands in the bubble @@ -844,7 +889,7 @@ private void handleEventOnUiThread(KANTVEventType eventType, int what, int arg1, flushStreamingChunk(); if (chatAdapter != null) { strInferenceResult += content; - chatAdapter.appendToLast("\n" + content); + chatAdapter.appendToLast("\n\n" + content); chatAdapter.markLastComplete(); } if (isBenchmarking.compareAndSet(true, false)) { @@ -879,7 +924,25 @@ private void handleEventOnUiThread(KANTVEventType eventType, int what, int arg1, } - private void initKANTVMgr() { + // Split out from initKANTVMgr() so callers can decide whether the + // permanent ASR listening thread should be started. + // + // `new KANTVMgr(...)` + `initASR()` are required for ALL bench types + // because the libkantv-media.so side of the GGML_JNI_NOTIFY event + // pipe is wired by KANTVMgr's constructor (native_setup) and + // initASR() builds the native dispatch state. Without them, tokens, + // perf data, and the "starting media encoding" hint that the C++ + // mtmd / llm code emits via GGML_JNI_NOTIFY never reach Java's + // MyEventListener.onEvent(), so the chat RecyclerView stays empty + // even though the inference completes successfully in native. + // + // `startASR()` is the part that actually starts the permanent + // listen loop. It is the one piece that is safe to gate on + // bench type: only ASR needs the always-on capture thread. LLM / + // MTMD / TTS / realtime-vision bench types don't want it because + // release() then deadlocks waiting for that thread to exit on the + // next send. + private void setupKANTVMgr(boolean startAsrListening) { if (mKANTVMgr != null) { release(); mKANTVMgr = null; @@ -889,7 +952,9 @@ private void initKANTVMgr() { mKANTVMgr = new KANTVMgr(mEventListener); if (mKANTVMgr != null) { mKANTVMgr.initASR(); - mKANTVMgr.startASR(); + if (startAsrListening) { + mKANTVMgr.startASR(); + } } KANTVLog.j(TAG, "KANTVMgr version:" + mKANTVMgr.getMgrVersion()); } catch (KANTVException ex) { @@ -900,6 +965,12 @@ private void initKANTVMgr() { } } + // Backwards-compatible alias used by ASR-only callers that want + // the full pipeline including the always-on listening thread. + private void initKANTVMgr() { + setupKANTVMgr(true); + } + public void release() { if (mKANTVMgr == null) { @@ -1423,6 +1494,20 @@ public void onNothingSelected(AdapterView parent) {} setTextGGMLInfo(m.getName()); } } + // NOTE: no unloadModel() here in the Bench dialog + // OK callback. The original B (proactive unload at + // model-switch time) was rejected because it has a + // UX cost: if the user picks a model, the dialog + // is closed, and then the user backs out of the + // AI Research page (without running any + // inference), the 4GB model has already been + // freed and needs to be reloaded on the next + // session start. The new design is to do the + // unload at Send time only, in runInference() + // (line ~1684), and only if the selected model + // is actually different from what's loaded. + // The C++ ensure_model_loaded() cache-miss path + // is the safety net for any race we miss. } // When the user switches bench types in the dialog we // bring the prompt box into the expected default @@ -1628,6 +1713,51 @@ private void runInference() { } KANTVLog.j(TAG, "exec ggml benchmark: type: " + KANTVAIUtils.getBenchmarkDesc(nBenchmarkIndex) + ", threads:" + nThreadCounts + ", model:" + strModeName); + + // Fallback "B" at Send time: if the model selected in the Bench + // dialog (or in LLM Setting) does not match what's currently + // loaded on the native side, proactively unload the old model + // *before* calling mtmd_inference / llm_inference. The C++ + // cache-miss path will then take the fast new-load branch + // (since loaded_model == nullptr) instead of unloading+loading + // in one shot, which on a 12GB phone was OOM-killing the + // process when switching from Qwen2.5-Omni (3GB) to + // gemma-3-4b Q8 (4.5GB), see project memory. + // + // This is a safety net for cases where the Bench-dialog B + // unload (line 1433) was skipped. Originally the Bench-dialog + // B was guarded by `nBenchmarkIndex == LLM`, so MTMD/ASR bench + // type selections never triggered it. Even after removing + // that guard, there are other ways B can be missed (e.g. a + // dialog Cancel, a stale strModeName from a previous bench + // type, a JNI exception, etc.) - so doing one last check here + // at the actual inference trigger is the safest place. + try { + if (!ggmljava.llm_is_running_state() + && strModeName != null && !strModeName.isEmpty()) { + String currentLoaded = ggmljava.llm_get_loaded_model_path(); + String newModelPath = KANTVUtils.getSDCardDataPath() + strModeName; + if (currentLoaded != null + && !currentLoaded.isEmpty() + && !currentLoaded.equals(newModelPath)) { + KANTVLog.j(TAG, "Send-time fallback: model changed from '" + + currentLoaded + "' to '" + newModelPath + + "', unloading old model proactively"); + ggmljava.unloadModel(); + // Give the kernel a moment to start reclaiming the + // 4GB before the C++ load fires. The 500ms matches + // the in-C++ sleep in ensure_model_loaded's slow + // path so total latency is bounded at ~1s. + try { Thread.sleep(500); } + catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } + } + } + } catch (Exception ex) { + KANTVLog.j(TAG, "Send-time unload fallback failed: " + ex.toString()); + } + String selectModelFilePath = ""; resetUIAndStatus(null, true, true); diff --git a/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/settings/LLMSettingFragment.java b/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/settings/LLMSettingFragment.java index d754efbce..4e41d4efd 100644 --- a/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/settings/LLMSettingFragment.java +++ b/android/kantvplayer/src/main/java/com/kantvai/kantvplayer/ui/fragment/settings/LLMSettingFragment.java @@ -51,6 +51,13 @@ public class LLMSettingFragment extends BaseSettingsFragment { private LLMSettingHeaderPreference mHeaderPreference; + // Tracks the model path that was last seen by the pref.llmmodel + // change listener. The listener fires both for the user's dropdown + // choice AND for the "heal on access" setValue() during fragment + // creation (see line 133), so we need to compare against the last + // seen value to avoid spurious unloadModel() calls. + private String mLastSelectedModelPath = null; + @Override public String getTitle() { @@ -221,6 +228,34 @@ public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, Strin KANTVLog.g(TAG, "LLM model name: " + KANTVAIModelMgr.getInstance().getModelName(mSettings.getLLMModel())); String modelPath = KANTVUtils.getSDCardDataPath() + KANTVAIModelMgr.getInstance().getModelName(mSettings.getLLMModel()); KANTVLog.g(TAG, "modelPath:" + modelPath); + + // If the user actually switched to a *different* model + // (not just the heal-on-access setValue that fires + // during fragment creation) and no inference is + // currently running, unload the model on the native + // side right now. + // + // Why here and not in ensure_model_loaded: the C++ + // cache-miss path also has an unload + 500ms sleep + // (Phase 2.5 / A), so the safety net is in place. + // The point of doing it here is *latency* - by the + // time the user navigates back to AI Research and + // fires an inference, the 4GB has already been + // released and the page-reclaimer has had seconds + // (not milliseconds) to give those pages back to + // the OS. The inference itself then takes the + // cache-miss path's fast new-load branch instead + // of unloading+loading in one go (which used to + // OOM on a 12GB OnePlus, see project memory). + if (mLastSelectedModelPath != null + && !mLastSelectedModelPath.equals(modelPath) + && !ggmljava.llm_is_running_state()) { + KANTVLog.j(TAG, "Model changed from '" + + mLastSelectedModelPath + "' to '" + modelPath + + "', unloading old model proactively"); + ggmljava.unloadModel(); + } + mLastSelectedModelPath = modelPath; } catch (Exception ex) { KANTVLog.g(TAG, "error: " + ex.toString()); KANTVUtils.showMsgBox(mActivity, "error: " + ex.toString()); diff --git a/core/jni/ggml-jni-context.cpp b/core/jni/ggml-jni-context.cpp index 8d86c1f1f..ee57d07ad 100644 --- a/core/jni/ggml-jni-context.cpp +++ b/core/jni/ggml-jni-context.cpp @@ -7,6 +7,9 @@ #include "ggml-cpu.h" #include "llamacpp/ggml/include/ggml-hexagon.h" +#include +#include + // common.h provides common_init() which used to be called at the top of // mtmd_inference_main / llm_inference_main; now it is called once from // ggml_jni_context::init() and must be in scope here. @@ -210,10 +213,43 @@ int ggml_jni_context::ensure_model_loaded(const char * model_path, // Slow path: either nothing is loaded yet, or the key changed. // Unload any old model first so the new load starts from a clean // state. unload_model() is a no-op if loaded_model == nullptr. + // + // The unload also has a forced sleep before the new load: freeing + // a 4GB llama_model gives the C++ heap the memory back, but the + // kernel is under no obligation to release the anonymous pages + // back to the page pool / ION allocator immediately. If we jump + // straight into llama_model_load_from_file (which mmaps the next + // 4GB model file) we end up with peak RSS of OLD_FREEING + + // NEW_MAPPING, which on a 12GB phone is enough to trigger + // OOM-killer (signal 9) before the new model finishes loading + // (this happened on a gemma-3-4b cache miss right after a + // Qwen2.5-Omni audio run, see project memory). + // + // 500ms is a heuristic - long enough for the page reclaimer to + // finish the unmap, short enough that the user doesn't notice. + // malloc_trim(0) below actively nudges glibc to release the freed + // heap back to the OS so the next 4GB allocation doesn't have to + // compete with the just-freed pages. if (loaded_model != nullptr) { LOGGD("ensure_model_loaded: cache miss, unloading previous model '%s' (ngl=%d)", loaded_model_path.c_str(), loaded_ngl); - unload_model(); + // Call the _unlocked variant because this function already holds + // m_model_mutex (line 184). Calling unload_model() here would + // re-acquire the same non-recursive std::mutex and deadlock the + // worker thread forever -- which is exactly what we saw on the + // Qwen2.5-Omni -> gemma-3-4b cache miss after a successful + // audio run: native inference stalled at the first cache-miss + // switch, the Java side never got a "starting media encoding" + // event, and the chat RecyclerView sat on "..." until force-stop. + // 2026-07-28 fix. + unload_model_unlocked(); +#if defined(__GLIBC__) + // glibc-specific: actively release freed heap back to OS. + // No-op on Android bionic (malloc_trim is a stub there). + extern int malloc_trim(size_t); + malloc_trim(0); +#endif + std::this_thread::sleep_for(std::chrono::milliseconds(500)); } // Actually load the model. The full loading sequence is identical to @@ -241,6 +277,44 @@ int ggml_jni_context::ensure_model_loaded(const char * model_path, common_params params; params.model.path = req_model_path; params.n_gpu_layers = n_gpu_layers; + // CRITICAL WARNING (do not repeat the previous fix): + // llama's `llama_load_mode` enum does NOT control whether the + // model file is mmap'd. The actual decision is hardcoded in + // `llama_model_base::load_tensors()` at llama-model.cpp:1252 + // as `const bool use_mmap_buffer = true;` regardless of + // load_mode. So setting `params.load_mode = LLAMA_LOAD_MODE_NONE` + // here does nothing for our peak-RSS problem. + // + // The `load_mode` enum actually only controls: + // LLAMA_LOAD_MODE_MLOCK -> use_mlock = true (force RAM, no swap) + // everything else -> uses the default mmap path + // + // The real peak-RSS driver on a 12GB phone is the 4GB Hexagon + // ION pool that gets allocated unconditionally by + // ggmlhexagon_init_rpcmempool() at ggml-hexagon-jz.cpp:1721 + // during llama_backend_init(), plus the mmap 4GB file + + // separate CPU-side weights copy for a Q8 model. That is + // >8GB, which on this device triggers the Oplus ION + // memory_leak alert (ion=4063MB, threshold=2048MB, see + // 18:17:08-18:17:10 logcat) and SurfaceFlinger removes the + // app's surfaces, freezing the UI. + // + // Practical mitigations to discuss: + // (1) use a Q4_K_M model instead of Q8_0 (cuts the CPU copy + // in half, ~2.5GB vs 4.5GB) + // (2) make Hexagon backend init conditional (e.g. skip + // ggml_backend_hexagon_init_ext() when the inference + // path is MTMD/CPU-only, so the 4GB ION pool is never + // reserved at app startup). This is a deeper change + // and may affect LLM/ASR paths. + // (3) drop the use_mmap_buffer = true hardcode in + // llama-model.cpp to honor a new params flag - this is + // upstream territory, not practical here. + // + // We deliberately do NOT set load_mode here. Leaving the + // default (MMAP) is correct for this llama.cpp version; any + // change would have to be at the hardcode site above. + llama_model_params mparams = common_model_params_to_llama(params); loaded_model = llama_model_load_from_file(req_model_path.c_str(), mparams); if (loaded_model == nullptr) { @@ -255,7 +329,13 @@ int ggml_jni_context::ensure_model_loaded(const char * model_path, // loaded_mctx as nullptr. if (!req_mmproj_path.empty()) { mtmd_context_params mparams = mtmd_context_params_default(); - mparams.use_gpu = false; // see ggml-jni-context.cpp mtmd_inference + // mparams.use_gpu = false to match the "force ngl=0" policy + // applied in mtmd-inference.cpp: the Hexagon DSP backend does + // not fully implement the vision encoder (mmproj) op patterns + // (convolutions + image-shaped attention) and aborts + // intermittently. Keeping the mmproj on the CPU here makes the + // backend choice consistent between the model and the mmproj. + mparams.use_gpu = false; mparams.print_timings = false; mparams.n_threads = std::thread::hardware_concurrency(); loaded_mctx = mtmd_init_from_file(req_mmproj_path.c_str(), @@ -284,6 +364,15 @@ int ggml_jni_context::ensure_model_loaded(const char * model_path, void ggml_jni_context::unload_model() { std::lock_guard lock(m_model_mutex); + unload_model_unlocked(); +} + +// Internal helper. Assumes the caller already holds m_model_mutex (this +// is the case when called from ensure_model_loaded() which is the +// primary call site). Exposed separately so that ensure_model_loaded() +// can call it without re-locking m_model_mutex and deadlocking on the +// non-recursive std::mutex (the original bug). +void ggml_jni_context::unload_model_unlocked() { if (loaded_mctx != nullptr) { LOGGD("unload_model: freeing mmproj context '%s'", loaded_mmproj_path.c_str()); mtmd_free(loaded_mctx); diff --git a/core/jni/ggml-jni-context.h b/core/jni/ggml-jni-context.h index 70cddbf65..106f9cdcd 100644 --- a/core/jni/ggml-jni-context.h +++ b/core/jni/ggml-jni-context.h @@ -89,6 +89,11 @@ class ggml_jni_context { // nothing is loaded (no-op). Called explicitly from the Java side on // model switch (LLMSettingFragment) and from process exit (cleanup()). void unload_model(); + // Internal: same as unload_model() but assumes the caller already + // holds m_model_mutex. Used by ensure_model_loaded() which is + // itself holding the lock; calling unload_model() from there + // would re-acquire a non-recursive std::mutex and deadlock. + void unload_model_unlocked(); bool is_model_loaded() const { return loaded_model != nullptr; } diff --git a/core/jni/mtmd-inference.cpp b/core/jni/mtmd-inference.cpp index a59ba3b4e..e52e94791 100644 --- a/core/jni/mtmd-inference.cpp +++ b/core/jni/mtmd-inference.cpp @@ -244,7 +244,12 @@ int mtmd_inference_main(int argc, char ** argv, int backend_type) { return 5; } // calculate bitmap hash (for KV caching) - std::string hash = fnv_hash(bmp.data(), bmp.nx() * bmp.ny() * 3); + // Use bmp.n_bytes() rather than nx*ny*3: nx*ny*3 is image-specific + // (RGB), but audio bitmaps store PCM f32 in 1D (nx=samples, ny=1, + // data size = nx*sizeof(float)) — the old formula would either + // over-read the buffer (unrelated memory) or compute a hash of + // unrelated bytes, silently breaking audio KV cache tracking. + std::string hash = fnv_hash(bmp.data(), bmp.n_bytes()); bmp.set_id(hash.c_str()); bitmaps.entries.push_back(std::move(bmp)); }