Add C# API for PocketTTS. - #3162
Conversation
dosubot
Bot
added
the
size:XL
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 significantly expands the C# API by integrating the PocketTTS text-to-speech model. It provides developers with the necessary tools and examples to generate speech from text, either saving it to a file or playing it directly, with enhanced configuration options. This addition broadens the range of available TTS models and improves the flexibility of the C# SDK. Highlights
Changelog
Activity
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 introduces a C# API for PocketTTS, complete with two example projects demonstrating its usage: one for generation and another for generation with playback. The core API additions and project structure are well-implemented. However, I've identified a critical bug in the audio playback logic that could lead to corrupted audio output. Additionally, there are several areas for improvement regarding performance, code style, and project configuration, including a high-risk issue with manual JSON serialization. My review provides specific suggestions to address these points and enhance the overall quality and robustness of the new examples.
| if (lastSampleArray != null) | ||
| { | ||
| int remaining = lastSampleArray.Length - lastIndex; | ||
| if (remaining >= needed) | ||
| { | ||
| float[] this_block = lastSampleArray.Skip(lastIndex).Take(needed).ToArray(); | ||
| lastIndex += needed; | ||
| if (lastIndex == lastSampleArray.Length) | ||
| { | ||
| lastSampleArray = null; | ||
| lastIndex = 0; | ||
| } | ||
|
|
||
| Marshal.Copy(this_block, 0, IntPtr.Add(output, i * sizeof(float)), needed); | ||
| return StreamCallbackResult.Continue; | ||
| } | ||
|
|
||
| float[] this_block2 = lastSampleArray.Skip(lastIndex).Take(remaining).ToArray(); | ||
| lastIndex = 0; | ||
| lastSampleArray = null; | ||
|
|
||
| Marshal.Copy(this_block2, 0, IntPtr.Add(output, i * sizeof(float)), remaining); | ||
| i += remaining; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This block of code has a critical bug and a performance issue.
- Bug: The
return StreamCallbackResult.Continue;on line 144 causes the callback to exit prematurely after filling only part of the audio buffer. This will result in audio glitches, as the rest of the buffer is not filled. - Performance: Using LINQ methods like
Skip(),Take(), andToArray()inside an audio callback (lines 135 and 147) is inefficient due to repeated memory allocations in a performance-sensitive context.
The suggested code below fixes the bug and improves performance by using Marshal.Copy with an offset, avoiding intermediate array allocations.
if (lastSampleArray != null)
{
int remaining = lastSampleArray.Length - lastIndex;
int toCopy = Math.Min(needed, remaining);
Marshal.Copy(lastSampleArray, lastIndex, IntPtr.Add(output, i * sizeof(float)), toCopy);
lastIndex += toCopy;
i += toCopy;
if (lastIndex >= lastSampleArray.Length)
{
lastSampleArray = null;
lastIndex = 0;
}
continue;
}| native.Extra = "{}"; | ||
| if (Extra != null && Extra.Count > 0) | ||
| { | ||
| string json = "{"; | ||
| bool first = true; | ||
| foreach (System.Collections.DictionaryEntry kv in Extra) | ||
| { | ||
| if (!first) json += ","; | ||
| string key = kv.Key.ToString(); | ||
| string val = kv.Value is string ? "\"" + kv.Value.ToString() + "\"" : kv.Value.ToString(); | ||
| json += "\"" + key + "\":" + val; | ||
| first = false; | ||
| } | ||
| json += "}"; | ||
| native.Extra = json; | ||
| } |
There was a problem hiding this comment.
Manually constructing a JSON string is brittle and can produce invalid JSON if values contain special characters that need escaping (e.g., quotes). This can lead to runtime errors on the native side.
It is highly recommended to use a robust JSON library like System.Text.Json (which is built into .NET) for serialization. This ensures the generated JSON is always well-formed.
Additionally, consider replacing the legacy Hashtable with Dictionary<string, object> for the Extra property to improve type safety and simplify serialization.
if (Extra != null && Extra.Count > 0)
{
var dictionary = new System.Collections.Generic.Dictionary<string, object>();
foreach (System.Collections.DictionaryEntry kv in Extra)
{
if (kv.Key is string key)
{
dictionary[key] = kv.Value;
}
}
native.Extra = System.Text.Json.JsonSerializer.Serialize(dictionary);
}
else
{
native.Extra = "{}";
}| var text = "Today as always, men fall into two groups: slaves and free men. Whoever " + | ||
| "does not have two-thirds of his day for himself, is a slave, whatever " + | ||
| "he may be: a statesman, a businessman, an official, or a scholar. " + | ||
| "Friends fell out often because life was changing so fast. The easiest " + | ||
| "thing in the world was to lose touch with someone."; |
There was a problem hiding this comment.
For multi-line strings, using a verbatim string literal (@"...") is cleaner and more readable than concatenating multiple string literals with +.
var text = @"Today as always, men fall into two groups: slaves and free men. Whoever does not have two-thirds of his day for himself, is a slave, whatever he may be: a statesman, a businessman, an official, or a scholar. Friends fell out often because life was changing so fast. The easiest thing in the world was to lose touch with someone.";| // https://learn.microsoft.com/en-us/dotnet/standard/collections/thread-safe/blockingcollection-overview | ||
| var dataItems = new BlockingCollection<float[]>(); | ||
|
|
||
| var MyCallback = (IntPtr samples, int n, float progress, IntPtr arg) => |
There was a problem hiding this comment.
According to C# naming conventions, local variable names should use camelCase. Please rename MyCallback to myCallback.
var myCallback = (IntPtr samples, int n, float progress, IntPtr arg) =>|
|
||
| var audio = tts.GenerateWithConfig(text, genConfig, callback); | ||
|
|
||
| var outputFilename = "./generated-pocket-en-paly.wav"; |
There was a problem hiding this comment.
There is a typo in the filename. It should be play, not paly.
var outputFilename = "./generated-pocket-en-play.wav";| while (!playFinished) | ||
| { | ||
| Thread.Sleep(100); // 100ms | ||
| } |
There was a problem hiding this comment.
This while loop with Thread.Sleep is a form of busy-waiting, which is inefficient as it consumes CPU cycles unnecessarily. A better approach is to use a synchronization primitive like System.Threading.ManualResetEvent to wait for the playback to finish.
To implement this:
- Replace
var playFinished = false;withusing var finishedEvent = new System.Threading.ManualResetEvent(false);. - In the
playCallback, replaceplayFinished = true;withfinishedEvent.Set();. - Replace this
whileloop with a single call:finishedEvent.WaitOne();.
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="PortAudioSharp2" Version="*" /> |
There was a problem hiding this comment.
Using a wildcard * for package versions is generally discouraged as it can lead to unexpected build breaks when a new, potentially breaking, version of the package is released. It is safer to pin to a specific version or use a controlled version range (e.g., [2.0.0, 3.0.0)).
| var text = "Today as always, men fall into two groups: slaves and free men. Whoever " + | ||
| "does not have two-thirds of his day for himself, is a slave, whatever " + | ||
| "he may be: a statesman, a businessman, an official, or a scholar. " + | ||
| "Friends fell out often because life was changing so fast. The easiest " + | ||
| "thing in the world was to lose touch with someone."; |
There was a problem hiding this comment.
For multi-line strings, using a verbatim string literal (@"...") is cleaner and more readable than concatenating multiple string literals with +.
var text = @"Today as always, men fall into two groups: slaves and free men. Whoever does not have two-thirds of his day for himself, is a slave, whatever he may be: a statesman, a businessman, an official, or a scholar. Friends fell out often because life was changing so fast. The easiest thing in the world was to lose touch with someone.";| "Friends fell out often because life was changing so fast. The easiest " + | ||
| "thing in the world was to lose touch with someone."; | ||
|
|
||
| var MyCallback = (IntPtr samples, int n, float progress, IntPtr arg) => |
There was a problem hiding this comment.
According to C# naming conventions, local variable names should use camelCase. Please rename MyCallback to myCallback.
var myCallback = (IntPtr samples, int n, float progress, IntPtr arg) =>There was a problem hiding this comment.
Pull request overview
Adds PocketTTS support to the repository’s C#/.NET surface area, including new model/config bindings and runnable .NET examples, and wires one of the examples into the CI .NET test script.
Changes:
- Extend .NET interop configs to support PocketTTS (model config) and richer TTS generation parameters (generation config + new generate API).
- Update existing .NET ASR/Whisper configs to match newly available native options (Whisper timestamps, FunASR Nano language/ITN/hotwords).
- Add PocketTTS .NET example projects (including a playback example) and include PocketTTS in the GitHub Actions .NET test script.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/dotnet/OfflineWhisperModelConfig.cs | Adds token/segment timestamp toggles to align C# config with native Whisper options. |
| scripts/dotnet/OfflineTtsPocketModelConfig.cs | Introduces PocketTTS model file-path config struct for C# interop. |
| scripts/dotnet/OfflineTtsModelConfig.cs | Adds PocketTTS config to the unified OfflineTTS model config. |
| scripts/dotnet/OfflineTtsGenerationConfig.cs | Adds a generation-time config object (incl. reference audio + “Extra” JSON) for PocketTTS/advanced TTS. |
| scripts/dotnet/OfflineTts.cs | Adds a new callback delegate type and a GenerateWithConfig P/Invoke wrapper. |
| scripts/dotnet/OfflineFunAsrNanoModel.cs | Extends FunASR Nano config with language/ITN/hotwords fields. |
| dotnet-examples/sherpa-onnx.sln | Registers the new PocketTTS example projects in the solution. |
| dotnet-examples/pocket-tts-zero-shot/run.sh | Adds a runnable script to download PocketTTS model assets and run the example. |
| dotnet-examples/pocket-tts-zero-shot/pocket-tts-zero-shot.csproj | New PocketTTS “zero-shot” example project. |
| dotnet-examples/pocket-tts-zero-shot/Program.cs | Demonstrates using PocketTTS + generation config + progress callback (non-playback). |
| dotnet-examples/pocket-tts-zero-shot-play/run.sh | Adds a runnable script to download PocketTTS model assets and run the playback example. |
| dotnet-examples/pocket-tts-zero-shot-play/pocket-tts-zero-shot-play.csproj | New PocketTTS playback example project (PortAudio). |
| dotnet-examples/pocket-tts-zero-shot-play/Program.cs | Demonstrates PocketTTS generation with progressive playback via PortAudio. |
| .github/scripts/test-dot-net.sh | Runs the new PocketTTS .NET example as part of CI .NET tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // IntPtr is actually a `const float*` from C++ | ||
| public delegate int OfflineTtsCallback(IntPtr samples, int n); | ||
| public delegate int OfflineTtsCallbackProgress(IntPtr samples, int n, float progress); |
There was a problem hiding this comment.
The callback delegates are passed to P/Invokes declared with CallingConvention.Cdecl, but the delegates themselves are not annotated with [UnmanagedFunctionPointer(CallingConvention.Cdecl)]. On Windows x86 (which this repo targets via win-x86), this calling-convention mismatch can cause stack imbalance/crashes when the native code invokes the callback. Add the attribute to these delegate types (including the newly added OfflineTtsCallbackProgressWithArg).
| // IntPtr is actually a `const float*` from C++ | |
| public delegate int OfflineTtsCallback(IntPtr samples, int n); | |
| public delegate int OfflineTtsCallbackProgress(IntPtr samples, int n, float progress); | |
| // IntPtr is actually a `const float*` from C++ | |
| [UnmanagedFunctionPointer(CallingConvention.Cdecl)] | |
| public delegate int OfflineTtsCallback(IntPtr samples, int n); | |
| [UnmanagedFunctionPointer(CallingConvention.Cdecl)] | |
| public delegate int OfflineTtsCallbackProgress(IntPtr samples, int n, float progress); | |
| [UnmanagedFunctionPointer(CallingConvention.Cdecl)] |
| public OfflineTtsGeneratedAudio GenerateWithConfig(String text, OfflineTtsGenerationConfig config, OfflineTtsCallbackProgressWithArg callback) | ||
| { | ||
| byte[] utf8Bytes = Encoding.UTF8.GetBytes(text); | ||
| byte[] utf8BytesWithNull = new byte[utf8Bytes.Length + 1]; // +1 for null terminator | ||
| Array.Copy(utf8Bytes, utf8BytesWithNull, utf8Bytes.Length); | ||
| utf8BytesWithNull[utf8Bytes.Length] = 0; // Null terminator | ||
|
|
||
| GCHandle? audioHandle; | ||
|
|
||
| OfflineTtsGenerationConfig.NativeStruct nativeConfig = config.ToNative(out audioHandle); | ||
|
|
||
|
|
||
| IntPtr p = SherpaOnnxOfflineTtsGenerateWithConfig(_handle.Handle, utf8BytesWithNull, ref nativeConfig, callback, IntPtr.Zero); | ||
|
|
There was a problem hiding this comment.
GenerateWithConfig() always passes IntPtr.Zero for the native void* arg, so the arg parameter in OfflineTtsCallbackProgressWithArg is effectively unusable. Consider adding an overload that accepts an IntPtr arg (or GCHandle-backed object) and forwards it to the native API, or remove the arg parameter from the delegate if it’s intentionally unsupported.
| float[] data = new float[n]; | ||
| Marshal.Copy(samples, data, 0, n); | ||
| // You can process samples here, e.g., play them. | ||
| // See ../kitten-tts-playback for how to play them |
There was a problem hiding this comment.
The comment points to ../kitten-tts-playback, but the example directory in this repo is ../kitten-tts-play. Update the path in the comment to avoid sending users to a non-existent location.
| // See ../kitten-tts-playback for how to play them | |
| // See ../kitten-tts-play for how to play them |
|
|
||
| var audio = tts.GenerateWithConfig(text, genConfig, callback); | ||
|
|
||
| var outputFilename = "./generated-pocket-en-paly.wav"; |
There was a problem hiding this comment.
Typo in output filename: generated-pocket-en-paly.wav looks like it should be generated-pocket-en-play.wav. This filename is user-visible and easy to copy/paste, so it’s worth fixing.
| var outputFilename = "./generated-pocket-en-paly.wav"; | |
| var outputFilename = "./generated-pocket-en-play.wav"; |
| native.Extra = "{}"; | ||
| if (Extra != null && Extra.Count > 0) | ||
| { | ||
| string json = "{"; | ||
| bool first = true; | ||
| foreach (System.Collections.DictionaryEntry kv in Extra) | ||
| { | ||
| if (!first) json += ","; | ||
| string key = kv.Key.ToString(); | ||
| string val = kv.Value is string ? "\"" + kv.Value.ToString() + "\"" : kv.Value.ToString(); | ||
| json += "\"" + key + "\":" + val; | ||
| first = false; | ||
| } | ||
| json += "}"; | ||
| native.Extra = json; | ||
| } |
There was a problem hiding this comment.
OfflineTtsGenerationConfig.ToNative() builds the Extra JSON string via manual string concatenation without JSON escaping (quotes, backslashes, control chars) and uses ToString() for non-strings (culture-dependent formatting). This can produce invalid JSON that the native side cannot parse (and can be corrupted by keys/values containing " etc.). Prefer serializing Extra with a proper JSON serializer (e.g., System.Text.Json) and using invariant formatting for numbers.
| OfflineTtsGenerationConfig.NativeStruct nativeConfig = config.ToNative(out audioHandle); | ||
|
|
||
|
|
||
| IntPtr p = SherpaOnnxOfflineTtsGenerateWithConfig(_handle.Handle, utf8BytesWithNull, ref nativeConfig, callback, IntPtr.Zero); |
There was a problem hiding this comment.
Replace this call with a call to managed code if possible.
| private static extern IntPtr SherpaOnnxOfflineTtsGenerateWithProgressCallback(IntPtr handle, [MarshalAs(UnmanagedType.LPArray, ArraySubType = UnmanagedType.I1)] byte[] utf8Text, int sid, float speed, OfflineTtsCallbackProgress callback); | ||
|
|
||
| [DllImport(Dll.Filename, CallingConvention = CallingConvention.Cdecl)] | ||
| private static extern IntPtr SherpaOnnxOfflineTtsGenerateWithConfig(IntPtr handle, [MarshalAs(UnmanagedType.LPArray, ArraySubType = UnmanagedType.I1)] byte[] utf8Text, ref OfflineTtsGenerationConfig.NativeStruct config, OfflineTtsCallbackProgressWithArg callback, IntPtr arg); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
| { | ||
| if (!first) json += ","; | ||
| string key = kv.Key.ToString(); | ||
| string val = kv.Value is string ? "\"" + kv.Value.ToString() + "\"" : kv.Value.ToString(); |
There was a problem hiding this comment.
Redundant call to 'ToString' on a String object.
| string val = kv.Value is string ? "\"" + kv.Value.ToString() + "\"" : kv.Value.ToString(); | |
| string val = kv.Value is string s ? "\"" + s + "\"" : kv.Value.ToString(); |
| bool first = true; | ||
| foreach (System.Collections.DictionaryEntry kv in Extra) | ||
| { | ||
| if (!first) json += ","; |
There was a problem hiding this comment.
String concatenation in loop: use 'StringBuilder'.
| if (!first) json += ","; | ||
| string key = kv.Key.ToString(); | ||
| string val = kv.Value is string ? "\"" + kv.Value.ToString() + "\"" : kv.Value.ToString(); | ||
| json += "\"" + key + "\":" + val; |
There was a problem hiding this comment.
String concatenation in loop: use 'StringBuilder'.
📝 WalkthroughWalkthroughAdds PocketTTS support to the .NET bindings: new interop structs and APIs for generation-with-config, two PocketTTS example apps (generation and playback) with run scripts and solution entries, updates to CI test script to run the pocket example first, and assorted marshaling and small C# edits. Changes
Sequence Diagram(s)sequenceDiagram
participant App as .NET App (PocketTtsDemo)
participant FS as FileSystem / Model artifacts
participant Lib as Native SherpaOnnx
participant Audio as PortAudio / Playback
App->>FS: Ensure model files present (run.sh / download)
App->>Lib: SherpaOnnxOfflineTtsGenerateWithConfig(text, config, callback, arg)
Lib-->>App: Invoke callback with audio chunks (samples, n, progress, arg)
App->>Audio: Stream audio chunks to PortAudio (or buffer to WAV)
App->>FS: Save final waveform to WAV
Lib-->>App: Native generation completes / returns
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@dotnet-examples/pocket-tts-zero-shot/Program.cs`:
- Around line 52-63: Rename the local callback variable MyCallback to myCallback
to match camelCase convention used in other examples and update any references
to it; inside the callback (the lambda assigned to myCallback) remove the unused
allocation and Marshal.Copy of float[] data (or replace it with a single-line
comment like "// process samples here (e.g., Marshal.Copy into a buffer and
play)"), leaving the Console.WriteLine and the return 1 intact so the callback
still reports progress and signals continue.
In `@scripts/dotnet/OfflineTts.cs`:
- Around line 67-90: The call to GetUtf8BytesWithNull in GenerateWithConfig is
unresolved; implement a private helper that builds a null-terminated UTF-8 byte
array (or replace the call with the same inline pattern used in the other
Generate* methods: Encoding.UTF8.GetBytes, copy into a new array length+1 and
set final byte to 0), ensure you pin/free that array the same way you do now,
and keep the existing GCHandle cleanup in the finally block; also fix the stray
indentation before the _callbackRef = null assignment in the other Generate
method so the assignment aligns correctly with surrounding code.
In `@scripts/dotnet/OfflineTtsGenerationConfig.cs`:
- Around line 86-94: The manual JSON builder in OfflineTtsGenerationConfig.cs
currently falls back to kv.Value.ToString() (see the block around JsonEscape,
JsonEscape method and the json.AppendFormat("{0}:{1}", key, val) call), which
produces invalid JSON for booleans and nulls; update the value formatting to
explicitly handle bool (emit "true"/"false" lowercase), handle null (emit
"null"), and otherwise fall back to the existing numeric/quoted-string logic so
non-string, non-numeric values are serialized to valid JSON.
- Around line 8-10: The file imports System.Web.Script.Serialization and uses
JavaScriptSerializer which is unavailable in net8.0; replace the using directive
with System.Text.Json and update all uses of JavaScriptSerializer (e.g., any
calls like new JavaScriptSerializer().Serialize/Deserialize in
OfflineTtsGenerationConfig) to use
System.Text.Json.JsonSerializer.Serialize/Deserialize<T>, adjust types and
options as needed (e.g., JsonSerializerOptions for camelCase or property
handling), and remove the incorrect `#if` !NET20 guard so the System.Text.Json
import is used for modern frameworks.
🧹 Nitpick comments (6)
dotnet-examples/pocket-tts-zero-shot-play/pocket-tts-zero-shot-play.csproj (1)
12-12: PinPortAudioSharp2to a specific version for reproducible builds.
Version="*"resolves to the latest available version at restore time, making builds non-reproducible and risking silent introduction of breaking changes. Consider pinning to a specific version (e.g.,Version="1.4.0"). Note: This pattern is used across all .NET example projects in the repository and could benefit from a coordinated update.dotnet-examples/pocket-tts-zero-shot-play/Program.cs (3)
44-44: Minor formatting nit: missing space before=.- genConfig.ReferenceSampleRate= reader.SampleRate; + genConfig.ReferenceSampleRate = reader.SampleRate;
38-47:OfflineTtsandWaveReaderimplementIDisposablebut are never disposed.
OfflineTtswraps a native handle and implementsIDisposable(seescripts/dotnet/OfflineTts.cs). Neitherttsnorreaderare disposed, which defers cleanup to the finalizer (non-deterministic). Consider wrapping inusingstatements. The same applies to thePortAudioSharp.Streamon line 172—it should be stopped and disposed after playback finishes.Suggested pattern
+ using var reader = new WaveReader(referenceWaveFilename); // ... + using var tts = new OfflineTts(config);
109-170: The PortAudio playback callback is duplicated across multiple demos.This block is essentially identical to the playback callback in
kitten-tts-play/Program.csandoffline-tts-play/Program.cs. Consider extracting it into a shared helper in the Common project to avoid maintaining three copies.dotnet-examples/pocket-tts-zero-shot/Program.cs (1)
39-45: SameIDisposableconcern as the play variant:readerandttsare not disposed.See the same comment on
pocket-tts-zero-shot-play/Program.cs. Consider usingusingdeclarations forWaveReaderandOfflineTts.scripts/dotnet/OfflineTts.cs (1)
50-51: Nit: extra leading space on line 51.Line 51 has an extra leading space compared to the surrounding indentation.
Proposed fix
- _callbackRef = null; + _callbackRef = null;
| var MyCallback = (IntPtr samples, int n, float progress, IntPtr arg) => | ||
| { | ||
| float[] data = new float[n]; | ||
| Marshal.Copy(samples, data, 0, n); | ||
| // You can process samples here, e.g., play them. | ||
| // See ../pocket-tts-zero-shot-play for how to play them | ||
| Console.WriteLine($"Progress {progress*100}%"); | ||
|
|
||
| // 1 means to keep generating | ||
| // 0 means to stop generating | ||
| return 1; | ||
| }; |
There was a problem hiding this comment.
Naming inconsistency: MyCallback should be myCallback.
This PR specifically renames MyCallback → myCallback in both offline-tts-play/Program.cs and kitten-tts-play/Program.cs to follow C# camelCase convention for local variables. This new file uses MyCallback, which is inconsistent with those changes.
Additionally, lines 54–55 allocate and populate float[] data but never use it—this is dead code that wastes allocations on every callback invocation. If the intent is to demonstrate where sample processing would happen, a comment alone would suffice.
Suggested fix
- var MyCallback = (IntPtr samples, int n, float progress, IntPtr arg) =>
+ var myCallback = (IntPtr samples, int n, float progress, IntPtr arg) =>
{
- float[] data = new float[n];
- Marshal.Copy(samples, data, 0, n);
// You can process samples here, e.g., play them.
// See ../pocket-tts-zero-shot-play for how to play them
Console.WriteLine($"Progress {progress*100}%");
// 1 means to keep generating
// 0 means to stop generating
return 1;
};
- var callback = new OfflineTtsCallbackProgressWithArg(MyCallback);
+ var callback = new OfflineTtsCallbackProgressWithArg(myCallback);🤖 Prompt for AI Agents
In `@dotnet-examples/pocket-tts-zero-shot/Program.cs` around lines 52 - 63, Rename
the local callback variable MyCallback to myCallback to match camelCase
convention used in other examples and update any references to it; inside the
callback (the lambda assigned to myCallback) remove the unused allocation and
Marshal.Copy of float[] data (or replace it with a single-line comment like "//
process samples here (e.g., Marshal.Copy into a buffer and play)"), leaving the
Console.WriteLine and the return 1 intact so the callback still reports progress
and signals continue.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/dotnet/OfflineTts.cs`:
- Around line 67-92: In GenerateWithConfig, add an explicit null terminator to
the utf8BytesWithNull buffer (set utf8BytesWithNull[utf8Bytes.Length] = 0) after
copying utf8Bytes so the behavior matches the other Generate* methods (e.g.,
Generate, GenerateWithVoice, GenerateWithModel) and avoids confusion for future
readers; locate the code in the GenerateWithConfig method and insert the single
assignment right after Array.Copy.
In `@scripts/dotnet/OfflineTtsGenerationConfig.cs`:
- Around line 155-173: The NativeStruct definition uses
[MarshalAs(UnmanagedType.LPStr)] for ReferenceText and Extra which marshals ANSI
instead of UTF-8 and will corrupt non-ASCII data; update the marshaling to use
UTF-8 by either changing the attributes to [MarshalAs(UnmanagedType.LPUTF8Str)]
(requires .NET 7+) for ReferenceText and Extra or, to be compatible with older
runtimes, remove those attributes and mimic GenerateWithConfig's approach by
adding byte[]/IntPtr fields for the UTF-8-encoded, null-terminated bytes and
pin/assign those buffers before calling into the native
SherpaOnnxGenerationConfig APIs so the native const char* receives correct UTF-8
data.
🧹 Nitpick comments (1)
scripts/dotnet/OfflineTtsGenerationConfig.cs (1)
71-116: NET20 JSON:kv.Valuecould benull, causingNullReferenceException.If a
Hashtableentry has anullvalue,kv.Value.ToString()on line 94 (andkv.Value is stringetc.) will throw. This is a minor edge case sinceExtrais likely only populated with non-null values in practice.Proposed defensive fix
foreach (DictionaryEntry kv in Extra) { if (!first) json.Append(","); first = false; string key = JsonEscape(kv.Key.ToString()); string val; - if (kv.Value is string) + if (kv.Value == null) + val = "null"; + else if (kv.Value is string) val = JsonEscape((string)kv.Value);
| [StructLayout(LayoutKind.Sequential)] | ||
| internal struct NativeStruct | ||
| { | ||
| public float SilenceScale; | ||
| public float Speed; | ||
| public int Sid; | ||
|
|
||
| public IntPtr ReferenceAudio; | ||
| public int ReferenceAudioLen; | ||
| public int ReferenceSampleRate; | ||
|
|
||
| [MarshalAs(UnmanagedType.LPStr)] | ||
| public string ReferenceText; | ||
|
|
||
| public int NumSteps; | ||
|
|
||
| [MarshalAs(UnmanagedType.LPStr)] | ||
| public string Extra; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other NativeStruct/config types in the codebase marshal strings
echo "=== String marshaling in StructLayout types ==="
rg -n 'UnmanagedType\.LP' --type=cs -B2 -A0 -g '*.cs' scripts/dotnet/
echo ""
echo "=== Check the C struct definition for SherpaOnnxGenerationConfig ==="
rg -n 'SherpaOnnxGenerationConfig' --type=cpp --type=c -B2 -A20 | head -100Repository: k2-fsa/sherpa-onnx
Length of output: 40019
🏁 Script executed:
#!/bin/bash
# Find and examine GenerateWithConfig method to see how text is handled
echo "=== GenerateWithConfig method implementation ==="
rg -n 'GenerateWithConfig' --type=cs -B2 -A30 scripts/dotnet/ | head -150Repository: k2-fsa/sherpa-onnx
Length of output: 3655
🏁 Script executed:
#!/bin/bash
# Look at the actual file to understand context better
echo "=== OfflineTtsGenerationConfig.cs content ==="
wc -l scripts/dotnet/OfflineTtsGenerationConfig.cs
cat -n scripts/dotnet/OfflineTtsGenerationConfig.csRepository: k2-fsa/sherpa-onnx
Length of output: 6883
Use UnmanagedType.LPUTF8Str for ReferenceText and Extra fields, or manually marshal as UTF-8 byte arrays.
UnmanagedType.LPStr marshals as ANSI (system code page), not UTF-8. The native C API expects UTF-8 strings (const char* in SherpaOnnxGenerationConfig). Non-ASCII characters in ReferenceText (e.g., Chinese text for PocketTTS zero-shot prompts) or Extra JSON will be corrupted on Windows.
The text parameter in GenerateWithConfig handles this correctly by manually encoding to UTF-8 bytes with a null terminator. Apply the same approach to these struct fields: either use UnmanagedType.LPUTF8Str (.NET 7+) or manually marshal to pinned UTF-8 byte arrays like the text parameter does.
🤖 Prompt for AI Agents
In `@scripts/dotnet/OfflineTtsGenerationConfig.cs` around lines 155 - 173, The
NativeStruct definition uses [MarshalAs(UnmanagedType.LPStr)] for ReferenceText
and Extra which marshals ANSI instead of UTF-8 and will corrupt non-ASCII data;
update the marshaling to use UTF-8 by either changing the attributes to
[MarshalAs(UnmanagedType.LPUTF8Str)] (requires .NET 7+) for ReferenceText and
Extra or, to be compatible with older runtimes, remove those attributes and
mimic GenerateWithConfig's approach by adding byte[]/IntPtr fields for the
UTF-8-encoded, null-terminated bytes and pin/assign those buffers before calling
into the native SherpaOnnxGenerationConfig APIs so the native const char*
receives correct UTF-8 data.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if !NET20 | ||
| using System.Text.Json; | ||
| #endif |
There was a problem hiding this comment.
OfflineTtsGenerationConfig uses System.Text.Json under #if !NET20, but the sherpa-onnx package multi-targets net35, net40, and net45 where System.Text.Json is not available by default and is not referenced in the project. This will break builds for those target frameworks; consider either avoiding System.Text.Json entirely (use the manual JSON builder for all legacy TFMs) or tightening the compilation symbol (e.g., only enable this path for net6+/net7+/net8+ or netstandard2.0 with an explicit package reference).
| // Keep delegates alive | ||
| private OfflineTtsCallback _callbackRef; | ||
| private OfflineTtsCallbackProgress _callbackProgressRef; | ||
| private OfflineTtsCallbackProgressWithArg _callbackWithArgRef; | ||
|
|
There was a problem hiding this comment.
GenerateWithCallback*() stores the delegate in instance fields (_callbackRef, _callbackProgressRef, _callbackWithArgRef) and then clears them after the P/Invoke. This makes the API non-reentrant and unsafe if OfflineTts.Generate*() is called concurrently (callbacks can be overwritten/null'ed mid-call). Prefer keeping the delegate alive per-invocation (e.g., via a local GCHandle/SafeHandle pattern or a lock) rather than shared instance fields.
| public OfflineTtsGeneratedAudio GenerateWithConfig(string text, OfflineTtsGenerationConfig config, OfflineTtsCallbackProgressWithArg callback) | ||
| { | ||
| _callbackWithArgRef = callback; | ||
| byte[] utf8Bytes = Encoding.UTF8.GetBytes(text); | ||
| byte[] utf8BytesWithNull = new byte[utf8Bytes.Length + 1]; // +1 for null terminator | ||
| Array.Copy(utf8Bytes, utf8BytesWithNull, utf8Bytes.Length); | ||
|
|
||
| GCHandle? audioHandle = null; | ||
| IntPtr p; | ||
|
|
||
| var nativeConfig = config.ToNative(out audioHandle); | ||
|
|
||
| try | ||
| { | ||
| p = SherpaOnnxOfflineTtsGenerateWithConfig(_handle.Handle, utf8BytesWithNull, ref nativeConfig, _callbackWithArgRef, IntPtr.Zero); | ||
| } |
There was a problem hiding this comment.
GenerateWithConfig() always passes IntPtr.Zero for the native arg, so the IntPtr arg parameter of OfflineTtsCallbackProgressWithArg is never useful to callers. Consider either exposing an overload that accepts an IntPtr arg (or GCHandle/object state) to pass through, or remove the arg from the managed callback signature to avoid a confusing API.
| while ((lastSampleArray != null || dataItems.Count != 0) && (i < expected)) | ||
| { | ||
| int needed = expected - i; | ||
|
|
||
| if (lastSampleArray != null) | ||
| { | ||
| int remaining = lastSampleArray.Length - lastIndex; | ||
| if (remaining >= needed) | ||
| { | ||
| float[] this_block = lastSampleArray.Skip(lastIndex).Take(needed).ToArray(); | ||
| lastIndex += needed; | ||
| if (lastIndex == lastSampleArray.Length) | ||
| { | ||
| lastSampleArray = null; | ||
| lastIndex = 0; | ||
| } | ||
|
|
||
| Marshal.Copy(this_block, 0, IntPtr.Add(output, i * sizeof(float)), needed); | ||
| return StreamCallbackResult.Continue; | ||
| } | ||
|
|
||
| float[] this_block2 = lastSampleArray.Skip(lastIndex).Take(remaining).ToArray(); | ||
| lastIndex = 0; | ||
| lastSampleArray = null; | ||
|
|
||
| Marshal.Copy(this_block2, 0, IntPtr.Add(output, i * sizeof(float)), remaining); | ||
| i += remaining; | ||
| continue; | ||
| } | ||
|
|
||
| if (dataItems.Count != 0) | ||
| { | ||
| lastSampleArray = dataItems.Take(); | ||
| lastIndex = 0; | ||
| } |
There was a problem hiding this comment.
The PortAudio callback uses dataItems.Count followed by dataItems.Take() inside the real-time audio thread. This has a race where Take() can block if the collection becomes empty after the Count check, which can glitch/hang audio. Use TryTake(out item, 0) (or a lock-free ring buffer) and never block in the audio callback.
| while ((lastSampleArray != null || dataItems.Count != 0) && (i < expected)) | |
| { | |
| int needed = expected - i; | |
| if (lastSampleArray != null) | |
| { | |
| int remaining = lastSampleArray.Length - lastIndex; | |
| if (remaining >= needed) | |
| { | |
| float[] this_block = lastSampleArray.Skip(lastIndex).Take(needed).ToArray(); | |
| lastIndex += needed; | |
| if (lastIndex == lastSampleArray.Length) | |
| { | |
| lastSampleArray = null; | |
| lastIndex = 0; | |
| } | |
| Marshal.Copy(this_block, 0, IntPtr.Add(output, i * sizeof(float)), needed); | |
| return StreamCallbackResult.Continue; | |
| } | |
| float[] this_block2 = lastSampleArray.Skip(lastIndex).Take(remaining).ToArray(); | |
| lastIndex = 0; | |
| lastSampleArray = null; | |
| Marshal.Copy(this_block2, 0, IntPtr.Add(output, i * sizeof(float)), remaining); | |
| i += remaining; | |
| continue; | |
| } | |
| if (dataItems.Count != 0) | |
| { | |
| lastSampleArray = dataItems.Take(); | |
| lastIndex = 0; | |
| } | |
| while (i < expected) | |
| { | |
| if (lastSampleArray == null) | |
| { | |
| if (!dataItems.TryTake(out lastSampleArray, 0)) | |
| { | |
| // No more data available right now; remaining samples will be zero-filled below. | |
| break; | |
| } | |
| lastIndex = 0; | |
| } | |
| int needed = expected - i; | |
| int remaining = lastSampleArray.Length - lastIndex; | |
| if (remaining >= needed) | |
| { | |
| float[] this_block = lastSampleArray.Skip(lastIndex).Take(needed).ToArray(); | |
| lastIndex += needed; | |
| if (lastIndex == lastSampleArray.Length) | |
| { | |
| lastSampleArray = null; | |
| lastIndex = 0; | |
| } | |
| Marshal.Copy(this_block, 0, IntPtr.Add(output, i * sizeof(float)), needed); | |
| return StreamCallbackResult.Continue; | |
| } | |
| float[] this_block2 = lastSampleArray.Skip(lastIndex).Take(remaining).ToArray(); | |
| lastIndex = 0; | |
| lastSampleArray = null; | |
| Marshal.Copy(this_block2, 0, IntPtr.Add(output, i * sizeof(float)), remaining); | |
| i += remaining; |
| var playFinished = false; | ||
|
|
||
| float[]? lastSampleArray = null; | ||
| int lastIndex = 0; // not played | ||
|
|
||
| PortAudioSharp.Stream.Callback playCallback = (IntPtr input, IntPtr output, | ||
| UInt32 frameCount, | ||
| ref StreamCallbackTimeInfo timeInfo, | ||
| StreamCallbackFlags statusFlags, | ||
| IntPtr userData | ||
| ) => | ||
| { | ||
| if (dataItems.IsCompleted && lastSampleArray == null && lastIndex == 0) | ||
| { | ||
| Console.WriteLine($"Finished playing"); | ||
| playFinished = true; | ||
| return StreamCallbackResult.Complete; | ||
| } |
There was a problem hiding this comment.
playFinished is written from the PortAudio callback thread and read from the main thread without any synchronization. This is a data race and the main thread may never observe the update. Use Volatile.Write/Read, Interlocked.Exchange, or a ManualResetEventSlim/TaskCompletionSource to signal completion.
| PortAudioSharp.Stream stream = new PortAudioSharp.Stream(inParams: null, outParams: param, sampleRate: tts.SampleRate, | ||
| framesPerBuffer: 0, | ||
| streamFlags: StreamFlags.ClipOff, | ||
| callback: playCallback, | ||
| userData: IntPtr.Zero | ||
| ); | ||
|
|
||
| stream.Start(); | ||
|
|
||
| var callback = new OfflineTtsCallbackProgressWithArg(myCallback); | ||
|
|
||
| var audio = tts.GenerateWithConfig(text, genConfig, callback); | ||
|
|
||
| var outputFilename = "./generated-pocket-en-play.wav"; | ||
| var ok = audio.SaveToWaveFile(outputFilename); | ||
|
|
||
| if (ok) | ||
| { | ||
| Console.WriteLine($"Wrote to {outputFilename} succeeded!"); | ||
| } | ||
| else | ||
| { | ||
| Console.WriteLine($"Failed to write {outputFilename}"); | ||
| } | ||
|
|
||
| dataItems.CompleteAdding(); | ||
|
|
||
| while (!playFinished) | ||
| { | ||
| Thread.Sleep(100); // 100ms |
There was a problem hiding this comment.
Disposable 'Stream' is created but not disposed.
| PortAudioSharp.Stream stream = new PortAudioSharp.Stream(inParams: null, outParams: param, sampleRate: tts.SampleRate, | |
| framesPerBuffer: 0, | |
| streamFlags: StreamFlags.ClipOff, | |
| callback: playCallback, | |
| userData: IntPtr.Zero | |
| ); | |
| stream.Start(); | |
| var callback = new OfflineTtsCallbackProgressWithArg(myCallback); | |
| var audio = tts.GenerateWithConfig(text, genConfig, callback); | |
| var outputFilename = "./generated-pocket-en-play.wav"; | |
| var ok = audio.SaveToWaveFile(outputFilename); | |
| if (ok) | |
| { | |
| Console.WriteLine($"Wrote to {outputFilename} succeeded!"); | |
| } | |
| else | |
| { | |
| Console.WriteLine($"Failed to write {outputFilename}"); | |
| } | |
| dataItems.CompleteAdding(); | |
| while (!playFinished) | |
| { | |
| Thread.Sleep(100); // 100ms | |
| using (PortAudioSharp.Stream stream = new PortAudioSharp.Stream(inParams: null, outParams: param, sampleRate: tts.SampleRate, | |
| framesPerBuffer: 0, | |
| streamFlags: StreamFlags.ClipOff, | |
| callback: playCallback, | |
| userData: IntPtr.Zero | |
| )) | |
| { | |
| stream.Start(); | |
| var callback = new OfflineTtsCallbackProgressWithArg(myCallback); | |
| var audio = tts.GenerateWithConfig(text, genConfig, callback); | |
| var outputFilename = "./generated-pocket-en-play.wav"; | |
| var ok = audio.SaveToWaveFile(outputFilename); | |
| if (ok) | |
| { | |
| Console.WriteLine($"Wrote to {outputFilename} succeeded!"); | |
| } | |
| else | |
| { | |
| Console.WriteLine($"Failed to write {outputFilename}"); | |
| } | |
| dataItems.CompleteAdding(); | |
| while (!playFinished) | |
| { | |
| Thread.Sleep(100); // 100ms | |
| } |
| Array.Copy(utf8Bytes, utf8BytesWithNull, utf8Bytes.Length); | ||
| utf8BytesWithNull[utf8Bytes.Length] = 0; // Null terminator | ||
| IntPtr p = SherpaOnnxOfflineTtsGenerateWithCallback(_handle.Handle, utf8BytesWithNull, speakerId, speed, callback); | ||
| IntPtr p = SherpaOnnxOfflineTtsGenerateWithCallback(_handle.Handle, utf8BytesWithNull, speakerId, speed, _callbackRef); |
There was a problem hiding this comment.
Replace this call with a call to managed code if possible.
|
|
||
| try | ||
| { | ||
| p = SherpaOnnxOfflineTtsGenerateWithConfig(_handle.Handle, utf8BytesWithNull, ref nativeConfig, _callbackWithArgRef, IntPtr.Zero); |
There was a problem hiding this comment.
Replace this call with a call to managed code if possible.
| private static extern IntPtr SherpaOnnxOfflineTtsGenerateWithProgressCallback(IntPtr handle, [MarshalAs(UnmanagedType.LPArray, ArraySubType = UnmanagedType.I1)] byte[] utf8Text, int sid, float speed, OfflineTtsCallbackProgress callback); | ||
|
|
||
| [DllImport(Dll.Filename, CallingConvention = CallingConvention.Cdecl)] | ||
| private static extern IntPtr SherpaOnnxOfflineTtsGenerateWithConfig(IntPtr handle, [MarshalAs(UnmanagedType.LPArray, ArraySubType = UnmanagedType.I1)] byte[] utf8Text, ref OfflineTtsGenerationConfig.NativeStruct config, OfflineTtsCallbackProgressWithArg callback, IntPtr arg); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
| if (Extra != null && Extra.Count > 0) | ||
| { | ||
| native.Extra = JsonSerializer.Serialize( | ||
| Extra, | ||
| new JsonSerializerOptions | ||
| { | ||
| PropertyNamingPolicy = JsonNamingPolicy.CamelCase | ||
| }); | ||
| } | ||
| else | ||
| { | ||
| native.Extra = "{}"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (Extra != null && Extra.Count > 0) | |
| { | |
| native.Extra = JsonSerializer.Serialize( | |
| Extra, | |
| new JsonSerializerOptions | |
| { | |
| PropertyNamingPolicy = JsonNamingPolicy.CamelCase | |
| }); | |
| } | |
| else | |
| { | |
| native.Extra = "{}"; | |
| } | |
| native.Extra = (Extra != null && Extra.Count > 0) | |
| ? JsonSerializer.Serialize( | |
| Extra, | |
| new JsonSerializerOptions | |
| { | |
| PropertyNamingPolicy = JsonNamingPolicy.CamelCase | |
| }) | |
| : "{}"; |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/dotnet/OfflineTts.cs`:
- Around line 114-140: The code calls config.ToNative(out audioHandle) before
entering the try, so if ToNative throws the GCHandle allocated into audioHandle
can leak; move the ToNative call and creation of nativeConfig inside the try
after initializing audioHandle to null so both callbackHandle and audioHandle
are freed in the finally; specifically, initialize "GCHandle? audioHandle =
null" as now, then inside the try call "var nativeConfig = config.ToNative(out
audioHandle)", allocate callbackHandle with GCHandle.Alloc(callback), then call
SherpaOnnxOfflineTtsGenerateWithConfig and return OfflineTtsGeneratedAudio,
leaving the existing finally block to free callbackHandle and audioHandle if
allocated.
In `@scripts/dotnet/OfflineTtsGenerationConfig.cs`:
- Around line 103-110: The current serialization block sets
JsonSerializerOptions.PropertyNamingPolicy which does not affect dictionary keys
(so Extra dictionary keys remain unchanged); update the JsonSerializerOptions
used when serializing Extra in the assignment to native.Extra to either remove
the ineffective PropertyNamingPolicy or replace it with DictionaryKeyPolicy =
JsonNamingPolicy.CamelCase if you intend to convert dictionary keys to
camelCase; ensure you reference the same symbols (Extra, native.Extra,
JsonSerializer.Serialize) and adjust only the JsonSerializerOptions passed to
Serialize.
| GCHandle callbackHandle = default(GCHandle); | ||
| GCHandle? audioHandle = null; | ||
|
|
||
| var nativeConfig = config.ToNative(out audioHandle); | ||
|
|
||
| try | ||
| { | ||
| callbackHandle = GCHandle.Alloc(callback); | ||
|
|
||
| IntPtr p = SherpaOnnxOfflineTtsGenerateWithConfig( | ||
| _handle.Handle, | ||
| utf8BytesWithNull, | ||
| ref nativeConfig, | ||
| callback, | ||
| IntPtr.Zero | ||
| ); | ||
|
|
||
| return new OfflineTtsGeneratedAudio(p); | ||
| } | ||
| finally | ||
| { | ||
| if (callbackHandle.IsAllocated) | ||
| callbackHandle.Free(); | ||
|
|
||
| if (audioHandle.HasValue) | ||
| audioHandle.Value.Free(); | ||
| } |
There was a problem hiding this comment.
audioHandle allocated outside the try block — resource leak on failure.
config.ToNative(out audioHandle) at line 117 may pin ReferenceAudio via GCHandle.Alloc internally, but if a subsequent statement in ToNative (or code between lines 117–120) throws, the finally block won't free audioHandle because execution never entered the try.
Move ToNative inside the try block:
Proposed fix
GCHandle callbackHandle = default(GCHandle);
GCHandle? audioHandle = null;
- var nativeConfig = config.ToNative(out audioHandle);
-
try
{
+ var nativeConfig = config.ToNative(out audioHandle);
callbackHandle = GCHandle.Alloc(callback);📝 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.
| GCHandle callbackHandle = default(GCHandle); | |
| GCHandle? audioHandle = null; | |
| var nativeConfig = config.ToNative(out audioHandle); | |
| try | |
| { | |
| callbackHandle = GCHandle.Alloc(callback); | |
| IntPtr p = SherpaOnnxOfflineTtsGenerateWithConfig( | |
| _handle.Handle, | |
| utf8BytesWithNull, | |
| ref nativeConfig, | |
| callback, | |
| IntPtr.Zero | |
| ); | |
| return new OfflineTtsGeneratedAudio(p); | |
| } | |
| finally | |
| { | |
| if (callbackHandle.IsAllocated) | |
| callbackHandle.Free(); | |
| if (audioHandle.HasValue) | |
| audioHandle.Value.Free(); | |
| } | |
| GCHandle callbackHandle = default(GCHandle); | |
| GCHandle? audioHandle = null; | |
| try | |
| { | |
| var nativeConfig = config.ToNative(out audioHandle); | |
| callbackHandle = GCHandle.Alloc(callback); | |
| IntPtr p = SherpaOnnxOfflineTtsGenerateWithConfig( | |
| _handle.Handle, | |
| utf8BytesWithNull, | |
| ref nativeConfig, | |
| callback, | |
| IntPtr.Zero | |
| ); | |
| return new OfflineTtsGeneratedAudio(p); | |
| } | |
| finally | |
| { | |
| if (callbackHandle.IsAllocated) | |
| callbackHandle.Free(); | |
| if (audioHandle.HasValue) | |
| audioHandle.Value.Free(); | |
| } |
🤖 Prompt for AI Agents
In `@scripts/dotnet/OfflineTts.cs` around lines 114 - 140, The code calls
config.ToNative(out audioHandle) before entering the try, so if ToNative throws
the GCHandle allocated into audioHandle can leak; move the ToNative call and
creation of nativeConfig inside the try after initializing audioHandle to null
so both callbackHandle and audioHandle are freed in the finally; specifically,
initialize "GCHandle? audioHandle = null" as now, then inside the try call "var
nativeConfig = config.ToNative(out audioHandle)", allocate callbackHandle with
GCHandle.Alloc(callback), then call SherpaOnnxOfflineTtsGenerateWithConfig and
return OfflineTtsGeneratedAudio, leaving the existing finally block to free
callbackHandle and audioHandle if allocated.
| native.Extra = (Extra != null && Extra.Count > 0) | ||
| ? JsonSerializer.Serialize( | ||
| Extra, | ||
| new JsonSerializerOptions | ||
| { | ||
| PropertyNamingPolicy = JsonNamingPolicy.CamelCase | ||
| }) | ||
| : "{}"; |
There was a problem hiding this comment.
PropertyNamingPolicy does not affect dictionary keys — CamelCase option is ineffective here.
JsonSerializerOptions.PropertyNamingPolicy applies to object property names, not to dictionary keys. For a Hashtable (serialized as a dictionary), the keys are emitted as-is. If camelCase keys are desired, use DictionaryKeyPolicy instead — or simply drop the option if keys are already in the expected format.
Proposed fix (if camelCase keys are intended)
? JsonSerializer.Serialize(
Extra,
new JsonSerializerOptions
{
- PropertyNamingPolicy = JsonNamingPolicy.CamelCase
+ DictionaryKeyPolicy = JsonNamingPolicy.CamelCase
})📝 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.
| native.Extra = (Extra != null && Extra.Count > 0) | |
| ? JsonSerializer.Serialize( | |
| Extra, | |
| new JsonSerializerOptions | |
| { | |
| PropertyNamingPolicy = JsonNamingPolicy.CamelCase | |
| }) | |
| : "{}"; | |
| native.Extra = (Extra != null && Extra.Count > 0) | |
| ? JsonSerializer.Serialize( | |
| Extra, | |
| new JsonSerializerOptions | |
| { | |
| DictionaryKeyPolicy = JsonNamingPolicy.CamelCase | |
| }) | |
| : "{}"; |
🤖 Prompt for AI Agents
In `@scripts/dotnet/OfflineTtsGenerationConfig.cs` around lines 103 - 110, The
current serialization block sets JsonSerializerOptions.PropertyNamingPolicy
which does not affect dictionary keys (so Extra dictionary keys remain
unchanged); update the JsonSerializerOptions used when serializing Extra in the
assignment to native.Extra to either remove the ineffective PropertyNamingPolicy
or replace it with DictionaryKeyPolicy = JsonNamingPolicy.CamelCase if you
intend to convert dictionary keys to camelCase; ensure you reference the same
symbols (Extra, native.Extra, JsonSerializer.Serialize) and adjust only the
JsonSerializerOptions passed to Serialize.
Summary by CodeRabbit
New Features
Enhancements