Add Android demo for streaming zipformer transducer ASR with QNN - #3654
Conversation
📝 WalkthroughWalkthroughThis PR introduces end-to-end Qualcomm QNN ASR support for Sherpa-ONNX Android by adding QNN configuration types across Java/Kotlin APIs, JNI bridges, Android app asset handling, system library declarations, build automation scripts with model parametrization, and a GitHub Actions CI/CD workflow for automated APK generation and publishing. ChangesQNN ASR Android Integration
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)sherpa-onnx/jni/online-recognizer.ccsherpa-onnx/jni/online-recognizer.cc:5:10: fatal error: 'sherpa-onnx/csrc/online-recognizer.h' file not found ... [truncated 2200 characters] ... lled from ClangFrontend__CTrans.CTrans_funct.instruction_log.(fun) in file "src/clang/cTrans.ml", line 4784, characters 10-1023 sherpa-onnx/csrc/qnn/utils.ccsherpa-onnx/csrc/qnn/utils.cc:4:10: fatal error: 'sherpa-onnx/csrc/qnn/utils.h' file not found ... [truncated 2200 characters] ... n) in file "src/clang/cTrans.ml", line 4784, characters 10-1023 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Qualcomm QNN HTP backend on Android devices, enabling streaming ASR using QNN models. Key changes include updating the Android manifest to expose the libcdsprpc.so vendor library, adding helper functions to copy assets to internal storage, and updating the JNI and Kotlin APIs to support QNN configurations and models. Feedback on these changes highlights potential null pointer dereferences in the JNI code when handling qnnConfig and new_path. Additionally, there are opportunities to optimize the asset existence check by avoiding AssetManager.list() and to handle empty paths more robustly in the asset copying utility.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fid = env->GetFieldID(transducer_config_cls, "qnnConfig", | ||
| "Lcom/k2fsa/sherpa/onnx/QnnConfig;"); | ||
| jobject qnn_config = env->GetObjectField(transducer_config, fid); | ||
| jclass qnn_config_cls = env->GetObjectClass(qnn_config); | ||
|
|
||
| SHERPA_ONNX_JNI_READ_STRING(ans.transducer.qnn_config.backend_lib, backendLib, | ||
| qnn_config_cls, qnn_config); | ||
|
|
||
| SHERPA_ONNX_JNI_READ_STRING(ans.transducer.qnn_config.context_binary, | ||
| contextBinary, qnn_config_cls, qnn_config); | ||
|
|
||
| SHERPA_ONNX_JNI_READ_STRING(ans.transducer.qnn_config.system_lib, systemLib, | ||
| qnn_config_cls, qnn_config); |
There was a problem hiding this comment.
The JNI code retrieves the qnnConfig object field from transducer_config but does not check if it is nullptr. If qnnConfig is null (e.g., if a user passes a null configuration or from other language bindings), calling env->GetObjectClass(qnn_config) will crash the application. A null check should be added to ensure safety.
fid = env->GetFieldID(transducer_config_cls, "qnnConfig",
"Lcom/k2fsa/sherpa/onnx/QnnConfig;");
jobject qnn_config = env->GetObjectField(transducer_config, fid);
if (qnn_config != nullptr) {
jclass qnn_config_cls = env->GetObjectClass(qnn_config);
SHERPA_ONNX_JNI_READ_STRING(ans.transducer.qnn_config.backend_lib, backendLib,
qnn_config_cls, qnn_config);
SHERPA_ONNX_JNI_READ_STRING(ans.transducer.qnn_config.context_binary,
contextBinary, qnn_config_cls, qnn_config);
SHERPA_ONNX_JNI_READ_STRING(ans.transducer.qnn_config.system_lib, systemLib,
qnn_config_cls, qnn_config);
}| private fun assetExists(assetManager: AssetManager, path: String): Boolean { | ||
| val dir = path.substringBeforeLast('/', "") | ||
| val fileName = path.substringAfterLast('/') | ||
|
|
||
| val files = assetManager.list(dir) ?: return false | ||
| return files.contains(fileName) | ||
| } |
There was a problem hiding this comment.
Using AssetManager.list() to check if a single asset exists is highly inefficient because it lists all files in the directory and performs a linear search. This can cause significant startup delays if there are many assets. A much faster and cleaner approach is to try opening the asset directly using AssetManager.open().
private fun assetExists(assetManager: AssetManager, path: String): Boolean {
if (path.isEmpty()) return false
return try {
assetManager.open(path).use { }
true
} catch (e: Exception) {
false
}
}| private fun copyAssetToInternalStorage(path: String, context: Context): String { | ||
| val targetRoot = context.filesDir | ||
| val outFile = File(targetRoot, path) | ||
|
|
||
| if (!assetExists(context.assets, path = path)) { | ||
| outFile.parentFile?.mkdirs() | ||
| Log.i(TAG, "$path does not exist, return ${outFile.absolutePath}") | ||
| return outFile.absolutePath | ||
| } |
There was a problem hiding this comment.
If path is empty, copyAssetToInternalStorage will attempt to resolve it against context.filesDir, which points to the files directory itself. This results in calling parentFile?.mkdirs() on the app's internal storage root and returning an incorrect path. Adding a guard clause to return early when path is empty prevents this issue.
| private fun copyAssetToInternalStorage(path: String, context: Context): String { | |
| val targetRoot = context.filesDir | |
| val outFile = File(targetRoot, path) | |
| if (!assetExists(context.assets, path = path)) { | |
| outFile.parentFile?.mkdirs() | |
| Log.i(TAG, "$path does not exist, return ${outFile.absolutePath}") | |
| return outFile.absolutePath | |
| } | |
| private fun copyAssetToInternalStorage(path: String, context: Context): String { | |
| if (path.isEmpty()) return path | |
| val targetRoot = context.filesDir | |
| val outFile = File(targetRoot, path) | |
| if (!assetExists(context.assets, path = path)) { | |
| outFile.parentFile?.mkdirs() | |
| Log.i(TAG, "$path does not exist, return ${outFile.absolutePath}") | |
| return outFile.absolutePath | |
| } |
| SHERPA_ONNX_EXTERN_C | ||
| JNIEXPORT void JNICALL | ||
| Java_com_k2fsa_sherpa_onnx_OnlineRecognizer_prependAdspLibraryPath( | ||
| JNIEnv *env, jclass /*cls*/, jstring new_path) { | ||
| const char *p = env->GetStringUTFChars(new_path, nullptr); | ||
| sherpa_onnx::PrependAdspLibraryPath(p); | ||
|
|
||
| env->ReleaseStringUTFChars(new_path, p); | ||
| } |
There was a problem hiding this comment.
If new_path is nullptr, calling env->GetStringUTFChars will cause a crash. Adding a null check for new_path is a recommended defensive programming practice to prevent native crashes.
SHERPA_ONNX_EXTERN_C
JNIEXPORT void JNICALL
Java_com_k2fsa_sherpa_onnx_OnlineRecognizer_prependAdspLibraryPath(
JNIEnv *env, jclass /*cls*/, jstring new_path) {
if (!new_path) {
return;
}
const char *p = env->GetStringUTFChars(new_path, nullptr);
if (p) {
sherpa_onnx::PrependAdspLibraryPath(p);
env->ReleaseStringUTFChars(new_path, p);
}
}There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt (1)
252-252:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe demo still boots the non-QNN model by default.
typeis hardcoded to0, so this activity never reaches the newprovider == "qnn"initialization path. As written, the “QNN demo” requires editing source before it actually exercises any QNN code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt` at line 252, The code hardcodes val type = 0 which prevents taking the provider == "qnn" path; replace the hardcoded assignment so the activity reads the desired provider at runtime (e.g., from an Intent extra, a runtime setting, or a build flag) and sets the same variable used in the provider check accordingly. Locate the val type declaration in MainActivity (the code that later checks provider == "qnn") and change it to derive its value from intent.getStringExtra("provider") or a UI toggle/BuildConfig flag, then ensure the provider comparison uses that runtime value so the QNN initialization branch can execute without editing source.
🧹 Nitpick comments (1)
sherpa-onnx/csrc/qnn/utils.cc (1)
21-93: ⚡ Quick winConsider applying the same pattern to other enum-to-string functions for consistency.
The explicit
default:case inQuantizationEncodingToStringis good practice. However, the other similar functions (TensorTypeToString,TensorDataTypeToString,TensorMemTypeToString) still rely on a return statement after the switch. Applying the same pattern across all four functions would improve consistency.Additionally, note that
TensorDataTypeToStringreturns"unknown"(lowercase) at line 82, while others return"Unknown"(capitalized).♻️ Proposed refactor for consistency
Update
TensorTypeToString:SHERPA_ONNX_TO_STRING(QNN_TENSOR_TYPE_OPTIONAL_APP_READWRITE); SHERPA_ONNX_TO_STRING(QNN_TENSOR_TYPE_UNDEFINED); + default: + return "Unknown"; } - return "Unknown"; }Update
TensorDataTypeToString:SHERPA_ONNX_TO_STRING(QNN_DATATYPE_BOOL_8); SHERPA_ONNX_TO_STRING(QNN_DATATYPE_STRING); SHERPA_ONNX_TO_STRING(QNN_DATATYPE_UNDEFINED); + default: + return "Unknown"; } - return "unknown"; }Update
TensorMemTypeToString:SHERPA_ONNX_TO_STRING(QNN_TENSORMEMTYPE_MEMHANDLE); SHERPA_ONNX_TO_STRING(QNN_TENSORMEMTYPE_RETRIEVE_RAW); SHERPA_ONNX_TO_STRING(QNN_TENSORMEMTYPE_UNDEFINED); + default: + return "Unknown"; } - return "Unknown"; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sherpa-onnx/csrc/qnn/utils.cc` around lines 21 - 93, The three functions TensorTypeToString, TensorDataTypeToString, and TensorMemTypeToString should follow the same pattern used in QuantizationEncodingToString: add an explicit default: case inside each switch that returns the standardized string "Unknown" (capitalized) and remove the trailing return after the switch (or make it unreachable), and fix TensorDataTypeToString to return "Unknown" instead of "unknown"; update the switch blocks in TensorTypeToString, TensorDataTypeToString, and TensorMemTypeToString accordingly so all enum-to-string helpers are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/apk-qnn-asr.yaml:
- Line 34: The workflow uses mutable action tags (e.g., actions/checkout@v4,
actions/setup-java@v4, hendrikmuhs/ccache-action@v1.2,
actions/upload-artifact@v4, r0adkll/sign-android-release@v1,
nick-fields/retry@v3); update each `uses:` entry to point to the corresponding
full 40-character commit SHA instead of the tag so the workflow is pinned to
immutable commits (replace occurrences of actions/checkout@v4,
actions/setup-java@v4, hendrikmuhs/ccache-action@v1.2,
actions/upload-artifact@v4, r0adkll/sign-android-release@v1,
nick-fields/retry@v3 with their respective full commit SHAs).
- Around line 15-16: The workflow currently grants repo write access via
"permissions: contents: write" and leaves checkout credentials persisted; change
the workflow-level permissions to the least privilege needed (e.g., remove or
set "contents: read" / minimal permissions since you only publish to Hugging
Face with HF_TOKEN) and update the checkout step (actions/checkout@v4) to
include "persist-credentials: false" so the write-capable token is not left
available to later steps; ensure HF_TOKEN is used only for the Hugging Face
publish step and no steps require repo write permissions.
In `@android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt`:
- Around line 42-46: The current helper silently returns outFile.absolutePath
when assetExists(...) is false, which masks missing required assets; modify the
helper (the function that checks assetExists(context.assets, path) and writes to
outFile) to accept a mustExist (or isOptional) boolean parameter and, for
mustExist==true, throw or return an error (or null) instead of returning
outFile.absolutePath so callers can fail fast; update callers that copy required
inputs (tokens.txt, libencoder.so, libdecoder.so, libjoiner.so) to call the
helper with mustExist=true and only allow mustExist=false for the generated
contextBinary path so missing optional assets still follow the old behavior.
In `@scripts/apk/build-apk-qnn-asr.sh.in`:
- Around line 30-46: The script downloads and extracts third-party archives
(qnn-include-2.40.0.251030.tar.bz2, qnn-libs-2.40.0.251030.tar.bz2 and
${model_name}.tar.bz2) without verifying integrity; update
build-apk-qnn-asr.sh.in to fetch an accompanying checksum or signature (e.g.
.sha256 or .asc) for each archive and verify it before extracting, aborting the
script on mismatch; implement verification steps around the curl+tar sequences
that set QNN_SDK_ROOT and the later qnn-libs/model extraction, and ensure the
check logic returns non-zero and logs an error if verification fails so no
unverified asset is used in packaging.
- Around line 72-74: The script uses a destructive `git checkout .` which will
wipe unrelated local changes; instead update the cleanup steps to only restore
the specific files and paths the script mutates (e.g. the MainActivity.kt edited
via sed in pushd android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx and
any copied assets/libs touched later around the same block and lines
referenced), or switch the build to a temporary worktree; replace the global
reset with targeted restores (use git restore or equivalent for MainActivity.kt
and explicit removals for copied assets/libs) and remove the `git checkout .`
occurrences (also where similar resets appear later at lines 97-98) so unrelated
tracked modifications are preserved.
In
`@sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.java`:
- Around line 64-66: In setQnnConfig(QnnConfig qnnConfig) ensure you never
accept null: either validate and throw IllegalArgumentException when qnnConfig
is null or replace a null argument with a default instance (e.g.
QnnConfig.builder().build()) before assigning to the builder field; update the
Builder.setQnnConfig method (and constructor/field initialization in
OnlineTransducerModelConfig.Builder if needed) so the builder's qnnConfig is
guaranteed non-null to avoid null reaching GetOnlineModelConfig()/GetObjectClass
in JNI.
---
Outside diff comments:
In `@android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt`:
- Line 252: The code hardcodes val type = 0 which prevents taking the provider
== "qnn" path; replace the hardcoded assignment so the activity reads the
desired provider at runtime (e.g., from an Intent extra, a runtime setting, or a
build flag) and sets the same variable used in the provider check accordingly.
Locate the val type declaration in MainActivity (the code that later checks
provider == "qnn") and change it to derive its value from
intent.getStringExtra("provider") or a UI toggle/BuildConfig flag, then ensure
the provider comparison uses that runtime value so the QNN initialization branch
can execute without editing source.
---
Nitpick comments:
In `@sherpa-onnx/csrc/qnn/utils.cc`:
- Around line 21-93: The three functions TensorTypeToString,
TensorDataTypeToString, and TensorMemTypeToString should follow the same pattern
used in QuantizationEncodingToString: add an explicit default: case inside each
switch that returns the standardized string "Unknown" (capitalized) and remove
the trailing return after the switch (or make it unreachable), and fix
TensorDataTypeToString to return "Unknown" instead of "unknown"; update the
switch blocks in TensorTypeToString, TensorDataTypeToString, and
TensorMemTypeToString accordingly so all enum-to-string helpers are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 843eb1c9-8b1e-41e2-b1cb-4d25f7c7bb00
📒 Files selected for processing (11)
.github/workflows/apk-qnn-asr.yamlandroid/SherpaOnnx/app/src/main/AndroidManifest.xmlandroid/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.ktandroid/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/QnnConfig.ktscripts/apk/build-apk-qnn-asr.sh.inscripts/apk/generate-qnn-asr-apk-script.pysherpa-onnx/csrc/qnn/utils.ccsherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.javasherpa-onnx/jni/online-recognizer.ccsherpa-onnx/jni/sherpa-onnx-symbols.expsherpa-onnx/kotlin-api/OnlineRecognizer.kt
| permissions: | ||
| contents: write |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/apk-qnn-asr.yaml"
echo "== File = $FILE =="
ls -l "$FILE"
echo
echo "== Permissions block =="
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,80p'
echo
echo "== Checkout config lines (approx 20-80) =="
rg -n "actions/checkout|persist-credentials|permissions:" "$FILE" || true
echo
echo "== GITHUB_TOKEN / GitHub write operations =="
rg -n "GITHUB_TOKEN|github\\.com|repos/|create-release|actions:|releases|tags|git push|octokit|gh " "$FILE" || true
echo
echo "== Git commands (git clone/push) =="
rg -n "git (clone|push|remote add|config)" "$FILE" || true
echo
echo "== Context around git push =="
rg -n "git push" "$FILE" || true
# show around each match
for m in $(rg -n "git push" "$FILE" | cut -d: -f1); do
echo "---- around git push at line $m ----"
start=$((m-15)); end=$((m+25))
sed -n "${start},${end}p" "$FILE" | nl -ba | sed -n "$((m-start+1-2)),$((m-start+1+25+2))p" || true
doneRepository: k2-fsa/sherpa-onnx
Length of output: 281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/apk-qnn-asr.yaml"
echo "== File = $FILE =="
ls -l "$FILE"
echo
echo "== Permissions block =="
sed -n '1,120p' "$FILE" | awk '{printf "%6d %s\n", NR, $0}'
echo
echo "== Checkout config lines (approx 1-200) =="
rg -n "actions/checkout|persist-credentials|permissions:" "$FILE" || true
echo
echo "== GITHUB_TOKEN / GitHub write operations (1-250) =="
rg -n "GITHUB_TOKEN|github\.com|create-release|releases|tags|git push|gh |octokit|actions:" "$FILE" || true
echo
echo "== Git commands (clone/push/remote/config) (1-250) =="
rg -n "git (clone|push|remote add|config)" "$FILE" || true
echo
echo "== Context around each 'git push' match =="
push_lines=$(rg -n "git push" "$FILE" | cut -d: -f1 || true)
if [ -z "${push_lines}" ]; then
echo "(no 'git push' found)"
else
while read -r m; do
[ -z "$m" ] && continue
start=$((m-15)); end=$((m+25))
echo "---- around git push at line $m ----"
sed -n "${start},${end}p" "$FILE" | awk '{printf "%6d %s\n", NR+('$start'-1), $0}'
done <<< "$push_lines"
fiRepository: k2-fsa/sherpa-onnx
Length of output: 5590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/apk-qnn-asr.yaml"
echo "== File tail (120-end) =="
wc -l "$FILE"
tail -n 120 "$FILE" | awk '{printf "%6d %s\n", NR, $0}'
echo
echo "== Remaining checkout/permissions overrides =="
rg -n "permissions:|actions/checkout@|persist-credentials" "$FILE" || true
echo
echo "== Any explicit GitHub token/release creation in whole file =="
rg -n "GITHUB_TOKEN|create-release|releases|tags|gh |octokit|actions:" "$FILE" || trueRepository: k2-fsa/sherpa-onnx
Length of output: 4841
Drop GitHub write access and disable persisted checkout credentials.
.github/workflows/apk-qnn-asr.yaml sets workflow-level permissions: contents: write, but it only publishes to Hugging Face via HF_TOKEN (no steps push back to GitHub). Also, actions/checkout@v4 doesn’t set persist-credentials: false, so the write-capable token can remain in the repo for later steps/actions.
Minimal tightening
permissions:
- contents: write
+ contents: read
@@
- uses: actions/checkout@v4
with:
fetch-depth: 0
+ persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/apk-qnn-asr.yaml around lines 15 - 16, The workflow
currently grants repo write access via "permissions: contents: write" and leaves
checkout credentials persisted; change the workflow-level permissions to the
least privilege needed (e.g., remove or set "contents: read" / minimal
permissions since you only publish to Hugging Face with HF_TOKEN) and update the
checkout step (actions/checkout@v4) to include "persist-credentials: false" so
the write-capable token is not left available to later steps; ensure HF_TOKEN is
used only for the Hugging Face publish step and no steps require repo write
permissions.
| index: ["0"] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '^\s*-\s*uses:\s+[^@]+@([^0-9a-f]{40}|[0-9a-f]{1,39})\s*$' .github/workflows/apk-qnn-asr.yamlRepository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/apk-qnn-asr.yaml"
echo "== File = $FILE =="
rg -n '^\s*-\s*uses:\s+' "$FILE" || true
echo
echo "== Non-SHA refs (anything not 40-hex) =="
rg -n '^\s*-\s*uses:\s+[^@]+@(?!(?:[0-9a-f]{40})$)[^\s]+' "$FILE" || true
echo
echo "== SHA refs =="
rg -n '^\s*-\s*uses:\s+[^@]+@[0-9a-f]{40}$' "$FILE" || trueRepository: k2-fsa/sherpa-onnx
Length of output: 631
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/apk-qnn-asr.yaml"
echo "== Total uses occurrences =="
rg -n 'uses:' "$FILE" || true
echo
echo "== Pinned to 40-hex SHAs? =="
rg -n '^\s*-\s*uses:\s+[^@]+@[0-9a-f]{40}\s*$' "$FILE" || true
echo
echo "== Non-SHA action refs (not 40-hex at end) =="
rg -n '^\s*-\s*uses:\s+[^@]+@\S+\s*$' "$FILE" || trueRepository: k2-fsa/sherpa-onnx
Length of output: 595
Pin all GitHub Actions uses: to full commit SHAs.
This workflow produces signed/uploaded APK artifacts, but it uses mutable action tags (@v4, @v1, @v1.2, @v3), e.g.:
actions/checkout@v4(34)actions/setup-java@v4(44)hendrikmuhs/ccache-action@v1.2(50)actions/upload-artifact@v4(85)r0adkll/sign-android-release@v1(106)nick-fields/retry@v3(141)
Pin each uses: entry to an immutable 40-character commit SHA.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 34-36: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 34-34: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/apk-qnn-asr.yaml at line 34, The workflow uses mutable
action tags (e.g., actions/checkout@v4, actions/setup-java@v4,
hendrikmuhs/ccache-action@v1.2, actions/upload-artifact@v4,
r0adkll/sign-android-release@v1, nick-fields/retry@v3); update each `uses:`
entry to point to the corresponding full 40-character commit SHA instead of the
tag so the workflow is pinned to immutable commits (replace occurrences of
actions/checkout@v4, actions/setup-java@v4, hendrikmuhs/ccache-action@v1.2,
actions/upload-artifact@v4, r0adkll/sign-android-release@v1,
nick-fields/retry@v3 with their respective full commit SHAs).
| if (!assetExists(context.assets, path = path)) { | ||
| outFile.parentFile?.mkdirs() | ||
| Log.i(TAG, "$path does not exist, return ${outFile.absolutePath}") | ||
| return outFile.absolutePath | ||
| } |
There was a problem hiding this comment.
Don't treat missing required assets as success.
Returning outFile.absolutePath when the asset is absent is only valid for first-run generated QNN context binaries. Reusing the same helper for required inputs like tokens.txt, libencoder.so, libdecoder.so, and libjoiner.so hides packaging mistakes and defers the failure to native init with a much less actionable error. Split required vs. optional copies, or add a mustExist flag and use the optional path only for contextBinary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt`
around lines 42 - 46, The current helper silently returns outFile.absolutePath
when assetExists(...) is false, which masks missing required assets; modify the
helper (the function that checks assetExists(context.assets, path) and writes to
outFile) to accept a mustExist (or isOptional) boolean parameter and, for
mustExist==true, throw or return an error (or null) instead of returning
outFile.absolutePath so callers can fail fast; update callers that copy required
inputs (tokens.txt, libencoder.so, libdecoder.so, libjoiner.so) to call the
helper with mustExist=true and only allow mustExist=false for the generated
contextBinary path so missing optional assets still follow the old behavior.
| curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models-qnn/qnn-include-2.40.0.251030.tar.bz2 | ||
| tar xf qnn-include-2.40.0.251030.tar.bz2 | ||
| rm qnn-include-2.40.0.251030.tar.bz2 | ||
| ls -lh qnn-include-2.40.0.251030 | ||
|
|
||
| export QNN_SDK_ROOT=$PWD/qnn-include-2.40.0.251030 | ||
|
|
||
| log "====================arm64-v8a=================" | ||
| ./build-android-arm64-v8a.sh | ||
|
|
||
| cp -v ./build-android-arm64-v8a/install/lib/*.so ./android/SherpaOnnx/app/src/main/jniLibs/arm64-v8a/ | ||
|
|
||
| log "=======Download qnn libs============" | ||
| curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models-qnn/qnn-libs-2.40.0.251030.tar.bz2 | ||
| tar xvf qnn-libs-2.40.0.251030.tar.bz2 | ||
| rm qnn-libs-2.40.0.251030.tar.bz2 | ||
| cp -v qnn-libs-2.40.0.251030/*.so ./android/SherpaOnnx/app/src/main/jniLibs/arm64-v8a/ |
There was a problem hiding this comment.
Verify downloaded QNN and model archives before extracting them.
These curl | tar paths trust mutable release assets with no checksum or signature check, and the workflow later signs and publishes the resulting APKs. A replaced asset here becomes a signed binary distribution.
Suggested hardening
-curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models-qnn/qnn-include-2.40.0.251030.tar.bz2
-tar xf qnn-include-2.40.0.251030.tar.bz2
+curl -fSL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models-qnn/qnn-include-2.40.0.251030.tar.bz2
+echo "<sha256> qnn-include-2.40.0.251030.tar.bz2" | sha256sum -c -
+tar xf qnn-include-2.40.0.251030.tar.bz2Apply the same pattern to qnn-libs-2.40.0.251030.tar.bz2 and ${model_name}.tar.bz2.
Also applies to: 61-62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/apk/build-apk-qnn-asr.sh.in` around lines 30 - 46, The script
downloads and extracts third-party archives (qnn-include-2.40.0.251030.tar.bz2,
qnn-libs-2.40.0.251030.tar.bz2 and ${model_name}.tar.bz2) without verifying
integrity; update build-apk-qnn-asr.sh.in to fetch an accompanying checksum or
signature (e.g. .sha256 or .asc) for each archive and verify it before
extracting, aborting the script on mismatch; implement verification steps around
the curl+tar sequences that set QNN_SDK_ROOT and the later qnn-libs/model
extraction, and ensure the check logic returns non-zero and logs an error if
verification fails so no unverified asset is used in packaging.
| git checkout . | ||
| pushd android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx | ||
| sed -i.bak s/"type = 0/type = $type/" ./MainActivity.kt |
There was a problem hiding this comment.
Avoid git checkout . here; it will wipe unrelated local work.
This script is documented for local use, and both resets discard every tracked modification in the repo, not just the temporary edits from this build. Restore only the files this script changes, or build from a temp worktree instead.
Safer cleanup sketch
-git checkout .
+git restore -- \
+ android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx/MainActivity.kt \
+ android/SherpaOnnx/gradle.propertiesIf you also want to clean copied assets/libs, remove those paths explicitly instead of resetting the whole tree.
Also applies to: 97-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/apk/build-apk-qnn-asr.sh.in` around lines 72 - 74, The script uses a
destructive `git checkout .` which will wipe unrelated local changes; instead
update the cleanup steps to only restore the specific files and paths the script
mutates (e.g. the MainActivity.kt edited via sed in pushd
android/SherpaOnnx/app/src/main/java/com/k2fsa/sherpa/onnx and any copied
assets/libs touched later around the same block and lines referenced), or switch
the build to a temporary worktree; replace the global reset with targeted
restores (use git restore or equivalent for MainActivity.kt and explicit
removals for copied assets/libs) and remove the `git checkout .` occurrences
(also where similar resets appear later at lines 97-98) so unrelated tracked
modifications are preserved.
| public Builder setQnnConfig(QnnConfig qnnConfig) { | ||
| this.qnnConfig = qnnConfig; | ||
| return this; |
There was a problem hiding this comment.
Reject null in setQnnConfig().
A Java caller can currently do setQnnConfig(null), but GetOnlineModelConfig() later unconditionally calls GetObjectClass(qnn_config) in JNI. That turns a nullable builder input into a native crash during recognizer creation. Keep this builder field non-null or fall back to QnnConfig.builder().build().
Suggested fix
+import java.util.Objects;
+
public Builder setQnnConfig(QnnConfig qnnConfig) {
- this.qnnConfig = qnnConfig;
+ this.qnnConfig = Objects.requireNonNull(qnnConfig, "qnnConfig");
return this;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Builder setQnnConfig(QnnConfig qnnConfig) { | |
| this.qnnConfig = qnnConfig; | |
| return this; | |
| import java.util.Objects; | |
| public Builder setQnnConfig(QnnConfig qnnConfig) { | |
| this.qnnConfig = Objects.requireNonNull(qnnConfig, "qnnConfig"); | |
| return this; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.java`
around lines 64 - 66, In setQnnConfig(QnnConfig qnnConfig) ensure you never
accept null: either validate and throw IllegalArgumentException when qnnConfig
is null or replace a null argument with a default instance (e.g.
QnnConfig.builder().build()) before assigning to the builder field; update the
Builder.setQnnConfig method (and constructor/field initialization in
OnlineTransducerModelConfig.Builder if needed) so the builder's qnnConfig is
guaranteed non-null to avoid null reaching GetOnlineModelConfig()/GetObjectClass
in JNI.
You can download pre-built APKs at
from https://huggingface.co/csukuangfj2/sherpa-onnx-apk/tree/main/qnn-asr/1.13.2
cc @ZhangWei125521
Summary by CodeRabbit