-
Notifications
You must be signed in to change notification settings - Fork 2k
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
perf: Optimization of recording function on mobile devices #2719
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 |
@@ -572,7 +579,7 @@ const uploadRecording = async (audioBlob: Blob) => { | |||
autoSendMessage() | |||
}) | |||
} else { | |||
isMicrophone.value = false | |||
switchMicrophone(false) | |||
} | |||
}) | |||
.catch((error) => { |
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 provided code seems to be correctly implementing a mobile audio recording feature using El-Cli components. However, here are some recommendations for improvement:
1. Error Handling
Ensure that all error handling is comprehensive. The current implementation doesn't have extensive error handling logic.
2. Callback Function
In the open
method of RecorderManage
, consider adding more detailed logging or notifications when callbacks are used.
3. Cleanup and Rescanning Permissions
If permissions change after initialization, add mechanisms such as rescans during permission denial scenarios to ensure functionality remains consistent.
4. Refactoring
Consider refactoring repetitive code patterns like the microphone toggle button into a reusable component if applicable.
Suggestions
Improved Audio Recording Management:
class RecorderManage {
private recorder?: Recordable
uploadRecording: (blob: Blob, duration: number) =>void;
constructor(uploadRecording: (blob: Blob, duration: number) => void) {
this.uploadRecording = uploadRecording;
}
open(callback?: () => void): Promise<void> {
return new Promise((resolve, reject) => {
const config: RecorderConfig = {
type: 'mp3',
bitRate: 128,
};
Recorder.create(config).then(recorder => {
this.recorder = recorder;
recorder.start();
resolve();
if (callback) {
callback();
}
}).catch(error => {
console.error("Failed to start recording:", error);
reject(error);
});
setTimeout(() => {
recorder.stop().then(blob => {
let startTime = Date.now() - timestamp;
startTime /= 1000; // Convert milliseconds to seconds
uploadRecording(blob, startTime);
this.recorder!.stop(); // Explicitly stop again to release resources
this.recorder = undefined;
resolve();
}).catch(error => {
console.error("Failed to process recorded file:", error);
reject(error);
});
}, recTime * SECOND);
}); // Close Promise
}
close(): void {
this.recorder?.clearDataForTest();
this.recorder?.destroy();
this.recorder = undefined;
}
destroy(): void {
this.close();
}
private errorHandler(errType: string | null, errStack: StackFrame[]): void {
throw new Error(`${errType}: ${JSON.stringify(stack)}\n`);
}
private errorCallBack(e: unknown): void {
if (!(e instanceof DOMException)) {
throw e;
}
console.warn("Error during recording:", { e });
}
}
Enhanced Code Clarity:
- Ensure comments explain key parts of the code.
- Consider breaking down complex logic into smaller functions for better readability.
This revised version addresses some issues with error management and provides additional flexibility in managing the recording lifecycle. Adjustments may need to be made based on specific requirements and application context.
perf: Optimization of recording function on mobile devices