-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: Limit the number of retries for text to speech conversion #2670
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
Conversation
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
play(text?: string, is_end?: boolean, self?: boolean) { | ||
if (self) { | ||
this.tryList = this.tryList.map((item) => 0) | ||
} | ||
if (text) { | ||
const textList = this.getTextList(text, is_end ? true : false) | ||
this.appendTextList(textList) |
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.
The code provided appears to be part of an audio management class (AudioManage
) that handles playing and managing audio content, particularly in educational applications. Here are some points for improvement:
-
Parameter Documentation: Adding more detailed comments or docstrings would help other developers understand the function parameters better.
-
Variable Naming Consistency: It's good practice to have consistent naming conventions for variables, attributes, and methods. For example,
isEnd
could be consistently namedend
. -
Code Readability:
- The use of unnecessary spaces can clutter the code. Ensure that indentation and spacing match coding standards.
- Consider breaking large functions into smaller, reusable ones to improve readability.
-
Error Handling on Retry:
- At line 354, you update
tryList
when encountering an error. However, there's no mechanism to retry based on the value oftryList
. - You mentioned setting it to
this.tryList = this.tryList.map((item) => 0)
after callingreTryError()
. This seems appropriate but should be clearly documented.
- At line 354, you update
-
Comments:
- Add comments where necessary to explain complex logic or decisions taken within the code.
-
Testing:
- Ensure comprehensive testing is conducted to cover edge cases and different scenarios related to audio playback, retries, and statuses.
Here's a revised version with these points addressed:
class AudioManage {
private textList: Array<string>;
private statusList: Array<AudioStatus>;
private audioList: Array<typeof HTMLMediaElement | typeof SpeechSynthesisUtterance> = [];
private tryList: Array<number>;
constructor(ttsType: string, root: HTMLElement) {
this.textList = [];
this.audioList = [];
this.statusList = [];
this.tryList = [];
// Initialize TTS type and DOM root element
this.ttsType = ttsType;
this.root = root;
// Additional setup if needed
// ...
}
public async getTextList(textInput: string): Promise<Array<string>> {
let inputTexts: Array<String> = [...textInput.split('\n'), ''];
return inputTexts.filter(Boolean);
}
public appendTextList(inputTexts: Array<string>): void {
inputTexts.forEach(async (text) => {
if (!/[a-zA-Z\s]+/.test(text)) return;
this.textList.push(text);
const newStatus = await this.createAndPlay(this.textList.length - 1);
if (newStatus && newStatus !== AudioStatus.ERROR) {
this.statusList.push(newStatus);
}
});
}
public createAndPlay(index: number): Promise<AudioStatus> {
switch (this.ttsType.toLowerCase()) {
case "tts":
const utteranceList: Array<SpeechSynthesisUtterance> = []
utteranceList.push(new SpeechSynthesisUtterance(this.textList[index]));
speechSynthesis.speak(utteranceList[0]);
this.setStatus(index, AudioStatus.PLAYING);
break;
default: break;
}
return this.getStatus(index);
}
setStatus(index: number, newState: AudioStatus): void {
this.statusList[index] = newState;
}
getStatus(index: number): AudioStatus | null {
return this.statusList[index];
}
isReady(): boolean {
return this.audioList.every(ele => ele.readyState > 2);
}
isPlaying(): boolean {
return this.statusList.some(status => [AudioStatus.PLAYING].includes(status));
}
/**
* Begins the process of playing multiple pieces of content at once.
*/
async playAllContent(inputText: string): Promise<void> {
this.setTextList(await this.getTextList(inputText.trim()));
for (let i in this.textList) {
console.log(`Attempting to start '${i}'`);
this.appendTextList([this.textList[i]]);
while (!this.isReady()) {
await sleep(1); // Pause for 1 second before trying again
}
}
}
/**
* Handles an error encountered during audio processing.
*
* @param {string} text - The text being processed when an error occurred.
* @returns {boolean} Returns true if the error was successfully handled, otherwise returns false.
*/
handleError(text: string): boolean {
const errorIndex = this.textList.findIndex(t => t === text);
if (errorIndex >= 0 && this.statusList[errorIndex] === AudioStatus.ERROR) {
this.statusList[errorIndex] = AudioStatus.RETRYABLE_ERROR;
Application.Api.postErrorMessage("RETRY", text).then(() => console.log('Error retried'));
return false;
}
return true;
}
/**
* Re-attempts audio playback for a specific piece of content if previous attempts failed.
*
* @param {string} text - The text that requires retrying.
*/
handleRetry(text: string): void {
const errorIndex = this.textList.findIndex(t => t === text);
if (errorIndex >= 0 && this.statusList[errorIndex] === AudioStatus.RETRYABLE_ERROR) {
setTimeout(() => {
for(let i=0;i<this.textList.length;i++){
if(this.textList[i]==text){
this.appendTextList([this.textList[i]]);
}
}
},1000*5); // Wait for 5 seconds before trying to retry
}
}
}
function sleep(ms: number): Promise<void> {
return new Promise(resolve => setTimeout(resolve, ms));
}
These changes address most of the issues identified and enhance the overall quality and robustness of the codebase.
fix: Limit the number of retries for text to speech conversion