Remove cppinyin from zipvoice - #2892
Conversation
dosubot
Bot
added
the
size:XL
|
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. WalkthroughThis PR removes cppinyin and the OfflineTtsZipvoiceFrontend, migrates TTS tokenization to MatchaTtsLexicon, adds a lexicon field to ZipVoice model config, and updates build/linking scripts to stop including cppinyin. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @csukuangfj, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on optimizing and simplifying the ZipVoice Text-to-Speech (TTS) system by removing the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the cppinyin dependency for zipvoice TTS, which significantly improves performance by reducing the Real-Time Factor (RTF). The implementation correctly replaces the custom zipvoice frontend with the more general MatchaTtsLexicon, promoting code reuse. The changes across the build files and source code are consistent with the dependency removal.
I have identified an area for improvement regarding the now-deprecated --zipvoice-pinyin-dict parameter, which could lead to user confusion. I've suggested adding a warning to guide users toward the new --zipvoice-lexicon parameter. This change also revealed that the Pascal API has not been fully updated to reflect these changes, which could lead to bugs.
| @@ -96,12 +97,6 @@ bool OfflineTtsZipvoiceModelConfig::Validate() const { | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The --zipvoice-pinyin-dict parameter is now deprecated and its value is ignored. To avoid user confusion, it would be beneficial to add a warning if this parameter is still provided. This also helps in guiding users to the new --zipvoice-lexicon parameter.
Additionally, the Pascal API in sherpa-onnx/pascal-api/sherpa_onnx.pas seems to be outdated. It still references PinyinDict and is missing the new Lexicon field in TSherpaOnnxOfflineTtsZipVoiceModelConfig. This should be updated to maintain consistency across APIs and prevent bugs.
| if (!pinyin_dict.empty()) { | |
| SHERPA_ONNX_LOGW( | |
| "--zipvoice-pinyin-dict is deprecated and ignored. Please use " | |
| "--zipvoice-lexicon instead."); | |
| } | |
| "Path to tokens.txt for ZipVoice models"); | ||
| po->Register("zipvoice-data-dir", &data_dir, | ||
| "Path to the directory containing dict for espeak-ng."); | ||
| po->Register("zipvoice-lexicon", &lexicon, "Plah to lexicon.txt for Chinese"); |
There was a problem hiding this comment.
There is a typo in the help string for zipvoice-lexicon. 'Plah' should be 'Path'.
| po->Register("zipvoice-lexicon", &lexicon, "Plah to lexicon.txt for Chinese"); | |
| po->Register("zipvoice-lexicon", &lexicon, "Path to lexicon.txt for Chinese"); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc (1)
42-124: Consider adding validation for thelexiconfield.The new
lexiconfield is registered and used inInitFrontendbut is not validated inValidate(). If lexicon is required for ZipVoice to function correctly, consider adding a check similar to other required files. If it's optional (e.g., only needed for Chinese), document this behavior.+ if (!lexicon.empty() && !FileExists(lexicon)) { + SHERPA_ONNX_LOGE("--zipvoice-lexicon: '%s' does not exist", lexicon.c_str()); + return false; + } + if (feat_scale <= 0) {sherpa-onnx/csrc/matcha-tts-lexicon.h (1)
22-29: LGTM!The
skip_replacementparameter is consistently added to both constructor signatures. The implementations in the corresponding.ccfile correctly forward this parameter.Consider adding a brief doc comment explaining what
skip_replacementcontrols, as this is now part of the public API.sherpa-onnx/csrc/offline-tts-matcha-impl.h (1)
380-411: MatchaTtsLexicon ctor updates look consistent and preserve behaviorBoth InitFrontend overloads now pass
falsefor the newskip_replacementparameter:
- Manager-based:
MatchaTtsLexicon(mgr, lexicon, tokens, data_dir, debug, false)- Non-manager:
MatchaTtsLexicon(lexicon, tokens, data_dir, debug, false)This matches the updated constructor signature (
..., data_dir, bool debug, bool skip_replacement) and keeps the previous default behavior (replacements enabled) for existing Matcha models, which is what you want for a drop-in change.If you foresee some Matcha models wanting different behavior, consider eventually threading this flag from config instead of hard-coding
false, but that’s strictly optional for now.sherpa-onnx/csrc/matcha-tts-lexicon.cc (1)
125-135: skip_replacement toggle is correctly wired through phoneme processingThe new
skip_replacementflow inProcessPhonemesand its use viaskip_replacement_inConvertWordToIdslook sound:
ProcessPhonemes(phonemes, skip_replacement):
- When
skip_replacementisfalse, behavior matches the previous implementation (UTF-8 conversion, full replacement pipeline, and UTF-8 splitting).- When
true, it returns the raw UTF‑8 phoneme tokens, cleanly bypassing replacements.skip_replacement_is stored inImpland used only in the espeak OOV path, which is exactly where the replacement behavior matters.This gives a clear, low-risk control knob while preserving existing behavior for callers that pass
false.A tiny nit: the trailing comment on the
Implclosing brace still says// namespace sherpa_onnx, which is misleading — consider updating it the next time you touch this file.Also applies to: 367-375, 483-485
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
CMakeLists.txt(0 hunks)build-ios.sh(1 hunks)build-swift-macos.sh(0 hunks)c-api-examples/Makefile(1 hunks)cmake/cppinyin.cmake(0 hunks)cmake/cppinyin.patch(0 hunks)cmake/sherpa-onnx-static.pc.in(1 hunks)mfc-examples/NonStreamingSpeechRecognition/sherpa-onnx-deps.props(0 hunks)mfc-examples/NonStreamingTextToSpeech/sherpa-onnx-deps.props(0 hunks)mfc-examples/StreamingSpeechRecognition/sherpa-onnx-deps.props(0 hunks)sherpa-onnx/csrc/CMakeLists.txt(0 hunks)sherpa-onnx/csrc/matcha-tts-lexicon.cc(6 hunks)sherpa-onnx/csrc/matcha-tts-lexicon.h(1 hunks)sherpa-onnx/csrc/offline-tts-matcha-impl.h(2 hunks)sherpa-onnx/csrc/offline-tts-zipvoice-frontend-test.cc(0 hunks)sherpa-onnx/csrc/offline-tts-zipvoice-frontend.cc(0 hunks)sherpa-onnx/csrc/offline-tts-zipvoice-frontend.h(0 hunks)sherpa-onnx/csrc/offline-tts-zipvoice-impl.h(3 hunks)sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc(2 hunks)sherpa-onnx/csrc/offline-tts-zipvoice-model-config.h(2 hunks)sherpa-onnx/csrc/offline-tts-zipvoice-model.cc(2 hunks)sherpa-onnx/pascal-api/sherpa_onnx.pas(0 hunks)
💤 Files with no reviewable changes (12)
- mfc-examples/StreamingSpeechRecognition/sherpa-onnx-deps.props
- CMakeLists.txt
- cmake/cppinyin.cmake
- build-swift-macos.sh
- mfc-examples/NonStreamingTextToSpeech/sherpa-onnx-deps.props
- sherpa-onnx/pascal-api/sherpa_onnx.pas
- mfc-examples/NonStreamingSpeechRecognition/sherpa-onnx-deps.props
- sherpa-onnx/csrc/offline-tts-zipvoice-frontend.h
- sherpa-onnx/csrc/offline-tts-zipvoice-frontend-test.cc
- cmake/cppinyin.patch
- sherpa-onnx/csrc/CMakeLists.txt
- sherpa-onnx/csrc/offline-tts-zipvoice-frontend.cc
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-06T04:23:50.237Z
Learnt from: litongjava
Repo: k2-fsa/sherpa-onnx 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:
build-ios.shcmake/sherpa-onnx-static.pc.insherpa-onnx/csrc/offline-tts-zipvoice-impl.h
📚 Learning: 2025-08-06T04:18:47.981Z
Learnt from: litongjava
Repo: k2-fsa/sherpa-onnx 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:
build-ios.shcmake/sherpa-onnx-static.pc.insherpa-onnx/csrc/offline-tts-zipvoice-impl.h
🧬 Code graph analysis (5)
sherpa-onnx/csrc/offline-tts-zipvoice-impl.h (2)
sherpa-onnx/csrc/offline-tts-zipvoice-model.cc (2)
tokens(60-191)tokens(60-61)sherpa-onnx/csrc/matcha-tts-lexicon.h (1)
MatchaTtsLexicon(18-38)
sherpa-onnx/csrc/matcha-tts-lexicon.h (1)
sherpa-onnx/csrc/matcha-tts-lexicon.cc (7)
MatchaTtsLexicon(487-487)MatchaTtsLexicon(489-494)MatchaTtsLexicon(497-502)MatchaTtsLexicon(510-514)MatchaTtsLexicon(518-522)lexicon(405-417)lexicon(405-405)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc (1)
sherpa-onnx/csrc/matcha-tts-lexicon.cc (2)
lexicon(405-417)lexicon(405-405)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.h (2)
sherpa-onnx/csrc/matcha-tts-lexicon.cc (2)
lexicon(405-417)lexicon(405-405)sherpa-onnx/csrc/offline-tts-zipvoice-model.cc (2)
tokens(60-191)tokens(60-61)
sherpa-onnx/csrc/matcha-tts-lexicon.cc (1)
sherpa-onnx/csrc/matcha-tts-lexicon.h (1)
MatchaTtsLexicon(18-38)
⏰ 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). (20)
- GitHub Check: Release shared tts-ON
- GitHub Check: Release shared tts-OFF
- GitHub Check: Debug static tts-OFF
- GitHub Check: Debug shared tts-OFF
- GitHub Check: rknn shared ON
- GitHub Check: Release shared-OFF tts-OFF
- GitHub Check: ubuntu-24.04 3.10
- GitHub Check: Debug shared-OFF tts-OFF
- GitHub Check: Release shared-ON tts-ON
- GitHub Check: Debug shared-OFF tts-ON
- GitHub Check: ubuntu-24.04 3.8
- GitHub Check: ubuntu-24.04 3.9
- GitHub Check: Release shared-ON tts-OFF
- GitHub Check: Debug shared-ON tts-ON
- GitHub Check: ubuntu-24.04 3.11
- GitHub Check: ubuntu-24.04 3.12
- GitHub Check: Debug shared-ON tts-OFF
- GitHub Check: ubuntu-24.04 3.13
- GitHub Check: Release shared-OFF tts-ON
- GitHub Check: swift (macos-latest)
🔇 Additional comments (12)
sherpa-onnx/csrc/offline-tts-zipvoice-model.cc (2)
71-71: LGTM!Using
SHERPA_ONNX_EXIT(-1)instead of rawexit(-1)is correct for cross-platform consistency with the project's macro conventions.
111-114: LGTM!The explicit range-based for loop is clearer and maintains the same behavior as the previous implementation.
sherpa-onnx/csrc/offline-tts-zipvoice-impl.h (3)
18-18: LGTM!The new include for
matcha-tts-lexicon.hcorrectly supports the frontend replacement fromOfflineTtsZipvoiceFrontendtoMatchaTtsLexicon.
86-95: LGTM!The change from single-element extraction (
text_token_ids[0].tokens) to concatenating allTokenIDsentries correctly supports multi-segment text inputs, which is important when the frontend splits text into multiple token sequences.
104-112: Verify thatskip_replacement=trueis the intended behavior for ZipVoice.The
MatchaTtsLexiconis instantiated withskip_replacementhardcoded totrue. Ensure this is intentional for the ZipVoice use case, as it affects phoneme processing behavior.sherpa-onnx/csrc/offline-tts-zipvoice-model-config.h (2)
22-22: LGTM!The new
lexiconmember is correctly placed afterdata_dir, aligning with the PR's goal to support lexicon-based text processing.
34-50: LGTM!The constructor signature and initializer list are correctly updated to include the new
lexiconparameter in proper order.sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc (1)
136-136: LGTM!The
ToString()method correctly outputs the newlexiconfield for debugging and logging purposes.sherpa-onnx/csrc/matcha-tts-lexicon.cc (1)
145-199: Constructor and explicit instantiation updates for skip_replacement are consistentThe additional
bool skip_replacementparameter is threaded cleanly:
MatchaTtsLexicon::Implconstructors now take(..., data_dir, bool debug, bool skip_replacement)and initialize bothdebug_andskip_replacement_.- Public
MatchaTtsLexiconconstructors mirror that signature and forward toImpl.- The templated
MatchaTtsLexicon(Manager *mgr, ...)and the explicit instantiations forAAssetManagerandNativeResourceManagerare all updated to includeskip_replacementat the end, matching the header declaration.This ensures all creation paths (regular, Android, OHOS) see the new flag consistently, with no dangling old-signature usage.
Also applies to: 489-503, 510-515, 518-523
cmake/sherpa-onnx-static.pc.in (1)
22-25: cppinyin_core cleanly removed from static pkg-config LibsThe
Libsline now omits-lcppinyin_corewhile preserving the existing library list and rpath flag, which aligns with dropping cppinyin from the build without disturbing other consumers.Please regenerate and use the resulting
sherpa-onnx-static.pcin a downstream project once to confirm static linking works as expected withoutcppinyin_core.c-api-examples/Makefile (1)
4-11: Example linker flags updated consistently with cppinyin removalThe
LDFLAGSlibrary list dropscppinyin_coreand remains consistent with the core/static configuration; rpaths still cover both the sherpa-onnx and ONNX Runtime library directories, so example binaries should keep running.After this change, please rebuild
decode-file-c-apiandoffline-tts-c-apito confirm there are no missing symbols at link or run time.build-ios.sh (1)
129-137: Universal simulator archive list correctly drops libcppinyin_core.aThe loop that creates universal simulator archives now excludes
libcppinyin_core.awhile keeping the set of libraries aligned with those later merged intolibsherpa-onnx.a, so xcframework generation should no longer depend on cppinyin.Please run
build-ios.shonce on a clean tree to verifylipo,libtool, andxcodebuild -create-xcframeworkall complete without missing-archive errors.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc (1)
21-23: Add deprecation warning for --zipvoice-pinyin-dict.The parameter is still registered but silently ignored after the migration to lexicon-based tokenization. Users who provide this parameter will not receive any feedback that it's no longer used, potentially causing confusion.
Based on previous review feedback that has not yet been addressed.
Consider adding a warning in the Validate() method:
+ if (!pinyin_dict.empty()) { + SHERPA_ONNX_LOGW( + "--zipvoice-pinyin-dict is deprecated and ignored. Please use " + "--zipvoice-lexicon instead."); + } + if (feat_scale <= 0) {
🧹 Nitpick comments (1)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc (1)
42-125: Consider validating lexicon parameter if provided.While
InitLexiconwill handle errors at runtime, validating the lexicon path(s) during configuration would provide earlier feedback to users. Following the pattern used fordata_dir(lines 83-98), you could check file existence when lexicon is non-empty.Example validation (can be added after the data_dir validation block):
if (!lexicon.empty()) { std::vector<std::string> files; SplitStringToVector(lexicon, ",", false, &files); for (const auto &f : files) { if (!FileExists(f)) { SHERPA_ONNX_LOGE("--zipvoice-lexicon: '%s' does not exist", f.c_str()); return false; } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc (1)
sherpa-onnx/csrc/matcha-tts-lexicon.cc (2)
lexicon(405-417)lexicon(405-405)
⏰ 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). (19)
- GitHub Check: Release static tts-OFF
- GitHub Check: Debug static tts-OFF
- GitHub Check: swift (macos-latest)
- GitHub Check: rknn shared OFF
- GitHub Check: rknn shared ON
- GitHub Check: Release shared-ON tts-ON
- GitHub Check: Debug shared-ON tts-ON
- GitHub Check: Debug shared-OFF tts-OFF
- GitHub Check: Debug shared-OFF tts-ON
- GitHub Check: Release shared-ON tts-OFF
- GitHub Check: Release shared-OFF tts-ON
- GitHub Check: Debug shared-ON tts-OFF
- GitHub Check: Release shared-OFF tts-OFF
- GitHub Check: ubuntu-24.04 3.13
- GitHub Check: ubuntu-24.04 3.11
- GitHub Check: ubuntu-24.04 3.8
- GitHub Check: ubuntu-24.04 3.12
- GitHub Check: ubuntu-24.04 3.10
- GitHub Check: ubuntu-24.04 3.9
🔇 Additional comments (2)
sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc (2)
26-27: Good clarification that flow-matching-model is the decoder.This documentation improvement helps users understand the model architecture.
127-143: ToString correctly reflects the lexicon field.The serialization properly outputs the new
lexiconfield, maintaining consistency with the migration frompinyin_dictto lexicon-based tokenization.
There was a problem hiding this comment.
Pull request overview
This PR removes the cppinyin dependency from the ZipVoice TTS implementation, improving performance (RTF reduced from 1.117 to 0.897) by switching from a custom frontend implementation to the existing MatchaTtsLexicon class. The change simplifies the codebase by eliminating redundant Chinese text processing code and standardizing on a common lexicon-based approach.
Key Changes:
- Replaced custom ZipVoice frontend with MatchaTtsLexicon, adding lexicon parameter support
- Removed cppinyin library dependency from build system across all platforms (CMake, iOS, macOS, Pascal, MFC examples)
- Enhanced MatchaTtsLexicon with skip_replacement parameter to support different phoneme processing requirements
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sherpa-onnx/csrc/offline-tts-zipvoice-impl.h | Replaced OfflineTtsZipvoiceFrontend with MatchaTtsLexicon and added token flattening logic |
| sherpa-onnx/csrc/offline-tts-zipvoice-model-config.h | Added lexicon field to config struct |
| sherpa-onnx/csrc/offline-tts-zipvoice-model-config.cc | Added lexicon parameter registration and removed pinyin_dict validation |
| sherpa-onnx/csrc/offline-tts-zipvoice-model.cc | Replaced exit() with SHERPA_ONNX_EXIT() and improved code style |
| sherpa-onnx/csrc/offline-tts-zipvoice-frontend.h | Deleted - custom frontend no longer needed |
| sherpa-onnx/csrc/offline-tts-zipvoice-frontend.cc | Deleted - custom frontend implementation removed |
| sherpa-onnx/csrc/offline-tts-zipvoice-frontend-test.cc | Deleted - test file for removed frontend |
| sherpa-onnx/csrc/matcha-tts-lexicon.h | Added skip_replacement parameter to support ZipVoice requirements |
| sherpa-onnx/csrc/matcha-tts-lexicon.cc | Implemented skip_replacement logic to bypass phoneme replacements |
| sherpa-onnx/csrc/offline-tts-matcha-impl.h | Updated calls to MatchaTtsLexicon with skip_replacement=false |
| sherpa-onnx/csrc/CMakeLists.txt | Removed offline-tts-zipvoice-frontend.cc and cppinyin_core dependency |
| CMakeLists.txt | Removed cppinyin.cmake include |
| cmake/cppinyin.cmake | Deleted - cppinyin download/build script no longer needed |
| cmake/cppinyin.patch | Deleted - cppinyin patch file no longer needed |
| cmake/sherpa-onnx-static.pc.in | Removed -lcppinyin_core from static library linking |
| build-ios.sh | Removed libcppinyin_core.a from iOS library merging |
| build-swift-macos.sh | Removed libcppinyin_core.a from macOS library merging |
| c-api-examples/Makefile | Removed -lcppinyin_core from linker flags |
| sherpa-onnx/pascal-api/sherpa_onnx.pas | Removed cppinyin_core library linkage |
| mfc-examples/*/sherpa-onnx-deps.props | Removed cppinyin_core.lib from MFC example dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Path to the pinyin dictionary for cppinyin (i.e converting " | ||
| "Chinese into phones)."); |
There was a problem hiding this comment.
The help text for zipvoice-pinyin-dict references cppinyin, which is being removed in this PR. Since pinyin_dict is no longer used, consider updating this help text to indicate it is deprecated or remove the parameter entirely in a future cleanup.
| "Path to the pinyin dictionary for cppinyin (i.e converting " | |
| "Chinese into phones)."); | |
| "DEPRECATED: This parameter is no longer used and will be removed in a future release."); |
| "Path to tokens.txt for ZipVoice models"); | ||
| po->Register("zipvoice-data-dir", &data_dir, | ||
| "Path to the directory containing dict for espeak-ng."); | ||
| po->Register("zipvoice-lexicon", &lexicon, "Path to lexicon.txt for Chinese"); |
There was a problem hiding this comment.
Typo in help text: "Plah" should be "Path".
| std::string data_dir; | ||
| std::string lexicon; | ||
|
|
||
| // Used for converting Chinese characters to pinyin |
There was a problem hiding this comment.
The comment "Used for converting Chinese characters to pinyin" for the pinyin_dict field is outdated and misleading. Since this PR removes cppinyin dependency and the pinyin_dict is no longer validated or used in the implementation, this comment should either be removed or updated to indicate it is deprecated/unused.
| // Used for converting Chinese characters to pinyin | |
| // Deprecated/unused: previously used for converting Chinese characters to pinyin |
| } | ||
| } | ||
|
|
||
| std::vector<TokenIDs> OfflineTtsZipvoiceFrontend::ConvertTextToTokenIds( |
There was a problem hiding this comment.
Have you read all this logic? You just removed all of them? If you don't like cppinyin, you can replace it with jieba + table lookup, but I think you should be careful to remove the whole frontend.
There was a problem hiding this comment.
Yes, this part of the code mirrors the Python frontend and supports constructs such as [] and <>.
Users can update lexicon.txt to specify the correct pronunciations of Chinese words. This model does not support [S1], [S2], or similar tags; removing them does not affect model behavior and results in less code and lower maintenance effort.
This is not about a preference for or against cppinyin; the goal is to keep the code concise and reusable across models.
There was a problem hiding this comment.
OK, [S1], [S2] is for zipvoice dialog, have not supported yet. Well, it's up to you, most of time, it is you who maintain this repo.
For the following test,