pushlish java-api to maven central #2440
Conversation
|
Warning Rate limit exceeded@litongjava has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis update introduces several new configuration and documentation files to the Changes
Sequence Diagram(s)Not applicable: Changes primarily involve configuration, documentation, validation additions, resource management refactoring, and native library loading utility introduction without new control flow or feature interactions. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
sherpa-onnx/java-api/Makefile (1)
121-124: Rule misses nested package dirsPattern rule assumes all
.javalive exactly one level below$(package_dir); nested sub-packages (e.g.,util/Helper.java) won’t compile.
Usejavac’s ability to ingest source file list instead:$(out_jar): $(java_files) @mkdir -p $(out_dir) javac -d $(out_dir) $(java_files) jar cvf $@ -C $(out_dir) .
🧹 Nitpick comments (20)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/EndpointRule.java (2)
30-32: Rename boolean getter to follow JavaBean conventionUsing
getMustContainNonSilence()for a boolean slightly deviates from the commonis*prefix convention (isMustContainNonSilence()).
Renaming improves interoperability with frameworks/tools that rely on the JavaBean spec for property introspection.
39-41: Add basic value validation inBuilder.build()
minTrailingSilenceandminUtteranceLengthlogically cannot be negative. Enforcing this early prevents invalid objects from being instantiated.public EndpointRule build() { - return new EndpointRule(this); + if (minTrailingSilence < 0 || minUtteranceLength < 0) { + throw new IllegalArgumentException( + "minTrailingSilence and minUtteranceLength must be non-negative"); + } + return new EndpointRule(this); }sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineLMConfig.java (1)
32-44: Consider adding basic argument validation insideBuilder.build()
Right now any value is accepted, including an emptymodelstring or a non-positivescale, both of which will likely yield runtime errors further downstream. A couple of cheap null / range checks here would fail-fast and surface misconfiguration closer to the call site.public OnlineLMConfig build() { - return new OnlineLMConfig(this); + // Fail-fast validation + if (model == null || model.isEmpty()) { + throw new IllegalArgumentException("model path must be non-empty"); + } + if (scale <= 0) { + throw new IllegalArgumentException("scale must be positive"); + } + return new OnlineLMConfig(this); }Verify that similar config builders across the codebase perform (or intentionally omit) such checks for consistency.
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.java (2)
34-37: Prefernulldefault over empty string or require explicit setUsing
""as a sentinel value makes it impossible to distinguish “unset” from “set to empty”, and forces the check above. Consider:- private String encoder = ""; - private String decoder = ""; - private String joiner = ""; + private String encoder; + private String decoder; + private String joiner;(Combined with the validation in
build()this keeps the API safe and intention-revealing.)
6-32: Consider overridingtoString(),equals(),hashCode()
OnlineTransducerModelConfigwill likely be logged, compared, or stored in collections.
Overriding these methods improves debuggability and correct behaviour in hash-based containers.
Simple implementation viaObjects.hash()is enough.sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.java (2)
6-16: Mark the configuration classfinalto guarantee immutability
FeatureConfigis designed to be immutable, yet it is still inheritable. Making the classfinal(or providing a private constructor plus a final class) prevents accidental subclassing that could break invariants or introduce mutability.-public class FeatureConfig { +public final class FeatureConfig {
34-37: Expose default constants instead of repeating literalsThe defaults (
16000,80,0.0f) are duplicated in tests or downstream code more easily if surfaced as publicstatic finalconstants. This avoids “magic numbers” and helps keep the defaults in a single place.public static class Builder { - private int sampleRate = 16000; - private int featureDim = 80; - private float dither = 0.0f; + public static final int DEFAULT_SAMPLE_RATE = 16000; + public static final int DEFAULT_FEATURE_DIM = 80; + public static final float DEFAULT_DITHER = 0.0f; + + private int sampleRate = DEFAULT_SAMPLE_RATE; + private int featureDim = DEFAULT_FEATURE_DIM; + private float dither = DEFAULT_DITHER;sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineParaformerModelConfig.java (2)
27-44: Builder could expose clearer “withX” methods & stronger typing
- Naming the mutators
setEncoder/setDecoderinside a builder is slightly misleading—withEncoder/withDecoder(orencoder()/decoder()) is the common convention.- These are filesystem paths; consider
java.nio.file.Pathinstead of rawStringto gain compile-time safety and platform independence.No functional change is required, but adopting these adjustments will improve API ergonomics and reduce misuse.
6-26: OverridetoString,equals, andhashCodefor better debuggability & collection support
This config object is immutable and will likely be logged, compared, or stored in collections. Implementing the standard trio will aid diagnostics and prevent subtle bugs when used as map keys or set elements.public class OnlineParaformerModelConfig { private final String encoder; private final String decoder; + @Override + public String toString() { + return "OnlineParaformerModelConfig{encoder='" + encoder + "', decoder='" + decoder + "'}"; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof OnlineParaformerModelConfig)) return false; + OnlineParaformerModelConfig that = (OnlineParaformerModelConfig) o; + return encoder.equals(that.encoder) && decoder.equals(that.decoder); + } + + @Override + public int hashCode() { + return java.util.Objects.hash(encoder, decoder); + }sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerConfig.java (2)
7-22: Expose the remaining configuration fields or document why they are intentionally hiddenOnly
getModelConfig()is public, while callers have no accessor for the other 11 immutable fields.
Unless the intent is to keep those details internal, provide getters (ortoString()/equals/hashCode) so that downstream code can inspect, log, or compare full configs.+ public FeatureConfig getFeatConfig() { return featConfig; } + public OnlineLMConfig getLmConfig() { return lmConfig; } + public OnlineCtcFstDecoderConfig getCtcFstDecoderConfig() { return ctcFstDecoderConfig; } + public EndpointConfig getEndpointConfig() { return endpointConfig; } + public HomophoneReplacerConfig getHr() { return hr; } + public boolean isEnableEndpoint() { return enableEndpoint; } + public String getDecodingMethod() { return decodingMethod; } + public int getMaxActivePaths() { return maxActivePaths; } + public String getHotwordsFile() { return hotwordsFile; } + public float getHotwordsScore() { return hotwordsScore; } + public String getRuleFsts() { return ruleFsts; } + public String getRuleFars() { return ruleFars; } + public float getBlankPenalty() { return blankPenalty; }
93-97: Renamehrto a clearer field name
hris cryptic compared to the otherwise descriptive field names.
ConsiderhomophoneReplacerConfigto improve readability.- private HomophoneReplacerConfig hr = HomophoneReplacerConfig.builder().build(); + private HomophoneReplacerConfig homophoneReplacerConfig = HomophoneReplacerConfig.builder().build();(and adjust usages accordingly)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java (2)
7-7: Clarify thread-safety of the native pointer
ptris mutated byrelease()and read by every public method, yet it is notvolatileand no synchronization is used. If anOnlineRecognizerinstance is accessed from multiple threads (for example, decoding while another thread closes it), visibility issues or use-after-free native crashes become possible.
Consider either documenting single-threaded usage, guarding all accesses with a lock, or makingptrvolatileand using an atomic “closed” flag.
57-63: Avoid unchecked casts from rawObject[]
Returning a rawObject[]from JNI forces multiple unchecked casts and hides type mismatches until runtime. Wrap the JNI layer to return a dedicated DTO or unpack in native code and return a strongly-typed Java object directly.sherpa-onnx/java-api/.build.txt (1)
1-17: Hard-coded JDK paths & ancient 8u121 limit portabilityHard-wiring
JAVA_HOMEto…/jdk1.8.0_121means the script will break on systems without that exact path and prevents use of a patched JDK.
Suggest:-[win.env] -set JAVA_HOME=D:\java\jdk1.8.0_121 +[win.env] +REM Override if JAVA_HOME already present +IF NOT DEFINED JAVA_HOME ( + set "JAVA_HOME=%ProgramFiles%\Java\jdk1.8.0_**latest**" +)Repeat for Linux/macOS (
${JAVA_HOME:-/usr/lib/jvm/java-1.8-openjdk}etc.).
Also drop-DskipTestsonce unit tests land and remove-Dgpg.skip=truein CI so artifacts are signed for Central.sherpa-onnx/java-api/Makefile (1)
7-105: Manual file list is brittle – generate itMaintaining ~100 filenames by hand is error-prone and new classes will silently be excluded.
-# Hard-coded list … -java_files := LibraryLoader.java -… +# Automatically collect all .java under src/main/java +java_files := $(shell find src/main/java/$(package_dir) -name '*.java')This eliminates duplicate bookkeeping and matches the Maven layout you just adopted.
sherpa-onnx/java-api/.gitignore (1)
5-5: Eliminate duplicate ignore entries
classes/,local.properties, andbuild/each appear twice. Duplicates add visual clutter and make future edits harder to reason about.-classes/ ... -local.properties ... -build/Remove the second occurrence of each to keep the file concise.
Also applies to: 117-117, 12-12, 140-140, 93-93, 144-144
sherpa-onnx/java-api/readme.md (4)
3-3: Use a proper heading rather than italic text
*Applicable to Windows / macOS / Linux ...*is meant as a section title, not emphasis.
Replace with a heading for better structure and to satisfy markdown-lint MD036.-*Applicable to Windows / macOS / Linux (Windows used as example for dynamic library loading)* +### Applicability +Windows / macOS / Linux +(Windows is used below when showing dynamic-library examples)
61-63: Add a language specifier to fenced blockThis code block triggers markdown-lint MD040. Prefix it with
sh(orbaton Windows) so tooling knows how to highlight it.-``` +```sh -Djava.library.path=path\to\directory\containing\sherpa-onnx-jni.dll +java -Djava.library.path=path\to\directory\containing\sherpa-onnx-jni.dll \ + -jar your-app.jar
97-100: Specify language for bare-URL code block-``` +```text https://k2-fsa.github.io/sherpa/onnx/tts/pretrained_models/kokoro.htmlPrevents MD040 and renders consistently across viewers.
174-177: Follow Java naming conventions
real_time_factoruses snake_case, which is uncommon in Java. Consider camelCase for consistency with the rest of the snippet.-float real_time_factor = timeElapsedSeconds / audioDuration; +float realTimeFactor = timeElapsedSeconds / audioDuration; ... -System.out.printf("-- real-time factor (RTF): %.3f\n", real_time_factor); +System.out.printf("-- real-time factor (RTF): %.3f\n", realTimeFactor);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
sherpa-onnx/java-api/.build.txt(1 hunks)sherpa-onnx/java-api/.gitignore(1 hunks)sherpa-onnx/java-api/Makefile(2 hunks)sherpa-onnx/java-api/pom.xml(1 hunks)sherpa-onnx/java-api/readme.md(1 hunks)sherpa-onnx/java-api/readme.zh.md(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/EndpointConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/EndpointRule.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineLMConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineModelConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineParaformerModelConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/LibraryLoader.java (1)
LibraryLoader(3-23)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.java (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/LibraryLoader.java (1)
LibraryLoader(3-23)
🪛 markdownlint-cli2 (0.17.2)
sherpa-onnx/java-api/readme.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
190-190: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (3)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.java (1)
9-16: Library may remain unloaded for thelong ptr-based ctorThe no-arg constructor ensures the JNI library is loaded (
LibraryLoader.maybeLoad()), but the(long ptr)constructor omits the call.
If user code instantiatesOnlineStreamdirectly with a native pointer before anything else has triggeredLibraryLoader,UnsatisfiedLinkErrorwill occur when the first native method is invoked.Either always call
LibraryLoader.maybeLoad()in a static block, or invoke it from the second constructor as well.sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineModelConfig.java (1)
6-152: No functional changes detected – looks goodThe class remains a straightforward immutable DTO + builder; no behavioural regressions introduced.
sherpa-onnx/java-api/readme.zh.md (1)
1-200: LGTM – valuable, detailed Chinese guideClear instructions, examples, and troubleshooting steps — great addition.
sherpa-onnx/java-api/pom.xml
Outdated
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <groupId>com.litongjava</groupId> | ||
| <artifactId>sherpa-onnx-java-api</artifactId> | ||
| <version>1.0.0</version> | ||
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <java.version>1.8</java.version> | ||
| <maven.compiler.source>${java.version}</maven.compiler.source> | ||
| <maven.compiler.target>${java.version}</maven.compiler.target> | ||
| </properties> | ||
| </project> |
There was a problem hiding this comment.
Blocking: POM is not Maven-Central compliant & groupId diverges from package namespace
To deploy to Central you must supply the mandatory metadata sections (<name>, <description>, <url>, <licenses>, <scm>, <developers>).
Additionally, the Java sources live under com.k2fsa.*, while the artifact is published under com.litongjava; this mismatch will confuse consumers and clashes with the reverse-DNS convention.
<artifactId>sherpa-onnx-java-api</artifactId>
<version>1.0.0</version>
+ <name>Sherpa-ONNX Java API</name>
+ <description>Java bindings for Sherpa-ONNX streaming/offline ASR & TTS</description>
+ <url>https://github.com/k2-fsa/sherpa-onnx</url>
+
+ <licenses>
+ <license>
+ <name>Apache License, Version 2.0</name>
+ <url>https://www.apache.org/licenses/LICENSE-2.0</url>
+ </license>
+ </licenses>
+
+ <scm>
+ <connection>scm:git:git://github.com/k2-fsa/sherpa-onnx.git</connection>
+ <developerConnection>scm:git:ssh://github.com:k2-fsa/sherpa-onnx.git</developerConnection>
+ <url>https://github.com/k2-fsa/sherpa-onnx</url>
+ </scm>
+
+ <developers>
+ <developer>
+ <id>litongjava</id>
+ <name>Li Tong</name>
+ </developer>
+ </developers>Consider changing <groupId> to com.k2fsa (or relocating the packages) so the coordinates and byte-code agree.
Failing to add the sections above will make OSSRH staging reject the deployment.
🤖 Prompt for AI Agents
In sherpa-onnx/java-api/pom.xml lines 1 to 12, the POM lacks mandatory metadata
sections required for Maven Central deployment and has a groupId that does not
match the Java package namespace. Add the required sections: <name>,
<description>, <url>, <licenses>, <scm>, and <developers> with appropriate
project information. Also, update the <groupId> to com.k2fsa to align with the
Java package namespace or alternatively refactor the Java packages to match the
current groupId. This will ensure compliance with Maven Central requirements and
avoid deployment rejection.
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/EndpointConfig.java
Outdated
Show resolved
Hide resolved
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.java
Outdated
Show resolved
Hide resolved
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineParaformerModelConfig.java
Outdated
Show resolved
Hide resolved
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java
Outdated
Show resolved
Hide resolved
| // You'd better call it manually if it is not used anymore | ||
| public void release() { | ||
| if (this.ptr == 0) { | ||
| return; | ||
| } | ||
| delete(this.ptr); | ||
| this.ptr = 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename release() to close() and make it idempotent-safe
Once AutoCloseable is implemented (see previous comment) the canonical resource-release method should be close(). Keep the current body but delegate from release() for back-compatibility:
+ @Deprecated
+ public void release() {
+ close();
+ }
+
@Override
- public void release() {
+ public void close() {
if (this.ptr == 0) {
return;
}
delete(this.ptr);
this.ptr = 0;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java
around lines 48 to 55, rename the method release() to close() to follow the
AutoCloseable convention. Keep the existing logic inside close() and modify
release() to simply call close() for backward compatibility, ensuring the
resource release method is idempotent-safe and consistent with Java standards.
| public static class Builder { | ||
| private FeatureConfig featConfig = FeatureConfig.builder().build(); | ||
| private OnlineModelConfig modelConfig = OnlineModelConfig.builder().build(); | ||
| private OnlineLMConfig lmConfig = OnlineLMConfig.builder().build(); | ||
| private OnlineCtcFstDecoderConfig ctcFstDecoderConfig = OnlineCtcFstDecoderConfig.builder().build(); | ||
| private EndpointConfig endpointConfig = EndpointConfig.builder().build(); | ||
| private HomophoneReplacerConfig hr = HomophoneReplacerConfig.builder().build(); | ||
| private boolean enableEndpoint = true; | ||
| private String decodingMethod = "greedy_search"; | ||
| private int maxActivePaths = 4; | ||
| private String hotwordsFile = ""; | ||
| private float hotwordsScore = 1.5f; | ||
| private String ruleFsts = ""; | ||
| private String ruleFars = ""; | ||
| private float blankPenalty = 0.0f; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add basic validation inside the builder before constructing an instance
Currently any null, negative, or otherwise invalid value is silently accepted.
Recommend validating at least:
decodingMethodnotnull/blankmaxActivePaths > 0hotwordsScore >= 1.0(or documented lower-bound)blankPenalty >= 0
public OnlineRecognizerConfig build() {
- return new OnlineRecognizerConfig(this);
+ Objects.requireNonNull(decodingMethod, "decodingMethod");
+ if (maxActivePaths <= 0) {
+ throw new IllegalArgumentException("maxActivePaths must be > 0");
+ }
+ if (hotwordsScore < 0) {
+ throw new IllegalArgumentException("hotwordsScore must be >= 0");
+ }
+ if (blankPenalty < 0) {
+ throw new IllegalArgumentException("blankPenalty must be >= 0");
+ }
+ return new OnlineRecognizerConfig(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 static class Builder { | |
| private FeatureConfig featConfig = FeatureConfig.builder().build(); | |
| private OnlineModelConfig modelConfig = OnlineModelConfig.builder().build(); | |
| private OnlineLMConfig lmConfig = OnlineLMConfig.builder().build(); | |
| private OnlineCtcFstDecoderConfig ctcFstDecoderConfig = OnlineCtcFstDecoderConfig.builder().build(); | |
| private EndpointConfig endpointConfig = EndpointConfig.builder().build(); | |
| private HomophoneReplacerConfig hr = HomophoneReplacerConfig.builder().build(); | |
| private boolean enableEndpoint = true; | |
| private String decodingMethod = "greedy_search"; | |
| private int maxActivePaths = 4; | |
| private String hotwordsFile = ""; | |
| private float hotwordsScore = 1.5f; | |
| private String ruleFsts = ""; | |
| private String ruleFars = ""; | |
| private float blankPenalty = 0.0f; | |
| public OnlineRecognizerConfig build() { | |
| Objects.requireNonNull(decodingMethod, "decodingMethod"); | |
| if (maxActivePaths <= 0) { | |
| throw new IllegalArgumentException("maxActivePaths must be > 0"); | |
| } | |
| if (hotwordsScore < 0) { | |
| throw new IllegalArgumentException("hotwordsScore must be >= 0"); | |
| } | |
| if (blankPenalty < 0) { | |
| throw new IllegalArgumentException("blankPenalty must be >= 0"); | |
| } | |
| return new OnlineRecognizerConfig(this); | |
| } |
🤖 Prompt for AI Agents
In
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerConfig.java
between lines 48 and 63, the Builder class lacks validation for its fields
before building an instance. Add validation checks in the build() method or
before returning the constructed object to ensure decodingMethod is not null or
blank, maxActivePaths is greater than 0, hotwordsScore is at least 1.0, and
blankPenalty is non-negative. If any validation fails, throw an
IllegalArgumentException with a clear message to prevent invalid configurations.
| public void release() { | ||
| // stream object must be release after used | ||
| if (this.ptr == 0) { | ||
| return; | ||
| } | ||
| delete(this.ptr); | ||
| this.ptr = 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
release() is not thread-safe – possible double free of native resource
release() and finalize() can be executed concurrently from different threads.
Since ptr is a plain long, the following race is possible:
- Thread-A calls
release(), passes theif (ptr == 0)guard, still seesptr != 0. - The GC finalizer thread calls
finalize()almost simultaneously, also passes the guard. - Both threads call the native
delete(ptr)– double free / corruption.
Guard the critical section with synchronization or an AtomicLong, and/or make release() idempotent using compareAndSet.
🤖 Prompt for AI Agents
In sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.java
around lines 34 to 41, the release() method is not thread-safe and can cause a
double free of the native resource if called concurrently with finalize(). To
fix this, synchronize the critical section in release() or use an AtomicLong for
ptr and implement a compareAndSet operation to ensure only one thread can delete
the native resource and set ptr to 0, making release() idempotent and
thread-safe.
| @Override | ||
| protected void finalize() throws Throwable { | ||
| release(); | ||
| super.finalize(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
finalize() is deprecated – migrate to AutoCloseable
finalize() has been deprecated since Java 9 and removed in recent JDKs. Relying on it delays resource release, hurts GC performance, and will break on newer runtimes.
Implement AutoCloseable and encourage try-with-resources instead:
-public class OnlineStream {
+public class OnlineStream implements AutoCloseable {
...
- public void release() {
+ @Override
+ public void close() {
if (this.ptr == 0) {
return;
}
delete(this.ptr);
this.ptr = 0;
}
- @Override
- protected void finalize() throws Throwable {
- release();
- super.finalize();
- }
+ /* `finalize` removed – rely on explicit `close()` */Clients can then use:
try (OnlineStream stream = recognizer.createStream()) {
...
}🤖 Prompt for AI Agents
In sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.java
around lines 43 to 47, the finalize() method is deprecated and should be
removed. Instead, make the OnlineStream class implement AutoCloseable, rename
the release() method to close(), and ensure close() properly releases resources.
This allows clients to use try-with-resources for automatic and timely resource
management.
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.java
Outdated
Show resolved
Hide resolved
|
上面的警告是之前代码中就有的 |
|
感谢贡献！ 目前这个PR的实现，需要用户做大量的工作。请设法把不同平台的库，放在包里面，代码自动加载不同平台的库，方便用户使用。 用户不需要手动去下载不同平台的库并设置环境变量。不然，现在的方法就够用了，没必要用maven或者其他包管理工具 |
|
可以参考onnxruntime是如何做的 |
|
1.没有问题,我来做 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.java (1)
38-54: Add input validation to prevent invalid configurationsThe builder methods don't validate input values, allowing negative or zero values that could cause downstream failures. Consider adding validation either in the setters or in the
build()method.Add validation in the
build()method:public FeatureConfig build() { + if (sampleRate <= 0) { + throw new IllegalArgumentException("sampleRate must be positive, got: " + sampleRate); + } + if (featureDim <= 0) { + throw new IllegalArgumentException("featureDim must be positive, got: " + featureDim); + } + if (dither < 0f) { + throw new IllegalArgumentException("dither must be non-negative, got: " + dither); + } - return new FeatureConfig(new Builder()); + return new FeatureConfig(this); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.java(1 hunks)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/FeatureConfig.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java (1)
36-42: Improve architecture detection robustness.The current architecture detection using
contains("64")is too broad and may incorrectly classify some architectures. Consider using more specific patterns.- if (osArch.contains("aarch64") || osArch.contains("arm")) { + if (osArch.contains("aarch64") || osArch.contains("arm64") || osArch.contains("arm")) { archName = WIN_ARM64; - } else if (osArch.contains("64")) { + } else if (osArch.contains("x86_64") || osArch.contains("amd64")) { archName = WIN_X64; } else { archName = WIN_X86; }Apply similar improvements to the Linux and macOS detection blocks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
sherpa-onnx/java-api/pom.xml(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/LibraryLoader.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineParaformerModelConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java(1 hunks)sherpa-onnx/java-api/src/main/resources/.gitignore(1 hunks)sherpa-onnx/java-api/src/main/resources/readme.md(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- sherpa-onnx/java-api/src/main/resources/.gitignore
- sherpa-onnx/java-api/src/main/resources/readme.md
- sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java
- sherpa-onnx/java-api/pom.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineParaformerModelConfig.java
- sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerConfig.java
- sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineTransducerModelConfig.java
- sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java
- sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineStream.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/LibraryLoader.java (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java (1)
LibraryUtils(11-100)
🔇 Additional comments (5)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/LibraryLoader.java (2)
3-3: LGTM!The import is correctly added and properly formatted.
11-12: LGTM! Clean delegation to platform-aware library loading.The refactoring delegates native library loading to
LibraryUtils, which provides automatic platform detection and resource extraction. This aligns well with the PR objective of simplifying user experience by eliminating manual platform-specific library management.sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java (3)
12-22: LGTM! Comprehensive platform support.The platform constants cover all major OS and architecture combinations mentioned in the PR discussion (Windows x64/x86/arm64, Linux x64/arm64/x86, macOS x64/arm64). The naming convention is consistent and clear.
66-79: LGTM! Solid library extraction and loading implementation.The directory creation, resource extraction, and library loading logic is well-implemented with proper error handling. The approach of extracting to user home directory follows common patterns used by other Java libraries (like ONNX Runtime mentioned in PR comments).
89-99: LGTM! Excellent resource management and error handling.The
extractResourcemethod properly uses try-with-resources for automatic stream closing, handles missing resources gracefully, and provides clear error messages. The use ofStandardCopyOption.REPLACE_EXISTINGensures updates work correctly.
| String osName = System.getProperty("os.name").toLowerCase(); | ||
| String osArch = System.getProperty("os.arch").toLowerCase(); | ||
| String userHome = System.getProperty("user.home"); | ||
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null checks for system properties.
System properties could potentially be null in some environments, which would cause NullPointerException when calling .toLowerCase().
- String osName = System.getProperty("os.name").toLowerCase();
- String osArch = System.getProperty("os.arch").toLowerCase();
- String userHome = System.getProperty("user.home");
+ String osName = System.getProperty("os.name");
+ String osArch = System.getProperty("os.arch");
+ String userHome = System.getProperty("user.home");
+
+ if (osName == null || osArch == null || userHome == null) {
+ throw new RuntimeException("Unable to detect system properties: os.name=" + osName + ", os.arch=" + osArch + ", user.home=" + userHome);
+ }
+
+ osName = osName.toLowerCase();
+ osArch = osArch.toLowerCase();📝 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.
| String osName = System.getProperty("os.name").toLowerCase(); | |
| String osArch = System.getProperty("os.arch").toLowerCase(); | |
| String userHome = System.getProperty("user.home"); | |
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome); | |
| String osName = System.getProperty("os.name"); | |
| String osArch = System.getProperty("os.arch"); | |
| String userHome = System.getProperty("user.home"); | |
| if (osName == null || osArch == null || userHome == null) { | |
| throw new RuntimeException("Unable to detect system properties: os.name=" + osName | |
| + ", os.arch=" + osArch + ", user.home=" + userHome); | |
| } | |
| osName = osName.toLowerCase(); | |
| osArch = osArch.toLowerCase(); | |
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome); |
🤖 Prompt for AI Agents
In
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java
around lines 25 to 28, add null checks for the system properties "os.name",
"os.arch", and "user.home" before calling toLowerCase() or using them. Assign
default values or handle null cases to prevent NullPointerException when these
properties are missing.
There was a problem hiding this comment.
Can you fix this comment?
There was a problem hiding this comment.
@csukuangfj I'll implement the null check fix for the system properties to prevent potential NullPointerException:
String osName = System.getProperty("os.name");
String osArch = System.getProperty("os.arch");
String userHome = System.getProperty("user.home");
if (osName == null || osArch == null || userHome == null) {
throw new RuntimeException("Unable to detect system properties: os.name=" + osName
+ ", os.arch=" + osArch + ", user.home=" + userHome);
}
osName = osName.toLowerCase();
osArch = osArch.toLowerCase();
System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome);This change adds defensive programming by:
- Retrieving system properties first without calling
toLowerCase() - Checking if any of the critical properties are null
- Throwing a descriptive RuntimeException if any are missing
- Only calling
toLowerCase()after confirming the properties are not null
This prevents potential NullPointerException in environments where these system properties might not be available.
There was a problem hiding this comment.
This is an incorrect comment; the three variables above cannot be empty.
String osName = System.getProperty("os.name");
String osArch = System.getProperty("os.arch");
String userHome = System.getProperty("user.home");
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| package com.k2fsa.sherpa.onnx.core; | ||
|
|
||
| public interface Core { | ||
| String WIN_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so"; |
There was a problem hiding this comment.
Is it sherpa-onnx-jni.dll for windows?
| String osName = System.getProperty("os.name").toLowerCase(); | ||
| String osArch = System.getProperty("os.arch").toLowerCase(); | ||
| String userHome = System.getProperty("user.home"); | ||
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome); |
There was a problem hiding this comment.
Can you fix this comment?
| String osName = System.getProperty("os.name").toLowerCase(); | ||
| String osArch = System.getProperty("os.arch").toLowerCase(); | ||
| String userHome = System.getProperty("user.home"); | ||
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome); |
There was a problem hiding this comment.
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome); | |
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s\n", osName, osArch, userHome); |
There was a problem hiding this comment.
No I can't
In a printf‐style format string, %n and the literal newline \n are not exactly the same:
-
%nis a format specifier-
It’s handled by Java’s
Formatterand is mapped to the platform’s line‐separator (System.lineSeparator()).- On Windows, that’s
"\r\n" - On Unix/Linux/macOS, that’s
"\n"
- On Windows, that’s
-
Usage example:
System.out.printf("First line%nSecond line%n"); // → on Windows: prints "First line\r\nSecond line\r\n"
-
-
\nis just a literal line‐feed character-
It’s escaped by the Java compiler into a single
'\n'in the format string. -
It does not get translated to
"\r\n"on Windows (so you end up with only\n). -
Usage example:
System.out.printf("First line\nSecond line\n"); // → on Windows: prints "First line\nSecond line\n" (no carriage return)
-
Why prefer %n in printf‐style output?
- Portability: You don’t have to worry about which OS you’re on—
%nwill always use the correct separator. - Semantics: It’s a format-specifier designed exactly for “new-line here” in
printf‐style calls. - Readability: It makes it clear that this newline is part of your formatted output, not just a Java‐string escape.
Quick summary
| Feature | %n |
\n |
|---|---|---|
| Type | Formatter specifier |
Java string literal escape |
| Result on Windows | "\r\n" |
"\n" |
| Result on Unix | "\n" |
"\n" |
| Portability | ✅ always correct | ❌ may be wrong on Windows |
Bottom line: in System.out.printf(...), use %n whenever you need a newline in cross-platform code.
| } else { | ||
| archName = WIN_X86; | ||
| } | ||
|
|
There was a problem hiding this comment.
Please remove extra blank lines.
Can you use IDE tools to reformat the file?
There was a problem hiding this comment.
I link two space, how about you?
There was a problem hiding this comment.
建议用自动化工具，自动 format 你自己写的代码.
上面移动过的文件，建议只移动，别重新 format 了， 加大了 review 的难度.
|
By the way, I hope you can follow the approach used in onnxruntime. 1For instance, there is no need to use You can use the function 2Please replace part of the code from with the code from https://github.com/microsoft/onnxruntime/blob/b1546da6f524f4995ed3273648ebf1152f9f727a/java/src/main/java/ai/onnxruntime/OnnxRuntime.java#L118 3Please follow You can define
4Please reuse as much code as possible from onnxruntime. It is well tested. You don't need to reinvent the wheel. |
|
下面是 解压 我们可以有
(不需要考虑 linux-x86. 没有与之对应的 onnxruntime lib 库. 也不需要考虑 linux-arm32) |
| public class LibraryLoader { | ||
| private static volatile boolean autoLoadEnabled = true; | ||
| private static volatile boolean isLoaded = false; | ||
|
|
||
| static synchronized void loadLibrary() { | ||
| if (!isLoaded) { | ||
| System.loadLibrary("sherpa-onnx-jni"); | ||
| //System.loadLibrary("sherpa-onnx-jni"); |
There was a problem hiding this comment.
Please remove code not used.
There was a problem hiding this comment.
This is required. The loading method is different—I'm using System.load instead of the previous System.loadLibrary.
System.load only requires specifying the full file path, including the file extension.
When the program starts, it copies files from the jar to the model directory in the user's directory and then loads them by calling System.load.
| private final String decoder; | ||
|
|
||
| private OnlineParaformerModelConfig(Builder builder) { | ||
| this.encoder = java.util.Objects.requireNonNull(builder.encoder, "encoder cannot be null"); |
There was a problem hiding this comment.
Is there a reason to check whether it is null?
Bu default, encoder and decoder are initialized to an empty string and they cannot be null.
There was a problem hiding this comment.
just make sure ii's not null
There was a problem hiding this comment.
Can you give an example in which case it is null?
| if (this.encoder.isEmpty() || this.decoder.isEmpty()) { | ||
| throw new IllegalArgumentException("encoder/decoder path must not be empty"); | ||
| } | ||
| } |
There was a problem hiding this comment.
| if (this.encoder.isEmpty() || this.decoder.isEmpty()) { | |
| throw new IllegalArgumentException("encoder/decoder path must not be empty"); | |
| } | |
| } |
Please remove this check.
sherpa-onnx supports lots of models. At any time, only one model can be configured and all other models have their default values, e.g., model files are initialized to empty strings.
| private float blankPenalty = 0.0f; | ||
|
|
||
| public OnlineRecognizerConfig build() { | ||
| Objects.requireNonNull(decodingMethod, "decodingMethod"); |
There was a problem hiding this comment.
Please remove checks and keep the original code.
The C++ code will validate the parameters.
There was a problem hiding this comment.
Ok, removed Objects.requireNonNull(decodingMethod, "decodingMethod");
| private String joiner = ""; | ||
|
|
||
| public OnlineTransducerModelConfig build() { | ||
| if (encoder.isEmpty() || decoder.isEmpty() || joiner.isEmpty()) { |
There was a problem hiding this comment.
Please remove the checks and keep the original code.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
sherpa-onnx/java-api/src/main/resources/readme.md (4)
1-2: Fix typos and improve clarity of the first sentence
downlaodanddonwloadare miss-spelled. The sentence is also missing an article and a clearer subject.-please downlaod file and put in folder -[donwload link](https://huggingface.co/csukuangfj/sherpa-onnx-libs/tree/main/jni) +Please download the required archive files and place them in the appropriate folder. +[Download link](https://huggingface.co/csukuangfj/sherpa-onnx-libs/tree/main/jni)
4-9: Add markdown code fencing for the archive list
Rendering the list inside a fenced block avoids accidental formatting issues and improves readability (especially for long version strings).-- sherpa-onnx-v1.12.7-linux-aarch64-jni.tar.bz2 ... -- sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2 +```text +sherpa-onnx-v1.12.7-linux-aarch64-jni.tar.bz2 +sherpa-onnx-v1.12.7-linux-x64-jni.tar.bz2 +sherpa-onnx-v1.12.7-osx-arm64-jni.tar.bz2 +sherpa-onnx-v1.12.7-osx-x86_64-jni.tar.bz2 +sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2 +```
11-16: Prefix folder names with the parentlibs/directory
The later tree shows these folders underlibs/, but the bullet list omits that context. Align the two to avoid user confusion.-- linux_arm64 -- linux_x64 -- darwin_arm64 -- darwin_x64 -- windows_x64 +libs/ + linux_arm64 + linux_x64 + darwin_arm64 + darwin_x64 + windows_x64
18-34: Directory tree is indented with a leading dot that renders oddly
Consider wrapping the tree inside a fenced block and adding a title line for clarity.-``` -. -├── .gitignore +```text +<project-root> +├── .gitignoresherpa-onnx/java-api/readme.md (5)
3-4: Replace italic heading with a proper second-level heading
Markdown-lint flags the use of emphasis instead of a heading. Converting it to##keeps semantics while removing the MD036 violation.-*Applicable to Windows / macOS / Linux (using Windows as an example for dynamic library loading)* +## Applicable Platforms +Windows / macOS / Linux (examples below use Windows for dynamic-library loading)
41-46: Pinning to a specific ORT version without explanation
Locking tov1.17.1is fine, but explain why (compatibility, tested version, etc.) or direct users to check for newer versions. This prevents outdated docs in the future.
93-101: Specify language for fenced error block
Addtextafter the triple back-ticks to silence MD040 and improve highlighting.-```text +```text
123-136: Missing language specifiers for shell snippets
Several fenced blocks lack a language tag. Addingbashenables syntax highlighting and resolves MD040 warnings.Example:
-```bash +# Correct – already taggedRepeat for all subsequent shell blocks that miss a tag.
211-229: Add a language specifier for the output block
Mark the block astextto satisfy MD040.-``` +```text -- elapsed : 6.739 seconds ... -- Saved to : tts-kokoro-en.wavsherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java (1)
3-3: Consider using a final class instead of an interface for constants.While using an interface for constants is syntactically valid, it's generally considered an anti-pattern. A final utility class with a private constructor would be more appropriate for housing static constants.
-public interface Core { +public final class Core { + private Core() { + // Utility class + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
sherpa-onnx/java-api/readme.md(1 hunks)sherpa-onnx/java-api/readme.zh.md(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java(1 hunks)sherpa-onnx/java-api/src/main/resources/.gitignore(1 hunks)sherpa-onnx/java-api/src/main/resources/readme.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sherpa-onnx/java-api/src/main/resources/.gitignore
- sherpa-onnx/java-api/readme.zh.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
sherpa-onnx/java-api/readme.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
131-131: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
211-211: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
sherpa-onnx/java-api/src/main/resources/readme.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (2)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java (2)
1-1: LGTM!Package declaration follows standard Java naming conventions.
7-7: LGTM!Interface closing is correct.
sherpa-onnx/java-api/readme.md
Outdated
| Windows 10 includes ONNX Runtime by default—no extra installation is required. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
“Windows 10 includes ONNX Runtime by default” is inaccurate
ONNX Runtime is not bundled with Windows 10. Users will still need the shared library unless your JAR embeds it. Please revise to avoid misleading instructions.
🤖 Prompt for AI Agents
In sherpa-onnx/java-api/readme.md around lines 35 to 36, the statement claiming
that Windows 10 includes ONNX Runtime by default is incorrect. Update the text
to clarify that ONNX Runtime is not bundled with Windows 10 and users must
install the shared library separately unless it is embedded in the JAR. Remove
or revise the misleading statement to accurately reflect the installation
requirements.
There was a problem hiding this comment.
you are wrong.
Starting from Windows 10 v1809 and all versions of Windows 11, the system comes with built-in ONNX Runtime as part of Windows ML (WinRT API), exposed through Windows.AI.MachineLearning.dll. You can directly use WinML to load and run ONNX models without additional downloads or installations.
https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/run-onnx-models
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Windows 10 v1809 Windows 11 built-in ONNX Runtime Windows ML WinML native libraries
💡 Result:
Windows 10 version 1809 and Windows 11 both include built-in support for ONNX Runtime via Windows ML (WinML) native libraries. Here's a brief overview:
-
Windows ML (WinML): This is a Microsoft API that allows developers to run machine learning models locally on Windows devices. It supports running models in the ONNX (Open Neural Network Exchange) format, which is a common format for machine learning models.
-
ONNX Runtime: This is a cross-platform, high-performance scoring engine for Open Neural Network Exchange (ONNX) models. Microsoft has integrated ONNX Runtime into Windows via Windows ML for efficient model inference on Windows 10 (starting with version 1809) and Windows 11.
-
WinML Native Libraries: These are the native API libraries provided in Windows that allow direct access to Windows ML capabilities for running ONNX models. They provide a way for developers to leverage hardware acceleration and optimized inference on supported devices.
If you are developing applications on Windows 10 v1809 or Windows 11 that involve machine learning inference, you can use the Windows ML API and the built-in ONNX Runtime for efficient and easy deployment of ONNX models without needing to manually install additional runtimes.
Would you like details on how to use these libraries programmatically, or information on specific APIs available for Windows ML?
@litongjava Thank you for the correction and the clarification! You're absolutely right about Windows ML being built into Windows 10 v1809+ and Windows 11.
However, there's an important technical distinction to consider: While Windows does include ONNX Runtime through Windows ML (WinML), JNI libraries like sherpa-onnx-jni typically depend on the standalone ONNX Runtime C/C++ libraries rather than the Windows ML API. The Windows ML implementation uses WinRT APIs, whereas most cross-platform JNI libraries are compiled against the standard ONNX Runtime libraries for consistency across platforms.
So while Windows ML is available, users would still likely need the standalone ONNX Runtime shared libraries unless sherpa-onnx-jni was specifically compiled to use the Windows ML APIs. The documentation in the README appears to be referring to the standalone ONNX Runtime libraries that the JNI code expects.
Thank you for providing the Microsoft documentation link - it's a valuable resource for understanding Windows ML capabilities!
🐰💭 thoughtfully noting the technical nuances
✏️ Learnings added
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.656Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| String WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll"; | ||
| String UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so"; | ||
| String MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify native library naming conventions align with onnxruntime approach.
The library names follow standard platform conventions, which addresses the previous review question about Windows using "sherpa-onnx-jni.dll". However, given the PR discussion emphasizes following onnxruntime's approach, we should verify these naming conventions are consistent with that implementation.
🏁 Script executed:
#!/bin/bash
# Description: Verify if the native library naming matches onnxruntime's approach
# Expected: Find similar naming patterns in onnxruntime documentation or examples
echo "Checking onnxruntime native library naming patterns..."
curl -s "https://raw.githubusercontent.com/microsoft/onnxruntime/main/java/src/main/java/ai/onnxruntime/OrtEnvironment.java" | grep -i "library\|\.dll\|\.so\|\.dylib" || echo "Could not fetch onnxruntime source"
echo -e "\nAlso checking Maven Central for onnxruntime artifact structure..."
curl -s "https://repo1.maven.org/maven2/com/microsoft/onnxruntime/onnxruntime/maven-metadata.xml" | head -20 || echo "Could not fetch Maven metadata"Length of output: 1339
🏁 Script executed:
#!/bin/bash
# Download and inspect the onnxruntime Java artifact to confirm native‐lib naming
VERSION=1.22.0
ARTIFACT_URL="https://repo1.maven.org/maven2/com/microsoft/onnxruntime/onnxruntime/${VERSION}/onnxruntime-${VERSION}.jar"
echo "Downloading onnxruntime-${VERSION}.jar..."
curl -s -O "${ARTIFACT_URL}" || { echo "Failed to download jar"; exit 1; }
echo -e "\nListing .so, .dll, and .dylib entries in the jar:"
unzip -l "onnxruntime-${VERSION}.jar" | grep -E "\.so$|\.dll$|\.dylib$" || echo "No native library files found in jar"Length of output: 1912
Rename JNI library constants to match ONNX Runtime’s Java naming
ONNX Runtime’s Java bindings ship their JNI libraries as:
- Windows: onnxruntime4j_jni.dll
- Linux: libonnxruntime4j_jni.so
- macOS: libonnxruntime4j_jni.dylib
In sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java (lines 4–6), update:
--- a/sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java
@@ -4,6 +4,6 @@
- String WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll";
- String UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so";
- String MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib";
+ String WIN_NATIVE_LIBRARY_NAME = "sherpaonnx4j_jni.dll";
+ String UNIX_NATIVE_LIBRARY_NAME = "libsherpaonnx4j_jni.so";
+ String MACOS_NATIVE_LIBRARY_NAME = "libsherpaonnx4j_jni.dylib";Also adjust your native packaging under ai/k2fsa/sherpa/onnx/native/... so the delivered binaries match these names.
📝 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.
| String WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll"; | |
| String UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so"; | |
| String MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib"; | |
| String WIN_NATIVE_LIBRARY_NAME = "sherpaonnx4j_jni.dll"; | |
| String UNIX_NATIVE_LIBRARY_NAME = "libsherpaonnx4j_jni.so"; | |
| String MACOS_NATIVE_LIBRARY_NAME = "libsherpaonnx4j_jni.dylib"; |
🤖 Prompt for AI Agents
In sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java at
lines 4 to 6, rename the JNI library constants to match ONNX Runtime’s Java
naming conventions: change WIN_NATIVE_LIBRARY_NAME to "onnxruntime4j_jni.dll",
UNIX_NATIVE_LIBRARY_NAME to "libonnxruntime4j_jni.so", and
MACOS_NATIVE_LIBRARY_NAME to "libonnxruntime4j_jni.dylib". Additionally, update
the native binaries under ai/k2fsa/sherpa/onnx/native/... to use these exact
filenames to ensure consistency.
There was a problem hiding this comment.
my code is right, I copy the filename from the compiled file.
There was a problem hiding this comment.
@litongjava, you're absolutely right! If you copied the filenames directly from the compiled binaries, then the constants in Core.java are correct as they are. The library names must match the actual compiled files, not necessarily follow the same naming pattern as ONNX Runtime. Thank you for the clarification.
✏️ Learnings added
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.936Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.656Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.
There was a problem hiding this comment.
please downlaod file and check donwload link
•sherpa-onnx-v1.12.7-linux-aarch64-jni.tar.bz2
•sherpa-onnx-v1.12.7-linux-x64-jni.tar.bz2
•sherpa-onnx-v1.12.7-osx-arm64-jni.tar.bz2
•sherpa-onnx-v1.12.7-osx-x86_64-jni.tar.bz2
•sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Download and verify the actual JNI library filenames from Hugging Face
echo "Downloading Windows x64 JNI library archive..."
curl -L "https://huggingface.co/csukuangfj/sherpa-onnx-libs/resolve/main/jni/sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2" -o win-x64-jni.tar.bz2
echo "Extracting Windows archive..."
tar -xjf win-x64-jni.tar.bz2
echo "Listing contents to find the actual library filename:"
find . -name "*.dll" -type f | head -10
echo -e "\n=== Also checking Linux x64 for comparison ==="
curl -L "https://huggingface.co/csukuangfj/sherpa-onnx-libs/resolve/main/jni/sherpa-onnx-v1.12.7-linux-x64-jni.tar.bz2" -o linux-x64-jni.tar.bz2
echo "Extracting Linux archive..."
tar -xjf linux-x64-jni.tar.bz2
echo "Listing Linux .so files:"
find . -name "*.so" -type f | head -10
echo -e "\n=== Checking macOS arm64 for .dylib files ==="
curl -L "https://huggingface.co/csukuangfj/sherpa-onnx-libs/resolve/main/jni/sherpa-onnx-v1.12.7-osx-arm64-jni.tar.bz2" -o osx-arm64-jni.tar.bz2
echo "Extracting macOS archive..."
tar -xjf osx-arm64-jni.tar.bz2
echo "Listing macOS .dylib files:"
find . -name "*.dylib" -type f | head -10
Length of output: 2910
🏁 Script executed:
#!/bin/bash
# Check what was actually downloaded
echo "Checking downloaded file contents:"
ls -la *.tar.bz2
echo -e "\nChecking file types:"
file *.tar.bz2
echo -e "\nChecking first few bytes of files:"
hexdump -C win-x64-jni.tar.bz2 | head -3
echo -e "\n=== Trying to access Hugging Face repository listing directly ==="
curl -s "https://huggingface.co/csukuangfj/sherpa-onnx-libs/tree/main/jni" | grep -E "sherpa-onnx.*jni\.tar\.bz2" || echo "Could not find direct file listings"
echo -e "\n=== Trying alternative URL format ==="
curl -L "https://huggingface.co/csukuangfj/sherpa-onnx-libs/raw/main/jni/sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2" -o win-x64-jni-alt.tar.bz2
ls -la win-x64-jni-alt.tar.bz2
file win-x64-jni-alt.tar.bz2
echo -e "\n=== Checking if files exist using HEAD request ==="
curl -I "https://huggingface.co/csukuangfj/sherpa-onnx-libs/resolve/main/jni/sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2"
Length of output: 92647
🏁 Script executed:
#!/bin/bash
# Try downloading from the versioned directory structure
echo "Trying to download from jni/1.12.7/ directory..."
curl -L "https://huggingface.co/csukuangfj/sherpa-onnx-libs/resolve/main/jni/1.12.7/sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2" -o v1127-win-x64-jni.tar.bz2
echo "Checking file size:"
ls -la v1127-win-x64-jni.tar.bz2
echo -e "\nChecking content type:"
curl -I "https://huggingface.co/csukuangfj/sherpa-onnx-libs/resolve/main/jni/1.12.7/sherpa-onnx-v1.12.7-win-x64-jni.tar.bz2"
# If that doesn't work, let's try an older version that we know exists
echo -e "\n=== Trying v1.10.16 which we know exists ==="
curl -L "https://huggingface.co/csukuangfj/sherpa-onnx-libs/resolve/main/jni/sherpa-onnx-v1.10.16-win-x64-jni.tar.bz2" -o v10116-win-x64-jni.tar.bz2
echo "Checking v1.10.16 file size:"
ls -la v10116-win-x64-jni.tar.bz2
# Try to extract with available tools (without bzip2)
echo -e "\nAttempting to extract using Python..."
python3 << 'EOF'
import tarfile
import bz2
import os
# Try to extract the v1.10.16 file if it downloaded successfully
if os.path.getsize('v10116-win-x64-jni.tar.bz2') > 1000: # If it's a real file
try:
with tarfile.open('v10116-win-x64-jni.tar.bz2', 'r:bz2') as tar:
print("Archive contents:")
for member in tar.getmembers():
if member.isfile() and (member.name.endswith('.dll') or member.name.endswith('.so') or member.name.endswith('.dylib')):
print(f" {member.name}")
# Extract just the library files
for member in tar.getmembers():
if member.isfile() and member.name.endswith('.dll'):
tar.extract(member)
print(f"Extracted: {member.name}")
except Exception as e:
print(f"Error extracting: {e}")
else:
print("File too small, probably not a valid archive")
EOF
echo -e "\nLooking for extracted DLL files:"
find . -name "*.dll" -type f
Length of output: 5889
@litongjava Perfect! I've successfully downloaded and verified the actual JNI library files. The archives contain the exact filename sherpa-onnx-jni.dll in the lib/ directory, which confirms that your constants in Core.java are absolutely correct.
From the extracted v1.10.16 Windows archive, I can see:
sherpa-onnx-v1.10.16-win-x64-jni/lib/sherpa-onnx-jni.dll
The library name "sherpa-onnx-jni.dll" in your WIN_NATIVE_LIBRARY_NAME constant matches exactly with the compiled binary filename. You were right to copy the names directly from the compiled files rather than following other libraries' naming conventions.
The v1.12.7 files are indeed available in the versioned directory structure at jni/1.12.7/ as you mentioned, and I was able to successfully download the 20.7MB Windows archive.
Your implementation in Core.java is correct as-is!
✏️ Learnings added
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.225Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.936Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.656Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.
|
1.已经按照你的要求进行修改 3.确认一下 是否可以推动到 maven center repository |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
sherpa-onnx/java-api/readme.md (1)
35-36: Clarify standalone ONNX Runtime requirement for sherpa-onnx-jniThe JNI layer depends on the standalone
onnxruntime.dll, not the Windows ML WinRT APIs provided by Windows 10 v1809+ or Windows 11. Users must install or bundle the matching onnxruntime C/C++ shared library for their platform to avoidUnsatisfiedLinkErrorat runtime.Affected location:
- sherpa-onnx/java-api/readme.md (lines 35–36)
Suggested update:
> **Note:** Although Windows 10 v1809+ and Windows 11 include ONNX Runtime via Windows ML > (Windows.AI.MachineLearning.dll), the `sherpa-onnx-jni` module requires the standalone > `onnxruntime.dll` from the onnxruntime C/C++ distribution, matching the versions used > on other platforms.
🧹 Nitpick comments (6)
sherpa-onnx/java-api/readme.md (4)
3-4: Use a proper heading instead of emphasis for the target-platform noteMarkdown-lint flags the italicised sentence as “emphasis used instead of a heading” (MD036).
Changing it to a level-2 heading keeps the document structure consistent and improves navigation.-*Applicable to Windows / macOS / Linux (using Windows as an example for dynamic library loading)* +## Supported platforms +Windows / macOS / Linux +(examples use Windows for dynamic-library loading)
36-36: Fix reversed Markdown link syntaxThe current line has
(text)[url]; correct syntax is[text](url).-(run-onnx-models)[https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/run-onnx-models] +[Run ONNX models with Windows ML](https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/run-onnx-models)
98-117: Add language identifiers to fenced blocks for syntax highlightingThree code fences in this section omit a language tag, triggering MD040.
Mark them asbashto improve readability:-```text +```text ... -``` +``` -``` +```bash java -Djava.library.path=C:\full\path\to\jni -jar your-app.jar -``` +```
124-137: Convert bare URL to an inline linkReplacing the naked URL with Markdown link text avoids MD034 warnings and looks cleaner:
-https://k2-fsa.github.io/sherpa/onnx/tts/pretrained_models/kokoro.html +[Kokoro pretrained models page](https://k2-fsa.github.io/sherpa/onnx/tts/pretrained_models/kokoro.html)sherpa-onnx/java-api/readme.zh.md (2)
35-35: Fix Markdown link syntaxParentheses are reversed:
-(run-onnx-models)[https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/run-onnx-models] +[run-onnx-models](https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/run-onnx-models)
38-86: Re-evaluate manual ONNX Runtime installation stepsSections 3.1 (2) Linux and 3.1 (3) macOS walk users through downloading and copying
libonnxruntime.*into system paths.
The ongoing PR discussion states that platform libraries will be packaged inside the Maven artifact and auto-extracted at runtime (à la onnxruntime-java). If that approach is now implemented, these manual steps are obsolete and risk confusing users.Recommend replacing this whole block with a short note such as:
“The required native libraries are bundled in the JAR and loaded automatically. Manual installation is only needed when you intentionally override the built-in loader.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sherpa-onnx/java-api/readme.md(1 hunks)sherpa-onnx/java-api/readme.zh.md(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineParaformerModelConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineParaformerModelConfig.java
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
📚 Learning: in sherpa-onnx java api, the native library names in core.java (win_native_library_name = "sherpa-on...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Applied to files:
sherpa-onnx/java-api/readme.mdsherpa-onnx/java-api/readme.zh.md
📚 Learning: the sherpa-onnx jni library files are stored in hugging face repository at https://huggingface.co/cs...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
Applied to files:
sherpa-onnx/java-api/readme.mdsherpa-onnx/java-api/readme.zh.md
📚 Learning: windows 10 version 1809 and later, as well as windows 11, include built-in onnx runtime as part of w...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.680Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.
Applied to files:
sherpa-onnx/java-api/readme.mdsherpa-onnx/java-api/readme.zh.md
🪛 markdownlint-cli2 (0.17.2)
sherpa-onnx/java-api/readme.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
35-35: Reversed link syntax
(run-onnx-models)[https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/run-onnx-models]
(MD011, no-reversed-links)
114-114: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
132-132: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
212-212: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
sherpa-onnx/java-api/readme.zh.md
[grammar] ~100-~100: Ensure spelling is correct
Context: ... java.library.path 中找到本地库。 排查步骤： 1. 确认下载的是与你操作系统与架构匹配的版本（如 win-x64 vs arm64 等）。 2. 用绝对路径测试：将 `.dll...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
sherpa-onnx/java-api/readme.zh.md
Outdated
| #### 1. Windows 11 | ||
|
|
||
| Starting from Windows 10 v1809 and all versions of Windows 11, the system comes with built-in ONNX Runtime as part of Windows ML (WinRT API), exposed through Windows.AI.MachineLearning.dll. You can directly use WinML to load and run ONNX models without additional downloads or installations. | ||
| (run-onnx-models)[https://learn.microsoft.com/en-us/windows/ai/new-windows-ml/run-onnx-models] | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify Windows “built-in ONNX Runtime” vs JNI requirement
Lines 32-36 imply that Windows 11 users can rely on the system WinML DLL and skip any extra downloads.
However, Sherpa-ONNX’s JNI layer links against the standalone ONNX Runtime C library, not WinML. End-users will still need the native onnxruntime.dll (or have it bundled in the JAR, per the new auto-loader design).
Please double-check and either:
- Remove this subsection, or
- Add an explicit note that WinML is not sufficient and that the packaged/bundled
onnxruntime.dllis still required.
🏁 Script executed:
#!/bin/bash
set -e
echo "---- English readme (context around Windows 11) ----"
rg -n -C3 "#### 1\. Windows 11" sherpa-onnx/java-api/readme.md || true
echo ""
echo "---- Chinese readme (context around Windows 11) ----"
rg -n -C3 "#### 1\. Windows 11" sherpa-onnx/java-api/readme.zh.md || trueLength of output: 843
Clarify WinML vs. Sherpa-ONNX JNI requirement
The current Chinese README (sherpa-onnx/java-api/readme.zh.md lines 32–36) implies that Windows 10 v1809+/Windows 11’s built-in ONNX Runtime via Windows ML is sufficient to run Sherpa-ONNX. In reality, Sherpa-ONNX’s JNI layer links against the standalone ONNX Runtime C library (onnxruntime.dll), so end users must still ship or bundle that DLL (the auto-loader in Core.java only loads it, it doesn’t replace it).
Please update accordingly:
- File:
sherpa-onnx/java-api/readme.zh.md(lines 32–36)- Either remove this WinML subsection
- Or add a clear note:
注意：虽然 Windows ML (WinRT API) 内置了 ONNX Runtime，但 Sherpa-ONNX JNI 仍需独立的
onnxruntime.dll（可随 JAR 一并发布）。
• (Optional) Mirror the same clarification in the English README for consistency.
🤖 Prompt for AI Agents
In sherpa-onnx/java-api/readme.zh.md around lines 32 to 36, clarify that
although Windows 10 v1809+ and Windows 11 include ONNX Runtime via Windows ML,
Sherpa-ONNX JNI still requires the standalone onnxruntime.dll to be shipped or
bundled. Either remove the current WinML subsection or add a clear note stating
that Sherpa-ONNX JNI depends on the separate onnxruntime.dll, which must be
distributed with the JAR. Optionally, apply the same clarification to the
English README for consistency.
请看这里 |
还有这里。 改完后，请修改 这个 PR 添加的使用教程。不需要用户单独下载 动态库. |
|
please review code |
|
请用 onnxruntime 里的代码，去检测 os 和 arch, 别自己写。 尽可能的利用 onnxruntime 的代码。希望能理解这个。 |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
sherpa-onnx/java-api/src/main/resources/readme.md (1)
15-15: Inconsistent platform naming with LibraryUtils.The folder name
windows_x64doesn't match the platform identifierWIN_X64 = "win_x64"used in LibraryUtils.java.This should be
win_x64to match the LibraryUtils constants.
🧹 Nitpick comments (6)
sherpa-onnx/java-api/src/main/resources/readme.md (2)
1-2: Fix typo in "download".-please downlaod file and put in folder +Please download files and put in folder
18-34: Add language specifiers to code blocks for better rendering.-``` +```text . ├── .gitignore ├── native │ ├── darwin_arm64 │ │ ├── libsherpa-onnx-jni.dylib │ ├── darwin_x64 │ │ ├── libsherpa-onnx-jni.dylib │ ├── linux_arm64 │ │ ├── libsherpa-onnx-jni.so │ ├── linux_x64 │ │ ├── libsherpa-onnx-jni.so │ ├── win_x64 │ │ ├── sherpa-onnx-jni.dll ├── readme.md - -``` +```sherpa-onnx/java-api/readme.md (4)
3-3: Convert italic emphasis to proper heading.-*Applicable to Windows / macOS / Linux (using Windows as an example for dynamic library loading)* +## Applicable to Windows / macOS / Linux (using Windows as an example for dynamic library loading)
124-126: Add language specifier to code block.-``` +```text https://k2-fsa.github.io/sherpa/onnx/tts/pretrained_models/kokoro.html--- `141-148`: **Add language specifier to code block.** ```diff -``` +```text LICENSE README.md espeak-ng-data/ # speech data directory model.onnx # TTS model tokens.txt # token mapping voices.bin # voice embeddings--- `222-228`: **Add language specifier to code block.** ```diff -``` +```text -- elapsed : 6.739 seconds -- audio duration : 6.739 seconds -- real-time factor : 0.563 -- text : ... -- Saved to : tts-kokoro-en.wav</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 0d2e14ccdbfdc0af22300e20caf6c3cd31e4ea49 and dc97e0023da98f3de900dff35f677d6987e6c54a. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `sherpa-onnx/java-api/readme.md` (1 hunks) * `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java` (1 hunks) * `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java` (1 hunks) * `sherpa-onnx/java-api/src/main/resources/.gitignore` (1 hunks) * `sherpa-onnx/java-api/src/main/resources/readme.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (4)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: litongjava
PR: #2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.Learnt from: litongjava
PR: #2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.</details> <details> <summary>📚 Learning: in sherpa-onnx java api, the native library names in core.java (win_native_library_name = "sherpa-on...</summary>Learnt from: litongjava
PR: #2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.**Applied to files:** - `sherpa-onnx/java-api/src/main/resources/.gitignore` - `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java` - `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java` - `sherpa-onnx/java-api/readme.md` - `sherpa-onnx/java-api/src/main/resources/readme.md` </details> <details> <summary>📚 Learning: the sherpa-onnx jni library files are stored in hugging face repository at https://huggingface.co/cs...</summary>Learnt from: litongjava
PR: #2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.**Applied to files:** - `sherpa-onnx/java-api/src/main/resources/.gitignore` - `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java` - `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java` - `sherpa-onnx/java-api/readme.md` - `sherpa-onnx/java-api/src/main/resources/readme.md` </details> <details> <summary>📚 Learning: windows 10 version 1809 and later, as well as windows 11, include built-in onnx runtime as part of w...</summary>Learnt from: litongjava
PR: #2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.680Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.**Applied to files:** - `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java` - `sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java` - `sherpa-onnx/java-api/readme.md` - `sherpa-onnx/java-api/src/main/resources/readme.md` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>sherpa-onnx/java-api/readme.md</summary> 3-3: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) --- 124-124: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 141-141: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 222-222: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>sherpa-onnx/java-api/src/main/resources/readme.md</summary> 18-18: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (5)</summary><blockquote> <details> <summary>sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java (3)</summary> `27-46`: **LGTM - Well-structured native library loading logic.** The two-tier approach (external directory first, then JAR extraction) provides good flexibility for users. The use of `System.mapLibraryName()` ensures correct platform-specific naming. --- `84-95`: **Solid resource extraction with proper error handling.** The directory creation logic and resource extraction are well-implemented with appropriate error handling for missing resources and I/O failures. --- `104-114`: **Resource extraction method is robust.** The method properly handles resource streams with try-with-resources and provides clear error messages for debugging. </details> <details> <summary>sherpa-onnx/java-api/src/main/resources/.gitignore (1)</summary> `1-2`: **LGTM - Appropriate exclusions for native library directories.** Ignoring `lib/` and `native/` directories prevents extracted native libraries from being committed to version control, which aligns with the dynamic native library loading approach. </details> <details> <summary>sherpa-onnx/java-api/readme.md (1)</summary> `156-216`: **Excellent comprehensive Java example.** The code example is well-structured and demonstrates proper usage of the Sherpa-ONNX Java API with clear resource management, timing measurements, and proper cleanup. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java
Outdated
Show resolved
Hide resolved
| private static File init(String libFileName) { | ||
| String osName = System.getProperty("os.name").toLowerCase(); | ||
| String osArch = System.getProperty("os.arch").toLowerCase(); | ||
| String userHome = System.getProperty("user.home"); | ||
| System.out.printf("Detected OS=%s, ARCH=%s, HOME=%s%n", osName, osArch, userHome); | ||
|
|
||
| String archName; | ||
| if (osName.contains("win")) { | ||
| if (osArch.contains("aarch64") || osArch.contains("arm")) { | ||
| archName = WIN_ARM64; | ||
| } else if (osArch.contains("64")) { | ||
| archName = WIN_X64; | ||
| } else { | ||
| archName = WIN_X86; | ||
| } | ||
|
|
||
| } else if (osName.contains("mac")) { | ||
| if (osArch.contains("aarch64") || osArch.contains("arm")) { | ||
| archName = DARWIN_ARM64; | ||
| } else { | ||
| archName = DARWIN_X64; | ||
| } | ||
|
|
||
| } else if (osName.contains("nix") || osName.contains("nux") || osName.contains("aix") || osName.contains("linux")) { | ||
| if (osArch.contains("aarch64") || osArch.contains("arm")) { | ||
| archName = LINUX_ARM64; | ||
| } else if (osArch.contains("64")) { | ||
| archName = LINUX_X64; | ||
| } else { | ||
| archName = LINUX_X86; | ||
| } | ||
|
|
||
| } else { | ||
| throw new UnsupportedOperationException("Unsupported OS: " + osName); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Platform detection logic is comprehensive but has inconsistent naming.
The platform detection covers all major platforms and architectures effectively. However, there's an inconsistency: the constant WIN_X64 suggests "win_x64" but the readme.md shows "windows_x64" in the folder structure.
Ensure consistency between the platform identifiers and the actual directory structure. Based on the constants defined at the top, update the readme.md to use:
win_x64instead ofwindows_x64win_x86instead ofwindows_x86win_arm64instead ofwindows_arm64
🤖 Prompt for AI Agents
In
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java
lines 48 to 82, the platform detection constants like WIN_X64 use names such as
"win_x64" which differ from the folder names "windows_x64" in readme.md. To fix
this, update the readme.md file to replace all occurrences of "windows_x64",
"windows_x86", and "windows_arm64" with "win_x64", "win_x86", and "win_arm64"
respectively, ensuring consistency between the code constants and documentation
folder names.
|
1.我的这个库和onnxruntime的加载库的逻辑并不完全相同 |
|
并没有需要依赖 onnxruntime jar. 只是说，检测 os 和 arch 的代码，从 onnxruntime 的代码里，直接 copy过来，别自己写。 |
这个逻辑是现成的， 我想不出理由，为什么不用。 如果非要自己写，请问你是每个平台都测试了么 |
1.可以使用 |
|
修改完成,请review |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java (1)
12-27: Platform constants are well-definedThe platform identifiers follow a consistent naming convention and cover all major supported platforms and architectures comprehensively.
Note: There was a previous concern about naming inconsistency between these constants (e.g.,
WIN_X64= "win_x64") and directory structure shown in documentation ("windows_x64"). Ensure consistency between code constants and documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
📚 Learning: in sherpa-onnx java api, the native library names in core.java (win_native_library_name = "sherpa-on...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Applied to files:
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java
📚 Learning: the sherpa-onnx jni library files are stored in hugging face repository at https://huggingface.co/cs...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
Applied to files:
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java
📚 Learning: windows 10 version 1809 and later, as well as windows 11, include built-in onnx runtime as part of w...
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.680Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.
Applied to files:
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java
🔇 Additional comments (4)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java (4)
1-11: LGTM - Clean imports and package structureThe package declaration follows Java conventions and imports are well-organized with appropriate use of the Core interface for constants.
28-47: Excellent native library loading strategyThe implementation follows best practices by:
- Using
System.mapLibraryName()for platform-appropriate naming- Implementing a clear fallback strategy (external directory → JAR extraction)
- Using absolute paths with
System.load()for reliable loading- Including helpful debugging output
77-96: Library initialization logic is soundThe method correctly:
- Retrieves system properties (confirmed by previous discussion that these cannot be null)
- Creates necessary directory structure with proper error handling
- Constructs appropriate resource paths for extraction
Note: This method will work correctly once the separator inconsistency in
initOsArch()is resolved.
98-116: Excellent resource extraction implementationThe method demonstrates best practices:
- Proper resource management with try-with-resources
- Null check for missing resources with descriptive error messages
- Robust file copying with
StandardCopyOption.REPLACE_EXISTING- Comprehensive exception handling
The Javadoc example correctly shows underscore separator format, confirming the expected resource path structure.
| /* Computes and initializes OS_ARCH_STR (such as linux-x64) */ | ||
| private static String initOsArch() { | ||
| String detectedOS = null; | ||
| String os = System.getProperty("os.name", "generic").toLowerCase(Locale.ENGLISH); | ||
| if (os.contains("mac") || os.contains("darwin")) { | ||
| detectedOS = "osx"; | ||
| } else if (os.contains("win")) { | ||
| detectedOS = "win"; | ||
| } else if (os.contains("nux")) { | ||
| detectedOS = "linux"; | ||
| } else { | ||
| throw new IllegalStateException("Unsupported os:" + os); | ||
| } | ||
| String detectedArch = null; | ||
| String arch = System.getProperty("os.arch", "generic").toLowerCase(Locale.ENGLISH); | ||
| if (arch.startsWith("amd64") || arch.startsWith("x86_64")) { | ||
| detectedArch = "x64"; | ||
| } else if (arch.startsWith("x86")) { | ||
| // 32-bit x86 is not supported by the Java API | ||
| detectedArch = "x86"; | ||
| } else if (arch.startsWith("aarch64")) { | ||
| detectedArch = "aarch64"; | ||
| } else { | ||
| throw new IllegalStateException("Unsupported arch:" + arch); | ||
| } | ||
| return detectedOS + '-' + detectedArch; | ||
| } |
There was a problem hiding this comment.
Fix naming inconsistencies in platform detection
The method has inconsistencies that will cause runtime failures:
- Separator mismatch: Returns "linux-x64" (dash) but constants use "linux_x64" (underscore)
- Architecture naming mismatch: Maps "aarch64" → "aarch64" but constants use "arm64"
Apply this diff to fix the inconsistencies:
- return detectedOS + '-' + detectedArch;
+ // Convert architecture names to match constants
+ if ("aarch64".equals(detectedArch)) {
+ detectedArch = "arm64";
+ }
+ return detectedOS + '_' + detectedArch;🤖 Prompt for AI Agents
In
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java
between lines 49 and 75, fix the platform detection method by changing the
separator from dash to underscore in the returned string to match the constants,
and rename the architecture mapping from "aarch64" to "arm64" to align with the
constant naming. This will ensure consistent naming and prevent runtime
failures.
There was a problem hiding this comment.
I've changed folder name
There was a problem hiding this comment.
🧩 Analysis chain
I'd like to verify the changes you made to ensure the naming inconsistencies are resolved.
🏁 Script executed:
#!/bin/bash
# Check the current implementation of LibraryUtils.java to see the changes
cat sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/utils/LibraryUtils.java
Length of output: 4528
@litongjava Thank you for the update! However, I can see that the core issues in the initOsArch() method still exist. The method on Line 74 still returns strings like "linux-aarch64" (with dash separator) while your constants use underscores like "linux_arm64".
The inconsistencies that will cause runtime failures are still present:
- Line 74:
return detectedOS + '-' + detectedArch;uses dash but should use underscore - Line 66:
detectedArch = "aarch64";should be"arm64"to match your constants
This means the method will return "linux-aarch64" but try to load from "/native/linux-aarch64/" while your resources are likely organized as "/native/linux_arm64/".
Could you please apply the fix to the initOsArch() method:
// Convert architecture names to match constants
if ("aarch64".equals(detectedArch)) {
detectedArch = "arm64";
}
return detectedOS + '_' + detectedArch;This will ensure the platform detection matches your folder structure and constants.
🧠 Learnings used
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/readme.md:35-36
Timestamp: 2025-08-06T04:18:28.680Z
Learning: Windows 10 version 1809 and later, as well as Windows 11, include built-in ONNX Runtime as part of Windows ML (WinML) exposed through Windows.AI.MachineLearning.dll and WinRT APIs. However, JNI libraries typically depend on standalone ONNX Runtime C/C++ libraries rather than the Windows ML API.
1.conver java-api to maven project
2.pushlish java-api to maven central
Summary by CodeRabbit
New Features
Documentation
Chores
.gitignorefile to cover a wide range of development environments and tools..gitignoreand README files for JNI native library resources.Style
Bug Fixes