Skip to content

add TTS generation with GenerationConfig params C API - #3115

Merged
csukuangfj merged 1 commit into
k2-fsa:masterfrom
seven1240:master
Feb 4, 2026
Merged

csukuangfj merged 1 commit into
k2-fsa:masterfrom
seven1240:master

Conversation

@seven1240

@seven1240 seven1240 commented Feb 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Pocket TTS: offline English TTS with expanded pocket-model configuration options.
    • Generation config API: create/use generation configs (silence/speed/sid/steps/extra), supply reference audio/text, and new config-driven generation entry points with progress callbacks accepting a user argument.
    • OHOS and non-enabled build behavior: generation APIs present with clear runtime handling when TTS is not enabled.
  • Documentation

    • Added a C example demonstrating Pocket TTS usage, progress reporting, WAV output, and integrated build entry.

@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Feb 1, 2026
@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello @seven1240, 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 significantly enhances the Text-to-Speech (TTS) C API by introducing a robust GenerationConfig mechanism. This allows users to precisely control various aspects of TTS output, such as speaker characteristics and generation speed, through a structured configuration object. The changes also integrate full support for Pocket TTS models into the C API, accompanied by a new example that illustrates how to leverage these advanced features for English TTS generation.

Highlights

  • New C API for TTS Generation Configuration: Introduced a new SherpaOnnxTtsGenerationConfig C API struct and associated functions (Create, Destroy, Get/SetInt/Float/Str, ToString) to provide a flexible way to configure TTS generation parameters such as speaker ID, speed, and reference audio.
  • Pocket TTS Model Integration: Added support for Pocket TTS models within the C API, including new configuration structs (SherpaOnnxOfflineTtsPocketModelConfig in C and OfflineTtsPocketModelConfig in C++) to specify paths for various Pocket TTS model components.
  • Enhanced TTS Generation Functions: Implemented new SherpaOnnxOfflineTtsGenerateWithConfig variants that accept the GenerationConfig object, allowing for more granular control over the TTS output, including options for progress callbacks during generation.
  • New C API Example for Pocket TTS: Added a new example file, pocket-tts-en-c-api.c, demonstrating how to use the new GenerationConfig C API with Pocket TTS models for English text-to-speech generation, including the use of progress callbacks.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@coderabbitai

coderabbitai Bot commented Feb 1, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds Pocket TTS model config and a GenerationConfig-driven offline TTS generation entry point to the C API, implements an internal wrapper mapping GenerationConfig to the internal generator (with progress-callback support and non-enabled stubs), and adds a C example plus CMake target for pocket-tts-en-c-api.

Changes

Cohort / File(s) Summary
Build config
c-api-examples/CMakeLists.txt
Adds executable pocket-tts-en-c-api and links it to sherpa-onnx-c-api under the SHERPA_ONNX_ENABLE_TTS block.
C example
c-api-examples/pocket-tts-en-c-api.c
New example demonstrating GenerationConfig construction, optional reference-audio usage, progress-callback and direct generation flows, WAV output, logging, and proper cleanup/error handling.
C API headers
sherpa-onnx/c-api/c-api.h, sherpa-onnx/c-api/cxx-api.h
Adds GenerationConfig type and SherpaOnnxOfflineTtsPocketModelConfig / OfflineTtsPocketModelConfig; extends offline model config with a pocket field; declares SherpaOnnxOfflineTtsGenerateWithConfig.
C API implementation
sherpa-onnx/c-api/c-api.cc
Adds internal wrapper SherpaOnnxOfflineTtsGenerateInternal to translate public GenerationConfig to internal generator calls (including pocket model fields, reference audio, extra JSON), implements GenerateWithConfig (callback-with-arg), allocates/translates generated audio, and provides not-enabled stubs that log and return safe defaults.

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant Config as GenerationConfig
    participant TTS as SherpaOnnxOfflineTts
    participant Gen as InternalGenerator
    participant Audio as SherpaOnnxGeneratedAudio

    App->>Config: build & set params (sid, speed, reference_audio, extra...)
    App->>TTS: SherpaOnnxOfflineTtsGenerateWithConfig(text, Config, callback?, arg)
    activate TTS
    TTS->>Config: read params
    TTS->>Gen: convert to internal GenerationConfig (pocket fields, steps, extras)
    Gen->>Gen: run model inference / decoding
    Gen-->>TTS: progress updates (via callback)
    Gen->>Audio: allocate & fill audio buffer
    deactivate TTS
    TTS-->>App: return const SherpaOnnxGeneratedAudio*
    App->>Audio: write WAV / destroy audio
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 I stitched a pocket, tuned each byte,
configs and callbacks through the night,
reference hums and WAVs take flight,
tiny rabbit claps—what a delight,
🥕🎶

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding TTS generation with GenerationConfig parameters to the C API.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a C API for TTS generation using GenerationConfig parameters, along with a new example for PocketTTS. The changes are well-structured. My review focuses on improving error handling in the new example, fixing critical memory-related bugs in the C API implementation, and enhancing API documentation for memory management. I've identified a few critical issues that could lead to crashes or memory corruption, and some medium-severity issues to improve logging and API clarity.

Comment on lines +67 to +103
const SherpaOnnxOfflineTts *tts = SherpaOnnxCreateOfflineTts(&config);
// mapping of sid to voice name
// 0->af, 1->af_bella, 2->af_nicole, 3->af_sarah, 4->af_sky, 5->am_adam
// 6->am_michael, 7->bf_emma, 8->bf_isabella, 9->bm_george, 10->bm_lewis
int32_t sid = 0;
float speed = 1.0; // larger -> faster in speech speed
SherpaOnnxTtsGenerationConfig *gen_config =
SherpaOnnxTtsGenerationConfigCreate();
if (!gen_config) {
fprintf(stderr, "Error create config\n");
exit(-1);
}
SherpaOnnxTtsGenerationConfigSetInt(gen_config, "sid", sid);
SherpaOnnxTtsGenerationConfigSetFloat(gen_config, "speed", speed);
SherpaOnnxTtsGenerationConfigSetStr(
gen_config, "reference_audio_file",
"./sherpa-onnx-pocket-tts-2026-01-26/test_wavs/bria.wav");
SherpaOnnxTtsGenerationConfigSetFloat(gen_config, "max_reference_audio_len", 10.0);
const char *config_str = SherpaOnnxTtsGenerationConfigToString(gen_config);
fprintf(stderr, "generation config: %s\n", config_str);
free(config_str);

#if 0
// If you don't want to use a callback, then please enable this branch
const SherpaOnnxGeneratedAudio *audio =
SherpaOnnxOfflineTtsGenerateWithConfig(tts, text, gen_config);
#else
const SherpaOnnxGeneratedAudio *audio =
SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallback(
tts, text, gen_config, ProgressCallback);
#endif

SherpaOnnxTtsGenerationConfigDestroy(&gen_config);
SherpaOnnxWriteWave(audio->samples, audio->n, audio->sample_rate, filename);

SherpaOnnxDestroyOfflineTtsGeneratedAudio(audio);
SherpaOnnxDestroyOfflineTts(tts);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The main function has some potential issues with error handling and resource management:

  1. The return values of SherpaOnnxCreateOfflineTts (line 67) and SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallback (lines 94-96) are not checked for NULL. This could lead to a segmentation fault if either function fails and returns NULL.
  2. exit(-1) is used (line 77) when SherpaOnnxTtsGenerationConfigCreate fails. It's better to return from main and ensure all previously allocated resources are properly released to prevent memory leaks.
  const SherpaOnnxOfflineTts *tts = SherpaOnnxCreateOfflineTts(&config);
  if (!tts) {
    fprintf(stderr, "Failed to create TTS engine.\n");
    return -1;
  }
  // mapping of sid to voice name
  // 0->af, 1->af_bella, 2->af_nicole, 3->af_sarah, 4->af_sky, 5->am_adam
  // 6->am_michael, 7->bf_emma, 8->bf_isabella, 9->bm_george, 10->bm_lewis
  int32_t sid = 0;
  float speed = 1.0;  // larger -> faster in speech speed
  SherpaOnnxTtsGenerationConfig *gen_config =
      SherpaOnnxTtsGenerationConfigCreate();
  if (!gen_config) {
    fprintf(stderr, "Error create config\n");
    SherpaOnnxDestroyOfflineTts(tts);
    return -1;
  }
  SherpaOnnxTtsGenerationConfigSetInt(gen_config, "sid", sid);
  SherpaOnnxTtsGenerationConfigSetFloat(gen_config, "speed", speed);
  SherpaOnnxTtsGenerationConfigSetStr(
      gen_config, "reference_audio_file",
      "./sherpa-onnx-pocket-tts-2026-01-26/test_wavs/bria.wav");
  SherpaOnnxTtsGenerationConfigSetFloat(gen_config, "max_reference_audio_len", 10.0);
  const char *config_str = SherpaOnnxTtsGenerationConfigToString(gen_config);
  fprintf(stderr, "generation config: %s\n", config_str);
  free((void *)config_str);

#if 0
  // If you don't want to use a callback, then please enable this branch
  const SherpaOnnxGeneratedAudio *audio =
      SherpaOnnxOfflineTtsGenerateWithConfig(tts, text, gen_config);
#else
  const SherpaOnnxGeneratedAudio *audio =
      SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallback(
          tts, text, gen_config, ProgressCallback);
#endif

  SherpaOnnxTtsGenerationConfigDestroy(&gen_config);

  if (!audio) {
    fprintf(stderr, "Failed to generate audio.\n");
    SherpaOnnxDestroyOfflineTts(tts);
    return -1;
  }

  SherpaOnnxWriteWave(audio->samples, audio->n, audio->sample_rate, filename);

  SherpaOnnxDestroyOfflineTtsGeneratedAudio(audio);
  SherpaOnnxDestroyOfflineTts(tts);

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment on lines +1575 to +1582
const char *SherpaOnnxTtsGenerationConfigGetStr(
SherpaOnnxTtsGenerationConfig *config, const char *k, const char *def) {
if (!k) return def;
if (!strcmp(k, "reference_text"))
return config->config_.reference_text.c_str();

return config->config_.GetExtraString(std::string(k), def).c_str();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The function config->config_.GetExtraString(...) returns a std::string by value. Calling .c_str() on this temporary object results in a dangling pointer being returned to the caller, which will lead to undefined behavior when accessed.

To fix this, you should allocate memory for the string (e.g., using strdup) and document that the caller is responsible for freeing it.

const char *SherpaOnnxTtsGenerationConfigGetStr(
    SherpaOnnxTtsGenerationConfig *config, const char *k, const char *def) {
  if (!k) {
    if (def) return strdup(def);
    return nullptr;
  }
  if (!strcmp(k, "reference_text")) {
    return strdup(config->config_.reference_text.c_str());
  }

  std::string s = config->config_.GetExtraString(std::string(k), def);
  return strdup(s.c_str());
}

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment on lines +1751 to +1755
const char *SherpaOnnxTtsGenerationConfigToString(
SherpaOnnxTtsGenerationConfig *config) {
auto str = config->config_.ToString();
return strdup(str.c_str());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

When SHERPA_ONNX_ENABLE_TTS is disabled, SherpaOnnxTtsGenerationConfigCreate returns nullptr. This function will then attempt to dereference a null config pointer, leading to a crash. Even if config were not null, config->config_ is not accessible in this build configuration.

The function should handle the nullptr case and provide a safe fallback, like other stub functions in this block.

const char *SherpaOnnxTtsGenerationConfigToString(
    SherpaOnnxTtsGenerationConfig *config) {
  SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
  return strdup("");
}

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
bool is_ok = false;
auto samples = sherpa_onnx::ReadWave(std::string(v), &sample_rate, &is_ok);
if (!is_ok) {
SHERPA_ONNX_LOGE("Failed to read '%s'", k);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error log message for a failed ReadWave operation uses the key k (which is "reference_audio_file") instead of the filename v. This can make debugging more difficult. The log should print the filename that failed to be read.

      SHERPA_ONNX_LOGE("Failed to read '%s'", v);

Comment thread sherpa-onnx/c-api/c-api.h Outdated
Comment on lines +1224 to +1233
SHERPA_ONNX_API const char *SherpaOnnxTtsGenerationConfigGetStr(
SherpaOnnxTtsGenerationConfig *config, const char *k, const char *def);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetStr(
SherpaOnnxTtsGenerationConfig *config, const char *k, const char *v);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetInt(
SherpaOnnxTtsGenerationConfig *config, const char *k, int32_t v);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetFloat(
SherpaOnnxTtsGenerationConfig *config, const char *k, float v);
SHERPA_ONNX_API const char *SherpaOnnxTtsGenerationConfigToString(
SherpaOnnxTtsGenerationConfig *config);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The functions SherpaOnnxTtsGenerationConfigGetStr and SherpaOnnxTtsGenerationConfigToString return a const char* that is allocated with strdup and must be freed by the caller. This memory management requirement is not documented in the header file, which can lead to memory leaks. Please add comments to clarify that the caller is responsible for freeing the returned pointer, for instance by using free().

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Fix all issues with AI agents
In `@c-api-examples/pocket-tts-en-c-api.c`:
- Around line 81-83: The reference audio path used in
SherpaOnnxTtsGenerationConfigSetStr (gen_config) is incorrect: update the string
"./sherpa-onnx-pocket-tts-2026-01-26/test_wavs/bria.wav" to match the downloaded
folder name used elsewhere ("sherpa-onnx-pocket-tts-int8-2026-01-26"), i.e.,
change the path in the call to SherpaOnnxTtsGenerationConfigSetStr so it points
to "./sherpa-onnx-pocket-tts-int8-2026-01-26/test_wavs/bria.wav".
- Around line 67-101: SherpaOnnxCreateOfflineTts may return NULL and
SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallback may return NULL, but
the code unconditionally dereferences tts and audio before calling
SherpaOnnxWriteWave; add null checks after creating tts and after generation: if
SherpaOnnxCreateOfflineTts(...) returns NULL log an error and exit/return, and
if SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallback(...) returns NULL
log an error, destroy gen_config via SherpaOnnxTtsGenerationConfigDestroy, free
any allocated strings, and avoid calling SherpaOnnxWriteWave; keep references to
the symbols tts, audio, SherpaOnnxWriteWave,
SherpaOnnxTtsGenerationConfigDestroy, and SherpaOnnxCreateOfflineTts when
locating the code to modify.

In `@sherpa-onnx/c-api/c-api.cc`:
- Around line 1575-1582: SherpaOnnxTtsGenerationConfigGetStr returns a pointer
to a temporary std::string from config->config_.GetExtraString(...).c_str(),
causing a dangling pointer; fix by returning a stable buffer instead: either
cache the result inside SherpaOnnxTtsGenerationConfig (e.g., add a member like
last_extra_string and return last_extra_string.c_str()) when servicing
GetExtraString, or allocate and return a duplicated C string (e.g., via strdup)
and document that the caller must free it; update
SherpaOnnxTtsGenerationConfigGetStr to use the chosen stable storage and ensure
the reference for config->config_.reference_text remains valid.
- Around line 1706-1755: The disabled-TTS block is missing stubs for the new
"WithConfig" APIs and current SherpaOnnxTtsGenerationConfigToString illegally
dereferences config and touches internal GenerationConfig; add no-op stubs for
the new WithConfig functions (matching their header names) that log "TTS is not
enabled" and return appropriate null/zero values, and change
SherpaOnnxTtsGenerationConfigToString to not access config internals—log the
same message and return nullptr (or an empty strdup("")) instead; keep all other
existing stubs
(SherpaOnnxTtsGenerationConfigCreate/Destroy/GetInt/GetFloat/GetStr/SetStr/SetInt/SetFloat/SherpaOnnxDestroyOfflineTtsGeneratedAudio)
as no-ops that log and return defaults.
- Around line 1584-1625: The extra map updates in
SherpaOnnxTtsGenerationConfigSetStr, SherpaOnnxTtsGenerationConfigSetInt, and
SherpaOnnxTtsGenerationConfigSetFloat use extra.insert(...), which silently does
nothing when the key already exists; change these to either
config->config_.extra.insert_or_assign(std::string(k), std::string(v)) (or
std::to_string(v) for numeric overloads) or use
config->config_.extra[std::string(k)] = ... so repeated Set* calls overwrite
existing entries instead of leaving stale values.
- Around line 66-68: Guard the SherpaOnnxTtsGenerationConfig definition behind
the SHERPA_ONNX_ENABLE_TTS macro: when SHERPA_ONNX_ENABLE_TTS == 1 keep the
current struct that embeds sherpa_onnx::GenerationConfig (the type from
offline-tts.h), and for the disabled case provide an empty stub struct so builds
without TTS still compile; update the block surrounding
SherpaOnnxTtsGenerationConfig to use the macro guard and ensure references to
sherpa_onnx::GenerationConfig only appear inside the enabled branch.

In `@sherpa-onnx/c-api/c-api.h`:
- Around line 1232-1233: Update the declaration of
SherpaOnnxTtsGenerationConfigToString to include ownership documentation:
indicate that the function returns a heap-allocated (strdup-style) C string and
that the caller is responsible for calling free() on the returned pointer when
done; reference the same wording pattern used by nearby functions (e.g.,
SherpaOnnxOfflineTtsGenerateWithConfig) to keep the header consistent.

Comment thread c-api-examples/pocket-tts-en-c-api.c
Comment thread c-api-examples/pocket-tts-en-c-api.c Outdated
Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment thread sherpa-onnx/c-api/c-api.h Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@sherpa-onnx/c-api/c-api.cc`:
- Around line 1577-1584: In SherpaOnnxTtsGenerationConfigGetStr, guard the def
parameter before calling config->config_.GetExtraString to avoid constructing a
std::string from a nullptr; specifically, after the initial if (!k) check add an
if (!def) return nullptr; so that when k is non-null you either return the
special "reference_text" string or call GetExtraString with a valid def,
preventing undefined behavior from std::string(nullptr).

In `@sherpa-onnx/c-api/c-api.h`:
- Around line 1225-1237: Update the ownership comment typo for TTS config
string-returning functions: change "The use has to free the returned pointer if
it's not NULL" to "The user has to free the returned pointer if it's not NULL"
for the declarations SherpaOnnxTtsGenerationConfigGetStr and
SherpaOnnxTtsGenerationConfigToString so the documentation correctly indicates
the caller owns and must free the returned char*.

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment thread sherpa-onnx/c-api/c-api.h Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@c-api-examples/pocket-tts-en-c-api.c`:
- Around line 76-79: When SherpaOnnxTtsGenerationConfigCreate() returns NULL the
function returns immediately but leaks the previously-created tts object; update
the error path in the block where gen_config is checked to call
SherpaOnnxTtsDestroy(tts) (or the appropriate destroy function for the tts
variable) before returning, ensuring any other allocated resources created
earlier (e.g., model handles) are also released; locate the creation sequence
(SherpaOnnxCreate / SherpaOnnxTtsGenerationConfigCreate) and add the cleanup
call(s) in that failure branch.
- Around line 103-112: The "Saved to" message is printed even when audio ==
NULL; update the logic so that after calling SherpaOnnxWriteWave and
SherpaOnnxDestroyOfflineTtsGeneratedAudio (inside the if (audio) block) you
print the "Saved to: %s" message there, and in the else branch print an
error/failure message indicating generation failed; keep
SherpaOnnxDestroyOfflineTts(tts) and the other fprintfs for "Input text" and
"Speaker ID" unchanged, and reference the audio variable, SherpaOnnxWriteWave,
SherpaOnnxDestroyOfflineTtsGeneratedAudio, SherpaOnnxDestroyOfflineTts, and the
fprintf calls to locate where to move/add the messages.

Comment thread c-api-examples/pocket-tts-en-c-api.c Outdated
Comment thread c-api-examples/pocket-tts-en-c-api.c Outdated
Comment on lines +103 to +112
if (audio) {
SherpaOnnxWriteWave(audio->samples, audio->n, audio->sample_rate, filename);
SherpaOnnxDestroyOfflineTtsGeneratedAudio(audio);
}

SherpaOnnxDestroyOfflineTts(tts);

fprintf(stderr, "Input text is: %s\n", text);
fprintf(stderr, "Speaker ID is: %d\n", sid);
fprintf(stderr, "Saved to: %s\n", filename);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

"Saved to" message printed even when generation fails.

When audio is NULL, the file isn't written, but line 112 still reports "Saved to: %s", which is misleading.

🛡️ Proposed fix to provide accurate feedback
   if (audio) {
     SherpaOnnxWriteWave(audio->samples, audio->n, audio->sample_rate, filename);
     SherpaOnnxDestroyOfflineTtsGeneratedAudio(audio);
+    fprintf(stderr, "Saved to: %s\n", filename);
+  } else {
+    fprintf(stderr, "Audio generation failed\n");
   }

   SherpaOnnxDestroyOfflineTts(tts);

   fprintf(stderr, "Input text is: %s\n", text);
   fprintf(stderr, "Speaker ID is: %d\n", sid);
-  fprintf(stderr, "Saved to: %s\n", filename);

   return 0;
🤖 Prompt for AI Agents
In `@c-api-examples/pocket-tts-en-c-api.c` around lines 103 - 112, The "Saved to"
message is printed even when audio == NULL; update the logic so that after
calling SherpaOnnxWriteWave and SherpaOnnxDestroyOfflineTtsGeneratedAudio
(inside the if (audio) block) you print the "Saved to: %s" message there, and in
the else branch print an error/failure message indicating generation failed;
keep SherpaOnnxDestroyOfflineTts(tts) and the other fprintfs for "Input text"
and "Speaker ID" unchanged, and reference the audio variable,
SherpaOnnxWriteWave, SherpaOnnxDestroyOfflineTtsGeneratedAudio,
SherpaOnnxDestroyOfflineTts, and the fprintf calls to locate where to move/add
the messages.

Comment thread sherpa-onnx/c-api/c-api.h Outdated
int32_t n_prompt, int32_t prompt_sr,
float speed, int32_t num_steps);

SHERPA_ONNX_API typedef struct SherpaOnnxTtsGenerationConfig

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a struct in c-api.h for GenerationConfig
to include the following fields

float silence_scale = 0.2;
float speed = 1.0f; // used only by some models.
int32_t sid = 0; // used only by models support multi-speakers
std::vector<float> reference_audio; // mono, [-1, 1]
int32_t reference_sample_rate = 0; // sample rate of reference_audio
std::string reference_text; // not all models require this
int32_t num_steps = 5; // number of steps in flow matching

Note: Use

const float* reference_audio;
int32_t reference_audio_len;

to represent

std::vector<float> reference_audio; 

and use

const char*  reference_text;

Also, please don't wrap

std::string GetExtraString(const std::string &key,
const std::string &def = "") const;
int32_t GetExtraInt(const std::string &key, int32_t def) const;
float GetExtraFloat(const std::string &key, float def) const;
std::string ToString() const;

to c-api.

Please delete

  • SherpaOnnxTtsGenerationConfigCreate
  • SherpaOnnxTtsGenerationConfigDestroy
  • SherpaOnnxTtsGenerationConfigGetInt
  • SherpaOnnxTtsGenerationConfigGetFloat
  • SherpaOnnxTtsGenerationConfigGetStr
  • SherpaOnnxTtsGenerationConfigSetStr
  • SherpaOnnxTtsGenerationConfigSetInt
  • SherpaOnnxTtsGenerationConfigSetFloat
  • SherpaOnnxTtsGenerationConfigToString

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, c++ use an internal vector to hold the reference audio which is hard to manage the life cycle in C. without those function the user would need to use SherpaOnnxReadWave to read the wav into the reference audio buffer and remember to destroy it after use, is that what you mean? thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and, I just realized that, even you can pass a raw pointer from c to c++, the c++ class internally need a vector, so still need one more memory copy. My whole idea was to avoid unnecessary memory copy as long as possible. I'm not a c++ guru, is there any suggestion for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit which kind of works, but the GenerationConfig still need some APIs to

  • Set extra params
  • Manage an internal state to hold param values
  • Init and Cleanup functions

It's incomplete, let me know if it makes sense I'll complete this.

Thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I saw you said we could use a JSON extra opt, I'll try.

Comment thread sherpa-onnx/c-api/c-api.h Outdated
// The user has to use SherpaOnnxDestroyOfflineTtsGeneratedAudio() to free the
// returned pointer to avoid memory leak.
SHERPA_ONNX_API const SherpaOnnxGeneratedAudio *
SherpaOnnxOfflineTtsGenerateWithConfig(const SherpaOnnxOfflineTts *tts,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete

  • SherpaOnnxOfflineTtsGenerateWithConfig
  • SherpaOnnxOfflineTtsGenerateWithConfigAndCallback
  • SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallback

We need to keep only
SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallbackWithArg

Please rename SherpaOnnxOfflineTtsGenerateWithConfigAndProgressCallbackWithArg
to SherpaOnnxOfflineTtsGenerateWithConfig.

Comment thread sherpa-onnx/c-api/c-api.h Outdated
// Same as SherpaOnnxGeneratedAudioCallback but you can pass an additional
// `void* arg` to the callback.
SHERPA_ONNX_API const SherpaOnnxGeneratedAudio *
SherpaOnnxOfflineTtsGenerateWithConfigAndCallbackWithArg(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Please rename it to
SherpaOnnxOfflineTtsGenerateWithConfig
  1. Please change the arguments to
const SherpaOnnxOfflineTts *tts, const char *text,
    SherpaOnnxTtsGenerationConfig *config,
    const char* extra_opt,
    SherpaOnnxGeneratedAudioCallbackWithArg callback, void *arg)

where extra_opt is a json string. You can parse it to construct

std::unordered_map<std::string, std::string> extra;

Note that extra_opt, callback, and arg can be nullptr.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it's a JSON, can we just add to the config struct instead of add a new arg?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it's a JSON, can we just add to the config struct instead of add a new arg?

Yes, that is also a good idea. In that case, please add

const char* extra;

to the struct and document that it is a json string.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In `@sherpa-onnx/c-api/c-api.cc`:
- Around line 1550-1555: In SherpaOnnxTtsGenerationConfigCleanup, after casting
and deleting the heap object via sherpa_onnx::GenerationConfig *cfg and calling
delete cfg, assign config->pvt = nullptr to clear the dangling pointer (so
repeated calls won't attempt to delete freed memory); keep the existing
early-return check (if (!config || !config->pvt) return) intact.
- Around line 1519-1548: In SherpaOnnxTtsGenerationConfigSetReferenceAudioFile,
first add a null check for the input pointer "config" and return early if it's
null to avoid dereferencing it; remove the unreachable if (!cfg) null-check
after "new" and instead wrap the allocation of "sherpa_onnx::GenerationConfig
*cfg = new ..." in a try-catch for std::bad_alloc and log an appropriate error
via SHERPA_ONNX_LOGE (do not use the incorrect "TTS is not enabled" message);
remove the leftover debug log "blah read" (or replace it with a concise,
accurate informational log if needed) and keep existing error handling for
sherpa_onnx::ReadWave and assignment to config->reference_audio_len and
cfg->reference_audio unchanged.
- Around line 1378-1420: SherpaOnnxOfflineTtsGenerateInternal currently
dereferences the parameter config without checking for null; add a null-check at
the start of SherpaOnnxOfflineTtsGenerateInternal and handle it (either return
nullptr immediately or create a local sherpa_onnx::GenerationConfig to use as a
default) to avoid crashes; update logic that uses config->pvt and later deletes
cfg to account for the chosen handling so you don't dereference config when it's
null and ensure proper memory ownership for the local GenerationConfig.
- Around line 1575-1581: SherpaOnnxTtsGenerationConfigSetFloat lacks a null
check for the key parameter `k`, risking undefined behavior when calling
std::string(k); update SherpaOnnxTtsGenerationConfigSetFloat to mirror
SherpaOnnxTtsGenerationConfigSetStr and SherpaOnnxTtsGenerationConfigSetInt by
returning early if `k` is null (and still ensure config->pvt is checked), then
proceed to cast config->pvt to sherpa_onnx::GenerationConfig* and call
cfg->extra.insert_or_assign(std::string(k), std::to_string(v)).
- Around line 1663-1686: The stub implementations for the TTS-disabled build use
the wrong exported names and must be renamed to match the header declarations to
avoid link errors: rename SherpaOnnxTtsGenerationConfigSetAudioFile to
SherpaOnnxTtsGenerationConfigSetReferenceAudioFile, and rename
SherpaOnnxTtsGenerationConfigDestroy to SherpaOnnxTtsGenerationConfigCleanup;
ensure the other stub signatures (SherpaOnnxTtsGenerationConfigSetStr,
SherpaOnnxTtsGenerationConfigSetInt, SherpaOnnxTtsGenerationConfigSetFloat)
remain unchanged and keep their current behavior (logging the "TTS is not
enabled..." message) so the symbols match the header.
🧹 Nitpick comments (1)
sherpa-onnx/c-api/c-api.h (1)

1214-1224: Consider using SherpaOnnx prefix for naming consistency.

All other public structs in this header use the SherpaOnnx prefix (e.g., SherpaOnnxOfflineTtsConfig, SherpaOnnxGeneratedAudio), but GenerationConfig does not follow this convention. This inconsistency may confuse API users.

Additionally, consider documenting ownership semantics for the pointer fields (reference_audio, reference_text) and adding a note that pvt is for internal use only.

♻️ Suggested naming and documentation
-SHERPA_ONNX_API typedef struct GenerationConfig {
+// Configuration for TTS generation with reference audio support.
+// Note: The pvt field is for internal use only - do not modify directly.
+// Use the provided Set* functions to configure generation parameters.
+SHERPA_ONNX_API typedef struct SherpaOnnxTtsGenerationConfig {
   float silence_scale;
   float speed;                    // used only by some models.
   int32_t sid;                    // used only by models support multi-speakers
-  float *reference_audio;         // mono, [-1, 1]
+  float *reference_audio;         // mono, [-1, 1], owned by caller unless set via SetReferenceAudioFile
   int32_t reference_audio_len;    // length in samples
   int32_t reference_sample_rate;  // sample rate of reference_audio
-  const char *reference_text;     // not all models require this
+  const char *reference_text;     // not all models require this, owned by caller
   int32_t num_steps;              // number of steps in flow matching
   void *pvt;                      // private data used internally, DON'T touch
-} GenerationConfig;
+} SherpaOnnxTtsGenerationConfig;

Comment on lines +1378 to +1420
static const SherpaOnnxGeneratedAudio *SherpaOnnxOfflineTtsGenerateInternal(
const SherpaOnnxOfflineTts *tts, const char *text, GenerationConfig *config,
std::function<int32_t(const float *, int32_t, float)> callback) {
sherpa_onnx::GenerationConfig *cfg;
if (config->pvt) {
cfg = (sherpa_onnx::GenerationConfig *)config->pvt;
} else {
cfg = new sherpa_onnx::GenerationConfig; // generate a config on the fly
if (config->reference_audio_len > 0) {
cfg->reference_audio.assign(
config->reference_audio,
config->reference_audio + config->reference_audio_len);
}
}
if (config->silence_scale > 0) cfg->silence_scale = config->silence_scale;
if (config->speed > 0) cfg->speed = config->speed;
cfg->sid = config->sid;
if (config->reference_sample_rate > 0)
cfg->reference_sample_rate = config->reference_sample_rate;
if (config->reference_text)
cfg->reference_text = std::string(config->reference_text);
if (config->num_steps > 0) cfg->num_steps = config->num_steps;

sherpa_onnx::GeneratedAudio audio = tts->impl->Generate(text, *cfg, callback);
if (!config->pvt && cfg) {
delete cfg; // free the local generated config
}

if (audio.samples.empty()) {
return nullptr;
}

SherpaOnnxGeneratedAudio *ans = new SherpaOnnxGeneratedAudio;

float *samples = new float[audio.samples.size()];
std::copy(audio.samples.begin(), audio.samples.end(), samples);

ans->samples = samples;
ans->n = audio.samples.size();
ans->sample_rate = audio.sample_rate;

return ans;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add null check for config parameter to prevent crashes.

The function dereferences config on line 1382 without first checking if it's null. If a caller passes a null config, this will cause a crash.

🛡️ Proposed fix
 static const SherpaOnnxGeneratedAudio *SherpaOnnxOfflineTtsGenerateInternal(
     const SherpaOnnxOfflineTts *tts, const char *text, GenerationConfig *config,
     std::function<int32_t(const float *, int32_t, float)> callback) {
+  if (!config) {
+    SHERPA_ONNX_LOGE("config is null");
+    return nullptr;
+  }
+
   sherpa_onnx::GenerationConfig *cfg;
   if (config->pvt) {
🤖 Prompt for AI Agents
In `@sherpa-onnx/c-api/c-api.cc` around lines 1378 - 1420,
SherpaOnnxOfflineTtsGenerateInternal currently dereferences the parameter config
without checking for null; add a null-check at the start of
SherpaOnnxOfflineTtsGenerateInternal and handle it (either return nullptr
immediately or create a local sherpa_onnx::GenerationConfig to use as a default)
to avoid crashes; update logic that uses config->pvt and later deletes cfg to
account for the chosen handling so you don't dereference config when it's null
and ensure proper memory ownership for the local GenerationConfig.

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment on lines +1519 to +1548
void SherpaOnnxTtsGenerationConfigSetReferenceAudioFile(
GenerationConfig *config, const char *audio_file) {
if (!audio_file) return;
if (config->pvt) {
sherpa_onnx::GenerationConfig *cfg =
(sherpa_onnx::GenerationConfig *)config->pvt;
config->pvt = nullptr;
delete cfg;
}
sherpa_onnx::GenerationConfig *cfg = new sherpa_onnx::GenerationConfig();
if (!cfg) {
SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
return;
}
config->pvt = (void *)cfg;
int32_t sample_rate;
bool is_ok = false;
auto samples =
sherpa_onnx::ReadWave(std::string(audio_file), &sample_rate, &is_ok);
if (!is_ok) {
SHERPA_ONNX_LOGE("Failed to read '%s'", audio_file);
return;
}
config->reference_audio_len = samples.size();
cfg->reference_audio = std::move(samples);
SHERPA_ONNX_LOGE("blah read '%d' %d", config->reference_audio_len,
sizeof(float));
cfg->reference_sample_rate = sample_rate;
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix multiple issues: null check, misleading error message, and debug log artifact.

Three issues in this function:

  1. Line 1522: Missing null check on config before dereferencing
  2. Lines 1529-1532: The if (!cfg) check is unreachable - new throws std::bad_alloc on failure, it doesn't return null. Also, the error message "TTS is not enabled" is incorrect since this code is inside the #if SHERPA_ONNX_ENABLE_TTS == 1 block.
  3. Lines 1544-1545: The log message "blah read" appears to be a leftover debug artifact and should be removed or replaced with a proper message.
🐛 Proposed fix
 void SherpaOnnxTtsGenerationConfigSetReferenceAudioFile(
     GenerationConfig *config, const char *audio_file) {
-  if (!audio_file) return;
+  if (!config || !audio_file) return;
   if (config->pvt) {
     sherpa_onnx::GenerationConfig *cfg =
         (sherpa_onnx::GenerationConfig *)config->pvt;
     config->pvt = nullptr;
     delete cfg;
   }
   sherpa_onnx::GenerationConfig *cfg = new sherpa_onnx::GenerationConfig();
-  if (!cfg) {
-    SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
-    return;
-  }
   config->pvt = (void *)cfg;
   int32_t sample_rate;
   bool is_ok = false;
   auto samples =
       sherpa_onnx::ReadWave(std::string(audio_file), &sample_rate, &is_ok);
   if (!is_ok) {
     SHERPA_ONNX_LOGE("Failed to read '%s'", audio_file);
     return;
   }
   config->reference_audio_len = samples.size();
   cfg->reference_audio = std::move(samples);
-  SHERPA_ONNX_LOGE("blah read '%d' %d", config->reference_audio_len,
-                   sizeof(float));
   cfg->reference_sample_rate = sample_rate;
   return;
 }
🤖 Prompt for AI Agents
In `@sherpa-onnx/c-api/c-api.cc` around lines 1519 - 1548, In
SherpaOnnxTtsGenerationConfigSetReferenceAudioFile, first add a null check for
the input pointer "config" and return early if it's null to avoid dereferencing
it; remove the unreachable if (!cfg) null-check after "new" and instead wrap the
allocation of "sherpa_onnx::GenerationConfig *cfg = new ..." in a try-catch for
std::bad_alloc and log an appropriate error via SHERPA_ONNX_LOGE (do not use the
incorrect "TTS is not enabled" message); remove the leftover debug log "blah
read" (or replace it with a concise, accurate informational log if needed) and
keep existing error handling for sherpa_onnx::ReadWave and assignment to
config->reference_audio_len and cfg->reference_audio unchanged.

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment on lines +1550 to +1555
void SherpaOnnxTtsGenerationConfigCleanup(GenerationConfig *config) {
if (!config || !config->pvt) return;
sherpa_onnx::GenerationConfig *cfg =
(sherpa_onnx::GenerationConfig *)config->pvt;
delete cfg;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Set config->pvt to nullptr after deletion to prevent potential double-free.

After deleting cfg, the config->pvt pointer still holds the address of freed memory. If SherpaOnnxTtsGenerationConfigCleanup is called again on the same config, it will attempt to delete already-freed memory.

🛡️ Proposed fix
 void SherpaOnnxTtsGenerationConfigCleanup(GenerationConfig *config) {
   if (!config || !config->pvt) return;
   sherpa_onnx::GenerationConfig *cfg =
       (sherpa_onnx::GenerationConfig *)config->pvt;
   delete cfg;
+  config->pvt = nullptr;
 }
🤖 Prompt for AI Agents
In `@sherpa-onnx/c-api/c-api.cc` around lines 1550 - 1555, In
SherpaOnnxTtsGenerationConfigCleanup, after casting and deleting the heap object
via sherpa_onnx::GenerationConfig *cfg and calling delete cfg, assign
config->pvt = nullptr to clear the dangling pointer (so repeated calls won't
attempt to delete freed memory); keep the existing early-return check (if
(!config || !config->pvt) return) intact.

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment on lines +1575 to +1581
void SherpaOnnxTtsGenerationConfigSetFloat(GenerationConfig *config,
const char *k, float v) {
if (!config->pvt) return;
sherpa_onnx::GenerationConfig *cfg =
(sherpa_onnx::GenerationConfig *)config->pvt;
cfg->extra.insert_or_assign(std::string(k), std::to_string(v));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add null check for k parameter to prevent undefined behavior.

SherpaOnnxTtsGenerationConfigSetStr (line 1559) and SherpaOnnxTtsGenerationConfigSetInt (line 1568) both check if k is null, but SherpaOnnxTtsGenerationConfigSetFloat does not. Passing a null k will cause std::string(k) on line 1580 to exhibit undefined behavior.

🛡️ Proposed fix
 void SherpaOnnxTtsGenerationConfigSetFloat(GenerationConfig *config,
                                            const char *k, float v) {
-  if (!config->pvt) return;
+  if (!k || !config || !config->pvt) return;
   sherpa_onnx::GenerationConfig *cfg =
       (sherpa_onnx::GenerationConfig *)config->pvt;
   cfg->extra.insert_or_assign(std::string(k), std::to_string(v));
 }
🤖 Prompt for AI Agents
In `@sherpa-onnx/c-api/c-api.cc` around lines 1575 - 1581,
SherpaOnnxTtsGenerationConfigSetFloat lacks a null check for the key parameter
`k`, risking undefined behavior when calling std::string(k); update
SherpaOnnxTtsGenerationConfigSetFloat to mirror
SherpaOnnxTtsGenerationConfigSetStr and SherpaOnnxTtsGenerationConfigSetInt by
returning early if `k` is null (and still ensure config->pvt is checked), then
proceed to cast config->pvt to sherpa_onnx::GenerationConfig* and call
cfg->extra.insert_or_assign(std::string(k), std::to_string(v)).

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment on lines +1663 to +1686
void SherpaOnnxTtsGenerationConfigSetAudioFile(GenerationConfig *config) {
SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
return;
}

void SherpaOnnxTtsGenerationConfigDestroy(GenerationConfig *config) {
SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
}

void SherpaOnnxTtsGenerationConfigSetStr(GenerationConfig *config,
const char *k, const char *v) {
SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
}

void SherpaOnnxTtsGenerationConfigSetInt(GenerationConfig *config,
const char *k, int32_t v) {
SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
}

void SherpaOnnxTtsGenerationConfigSetFloat(GenerationConfig *config,
const char *k, float v) {
SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix function name mismatches in TTS-disabled stubs to prevent link errors.

The stub function names don't match the declarations in the header:

  • SherpaOnnxTtsGenerationConfigSetAudioFile should be SherpaOnnxTtsGenerationConfigSetReferenceAudioFile
  • SherpaOnnxTtsGenerationConfigDestroy should be SherpaOnnxTtsGenerationConfigCleanup

This will cause link errors when building with TTS disabled.

🐛 Proposed fix
-void SherpaOnnxTtsGenerationConfigSetAudioFile(GenerationConfig *config) {
+void SherpaOnnxTtsGenerationConfigSetReferenceAudioFile(
+    GenerationConfig *config, const char *audio_file) {
   SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
   return;
 }

-void SherpaOnnxTtsGenerationConfigDestroy(GenerationConfig *config) {
+void SherpaOnnxTtsGenerationConfigCleanup(GenerationConfig *config) {
   SHERPA_ONNX_LOGE("TTS is not enabled. Please rebuild sherpa-onnx");
 }
🤖 Prompt for AI Agents
In `@sherpa-onnx/c-api/c-api.cc` around lines 1663 - 1686, The stub
implementations for the TTS-disabled build use the wrong exported names and must
be renamed to match the header declarations to avoid link errors: rename
SherpaOnnxTtsGenerationConfigSetAudioFile to
SherpaOnnxTtsGenerationConfigSetReferenceAudioFile, and rename
SherpaOnnxTtsGenerationConfigDestroy to SherpaOnnxTtsGenerationConfigCleanup;
ensure the other stub signatures (SherpaOnnxTtsGenerationConfigSetStr,
SherpaOnnxTtsGenerationConfigSetInt, SherpaOnnxTtsGenerationConfigSetFloat)
remain unchanged and keep their current behavior (logging the "TTS is not
enabled..." message) so the symbols match the header.

Comment thread c-api-examples/pocket-tts-en-c-api.c Outdated
Comment on lines +89 to +93
reference_audio_file);
SherpaOnnxTtsGenerationConfigSetInt(&cfg, "sid", sid);
SherpaOnnxTtsGenerationConfigSetFloat(&cfg, "speed", speed);
SherpaOnnxTtsGenerationConfigSetFloat(&cfg, "max_reference_audio_len",
10.0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you delete this else branch?

Comment thread c-api-examples/pocket-tts-en-c-api.c Outdated
NULL);
#endif

SherpaOnnxTtsGenerationConfigCleanup(&cfg);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete it. Normally, we don't need to clean up a struct config. We assume the config contains pointers to memory allocated by the user.

Comment thread sherpa-onnx/c-api/c-api.h Outdated
Comment on lines +1226 to +1236
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetReferenceAudioFile(
GenerationConfig *config, const char *audio_file);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigCleanup(
GenerationConfig *config);

SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetStr(
GenerationConfig *config, const char *k, const char *v);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetInt(
GenerationConfig *config, const char *k, int32_t v);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetFloat(
GenerationConfig *config, const char *k, float v);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you delete these functions?

Suggested change
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetReferenceAudioFile(
GenerationConfig *config, const char *audio_file);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigCleanup(
GenerationConfig *config);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetStr(
GenerationConfig *config, const char *k, const char *v);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetInt(
GenerationConfig *config, const char *k, int32_t v);
SHERPA_ONNX_API void SherpaOnnxTtsGenerationConfigSetFloat(
GenerationConfig *config, const char *k, float v);

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
return SherpaOnnxOfflineTtsGenerateInternal(tts, text, config, wrapper);
}

void SherpaOnnxTtsGenerationConfigSetReferenceAudioFile(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete this function.

We just set a pointer to the audio in the struct.
The struct does not own the memory pointed to by the pointer.

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
delete cfg;
}

void SherpaOnnxTtsGenerationConfigSetStr(GenerationConfig *config,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove it. We don't need such functions.

@seven1240

Copy link
Copy Markdown
Contributor Author

Thanks for the comments, I'll try with a new patch, in the mean time, do we already have an internal JSON parser?

@csukuangfj

Copy link
Copy Markdown
Collaborator

internal JSON parser

Yes! Please see how we read the two json files in the struct.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@c-api-examples/pocket-tts-en-c-api.c`:
- Around line 79-82: When SherpaOnnxReadWave returns NULL you currently return
-1 without freeing the previously created tts object; call the appropriate
destructor (e.g., SherpaOnnxDestroyTts(tts)) before returning to avoid leaking
tts, updating the error path around the call to SherpaOnnxReadWave (variable:
wave) and ensuring tts is destroyed when wave is NULL.

In `@sherpa-onnx/c-api/c-api.cc`:
- Around line 1507-1517: The wrapper lambda inside
SherpaOnnxOfflineTtsGenerateWithConfig currently returns 0 when callback is null
(in the lambda capturing callback and arg), which signals "stop generating" and
prevents audio generation; change the lambda so that if callback is null it
returns 1 to indicate "continue", otherwise call and return callback(samples, n,
progress, arg), ensuring SherpaOnnxOfflineTtsGenerateInternal receives a
progress callback that doesn't prematurely stop generation.
- Around line 1397-1402: Wrap the call to nlohmann::json::parse and the
subsequent loop over json.items() in a try/catch block that catches
nlohmann::json::parse_error (and optionally std::exception) to prevent
exceptions crossing the C API boundary; on parse_error capture e.what(), log or
propagate a C-API friendly error, and skip populating cfg.extra (or leave it
empty) so that cfg.extra.insert_or_assign is only called for successfully parsed
JSON. Ensure you reference the same symbols: config->extra,
nlohmann::json::parse, and cfg.extra.insert_or_assign when implementing the
try/catch and error handling.
🧹 Nitpick comments (2)
c-api-examples/pocket-tts-en-c-api.c (1)

72-73: Unused variables sid and speed.

The variables sid and speed are declared and printed but never used in the generation config. If these are intentional placeholders for future use, consider removing them or using them in cfg.

sherpa-onnx/c-api/c-api.h (1)

1214-1224: Consider renaming GenerationConfig to follow naming convention.

All other public types in this header use the SherpaOnnx prefix (e.g., SherpaOnnxOfflineTtsConfig, SherpaOnnxGeneratedAudio). The unprefixed GenerationConfig name may cause naming conflicts with user code or other libraries.

Consider renaming to SherpaOnnxTtsGenerationConfig for consistency.

Comment thread c-api-examples/pocket-tts-en-c-api.c
Comment thread sherpa-onnx/c-api/c-api.cc
Comment thread sherpa-onnx/c-api/c-api.cc

@csukuangfj csukuangfj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Left some minor comments. Otherwise, it looks great to me.

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
return callback(samples, n, progress, arg);
};

return SherpaOnnxOfflineTtsGenerateInternal(tts, text, config, wrapper);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return SherpaOnnxOfflineTtsGenerateInternal(tts, text, config, wrapper);
return SherpaOnnxOfflineTtsGenerateInternal(tts, text, config, std::move(wrapper));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed as suggested, but std::move are not used by other wrappers in the same file.

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
if (config->reference_sample_rate > 0)
cfg.reference_sample_rate = config->reference_sample_rate;
if (config->reference_text)
cfg.reference_text = std::string(config->reference_text);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cfg.reference_text = std::string(config->reference_text);
cfg.reference_text = config->reference_text;

Comment thread sherpa-onnx/c-api/c-api.cc Outdated
Comment on lines +1397 to +1398
if (config->extra) {
if (config->extra) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one if is enough.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, was a mistake :)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@sherpa-onnx/c-api/c-api.cc`:
- Around line 1384-1388: Add a null-pointer check for config->reference_audio
before calling cfg.reference_audio.assign: when handling the block that
currently tests config->reference_audio_len > 0, also verify
config->reference_audio != nullptr (similar to the prompt_samples check at the
other location) and only call
cfg.reference_audio.assign(config->reference_audio, config->reference_audio +
config->reference_audio_len) if both conditions hold; otherwise skip the assign
(or handle the invalid state consistently with the existing prompt_samples
handling).

Comment thread sherpa-onnx/c-api/c-api.cc Outdated

@csukuangfj csukuangfj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@csukuangfj
csukuangfj merged commit 9787280 into k2-fsa:master Feb 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants