(model/cosyvoice): support flush api#211
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a TextStreamItem wrapper class to the SpeechSynthesizer to support stream flushing and optional parameters. The internal reactive streams and emitters have been updated to use this new type, and new streamingFlush methods were added. Feedback recommends refactoring the streamingFlush methods to eliminate code duplication, making TextStreamItem a static inner class with final fields for immutability, and using JsonUtils.merge to simplify the JSON parameter processing.
| public void streamingFlush() { | ||
| synchronized (this) { | ||
| if (state != SpeechSynthesisState.TTS_STARTED) { | ||
| throw new ApiException( | ||
| new InputRequiredException( | ||
| "State invalid: expect stream input tts state is started but " + state.getValue())); | ||
| } | ||
| if (textEmitter == null) { | ||
| log.debug("submitFlush to new emitter"); | ||
| cmdBuffer.add(AsyncCmdBuffer.builder().item(new TextStreamItem(true)).build()); | ||
| } else { | ||
| log.debug("submitFlush to emitter"); | ||
| textEmitter.onNext(new TextStreamItem(true)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
| private class TextStreamItem { | ||
| public TextStreamItem(String text) { | ||
| this.text = text; | ||
| this.flush = false; | ||
| } | ||
|
|
||
| public TextStreamItem(boolean flush) { | ||
| this.text = null; | ||
| this.flush = flush; | ||
| } | ||
|
|
||
| public TextStreamItem(boolean flush, JsonObject params) { | ||
| this.text = null; | ||
| this.flush = flush; | ||
| this.params = params; | ||
| } | ||
|
|
||
| public String text; | ||
| public boolean flush; | ||
| public JsonObject params; | ||
| } |
There was a problem hiding this comment.
TextStreamItem should be declared as a static inner class because it does not require access to the outer SpeechSynthesizer instance. Additionally, making its fields final ensures immutability and thread safety, which is a best practice when objects are passed through reactive streams.
private static class TextStreamItem {
public final String text;
public final boolean flush;
public final JsonObject params;
public TextStreamItem(String text) {
this.text = text;
this.flush = false;
this.params = null;
}
public TextStreamItem(boolean flush) {
this.text = null;
this.flush = flush;
this.params = null;
}
public TextStreamItem(boolean flush, JsonObject params) {
this.text = null;
this.flush = flush;
this.params = params;
}
}| if (item.params != null && !item.params.isJsonNull()) { | ||
| for (String key : item.params.keySet()) { | ||
| Object value = item.params.get(key); | ||
| if (key != null && value != null) { | ||
| jsonObject.add(key, JsonUtils.toJsonElement(value)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The manual iteration and copying of parameters can be simplified by using the existing JsonUtils.merge utility. This is more concise, avoids redundant null checks on keys, and prevents unnecessary re-wrapping of JsonElement values via JsonUtils.toJsonElement.
if (item.params != null) {
JsonUtils.merge(jsonObject, item.params);
}
No description provided.