Refactor audio handling with range checks, buffer threshold, and lock… (#1200)
Some checks are pending
Build and Release / reuse (push) Waiting to run
Build and Release / clang-format (push) Waiting to run
Build and Release / get-info (push) Waiting to run
Build and Release / windows-sdl (push) Blocked by required conditions
Build and Release / windows-qt (push) Blocked by required conditions
Build and Release / macos-sdl (push) Blocked by required conditions
Build and Release / macos-qt (push) Blocked by required conditions
Build and Release / linux-sdl (push) Blocked by required conditions
Build and Release / linux-qt (push) Blocked by required conditions
Build and Release / pre-release (push) Blocked by required conditions

* Refactor audio handling with range checks, buffer threshold, and lock fixes

- Added range checks for handle to avoid invalid index access in AudioOutOutput, AudioOutSetVolume, and AudioOutGetStatus.
- Added a constant AUDIO_STREAM_BUFFER_THRESHOLD for the buffer threshold (was previously a magic number).
- Set the freq parameter correctly in the SDL_AudioSpec structure in AudioOutOpen.
- Fixed locking issues in AudioOutOutput to avoid unlocking before it's locked.

* Refactor audio handling with range checks, buffer threshold, and lock fixes

- Added range checks for handle to avoid invalid index access in AudioOutOutput, AudioOutSetVolume, and AudioOutGetStatus.

- Added a constant AUDIO_STREAM_BUFFER_THRESHOLD for the buffer threshold (was previously a magic number).

- Set the freq parameter correctly in the SDL_AudioSpec structure in AudioOutOpen.

- Fixed locking issues in AudioOutOutput to avoid unlocking before it's locked.

- Removed tab spaces to fix format clang error
This commit is contained in:
Mikasa-san 2024-10-02 19:34:16 +04:00 committed by GitHub
parent 93317911eb
commit 7e533ccf50
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -14,6 +14,8 @@
namespace Audio { namespace Audio {
constexpr int AUDIO_STREAM_BUFFER_THRESHOLD = 65536; // Define constant for buffer threshold
int SDLAudio::AudioOutOpen(int type, u32 samples_num, u32 freq, int SDLAudio::AudioOutOpen(int type, u32 samples_num, u32 freq,
Libraries::AudioOut::OrbisAudioOutParamFormat format) { Libraries::AudioOut::OrbisAudioOutParamFormat format) {
using Libraries::AudioOut::OrbisAudioOutParamFormat; using Libraries::AudioOut::OrbisAudioOutParamFormat;
@ -80,7 +82,7 @@ int SDLAudio::AudioOutOpen(int type, u32 samples_num, u32 freq,
SDL_zero(fmt); SDL_zero(fmt);
fmt.format = sampleFormat; fmt.format = sampleFormat;
fmt.channels = port.channels_num; fmt.channels = port.channels_num;
fmt.freq = 48000; fmt.freq = freq; // Set frequency from the argument
port.stream = port.stream =
SDL_OpenAudioDeviceStream(SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK, &fmt, NULL, NULL); SDL_OpenAudioDeviceStream(SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK, &fmt, NULL, NULL);
SDL_ResumeAudioDevice(SDL_GetAudioStreamDevice(port.stream)); SDL_ResumeAudioDevice(SDL_GetAudioStreamDevice(port.stream));
@ -92,20 +94,27 @@ int SDLAudio::AudioOutOpen(int type, u32 samples_num, u32 freq,
} }
s32 SDLAudio::AudioOutOutput(s32 handle, const void* ptr) { s32 SDLAudio::AudioOutOutput(s32 handle, const void* ptr) {
if (handle < 1 || handle > portsOut.size()) { // Add handle range check
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
}
if (ptr == nullptr) {
return 0; // Nothing to output
}
std::shared_lock lock{m_mutex}; std::shared_lock lock{m_mutex};
auto& port = portsOut[handle - 1]; auto& port = portsOut[handle - 1];
if (!port.isOpen) { if (!port.isOpen) {
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
} }
if (ptr == nullptr) {
return 0; const size_t data_size = port.samples_num * port.sample_size * port.channels_num;
}
lock.unlock(); SDL_bool result = SDL_PutAudioStreamData(port.stream, ptr, data_size);
// TODO mixing channels
SDL_bool result = SDL_PutAudioStreamData( lock.unlock(); // Unlock only after necessary operations
port.stream, ptr, port.samples_num * port.sample_size * port.channels_num);
// TODO find a correct value 8192 is estimated while (SDL_GetAudioStreamAvailable(port.stream) > AUDIO_STREAM_BUFFER_THRESHOLD) {
while (SDL_GetAudioStreamAvailable(port.stream) > 65536) {
SDL_Delay(0); SDL_Delay(0);
} }
@ -113,12 +122,17 @@ s32 SDLAudio::AudioOutOutput(s32 handle, const void* ptr) {
} }
bool SDLAudio::AudioOutSetVolume(s32 handle, s32 bitflag, s32* volume) { bool SDLAudio::AudioOutSetVolume(s32 handle, s32 bitflag, s32* volume) {
if (handle < 1 || handle > portsOut.size()) { // Add handle range check
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
}
using Libraries::AudioOut::OrbisAudioOutParamFormat; using Libraries::AudioOut::OrbisAudioOutParamFormat;
std::shared_lock lock{m_mutex}; std::shared_lock lock{m_mutex};
auto& port = portsOut[handle - 1]; auto& port = portsOut[handle - 1];
if (!port.isOpen) { if (!port.isOpen) {
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
} }
for (int i = 0; i < port.channels_num; i++, bitflag >>= 1u) { for (int i = 0; i < port.channels_num; i++, bitflag >>= 1u) {
auto bit = bitflag & 0x1u; auto bit = bitflag & 0x1u;
@ -152,6 +166,10 @@ bool SDLAudio::AudioOutSetVolume(s32 handle, s32 bitflag, s32* volume) {
} }
bool SDLAudio::AudioOutGetStatus(s32 handle, int* type, int* channels_num) { bool SDLAudio::AudioOutGetStatus(s32 handle, int* type, int* channels_num) {
if (handle < 1 || handle > portsOut.size()) { // Add handle range check
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
}
std::shared_lock lock{m_mutex}; std::shared_lock lock{m_mutex};
auto& port = portsOut[handle - 1]; auto& port = portsOut[handle - 1];
*type = port.type; *type = port.type;