Repository navigation
feat: add punctuation C++ API - #2510
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an OfflinePunctuation C++ wrapper: new config structs and OfflinePunctuation class in header; implements Create, private constructor, Destroy, and AddPunctuation in source mapping to C APIs; adds a punctuation example and a CMake target. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant CXX as OfflinePunctuation (C++)
participant C as C API
participant Model as Punctuation Engine
rect rgba(200,235,255,0.25)
note over Client,CXX: Create
Client->>CXX: OfflinePunctuation::Create(config)
CXX->>C: SherpaOnnxCreateOfflinePunctuation(mapped_config)
C-->>CXX: SherpaOnnxOfflinePunctuation*
CXX-->>Client: OfflinePunctuation wrapper
end
rect rgba(220,255,220,0.15)
note over Client,CXX: AddPunctuation
Client->>CXX: AddPunctuation(text)
CXX->>C: SherpaOfflinePunctuationAddPunct(text)
C->>Model: Process text
Model-->>C: punctuated char*
C-->>CXX: char*
CXX->>C: SherpaOfflinePunctuationFreeText(char*)
CXX-->>Client: std::string result
end
rect rgba(255,235,200,0.15)
note over Client,CXX: Destroy
Client->>CXX: Destroy(ptr)
CXX->>C: SherpaOnnxDestroyOfflinePunctuation(ptr)
C-->>CXX: (freed)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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/Issue comments)Type 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: 0
🧹 Nitpick comments (3)
sherpa-onnx/c-api/cxx-api.cc (2)
842-844: Use the passed-in pointer in Destroy() for consistency and clarityCurrent implementation ignores the parameter and uses p_. Most wrappers here call the underlying C destroy with the provided pointer. Align for consistency and to avoid surprises if Destroy() is ever called outside MoveOnly’s destructor.
Apply this diff:
void OfflinePunctuation::Destroy(const SherpaOnnxOfflinePunctuation *p) const { - SherpaOnnxDestroyOfflinePunctuation(p_); + SherpaOnnxDestroyOfflinePunctuation(p); }
846-851: Add a defensive null check in AddPunctuation()The C API appears to always return a valid buffer, but adding a guard costs nothing and avoids UB if a future change returns nullptr on error.
Apply this diff:
std::string OfflinePunctuation::AddPunctuation(const std::string &text) const { - const char *result = SherpaOfflinePunctuationAddPunct(p_, text.c_str()); - std::string ans(result); - SherpaOfflinePunctuationFreeText(result); - return ans; + const char *result = SherpaOfflinePunctuationAddPunct(p_, text.c_str()); + if (!result) { + return {}; + } + std::string ans(result); + SherpaOfflinePunctuationFreeText(result); + return ans; }sherpa-onnx/c-api/cxx-api.h (1)
697-699: Minor grammar: “punctuation” is uncountable hereNit: singular reads better.
- // Add punctuations to the input text and return it. + // Add punctuation to the input text and return it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
sherpa-onnx/c-api/cxx-api.cc(1 hunks)sherpa-onnx/c-api/cxx-api.h(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
sherpa-onnx/c-api/cxx-api.cc (1)
sherpa-onnx/c-api/c-api.cc (8)
SherpaOnnxCreateOfflinePunctuation(1898-1922)SherpaOnnxCreateOfflinePunctuation(1898-1899)SherpaOnnxDestroyOfflinePunctuation(1924-1927)SherpaOnnxDestroyOfflinePunctuation(1924-1925)SherpaOfflinePunctuationAddPunct(1929-1938)SherpaOfflinePunctuationAddPunct(1929-1930)SherpaOfflinePunctuationFreeText(1940-1940)SherpaOfflinePunctuationFreeText(1940-1940)
sherpa-onnx/c-api/cxx-api.h (3)
sherpa-onnx/c-api/cxx-api.cc (19)
OfflinePunctuation(839-840)Create(49-111)Create(49-50)Create(287-293)Create(287-288)Create(393-441)Create(393-393)Create(491-533)Create(491-491)Create(606-620)Create(606-607)Create(647-650)Create(647-647)Create(686-712)Create(686-687)Create(776-783)Create(776-779)AddPunctuation(846-851)AddPunctuation(846-846)sherpa-onnx/python/csrc/offline-punctuation.cc (1)
sherpa_onnx(11-51)sherpa-onnx/csrc/offline-punctuation.cc (1)
sherpa_onnx(15-53)
🔇 Additional comments (3)
sherpa-onnx/c-api/cxx-api.cc (1)
827-837: Create() config translation and lifetime handling look correct
- Zero-initialization + field mapping is consistent with the rest of the C++ wrappers.
- Passing c_str() is safe here since the C API consumes it during the call.
- Returning a MoveOnly wrapper around the C pointer follows the existing pattern.
sherpa-onnx/c-api/cxx-api.h (2)
679-689: Config structs are straightforward and align with the C APIThe fields match the underlying C config (ct_transformer, num_threads, debug, provider). Defaults mirror conventions elsewhere. Looks good.
690-703: C++ wrapper class shape matches existing MoveOnly patternCreate/Destroy/AddPunctuation + private explicit ctor are consistent with other components (e.g., TTS, VAD). Public API surface is minimal and clear.
csukuangfj
left a comment
csukuangfj
left a comment
There was a problem hiding this comment.
Looks good to me.
Thank you for your contribution!
Can you also add an example to cxx-api-examples to show how to use the added APIs?
There was a problem hiding this comment.
Pull Request Overview
This PR adds a C++ wrapper API for offline punctuation functionality, enabling automatic punctuation restoration for text input. The implementation follows the existing C++ API design patterns in the codebase.
Key changes:
- Added
OfflinePunctuationclass with configuration structs for model settings - Implemented core methods for creating, destroying, and processing text with punctuation
- Provided C++ wrapper interface that bridges to underlying C API functions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sherpa-onnx/c-api/cxx-api.h | Added OfflinePunctuation class declaration and configuration structs |
| sherpa-onnx/c-api/cxx-api.cc | Implemented OfflinePunctuation class methods with C API integration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| OfflinePunctuation::OfflinePunctuation(const SherpaOnnxOfflinePunctuation *p) | ||
| : MoveOnly<OfflinePunctuation, SherpaOnnxOfflinePunctuation>(p) {} | ||
|
|
||
| void OfflinePunctuation::Destroy(const SherpaOnnxOfflinePunctuation *p) const { |
There was a problem hiding this comment.
The Destroy method ignores the parameter p and uses the member variable p_ instead. This could lead to incorrect cleanup if the parameter doesn't match the instance's pointer. Either use the parameter p or remove it from the method signature.
| void OfflinePunctuation::Destroy(const SherpaOnnxOfflinePunctuation *p) const { | |
| void OfflinePunctuation::Destroy() const { |
sure! |
| : MoveOnly<OfflinePunctuation, SherpaOnnxOfflinePunctuation>(p) {} | ||
|
|
||
| void OfflinePunctuation::Destroy(const SherpaOnnxOfflinePunctuation *p) const { | ||
| SherpaOnnxDestroyOfflinePunctuation(p_); |
There was a problem hiding this comment.
Can you replace p_ with p?
Also, there are existing places in this file that should also be changed to p.
There was a problem hiding this comment.
done, although i dont know why; i think they are same when use the Destroy method.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
sherpa-onnx/c-api/cxx-api.cc (1)
842-844: Good: Destroy() uses the parameter, not the memberThis addresses prior feedback about using the method parameter (
p) instead of the member (p_) for destruction and aligns with the pattern requested in earlier review comments.
🧹 Nitpick comments (2)
sherpa-onnx/c-api/cxx-api.cc (2)
827-837: Minor: Align Create() local naming with file conventionMost Create() methods in this file use
auto p = ...; return Type(p);. Consider renamingpuncttopfor consistency and drop the explicit type.- const SherpaOnnxOfflinePunctuation *punct = SherpaOnnxCreateOfflinePunctuation(&c); - return OfflinePunctuation(punct); + auto p = SherpaOnnxCreateOfflinePunctuation(&c); + return OfflinePunctuation(p);
824-826: Follow-up: Please add a cxx-api example and minimal doc blurbPer the review thread, an example in cxx-api-examples showing OfflinePunctuation::Create and AddPunctuation would be helpful. A short README snippet mirroring other APIs would complete the feature.
I can scaffold the example (build targets, snippet, and a minimal README section). Want me to draft that?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
sherpa-onnx/c-api/cxx-api.cc(1 hunks)
🔇 Additional comments (1)
sherpa-onnx/c-api/cxx-api.cc (1)
846-851: Ignore C API prefix change suggestionThe functions
SherpaOfflinePunctuationAddPunctandSherpaOfflinePunctuationFreeTextare correctly declared insherpa-onnx/c-api/c-api.hand defined insherpa-onnx/c-api/c-api.cc. There is no symbol namedSherpaOnnxOfflinePunctuationAddPunct—renaming would break linkage. Also, the C API always returns a valid buffer (it throws on allocation failure), so no null‐pointer guard is required.Likely an incorrect or invalid review comment.
done |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
cxx-api-examples/punctuation-cxx-api.cc (3)
37-37: Remove incomplete usage note or provide sample output.The usage text says “The output text should look like below:” but shows nothing after. Either add a sample or drop the line to avoid confusion. Suggest removing it for now.
- The output text should look like below:
41-41: Align model path in code with the usage instructions.Your usage block shows extracting the ct-transformer model into a directory. The code points to ./models/punctuation.onnx, which may confuse users. Consider matching the path to the extracted model.
- punctuation_config.model.ct_transformer = "./models/punctuation.onnx"; + punctuation_config.model.ct_transformer = "./sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12/model.onnx";
35-35: Fix typo in the sample text: “Fantasitic” → “Fantastic”.Minor polish to the demo input text.
- "你好吗how are you Fantasitic 谢谢我很好你怎么样呢" + "你好吗how are you Fantastic 谢谢我很好你怎么样呢"- std::string text = "你好吗how are you Fantasitic 谢谢我很好你怎么样呢"; + std::string text = "你好吗how are you Fantastic 谢谢我很好你怎么样呢";Also applies to: 46-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
cxx-api-examples/punctuation-cxx-api.cc(1 hunks)
🔇 Additional comments (1)
cxx-api-examples/punctuation-cxx-api.cc (1)
39-50: Example flow looks good and clear.Creating the config, building the punctuator, applying AddPunctuation, and printing results is straightforward and matches the new API shape.
|
|
||
| #include "sherpa-onnx/c-api/cxx-api.h" | ||
|
|
||
| int32_t main() { |
There was a problem hiding this comment.
Use standard-conforming int main(); int32_t main() may fail without .
C++ requires main to return int. Using int32_t can be non-portable and requires <cstdint> to be included. Simplest fix: use int.
-int32_t main() {
+int main() {📝 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.
| int32_t main() { | |
| int main() { |
🤖 Prompt for AI Agents
In cxx-api-examples/punctuation-cxx-api.cc around line 21, the main function is
declared as int32_t main(), which is non-standard and can fail without including
<cstdint>; change the signature to int main() (and adjust any return statements
if necessary, e.g., return 0) so the program uses the standard-conforming return
type and no extra header is required.
| @@ -0,0 +1,50 @@ | |||
| // cxx-api-examples/fire-red-asr-cxx-api.cc | |||
There was a problem hiding this comment.
| // cxx-api-examples/fire-red-asr-cxx-api.cc | |
| // cxx-api-examples/punctuation-cxx-api.cc |
Please also change it to your info.
| // Copyright (c) 2025 Xiaomi Corporation | ||
|
|
||
| // | ||
| // This file demonstrates how to use FireRedAsr AED with sherpa-onnx's C++ API. |
There was a problem hiding this comment.
Please update the comment or you can delete it since you have already included in the usage info in the code.
| tar xvf sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12.tar.bz2 | ||
| rm sherpa-onnx-punct-ct-transformer-zh-en-vocab272727-2024-04-12.tar.bz2 | ||
|
|
||
| ./bin/sherpa-onnx-offline-punctuation \ |
| // | ||
| // clang-format on | ||
|
|
||
| #include <chrono> // NOLINT |
There was a problem hiding this comment.
please remove unused headers.
| @@ -0,0 +1,50 @@ | |||
| // cxx-api-examples/fire-red-asr-cxx-api.cc | |||
There was a problem hiding this comment.
Please also update cxx-api-examples/CMakeLists.txt to include this file.
| add_executable(nemo-canary-cxx-api ./nemo-canary-cxx-api.cc) | ||
| target_link_libraries(nemo-canary-cxx-api sherpa-onnx-cxx-api) | ||
|
|
||
| add_executable(punctuation-cxx-api.cc ./punctuation-cxx-api.cc) |
There was a problem hiding this comment.
| add_executable(punctuation-cxx-api.cc ./punctuation-cxx-api.cc) | |
| add_executable(punctuation-cxx-api ./punctuation-cxx-api.cc) |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
cxx-api-examples/CMakeLists.txt(1 hunks)cxx-api-examples/punctuation-cxx-api.cc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cxx-api-examples/punctuation-cxx-api.cc
|
Thanks! |
|
good job. I will appreciate if someone could contribute the API for onlinePunctuation of c++; |
1 similar comment
|
good job. I will appreciate if someone could contribute the API for onlinePunctuation of c++; |
描述
添加了punctuation的C++ API接口。
修改内容
cxx-api.h中添加了OfflinePunctuation类声明cxx-api.cc中实现了OfflinePunctuation类的方法Create、Destroy和AddPunctuation方法修改类型
测试
检查清单
Summary by CodeRabbit
New Features
Examples