Repository navigation
Add CXX API for KittenTTS - #2469
Conversation
✨ 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds CXX API support for KittenTTS to the sherpa-onnx project, enabling C++ developers to use the KittenTTS text-to-speech model through a C++ interface.
Key changes:
- Defines new C++ configuration structures for KittenTTS model parameters
- Implements configuration mapping between C++ and C APIs
- Adds a complete C++ example demonstrating KittenTTS usage
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sherpa-onnx/c-api/cxx-api.h | Defines OfflineTtsKittenModelConfig struct and integrates it into OfflineTtsModelConfig |
| sherpa-onnx/c-api/cxx-api.cc | Implements configuration mapping from C++ to C API for KittenTTS parameters |
| cxx-api-examples/kitten-tts-en-cxx-api.cc | Complete C++ example demonstrating KittenTTS usage with configuration and audio generation |
| cxx-api-examples/CMakeLists.txt | Adds build target for the new KittenTTS C++ example |
| c-api-examples/kitten-tts-en-c-api.c | Updates voice mapping comments to reflect current KittenTTS voice names |
| .github/workflows/cxx-api.yaml | Adds CI workflow step to test KittenTTS C++ API functionality |
| @@ -0,0 +1,73 @@ | |||
| // cxx-api-examples/kitten-tts-en-cxx-api.c | |||
There was a problem hiding this comment.
The file extension in the comment should be '.cc' to match the actual filename, not '.c'.
| // cxx-api-examples/kitten-tts-en-cxx-api.c | |
| // cxx-api-examples/kitten-tts-en-cxx-api.cc |
| WriteWave(filename, {audio.samples, audio.sample_rate}); | ||
|
|
||
| fprintf(stderr, "Input text is: %s\n", text.c_str()); | ||
| fprintf(stderr, "Speaker ID is is: %d\n", sid); |
There was a problem hiding this comment.
There is a duplicated word 'is' in the output message. It should be 'Speaker ID is: %d\n'.
| fprintf(stderr, "Speaker ID is is: %d\n", sid); | |
| fprintf(stderr, "Speaker ID is: %d\n", sid); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
c-api-examples/kitten-tts-en-c-api.c (1)
58-60: Kitten sid→voice mapping updated — looks good. Minor robustness nit.The new mapping in comments matches the Kitten voices. Consider guarding against out-of-range sids at runtime to keep the example resilient across model updates.
You could add (outside this block) after creating TTS:
int32_t num_speakers = SherpaOnnxOfflineTtsNumSpeakers(tts); if (sid < 0 || sid >= num_speakers) sid = 0;cxx-api-examples/kitten-tts-en-cxx-api.cc (7)
25-31: Silence unused parameters in ProgressCallbacksamples, num_samples, and arg are unused and will trigger warnings. Keep the signature but silence cleanly.
static int32_t ProgressCallback(const float *samples, int32_t num_samples, float progress, void *arg) { - fprintf(stderr, "Progress: %.3f%%\n", progress * 100); + (void)samples; + (void)num_samples; + (void)arg; + std::fprintf(stderr, "Progress: %.3f%%\n", progress * 100); // return 1 to continue generating // return 0 to stop generating return 1; }Note: This change assumes is included; otherwise switch to <stdio.h> and keep fprintf unqualified.
68-70: Qualify fprintf and fix a typo
- Prefer std::fprintf when including .
- Fix “is is”.
- fprintf(stderr, "Input text is: %s\n", text.c_str()); - fprintf(stderr, "Speaker ID is is: %d\n", sid); - fprintf(stderr, "Saved to: %s\n", filename.c_str()); + std::fprintf(stderr, "Input text is: %s\n", text.c_str()); + std::fprintf(stderr, "Speaker ID is: %d\n", sid); + std::fprintf(stderr, "Saved to: %s\n", filename.c_str());
57-58: Use a float literalMinor: make the literal type match the variable.
- float speed = 1.0; // larger -> faster in speech speed + float speed = 1.0f; // larger -> faster in speech speed
37-41: Avoid hardcoding model paths; add basic existence checks or CLI flagsHardcoded relative paths reduce portability and make CI/local runs brittle if the working directory differs. Consider either:
- Accepting a --model-dir argument (and compose the file paths), or
- Reading a KITTTEN_MODEL_DIR environment variable, and
- Checking that each file exists, failing with a clear message.
Example minimal change (C++17 filesystem):
+#include <filesystem> +namespace fs = std::filesystem; ... - config.model.kitten.model = "./kitten-nano-en-v0_1-fp16/model.fp16.onnx"; - config.model.kitten.voices = "./kitten-nano-en-v0_1-fp16/voices.bin"; - config.model.kitten.tokens = "./kitten-nano-en-v0_1-fp16/tokens.txt"; - config.model.kitten.data_dir = "./kitten-nano-en-v0_1-fp16/espeak-ng-data"; + std::string model_dir = (argc > 1) ? argv[1] : "./kitten-nano-en-v0_1-fp16"; + config.model.kitten.model = model_dir + "/model.fp16.onnx"; + config.model.kitten.voices = model_dir + "/voices.bin"; + config.model.kitten.tokens = model_dir + "/tokens.txt"; + config.model.kitten.data_dir = model_dir + "/espeak-ng-data"; + + for (auto&& p : {config.model.kitten.model, + config.model.kitten.voices, + config.model.kitten.tokens, + config.model.kitten.data_dir}) { + if (!fs::exists(p)) { + std::fprintf(stderr, "Path does not exist: %s\n", p.c_str()); + return 1; + } + }If you prefer not to add filesystem, at least accept a model_dir CLI arg.
59-64: Prefer runtime flag over compile-time #if to toggle callbackSwitching between callback/no-callback via #if 0 requires rebuilding. A simple runtime flag or heuristic is more user-friendly.
-#if 0 - // If you don't want to use a callback, then please enable this branch - GeneratedAudio audio = tts.Generate(text, sid, speed); -#else - GeneratedAudio audio = tts.Generate(text, sid, speed, ProgressCallback); -#endif + const bool use_callback = true; // or derive from argc/argv + GeneratedAudio audio = use_callback + ? tts.Generate(text, sid, speed, ProgressCallback) + : tts.Generate(text, sid, speed);
66-66: Consider basic result validation before writing WAVIf synthesis fails and returns empty audio, WriteWave will still emit a tiny file. A simple guard helps with debugging.
- WriteWave(filename, {audio.samples, audio.sample_rate}); + if (audio.samples.empty() || audio.sample_rate <= 0) { + std::fprintf(stderr, "No audio generated. Check model paths/config.\n"); + return 1; + } + WriteWave(filename, {audio.samples, audio.sample_rate});If the API guarantees non-empty audio on success, feel free to skip—just verify this assumption against the new Kitten TTS API behavior.
1-1: Fix filename in header commentNit: The header says .c, but this file is .cc.
-// cxx-api-examples/kitten-tts-en-cxx-api.c +// cxx-api-examples/kitten-tts-en-cxx-api.cc
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/cxx-api.yaml(1 hunks)c-api-examples/kitten-tts-en-c-api.c(1 hunks)cxx-api-examples/CMakeLists.txt(1 hunks)cxx-api-examples/kitten-tts-en-cxx-api.cc(1 hunks)sherpa-onnx/c-api/cxx-api.cc(1 hunks)sherpa-onnx/c-api/cxx-api.h(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-06T04:18:47.981Z
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:
cxx-api-examples/CMakeLists.txt
🔇 Additional comments (6)
cxx-api-examples/CMakeLists.txt (1)
127-129: Add Kitten TTS C++ example target — LGTM.Target and linkage mirror other TTS examples and are gated behind SHERPA_ONNX_ENABLE_TTS.
sherpa-onnx/c-api/cxx-api.cc (1)
395-400: Initialize Kitten TTS fields in OfflineTts::Create — LGTM.Field wiring (model/voices/tokens/data_dir/length_scale) is consistent with other TTS configs.
sherpa-onnx/c-api/cxx-api.h (2)
397-405: Introduce OfflineTtsKittenModelConfig — LGTM.Shape and defaults align with existing TTS configs. No ABI hazards beyond the expected header-only wrapper growth.
410-410: Extend OfflineTtsModelConfig withkitten— LGTM.Keeps ordering consistent with other TTS models; matches the implementation wiring in cxx-api.cc.
.github/workflows/cxx-api.yaml (2)
90-128: Add Kitten TTS CI step — LGTM.Build/link flags, model download, env library paths, run, and artifact upload follow existing patterns.
90-128: Artifact filename correctly matches example outputThe example C++ code defines the output filename as
./generated-kitten-en-cxx.wav, which exactly matches the path used in the workflow’s upload step. No changes are needed here.
| #include <string> | ||
|
|
||
| #include "sherpa-onnx/c-api/cxx-api.h" | ||
|
|
There was a problem hiding this comment.
Add missing headers for types and I/O
This file uses int32_t and fprintf but doesn’t include the corresponding headers. Depending on transitive includes is fragile and can break on some toolchains.
Apply:
#include <string>
+#include <cstdint>
+#include <cstdio>
#include "sherpa-onnx/c-api/cxx-api.h"Also qualify calls to fprintf (see below) or include <stdio.h> instead if you prefer using the global namespace.
📝 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.
| #include <string> | |
| #include "sherpa-onnx/c-api/cxx-api.h" | |
| #include <string> | |
| #include <cstdint> | |
| #include <cstdio> | |
| #include "sherpa-onnx/c-api/cxx-api.h" |
🤖 Prompt for AI Agents
In cxx-api-examples/kitten-tts-en-cxx-api.cc around lines 21 to 24, the file
uses int32_t and fprintf without including the necessary headers. Add #include
<cstdint> for int32_t and either #include <cstdio> and qualify fprintf with
std:: or include <stdio.h> to use fprintf in the global namespace. This ensures
the code compiles reliably across different toolchains.
| return 1; | ||
| } | ||
|
|
||
| int32_t main(int32_t argc, char *argv[]) { |
There was a problem hiding this comment.
Use a standard-conforming main signature
C++ requires main to return int (not int32_t). Using int32_t is non-standard and may fail on some platforms.
-int32_t main(int32_t argc, char *argv[]) {
+int main(int argc, char *argv[]) {📝 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(int32_t argc, char *argv[]) { | |
| int main(int argc, char *argv[]) { |
🤖 Prompt for AI Agents
In cxx-api-examples/kitten-tts-en-cxx-api.cc at line 33, the main function is
declared with a return type of int32_t, which is non-standard in C++. Change the
return type of main from int32_t to int to conform to the C++ standard and
ensure compatibility across platforms.
Summary by CodeRabbit
New Features
Documentation
Chores