-
-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds WithTimestamp functions for regular and streaming versions #99
Conversation
public readonly struct TimestampedTranscriptCharacter | ||
{ | ||
/// <summary> | ||
/// The character being spoken | ||
/// </summary> | ||
public readonly string Character; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
structs should not have strings. It is ok to make this a class.
/// </param> | ||
/// <param name="cancellationToken">Optional, <see cref="CancellationToken"/>.</param> | ||
/// <returns>A tuple containing the <see cref="VoiceClip"/> and an array of <see cref="TimestampedTranscriptCharacter"/>.</returns> | ||
public async Task<(VoiceClip voiceClip, TimestampedTranscriptCharacter[] timestampedTranscriptCharacters)> TextToSpeechWithTimestampsAsync(string text, Voice voice, VoiceSettings voiceSettings = null, Model model = null, OutputFormat outputFormat = OutputFormat.MP3_44100_128, int? optimizeStreamingLatency = null, CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for a public API I prefer not to return anon Tuples in this way. I'd prefer to create a class definition for the signature, to make breaking changes easier to deal with in future.
/// <returns>Downloaded clip path, and the loaded audio clip.</returns> | ||
public async Task<(VoiceClip voiceClip, TimestampedTranscriptCharacter[] timestampedTranscriptCharacters)> StreamTextToSpeechWithTimestampsAsync(TextToSpeechRequest request, Action<(AudioClip audioClip, TimestampedTranscriptCharacter[] timestampedTranscriptCharacters)> partialClipCallback, CancellationToken cancellationToken = default) | ||
{ | ||
var frequency = request.OutputFormat switch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a lot of duplicate code, maybe we can organize this a bit more to reduce duplicates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
Demo code here: