Repository navigation
Conversation
dosubot
Bot
added
the
size:XXL
WalkthroughAdds Piper-based offline TTS: new Piper model config, model/metadata classes, phonemization and phoneme→ID mapping, voice loading and synthesis, a Piper OfflineTts implementation (with Android support), CMake additions, and a new C++ example; also updates CLI usage text. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Example as piper-tts-cxx-api
participant API as OfflineTts (C++ API)
participant Impl as OfflineTtsPiperImpl
participant Voice as Piper::Voice
participant Phon as Piper Phonemizer
participant Map as Phoneme→ID
participant Model as Piper ONNX Model
User->>Example: run with text + Piper config
Example->>API: OfflineTts::Create(config)
API->>Impl: construct Piper backend
Impl->>Voice: loadVoice(model, config)
Note right of Voice: parse JSON, init ONNX session
Example->>API: Generate(text, sid, speed, callback)
API->>Impl: Generate(...)
Impl->>Phon: phonemize(text)
Phon-->>Impl: phrases of phonemes
Impl->>Map: phonemes_to_ids(phrase)
Map-->>Impl: phoneme ID sequence
loop per phrase
Impl->>Model: Run(ids, speaker, speed)
Model-->>Impl: audio chunk
Impl->>Example: optional progress callback(chunk)
end
Impl-->>API: GeneratedAudio
API-->>Example: write WAV, finish
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Nitpick comments (31)
sherpa-onnx/csrc/offline-tts-piper-model-meta-data.h (1)
8-11: Unify phoneme-id map type with Piper utilitiesThe metadata uses unordered_map<char32_t,int64_t>, but Piper’s phoneme ID utilities use piper::PhonemeIdMap (mapping phoneme -> vector). Aligning types avoids conversions and preserves multi-ID mappings.
#include <cstdint> #include <string> -#include <unordered_map> +#include "sherpa-onnx/csrc/piper-phoneme-ids.h" @@ - // Phoneme ID mapping - std::unordered_map<char32_t, int64_t> phoneme_id_map; + // Phoneme ID mapping (keep consistent with piper::PhonemeIdMap) + piper::PhonemeIdMap phoneme_id_map;Nit: consider renaming intersperse_pad to interspersePad to match Piper config naming for easier mental mapping.
Also applies to: 32-34
sherpa-onnx/csrc/offline-tts-impl.cc (1)
53-55: Wire-in Piper creation path — LGTM (minor message nit)
Factory branching for Piper mirrors other models correctly.Optional: unify the error message here (“Please provide a tts model.”) with Validate()’s phrasing (“Please provide exactly one tts model.”) for consistency.
Also applies to: 76-78
sherpa-onnx/csrc/offline-tts-model-config.h (1)
29-29: Changing default debug=true is a behavior change; confirm intent or keep false for backward compatibility.This flips the default verbosity for all OfflineTts backends. If not intentional, revert to false; otherwise, document in release notes.
Apply this diff to revert if unintended:
- bool debug = true; + bool debug = false;cxx-api-examples/piper-tts-cxx-api.cc (1)
45-47: Minor: comment contradicts value (debug=1 prints).Comment says “set to 0 to hide,” but the example sets 1. Either set to 0 or reword the comment.
sherpa-onnx/csrc/offline-tts-piper-model-config.cc (1)
13-22: CLI option descriptions — small polish.Consider aligning phrasing with other backends (e.g., “Path to Piper ONNX model” / “Path to Piper model .json”).
sherpa-onnx/csrc/offline-tts-piper-model-config.h (1)
14-27: Header is fine; consider adding brief field comments for parity with other configs.Add 1-line comments for model/model_config_file/data_dir to improve self-documentation.
sherpa-onnx/csrc/piper-phoneme-ids.cc (3)
16-16: Make the default map const and use a conventional kPrefix name.Prevents accidental mutation and clarifies intent.
Apply:
-static PhonemeIdMap DEFAULT_PHONEME_ID_MAP = { +static const PhonemeIdMap kDefaultPhonemeIdMap = {
196-206: Gracefully handle missing BOS/PAD/EOS in custom maps.Using at() will throw if a custom map omits these keys. Add count checks and warn/fallback to defaults.
I can patch this to log a warning and skip BOS/EOS (or fall back to kDefaultPhonemeIdMap) when keys are missing.
212-229: Minor perf: reserve and use find().
- Reserve approximate capacity for phonemeIds when interspersePad=true.
- Use find() to avoid double lookups.
sherpa-onnx/csrc/offline-tts-piper-model.h (3)
12-15: Harden Android guards and avoid unnecessary JNI include in a public header
- Use defined checks to avoid surprises when ANDROID_API is unset on non-Android builds.
- Drop android/asset_manager_jni.h from the header; it’s not needed for AAssetManager* and drags JNI deps into all includers.
-#if __ANDROID_API__ >= 9 -#include "android/asset_manager.h" -#include "android/asset_manager_jni.h" -#endif +#if defined(__ANDROID__) && defined(__ANDROID_API__) && (__ANDROID_API__ >= 9) +#include "android/asset_manager.h" +#endif ... -#if __ANDROID_API__ >= 9 +#if defined(__ANDROID_API__) && (__ANDROID_API__ >= 9) OfflineTtsPiperModel(AAssetManager *mgr, const OfflineTtsModelConfig &config); #endifAlso applies to: 27-31
23-55: Make copy/move semantics explicitThe unique_ptr member makes the class non-copyable implicitly; be explicit to prevent accidental copies and clarify intent.
class OfflineTtsPiperModel { public: explicit OfflineTtsPiperModel(const OfflineTtsModelConfig &config); + OfflineTtsPiperModel(const OfflineTtsPiperModel&) = delete; + OfflineTtsPiperModel& operator=(const OfflineTtsPiperModel&) = delete; + OfflineTtsPiperModel(OfflineTtsPiperModel&&) noexcept = default; + OfflineTtsPiperModel& operator=(OfflineTtsPiperModel&&) noexcept = default;
34-45: Clarify ownership/expectations for Ort::Value inputRun takes phoneme_ids by value and will std::move it; document that the caller should not reuse phoneme_ids after calling Run and that it must be a CPU tensor with shape (T,). Minor doc tweak.
sherpa-onnx/csrc/offline-tts-piper-impl.h (1)
12-15: Same Android guard tightening; remove JNI includeMirror the header changes here to keep build flags consistent and avoid unnecessary JNI includes.
-#if __ANDROID_API__ >= 9 -#include "android/asset_manager.h" -#include "android/asset_manager_jni.h" -#endif +#if defined(__ANDROID__) && defined(__ANDROID_API__) && (__ANDROID_API__ >= 9) +#include "android/asset_manager.h" +#endif ... -#if __ANDROID_API__ >= 9 +#if defined(__ANDROID_API__) && (__ANDROID_API__ >= 9) OfflineTtsPiperImpl(AAssetManager *mgr, const OfflineTtsConfig &config); #endifAlso applies to: 29-31
sherpa-onnx/csrc/piper-phonemize.cc (5)
40-42: Case-folding is a stubReturning lowercase is a placeholder. If this path is used for TextPhonemes, consider ICU/utf8proc for proper case folding.
98-103: Clear output on failure or guard partial resultsOn failure, phonemes may contain partial data. Either clear it before returning false or document partial results are possible.
bool phonemize(const std::string& text, const PhonemizeConfig& config, std::vector<std::vector<Phoneme>>& phonemes) { - + phonemes.clear();
113-120: Use info/debug logs instead of error logsThese are normal operational messages; LOGE is noisy.
- SHERPA_ONNX_LOGE("Using espeak-ng voice from loaded config: %s", espeak_config.voice.c_str()); + SHERPA_ONNX_LOGI("Using espeak-ng voice: %s", espeak_config.voice.c_str()); ... - SHERPA_ONNX_LOGE("Phonemized text into %zu sentence(s)", phonemes.size()); + SHERPA_ONNX_LOGI("Phonemized into %zu sentence(s)", phonemes.size());
25-32: Make DEFAULT_PHONEME_MAP immutable unless runtime mutation is intendedIf this is static configuration, prefer const (or function-local static) to avoid accidental mutation and data races.
-std::map<std::string, PhonemeMap> DEFAULT_PHONEME_MAP = { +const std::map<std::string, PhonemeMap> DEFAULT_PHONEME_MAP = { {"pt-br", {{U'c', {U'k'}}}} };
166-203: Make config parameter const and minor cleanupThe function doesn’t mutate config; take by const&, and use back() for clarity.
-void phonemize_codepoints(const std::string& text, CodepointsPhonemeConfig& config, +void phonemize_codepoints(const std::string& text, const CodepointsPhonemeConfig& config, std::vector<std::vector<Phoneme>>& phonemes) { ... - phonemes.emplace_back(); - auto sentPhonemes = &phonemes[phonemes.size() - 1]; + phonemes.emplace_back(); + auto* sentPhonemes = &phonemes.back();Note: Update the declaration in piper-phonemize.h accordingly.
sherpa-onnx/csrc/offline-tts-piper-impl.cc (3)
180-183: Lambda capture can be optimizedThe lambda captures
sentence_audio_bufferby reference, which is correct, but the callback pattern could be clearer.Consider making the capture more explicit:
- auto audio_callback = [&sentence_audio_buffer](std::vector<float>&& audio) { + auto audio_callback = [&sentence_audio_buffer](std::vector<float>&& audio) -> void { sentence_audio_buffer.insert(sentence_audio_buffer.end(), audio.begin(), audio.end()); };
255-256: Warning message might be too severeThe warning about missing piper-data-dir could be misleading if the model doesn't require espeak-ng initialization.
Consider using a debug-level log instead:
} else { - SHERPA_ONNX_LOGE("Warning: No piper-data-dir provided. Phonemization may fail."); + if (config_.model.debug) { + SHERPA_ONNX_LOGE("No piper-data-dir provided. Using model's built-in phonemization if available."); + } }
268-273: File reading error handling could be more specificThe error messages don't distinguish between file not found vs. read permission errors.
Consider adding more detailed error information when file reading fails to help with debugging.
sherpa-onnx/csrc/piper-phoneme-ids.h (2)
40-42: Make config parameter const.phonemes_to_ids doesn’t mutate PhonemeIdConfig; prefer const-ref.
Apply:
-void phonemes_to_ids(const std::vector<Phoneme> &phonemes, PhonemeIdConfig &config, +void phonemes_to_ids(const std::vector<Phoneme> &phonemes, const PhonemeIdConfig &config, std::vector<PhonemeId> &phonemeIds, std::map<Phoneme, std::size_t> &missingPhonemes);
9-11: Drop unused include (minor).isn’t used in this header.
Apply:
-#include <string>sherpa-onnx/csrc/piper-voice.cc (6)
277-281: Match printf-format type for ids[0].%lld expects long long; cast explicitly to avoid format warnings/UB on some platforms.
Apply:
- SHERPA_ONNX_LOGE("Parsed Unicode phoneme U+%04X -> %lld", - static_cast<uint32_t>(phoneme_char), ids[0]); + SHERPA_ONNX_LOGE("Parsed Unicode phoneme U+%04X -> %lld", + static_cast<uint32_t>(phoneme_char), + static_cast<long long>(ids[0]));
401-404: Keep input names in sync with provided tensors.When speakerId is absent, still declaring "sid" in names is brittle. Build names conditionally to match inputTensors size.
Apply:
- // Input and output names - std::vector<const char*> inputNames = {"input", "input_lengths", "scales", "sid"}; + // Input and output names (match tensors) + std::vector<const char*> inputNames = {"input", "input_lengths", "scales"}; + if (synthesisConfig.speakerId) { + inputNames.push_back("sid"); + } std::vector<const char*> outputNames = {"output"};Also applies to: 395-399
552-554: Use empty() for clarity.Minor readability tweak.
Apply:
- if (phrasePhonemes[phraseIdx]->size() <= 0) { + if (phrasePhonemes[phraseIdx]->empty()) { continue; }
35-41: Session options likely hurt performance; make configurable.Disabling all graph opts, mem arena, and patterns will slow inference. Consider enabling at least ORT_ENABLE_BASIC and keeping arena/patterns on, gated by config/env.
Example:
- options.SetGraphOptimizationLevel(GraphOptimizationLevel::ORT_DISABLE_ALL); - options.DisableCpuMemArena(); - options.DisableMemPattern(); + options.SetGraphOptimizationLevel(GraphOptimizationLevel::ORT_ENABLE_BASIC); + // Keep arenas/patterns enabled by default; allow disabling via config flags if needed.Also applies to: 313-322
329-347: Propagate meta/config flags to PhonemeIdConfig.Currently idConfig uses defaults (interspersePad/addBos/addEos). Read these from model config/meta to match Piper behavior.
Would you like me to thread OfflineTtsPiperModelMetaData.intersperse_pad/pad_id/bos_id/eos_id into PhonemeIdConfig here?
Also applies to: 495-561
431-436: Log levels: use info/debug for normal flow.SHERPA_ONNX_LOGE is for errors; most of these are status logs.
Apply, e.g., replace with SHERPA_ONNX_LOGI or SHERPA_ONNX_LOGD where available.
Also applies to: 579-581, 612-616
sherpa-onnx/csrc/piper-voice.h (2)
29-42: Don’t hard-disable ORT optimizations by default.Prefer a config knob; basic optimizations and arenas materially improve TTS throughput.
See proposed diff in piper-voice.cc to align defaults.
137-146: Tighten API: align input name handling with model I/O.Document that "sid" is optional and only supplied when present; consider exposing an introspection API to query inputs from ModelSession.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
cxx-api-examples/CMakeLists.txt(1 hunks)cxx-api-examples/piper-tts-cxx-api.cc(1 hunks)sherpa-onnx/c-api/cxx-api.h(1 hunks)sherpa-onnx/csrc/CMakeLists.txt(1 hunks)sherpa-onnx/csrc/offline-tts-impl.cc(3 hunks)sherpa-onnx/csrc/offline-tts-model-config.cc(3 hunks)sherpa-onnx/csrc/offline-tts-model-config.h(3 hunks)sherpa-onnx/csrc/offline-tts-piper-impl.cc(1 hunks)sherpa-onnx/csrc/offline-tts-piper-impl.h(1 hunks)sherpa-onnx/csrc/offline-tts-piper-model-config.cc(1 hunks)sherpa-onnx/csrc/offline-tts-piper-model-config.h(1 hunks)sherpa-onnx/csrc/offline-tts-piper-model-meta-data.cc(1 hunks)sherpa-onnx/csrc/offline-tts-piper-model-meta-data.h(1 hunks)sherpa-onnx/csrc/offline-tts-piper-model.cc(1 hunks)sherpa-onnx/csrc/offline-tts-piper-model.h(1 hunks)sherpa-onnx/csrc/piper-phoneme-ids.cc(1 hunks)sherpa-onnx/csrc/piper-phoneme-ids.h(1 hunks)sherpa-onnx/csrc/piper-phonemize.cc(1 hunks)sherpa-onnx/csrc/piper-phonemize.h(1 hunks)sherpa-onnx/csrc/piper-voice.cc(1 hunks)sherpa-onnx/csrc/piper-voice.h(1 hunks)sherpa-onnx/csrc/sherpa-onnx-offline-tts.cc(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
cxx-api-examples/piper-tts-cxx-api.cc (1)
sherpa-onnx/csrc/offline-tts-impl.cc (6)
Create(40-60)Create(40-41)Create(63-82)Create(63-64)Create(85-86)Create(90-91)
sherpa-onnx/csrc/offline-tts-piper-model-config.h (4)
sherpa-onnx/csrc/offline-tts-model-config.h (1)
sherpa_onnx(18-58)sherpa-onnx/csrc/offline-tts-kokoro-model-config.h (1)
sherpa_onnx(12-61)sherpa-onnx/csrc/offline-tts-matcha-model-config.h (1)
sherpa_onnx(12-54)sherpa-onnx/csrc/offline-tts-kitten-model-config.h (1)
sherpa_onnx(12-41)
sherpa-onnx/csrc/offline-tts-piper-model-meta-data.cc (2)
sherpa-onnx/csrc/offline-tts-model-config.cc (2)
ToString(64-79)ToString(64-64)sherpa-onnx/csrc/offline-tts-piper-model-config.cc (2)
ToString(36-45)ToString(36-36)
sherpa-onnx/csrc/piper-phonemize.h (3)
sherpa-onnx/csrc/piper-phoneme-ids.h (2)
sherpa_onnx(15-45)piper(16-44)sherpa-onnx/csrc/piper-voice.h (2)
sherpa_onnx(19-151)piper(20-71)sherpa-onnx/csrc/piper-phonemize.cc (4)
phonemize(99-164)phonemize(99-100)phonemize_codepoints(166-203)phonemize_codepoints(166-167)
sherpa-onnx/csrc/offline-tts-model-config.h (3)
sherpa-onnx/csrc/piper-phoneme-ids.h (1)
piper(16-44)sherpa-onnx/csrc/piper-phonemize.h (1)
piper(15-38)sherpa-onnx/csrc/piper-voice.h (1)
piper(20-71)
sherpa-onnx/csrc/piper-phoneme-ids.h (2)
sherpa-onnx/csrc/piper-phonemize.h (2)
sherpa_onnx(14-60)piper(15-38)sherpa-onnx/csrc/piper-phoneme-ids.cc (2)
phonemes_to_ids(187-254)phonemes_to_ids(187-189)
sherpa-onnx/csrc/offline-tts-piper-model.h (4)
sherpa-onnx/csrc/offline-tts-model-config.h (1)
sherpa_onnx(18-58)sherpa-onnx/csrc/offline-tts-piper-impl.h (1)
sherpa_onnx(23-53)sherpa-onnx/csrc/offline-tts-piper-model-meta-data.h (1)
sherpa_onnx(12-40)sherpa-onnx/csrc/offline-tts-piper-model.cc (13)
OfflineTtsPiperModel(335-336)OfflineTtsPiperModel(339-341)OfflineTtsPiperModel(344-344)Run(346-349)Run(346-347)phoneme_ids(57-71)phoneme_ids(57-57)Allocator(351-353)Allocator(351-351)GetMetaData(355-357)GetMetaData(355-355)Impl(27-39)Impl(42-54)
sherpa-onnx/csrc/offline-tts-piper-impl.cc (4)
sherpa-onnx/csrc/offline-tts-piper-impl.h (1)
OfflineTtsPiperImpl(25-51)sherpa-onnx/csrc/piper-phonemize.cc (2)
phonemize(99-164)phonemize(99-100)sherpa-onnx/csrc/piper-phoneme-ids.cc (2)
phonemes_to_ids(187-254)phonemes_to_ids(187-189)sherpa-onnx/csrc/piper-voice.cc (4)
synthesize(352-438)synthesize(352-354)loadVoice(324-350)loadVoice(324-325)
sherpa-onnx/csrc/piper-voice.cc (2)
sherpa-onnx/csrc/piper-phonemize.cc (4)
phonemize(99-164)phonemize(99-100)phonemize_codepoints(166-203)phonemize_codepoints(166-167)sherpa-onnx/csrc/piper-phoneme-ids.cc (2)
phonemes_to_ids(187-254)phonemes_to_ids(187-189)
sherpa-onnx/csrc/piper-phonemize.cc (1)
sherpa-onnx/csrc/offline-tts-piper-impl.cc (1)
CallPhonemizeEspeak(37-39)
sherpa-onnx/csrc/offline-tts-piper-model.cc (2)
sherpa-onnx/csrc/ten-vad-model.cc (1)
meta_data(257-314)sherpa-onnx/csrc/offline-tts-piper-model.h (1)
OfflineTtsPiperModel(23-55)
sherpa-onnx/csrc/offline-tts-piper-impl.h (6)
sherpa-onnx/csrc/offline-tts-model-config.h (1)
sherpa_onnx(18-58)sherpa-onnx/csrc/offline-tts-piper-model-config.h (1)
sherpa_onnx(12-33)sherpa-onnx/csrc/offline-tts-piper-model.h (2)
sherpa_onnx(21-57)OfflineTtsPiperModel(23-55)sherpa-onnx/csrc/piper-voice.h (2)
sherpa_onnx(19-151)piper(20-71)sherpa-onnx/csrc/offline-tts-impl.h (1)
OfflineTtsImpl(18-52)sherpa-onnx/csrc/offline-tts-frontend.h (1)
OfflineTtsFrontend(38-56)
sherpa-onnx/csrc/offline-tts-model-config.cc (3)
sherpa-onnx/csrc/piper-phoneme-ids.h (1)
piper(16-44)sherpa-onnx/csrc/piper-phonemize.h (1)
piper(15-38)sherpa-onnx/csrc/piper-voice.h (1)
piper(20-71)
sherpa-onnx/csrc/piper-voice.h (5)
sherpa-onnx/csrc/offline-tts-piper-impl.h (1)
sherpa_onnx(23-53)sherpa-onnx/csrc/offline-tts-piper-model-meta-data.h (1)
sherpa_onnx(12-40)sherpa-onnx/csrc/piper-phoneme-ids.h (2)
sherpa_onnx(15-45)piper(16-44)sherpa-onnx/csrc/piper-phonemize.h (2)
sherpa_onnx(14-60)piper(15-38)sherpa-onnx/csrc/piper-voice.cc (10)
loadVoice(324-350)loadVoice(324-325)textToAudio(440-617)textToAudio(440-442)synthesize(352-438)synthesize(352-354)parsePhonemeIdMapFromJson(129-290)parsePhonemeIdMapFromJson(129-129)parseEspeakVoiceFromJson(49-127)parseEspeakVoiceFromJson(49-49)
sherpa-onnx/csrc/offline-tts-piper-model-meta-data.h (4)
sherpa-onnx/csrc/offline-tts-piper-model.h (1)
sherpa_onnx(21-57)sherpa-onnx/csrc/piper-phoneme-ids.h (1)
sherpa_onnx(15-45)sherpa-onnx/csrc/piper-voice.h (1)
sherpa_onnx(19-151)sherpa-onnx/csrc/offline-tts-matcha-model-meta-data.h (1)
sherpa_onnx(11-28)
sherpa-onnx/csrc/offline-tts-piper-model-config.cc (2)
sherpa-onnx/csrc/offline-tts-model-config.cc (6)
Register(11-27)Register(11-11)Validate(29-62)Validate(29-29)ToString(64-79)ToString(64-64)sherpa-onnx/csrc/offline-tts-vits-model-config.cc (1)
Register(12-115)
🔇 Additional comments (16)
sherpa-onnx/csrc/offline-tts-impl.cc (1)
22-22: Include Piper backend — LGTM
The include adds the Piper path cleanly with no side effects.sherpa-onnx/c-api/cxx-api.h (1)
427-427: Incorrect — OfflineTtsPiperModelConfig already defined; include existing header instead of duplicatingOfflineTtsPiperModelConfig is defined in sherpa-onnx/csrc/offline-tts-piper-model-config.h; do not add the duplicate struct — add/include that header in sherpa-onnx/c-api/cxx-api.h (or ensure it's already included).
Likely an incorrect or invalid review comment.
cxx-api-examples/CMakeLists.txt (1)
151-152: Ensure example source exists and is wired in docscxx-api-examples/piper-tts-cxx-api.cc exists. Add or update distribution/examples docs (README.md or docs/) to reference the piper-tts-cxx-api target/example.
sherpa-onnx/csrc/offline-tts-piper-model-meta-data.cc (1)
11-29: ToString implementation — LGTMClear, matches metadata fields, and consistent with other ToString() outputs.
sherpa-onnx/csrc/offline-tts-model-config.cc (3)
17-17: Register Piper flags — LGTM
Registration mirrors other model configs.
55-57: Validate Piper path — LGTM
Short-circuiting to piper.Validate() matches existing patterns.
73-73: Include Piper in ToString — LGTM
Keeps config dumps complete for debugging.sherpa-onnx/csrc/offline-tts-model-config.h (3)
13-13: Piper config wire-up looks correct.Header include is appropriate and matches usage elsewhere.
26-26: New piper member added in model config — LGTM.Consistent with factory checks in OfflineTtsImpl.
39-47: Constructor signature expanded — verify all call sites & public API/ABIRepo search shows many bindings/examples construct OfflineTtsModelConfig (scripts/*, kotlin-api/Tts.kt, flutter/sherpa_onnx/lib/src/tts.dart, java-api, dotnet, python examples) and I found no internal C++ call sites that pass the positional ctor. Changing the C++ ctor is ABI-breaking for external consumers — either add a compatibility overload/default for the new piper parameter in sherpa-onnx/csrc/offline-tts-model-config.h or update all public bindings and bump the public API/ABI.
sherpa-onnx/csrc/piper-phoneme-ids.cc (2)
15-185: Source provenance/license check for the default mapping.If adapted from Piper, ensure license headers/attribution satisfy upstream’s license.
1-11: Include dependency on Phoneme definition — verified.
piper-phoneme-ids.h directly includes piper-phonemize.h (sherpa-onnx/csrc/piper-phoneme-ids.h:13), and piper-phonemize.h definestypedef char32_t Phoneme;(sherpa-onnx/csrc/piper-phonemize.h:17).sherpa-onnx/csrc/offline-tts-piper-impl.h (1)
50-51: Confirm mutability of voice_data_ is intentionalvoice_data_ is mutable; ensure it’s only mutated for stats/caching needed by const Generate. If more than that, consider removing const from Generate or narrowing what’s mutable.
sherpa-onnx/csrc/piper-phonemize.cc (1)
116-118: Verify ::piper::Phoneme and sherpa_onnx::piper::Phoneme are identical and ABI-safeLocation: sherpa-onnx/csrc/piper-phonemize.cc:116-118
ripgrep returned "No files were searched" — typedefs not found; confirm both aliases resolve to the same underlying type (e.g., char32_t) and are ABI-compatible.
- Add compile-time guard: static_assert(std::is_same_v<::piper::Phoneme, sherpa_onnx::piper::Phoneme>);
- If they differ, convert before calling sherpa_onnx::CallPhonemizeEspeak or unify the typedef.
sherpa-onnx/csrc/piper-phonemize.h (1)
1-63: Well-structured header fileThe header file is properly organized with include guards, clear namespace hierarchy, and appropriate forward declarations. The type definitions and configuration structures are well-documented.
sherpa-onnx/csrc/offline-tts-piper-model.cc (1)
297-317: Hardcoded phoneme mappings might not match actual model requirementsThe default phoneme ID mappings are hardcoded and might not match what specific Piper models expect.
These default mappings should be verified against actual Piper model requirements. Consider loading defaults from a configuration file or documenting the source of these mappings.
| auto tts = OfflineTts::Create(config); | ||
| int32_t sid = 0; | ||
| float speed = 1.0; // larger -> faster in speech speed | ||
|
|
||
| #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 |
There was a problem hiding this comment.
Use pointer semantics for OfflineTts and guard Create() result.
OfflineTts::Create typically returns a pointer-like object; current code uses dot-call and misses a null check.
Apply:
- auto tts = OfflineTts::Create(config);
- int32_t sid = 0;
- float speed = 1.0; // larger -> faster in speech speed
+ auto tts = OfflineTts::Create(config);
+ if (!tts) {
+ fprintf(stderr, "Failed to create OfflineTts (invalid config?)\n");
+ return 1;
+ }
+ int32_t sid = 0;
+ float speed = 1.0; // larger -> faster in speech speedAnd call through the pointer:
- GeneratedAudio audio = tts.Generate(text, sid, speed);
+ GeneratedAudio audio = tts->Generate(text, sid, speed);- GeneratedAudio audio = tts.Generate(text, sid, speed, ProgressCallback);
+ GeneratedAudio audio = tts->Generate(text, sid, speed, ProgressCallback);If the API requires a user_data argument for the callback, pass nullptr as the final arg.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cxx-api-examples/piper-tts-cxx-api.cc around lines 54-63, OfflineTts::Create
likely returns a pointer-like object that must be null-checked and used with
pointer semantics; update the code to store the Create() result in a
pointer/unique_ptr, check it for null and handle the error (log/return) before
proceeding, replace dot calls (tts.Generate) with pointer dereference syntax
(tts->Generate), and if the Generate callback signature requires a user_data
parameter, pass nullptr as the final argument when calling Generate with
ProgressCallback.
| offline-tts-piper-impl.cc | ||
| offline-tts-piper-model-config.cc | ||
| offline-tts-piper-model.cc | ||
| offline-tts-piper-model-meta-data.cc | ||
| piper-phoneme-ids.cc | ||
| piper-phonemize.cc | ||
| piper-voice.cc | ||
| offline-tts-vits-model-config.cc |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Piper sources added — looks good; verify external dependency availability
The Piper sources are correctly appended under SHERPA_ONNX_ENABLE_TTS. Ensure the external/library target piper_phonemize is available in all CI presets (Linux/macOS/Windows) to avoid link errors.
Suggested quick checks:
🏁 Script executed:
#!/bin/bash
# Confirm sources exist
fd -a 'offline-tts-piper-|piper-phoneme|piper-phonemize|piper-voice' sherpa-onnx/csrc
# Confirm the CMake target or import for piper_phonemize exists
rg -nP "add_library\(|target_link_libraries\([^)]*piper_phonemize" -C2Length of output: 16158
Piper sources added — piper_phonemize CMake target missing; add/import it or ensure CI provides it
- Sources were added to sherpa-onnx/csrc/CMakeLists.txt (files present under sherpa-onnx/csrc/).
- Repo search found no CMake add_library/target_link_libraries/import for "piper_phonemize" — builds will likely fail with link errors.
- Action: define/import a piper_phonemize target (or add appropriate find_package/install steps) and ensure all CI presets (Linux/macOS/Windows) provide the dependency.
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/CMakeLists.txt around lines 204 to 211, new Piper source
files were added but there is no CMake target named piper_phonemize, which will
cause linker errors; fix by declaring a piper_phonemize target (add_library or
add_executable as appropriate) that includes the piper_phonemize source and any
other Piper sources that belong to that component, set
target_include_directories and target_link_libraries to the correct internal and
external dependencies, export/install the target (install(TARGETS ...) and
export(...) if the project exports targets), and update the CI presets or
platform-specific CMake logic to ensure the dependency is available on
Linux/macOS/Windows (or replace with find_package()/add_subdirectory() if the
phonemize component is provided externally).
| if (!LoadVoiceConfig()) { | ||
| SHERPA_ONNX_LOGE("Failed to load voice configuration"); | ||
| } |
There was a problem hiding this comment.
Continue initialization despite configuration loading failure
The constructor logs an error but continues execution when LoadVoiceConfig() fails. This could lead to runtime crashes when trying to use an improperly initialized model.
Consider throwing an exception or properly handling the failure:
if (!LoadVoiceConfig()) {
SHERPA_ONNX_LOGE("Failed to load voice configuration");
+ throw std::runtime_error("Failed to load voice configuration");
}📝 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.
| if (!LoadVoiceConfig()) { | |
| SHERPA_ONNX_LOGE("Failed to load voice configuration"); | |
| } | |
| if (!LoadVoiceConfig()) { | |
| SHERPA_ONNX_LOGE("Failed to load voice configuration"); | |
| throw std::runtime_error("Failed to load voice configuration"); | |
| } |
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/offline-tts-piper-impl.cc around lines 52 to 54, the
constructor only logs an error when LoadVoiceConfig() fails and continues
execution which can cause crashes later; modify the constructor to handle the
failure explicitly by either throwing a descriptive std::runtime_error (or
another appropriate exception type used in the project) including the failure
context, or by returning/setting an initialization-failed flag and aborting
further setup so that downstream methods cannot be used on an uninitialized
object; ensure resources allocated before the call are cleaned up before
throwing or that the object is left in a safe, non-usable state.
| if (!LoadVoiceConfig()) { | ||
| SHERPA_ONNX_LOGE("Failed to load voice configuration"); | ||
| } |
There was a problem hiding this comment.
Duplicate error handling logic in Android constructor
Same issue as above - the Android constructor also continues after configuration loading failure.
Apply the same fix as suggested for the standard constructor.
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/offline-tts-piper-impl.cc around lines 65-67, the Android
constructor logs a failure when LoadVoiceConfig() returns false but then
continues initialization; mirror the fix applied to the standard constructor by,
immediately after logging the error, marking the object as uninitialized (call
the same SetInitialized(false) or ok_ = false used in the other constructor) and
exit the constructor body early (i.e., return) to stop further initialization.
| if (!sherpa_onnx::piper::phonemize(text, voice_data_.phonemizeConfig, phonemes)) { | ||
| SHERPA_ONNX_LOGE("Failed to phonemize text"); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Missing null check for voice configuration
The code uses voice_data_.phonemizeConfig without verifying if it was successfully loaded. If LoadVoiceConfig() failed in the constructor, this could cause undefined behavior.
Add a check before using the voice configuration:
+ if (voice_data_.phonemizeConfig.voice.empty()) {
+ SHERPA_ONNX_LOGE("Voice configuration not loaded");
+ return result;
+ }
+
// Phonemize text using loaded voice configuration
if (!sherpa_onnx::piper::phonemize(text, voice_data_.phonemizeConfig, phonemes)) {📝 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.
| if (!sherpa_onnx::piper::phonemize(text, voice_data_.phonemizeConfig, phonemes)) { | |
| SHERPA_ONNX_LOGE("Failed to phonemize text"); | |
| return result; | |
| } | |
| if (voice_data_.phonemizeConfig.voice.empty()) { | |
| SHERPA_ONNX_LOGE("Voice configuration not loaded"); | |
| return result; | |
| } | |
| // Phonemize text using loaded voice configuration | |
| if (!sherpa_onnx::piper::phonemize(text, voice_data_.phonemizeConfig, phonemes)) { | |
| SHERPA_ONNX_LOGE("Failed to phonemize text"); | |
| return result; | |
| } |
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/offline-tts-piper-impl.cc around lines 105 to 108, the code
calls sherpa_onnx::piper::phonemize(text, voice_data_.phonemizeConfig, phonemes)
without verifying that voice_data_.phonemizeConfig was successfully loaded; add
a pre-check that voice_data_.phonemizeConfig is valid (e.g., not null/empty or
the optional/flag set by LoadVoiceConfig()), and if it's invalid log an error
with SHERPA_ONNX_LOGE and return the current result (or an appropriate error
result) instead of calling phonemize; alternatively, attempt to reload the voice
config and only call phonemize when the config is confirmed valid.
| #include <algorithm> | ||
| #include <chrono> | ||
| #include <sstream> | ||
|
|
||
| #include "sherpa-onnx/csrc/macros.h" | ||
|
|
||
| namespace sherpa_onnx { |
There was a problem hiding this comment.
Include for uint8_t usage.
uint8_t is used in UTF-8 decoding; add the header to avoid build breaks.
Apply:
#include <algorithm>
#include <chrono>
+#include <cstdint>
#include <sstream>📝 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 <algorithm> | |
| #include <chrono> | |
| #include <sstream> | |
| #include "sherpa-onnx/csrc/macros.h" | |
| namespace sherpa_onnx { | |
| #include <algorithm> | |
| #include <chrono> | |
| #include <cstdint> | |
| #include <sstream> | |
| #include "sherpa-onnx/csrc/macros.h" | |
| namespace sherpa_onnx { |
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/piper-voice.cc around lines 7 to 13, the file uses uint8_t
for UTF-8 decoding but doesn't include <cstdint>, which can cause build
failures; add the header by inserting #include <cstdint> with the other includes
so uint8_t is defined before its usage.
| // JSON parsing helper functions - parse phoneme_id_map and espeak config from JSON | ||
| bool parsePhonemizeConfig(const std::string& configData, PhonemizeConfig& phonemizeConfig) { | ||
| phonemizeConfig.phonemeType = PhonemizerPhonemes; | ||
|
|
||
| // Parse espeak voice from JSON config | ||
| if (!parseEspeakVoiceFromJson(configData, phonemizeConfig.voice)) { | ||
| SHERPA_ONNX_LOGE("Failed to parse espeak voice from config, using default"); | ||
| phonemizeConfig.voice = "en-us"; // Fallback default | ||
| } | ||
|
|
||
| // Parse phoneme_id_map from JSON config | ||
| if (!parsePhonemeIdMapFromJson(configData, phonemizeConfig.phonemeIdMap)) { | ||
| SHERPA_ONNX_LOGE("Failed to parse phoneme_id_map from config, using defaults"); | ||
| // Fallback to default phoneme ID map | ||
| phonemizeConfig.phonemeIdMap = { | ||
| {U'_', {0}}, {U'^', {1}}, {U'$', {2}}, {U' ', {3}}, | ||
| {U'!', {4}}, {U'\'', {5}}, {U'(', {6}}, {U')', {7}}, | ||
| {U',', {8}}, {U'-', {9}}, {U'.', {10}}, {U':', {11}}, | ||
| {U';', {12}}, {U'?', {13}} | ||
| }; | ||
| } | ||
|
|
||
| if (phonemizeConfig.phonemeIdMap.empty()) { | ||
| SHERPA_ONNX_LOGE("Warning: Empty phoneme_id_map loaded from config"); | ||
| return false; | ||
| } | ||
|
|
||
| SHERPA_ONNX_LOGE("Loaded %zu phoneme mappings from config", phonemizeConfig.phonemeIdMap.size()); | ||
| SHERPA_ONNX_LOGE("Using espeak voice: %s", phonemizeConfig.voice.c_str()); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace handwritten JSON parsing with a real JSON library.
Manual brace/quote scanning is fragile (strings with braces, escaped quotes, whitespace). Parse failures will be silent and lead to wrong configs (e.g., partial phoneme maps).
I can replace parsePhonemizeConfig/parseEspeakVoiceFromJson/parsePhonemeIdMapFromJson with nlohmann::json or RapidJSON and wire fields: espeak.voice, phoneme_id_map (UTF-8 keys), intersperse_pad, pad/bos/eos IDs, sample_rate, num_speakers, noise params. Want me to draft that?
Also applies to: 48-127, 129-291
| // Parse comma-separated IDs | ||
| std::istringstream iss(ids_str); | ||
| std::string id_token; | ||
| while (std::getline(iss, id_token, ',')) { | ||
| // Remove whitespace | ||
| id_token.erase(std::remove_if(id_token.begin(), id_token.end(), ::isspace), id_token.end()); | ||
| if (!id_token.empty()) { | ||
| try { | ||
| ids.push_back(std::stoll(id_token)); | ||
| } catch (const std::exception& e) { | ||
| SHERPA_ONNX_LOGE("Failed to parse phoneme ID: %s", id_token.c_str()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix undefined behavior with std::isspace on signed char.
Passing char to ::isspace is UB for non-ASCII. Cast to unsigned char.
Apply:
- // Remove whitespace
- id_token.erase(std::remove_if(id_token.begin(), id_token.end(), ::isspace), id_token.end());
+ // Remove whitespace (avoid UB)
+ auto is_ws = [](unsigned char c) { return std::isspace(c); };
+ id_token.erase(std::remove_if(id_token.begin(), id_token.end(),
+ [&](char c){ return is_ws(static_cast<unsigned char>(c)); }),
+ id_token.end());
@@
- // Remove whitespace
- id_str.erase(std::remove_if(id_str.begin(), id_str.end(), ::isspace), id_str.end());
+ // Remove whitespace (avoid UB)
+ auto is_ws2 = [](unsigned char c) { return std::isspace(c); };
+ id_str.erase(std::remove_if(id_str.begin(), id_str.end(),
+ [&](char c){ return is_ws2(static_cast<unsigned char>(c)); }),
+ id_str.end());Also applies to: 224-226
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/piper-voice.cc around lines 202 to 213 (and also apply the
same change at lines 224-226), using ::isspace directly on char can cause
undefined behavior for signed char on non-ASCII input; change the call to cast
each character to unsigned char before calling ::isspace (e.g., replace
::isspace with ::isspace(static_cast<unsigned char>(c)) inside the remove_if
predicate) so whitespace removal is safe for non-ASCII bytes.
| #include <map> | ||
| #include <memory> | ||
| #include <optional> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
There was a problem hiding this comment.
Include missing headers (, ).
std::function and fixed-width integers are used in this header.
Apply:
#include <map>
#include <memory>
#include <optional>
+#include <functional>
+#include <cstdint>
#include <string>
#include <vector>📝 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 <map> | |
| #include <memory> | |
| #include <optional> | |
| #include <string> | |
| #include <vector> | |
| #include <map> | |
| #include <memory> | |
| #include <optional> | |
| #include <functional> | |
| #include <cstdint> | |
| #include <string> | |
| #include <vector> |
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/piper-voice.h around lines 9 to 14, the header uses
std::function and fixed-width integer types but doesn't include <functional> or
<cstdint>; add #include <functional> and #include <cstdint> alongside the
existing includes (e.g., after <memory> or <optional>) so the types are declared
and the header compiles cleanly.
| --piper-model=D:/Sound/PiperModel/alan/en_GB-alan-medium.onnx --piper-model-config-file=D:/Sound/PiperModel/alan/en_GB-alan-medium.onnx.json --piper-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low --debug=1 --output-filename="D:\Sound\sherpa-onnx\generated.wav" "How are you?" | ||
| --vits-model=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/en_US-amy-low.onnx --vits-tokens=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/tokens.txt --vits-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/espeak-ng-data --output-filename=./generated.wav --debug=1 "How are you?" |
There was a problem hiding this comment.
Fix Piper example path and align usage style
- The Piper example likely needs espeak-ng-data; otherwise phonemizer lookup fails. Also, the help later advises using single quotes, but these examples use double quotes. Consider clarifying that Windows uses double quotes and Linux/macOS use single quotes, or provide both.
Apply this minimal correction to the data dir in the example:
- --piper-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low --debug=1 --output-filename="D:\Sound\sherpa-onnx\generated.wav" "How are you?"
+ --piper-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/espeak-ng-data --debug=1 --output-filename="D:\Sound\sherpa-onnx\generated.wav" "How are you?"📝 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.
| --piper-model=D:/Sound/PiperModel/alan/en_GB-alan-medium.onnx --piper-model-config-file=D:/Sound/PiperModel/alan/en_GB-alan-medium.onnx.json --piper-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low --debug=1 --output-filename="D:\Sound\sherpa-onnx\generated.wav" "How are you?" | |
| --vits-model=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/en_US-amy-low.onnx --vits-tokens=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/tokens.txt --vits-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/espeak-ng-data --output-filename=./generated.wav --debug=1 "How are you?" | |
| --piper-model=D:/Sound/PiperModel/alan/en_GB-alan-medium.onnx --piper-model-config-file=D:/Sound/PiperModel/alan/en_GB-alan-medium.onnx.json --piper-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/espeak-ng-data --debug=1 --output-filename="D:\Sound\sherpa-onnx\generated.wav" "How are you?" | |
| --vits-model=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/en_US-amy-low.onnx --vits-tokens=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/tokens.txt --vits-data-dir=D:/Sound/sherpa-onnx/vits-piper-en_US-amy-low/espeak-ng-data --output-filename=./generated.wav --debug=1 "How are you?" |
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/sherpa-onnx-offline-tts.cc around lines 20-21, the Piper
example uses the vits data dir for --piper-data-dir and inconsistent quoting;
update the Piper example to point --piper-data-dir to the espeak-ng-data
subdirectory (same as VITS) so phonemizer lookups succeed, and align the
help/example quoting by either showing both variants or noting “use double
quotes on Windows, single quotes on Linux/macOS” so examples match platform
conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sherpa-onnx/c-api/cxx-api.h(1 hunks)sherpa-onnx/csrc/offline-tts-model-config.h(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sherpa-onnx/c-api/cxx-api.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: ubuntu-latest Debug shared tts-OFF
- GitHub Check: ubuntu-latest Release shared tts-OFF
- GitHub Check: ubuntu-24.04 3.12
- GitHub Check: ubuntu-24.04 3.9
- GitHub Check: ubuntu-24.04 3.11
- GitHub Check: ubuntu-24.04 3.10
- GitHub Check: ubuntu-24.04 3.8
- GitHub Check: ubuntu-24.04 3.13
- GitHub Check: Release shared-OFF tts-OFF
- GitHub Check: Debug shared-OFF tts-ON
- GitHub Check: Release shared-ON tts-ON
- GitHub Check: Release shared-OFF tts-ON
- GitHub Check: Release shared-ON tts-OFF
- GitHub Check: Debug shared-OFF tts-OFF
- GitHub Check: Debug shared-ON tts-ON
- GitHub Check: Debug shared-ON tts-OFF
🔇 Additional comments (3)
sherpa-onnx/csrc/offline-tts-model-config.h (3)
13-13: Add Piper config header — LGTMConsistent with existing model-config includes; required since
OfflineTtsPiperModelConfigis a by-value member.
47-50: Initializer list order matches declaration — goodNo -Wreorder risk; construction is deterministic.
26-26: piper wiring verified; confirm intentional ABI break
- Wiring present:
piper.Register(po)in sherpa-onnx/csrc/offline-tts-model-config.cc (~line 17);piper.Validate()is called whenpiper.modelis non-empty (~lines 55–56);piper.ToString()is included in ToString (~line 73).- Action: adding this member changes the struct ABI — confirm this is an intended breaking change. If this header is part of a public ABI, bump SONAME/package version or document/notify consumers.
| const OfflineTtsPiperModelConfig &piper, | ||
| int32_t num_threads, bool debug, | ||
| const std::string &provider) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Constructor parameter order is source-breaking; add a back-compat overload
Existing call-sites passing (vits, matcha, kokoro, zipvoice, kitten, num_threads, debug, provider) will fail since piper was inserted before num_threads. Add an overload with the original signature that delegates, preserving source compatibility.
@@
OfflineTtsModelConfig(const OfflineTtsVitsModelConfig &vits,
const OfflineTtsMatchaModelConfig &matcha,
const OfflineTtsKokoroModelConfig &kokoro,
const OfflineTtsZipvoiceModelConfig &zipvoice,
const OfflineTtsKittenModelConfig &kitten,
- const OfflineTtsPiperModelConfig &piper,
+ const OfflineTtsPiperModelConfig &piper,
int32_t num_threads, bool debug,
const std::string &provider)
: vits(vits),
matcha(matcha),
kokoro(kokoro),
zipvoice(zipvoice),
kitten(kitten),
piper(piper),
num_threads(num_threads),
debug(debug),
provider(provider) {}
+
+ // Back-compat overload: preserves the old parameter order.
+ OfflineTtsModelConfig(const OfflineTtsVitsModelConfig &vits,
+ const OfflineTtsMatchaModelConfig &matcha,
+ const OfflineTtsKokoroModelConfig &kokoro,
+ const OfflineTtsZipvoiceModelConfig &zipvoice,
+ const OfflineTtsKittenModelConfig &kitten,
+ int32_t num_threads, bool debug,
+ const std::string &provider)
+ : OfflineTtsModelConfig(vits, matcha, kokoro, zipvoice, kitten,
+ OfflineTtsPiperModelConfig{}, num_threads, debug,
+ provider) {}📝 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.
| const OfflineTtsPiperModelConfig &piper, | |
| int32_t num_threads, bool debug, | |
| const std::string &provider) | |
| OfflineTtsModelConfig(const OfflineTtsVitsModelConfig &vits, | |
| const OfflineTtsMatchaModelConfig &matcha, | |
| const OfflineTtsKokoroModelConfig &kokoro, | |
| const OfflineTtsZipvoiceModelConfig &zipvoice, | |
| const OfflineTtsKittenModelConfig &kitten, | |
| const OfflineTtsPiperModelConfig &piper, | |
| int32_t num_threads, bool debug, | |
| const std::string &provider) | |
| : vits(vits), | |
| matcha(matcha), | |
| kokoro(kokoro), | |
| zipvoice(zipvoice), | |
| kitten(kitten), | |
| piper(piper), | |
| num_threads(num_threads), | |
| debug(debug), | |
| provider(provider) {} | |
| // Back-compat overload: preserves the old parameter order. | |
| OfflineTtsModelConfig(const OfflineTtsVitsModelConfig &vits, | |
| const OfflineTtsMatchaModelConfig &matcha, | |
| const OfflineTtsKokoroModelConfig &kokoro, | |
| const OfflineTtsZipvoiceModelConfig &zipvoice, | |
| const OfflineTtsKittenModelConfig &kitten, | |
| int32_t num_threads, bool debug, | |
| const std::string &provider) | |
| : OfflineTtsModelConfig(vits, matcha, kokoro, zipvoice, kitten, | |
| OfflineTtsPiperModelConfig{}, num_threads, debug, | |
| provider) {} |
🤖 Prompt for AI Agents
sherpa-onnx/csrc/offline-tts-model-config.h around lines 39-41: the constructor
parameter order changed by inserting 'piper' before 'num_threads', breaking
existing call-sites; add a backward-compatible overload that keeps the original
signature (i.e., without the 'piper' parameter in that position) and implement
it to delegate to the new constructor (construct or forward default/empty piper
as appropriate) so existing callers continue to work while new API remains
available.
|
Thank you for your contribution! But we have already supported Piper TTS models. You can find the doc at Piper TTS models are based on VITS, and we have already supported VITS models in sherpa-onnx. |
3 participants
This PR adds support for the Pipe TTS model.
Usage Example
You can now run the Piper TTS model with sherpa-onnx-offline-tts as follows:
sherpa-onnx-offline-tts
--piper-model=./piper-model/alan/en_GB-alan-medium.onnx
--piper-model-config-file=./piper-model/alan/en_GB-alan-medium.onnx.json
--piper-data-dir=./sherpa-onnx/vits-piper-en_US-amy-low/espeak-ng-data
--output-filename=./generated.wav
"how old are you?"
This will generate a speech waveform (generated.wav) from the given text using the Piper TTS model.
Summary by CodeRabbit