-
Notifications
You must be signed in to change notification settings - Fork 2
Merge 'develop' for release 2.2.0 #26
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
base: main
Are you sure you want to change the base?
Conversation
…al mod_audio_stream repo (#24) Signed-off-by: Dario Pellegrino <dario.pellegrino@voismart.it>
refactor align forward-stream logic with improvements from the original mod_audio_stream
Signed-off-by: Dario Pellegrino <dario.pellegrino@voismart.it>
Signed-off-by: Dario Pellegrino <dario.pellegrino@voismart.it>
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "uuid_openai_audio_stream <uuid> start <wss-url> <mono | mixed | stereo> \n" \ | ||
| " [8k | 16k | 24k | <other rate>] [mute_user]\n" \ | ||
| " where <rate> = 8k|16k|24k or any multiple of 8000 (default: 8k)\n" \ |
Copilot
AI
Dec 16, 2025
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.
There's an inconsistency in formatting - line 173 has a trailing space after "stereo>" and before the newline, and the indentation uses different spacing. Consider aligning all continuation lines consistently for better readability.
| "uuid_openai_audio_stream <uuid> start <wss-url> <mono | mixed | stereo> \n" \ | |
| " [8k | 16k | 24k | <other rate>] [mute_user]\n" \ | |
| " where <rate> = 8k|16k|24k or any multiple of 8000 (default: 8k)\n" \ | |
| "uuid_openai_audio_stream <uuid> start <wss-url> <mono | mixed | stereo>\n" \ | |
| " [8k | 16k | 24k | <other rate>] [mute_user]\n" \ | |
| " where <rate> = 8k|16k|24k or any multiple of 8000 (default: 8k)\n" \ |
| std::vector<uint8_t> flush_buffer; | ||
| std::vector<spx_int16_t> resample_buffer; |
Copilot
AI
Dec 16, 2025
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 vectors flush_buffer and resample_buffer are declared outside the loop but are resized on each iteration (lines 946, 1006). While the comment suggests pre-allocation to avoid repeated allocations, the vectors will still reallocate if they need to grow. Consider using reserve() with a maximum expected size to pre-allocate capacity, or maintain the vectors at their maximum size and only use a portion of them as needed.
| std::vector<uint8_t> flush_buffer; | |
| std::vector<spx_int16_t> resample_buffer; | |
| std::vector<uint8_t> flush_buffer; | |
| flush_buffer.reserve(SWITCH_RECOMMENDED_BUFFER_SIZE); // Pre-allocate to avoid reallocations | |
| std::vector<spx_int16_t> resample_buffer; | |
| resample_buffer.reserve(SWITCH_RECOMMENDED_BUFFER_SIZE / 2); // Adjust as appropriate for 16-bit samples |
Summary