Fix issues and default dev in Mac CoreAudio driver

Fix some issues identified by Claude in the Mac CoreAudio sound driver,
including a race condition and some memory leaks.

Also fix selecting the default device.

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
This commit is contained in:
Rafael Kitover
2025-10-04 02:59:40 +00:00
parent fc34bbbd12
commit 4208f5af6e
2 changed files with 146 additions and 105 deletions

View File

@@ -20,6 +20,7 @@
#include <AudioToolbox/AudioToolbox.h>
#include <stdlib.h>
#include <memory.h>
#include <mutex>
#include <wx/arrstr.h>
#include <wx/log.h>
@@ -109,7 +110,8 @@ public:
int soundBufferLen = 0;
AudioTimeStamp starttime;
AudioTimeStamp timestamp;
AudioQueueTimelineRef timeline;
AudioQueueTimelineRef timeline = NULL;
std::mutex buffer_mutex;
private:
AudioDeviceID GetCoreAudioDevice(wxString name);
@@ -145,6 +147,7 @@ static void PlaybackBufferReadyCallback(void *inUserData, AudioQueueRef inAQ, Au
cadevice->buffers[curbuf]->mAudioDataByteSize = 0;
}
std::lock_guard<std::mutex> lock(cadevice->buffer_mutex);
if (cadevice->filled_buffers > 0) {
cadevice->filled_buffers--;
}
@@ -180,84 +183,72 @@ AudioDeviceID CoreAudioAudio::GetCoreAudioDevice(wxString name)
AudioBufferList *buflist = NULL;
OSStatus result = 0;
CFStringRef cfstr = NULL;
if (name == _("Default device")) {
if (AudioObjectGetPropertyDataSize(kAudioObjectSystemObject, &default_playback_device_address, 0, NULL, &size) != kAudioHardwareNoError) {
return 0;
} else if ((devs = (AudioDeviceID *)malloc(size)) == NULL) {
return 0;
} else if (AudioObjectGetPropertyData(kAudioObjectSystemObject, &default_playback_device_address, 0, NULL, &size, devs) != kAudioHardwareNoError) {
free(devs);
return 0;
}
dev = devs[0];
free(devs);
return dev;
} else {
if (AudioObjectGetPropertyDataSize(kAudioObjectSystemObject, &devlist_address, 0, NULL, &size) != kAudioHardwareNoError) {
return 0;
} else if ((devs = (AudioDeviceID *)malloc(size)) == NULL) {
return 0;
} else if (AudioObjectGetPropertyData(kAudioObjectSystemObject, &devlist_address, 0, NULL, &size, devs) != kAudioHardwareNoError) {
free(devs);
return 0;
}
const UInt32 total_devices = (UInt32) (size / sizeof(AudioDeviceID));
for (UInt32 i = 0; i < total_devices; i++)
{
if (AudioObjectGetPropertyDataSize(devs[i], &addr, 0, NULL, &size) != noErr) {
continue;
} else if ((buflist = (AudioBufferList *)malloc(size)) == NULL) {
continue;
}
result = AudioObjectGetPropertyData(devs[i], &addr, 0, NULL, &size, buflist);
if (result != noErr) {
free(buflist);
continue;
}
if (buflist->mNumberBuffers == 0) {
free(buflist);
continue;
}
size = sizeof(CFStringRef);
if (AudioObjectGetPropertyData(devs[i], &nameaddr, 0, NULL, &size, &cfstr) != kAudioHardwareNoError) {
free(buflist);
continue;
}
CFIndex len = CFStringGetMaximumSizeForEncoding(CFStringGetLength(cfstr), kCFStringEncodingUTF8);
const char *name = (const char *)malloc(len + 1);
CFStringGetCString(cfstr, (char *)name, len + 1, kCFStringEncodingUTF8);
if (name != NULL)
{
const wxString device_name(name, wxConvLibc);
if (device_name == name) {
dev = devs[i];
free(devs);
free(buflist);
free((void *)name);
return dev;
}
}
free(buflist);
free((void *)name);
}
if (AudioObjectGetPropertyDataSize(kAudioObjectSystemObject, &devlist_address, 0, NULL, &size) != kAudioHardwareNoError) {
return 0;
} else if ((devs = (AudioDeviceID *)malloc(size)) == NULL) {
return 0;
} else if (AudioObjectGetPropertyData(kAudioObjectSystemObject, &devlist_address, 0, NULL, &size, devs) != kAudioHardwareNoError) {
free(devs);
return 0;
}
const UInt32 total_devices = (UInt32) (size / sizeof(AudioDeviceID));
for (UInt32 i = 0; i < total_devices; i++)
{
if (AudioObjectGetPropertyDataSize(devs[i], &addr, 0, NULL, &size) != noErr) {
continue;
} else if ((buflist = (AudioBufferList *)malloc(size)) == NULL) {
continue;
}
result = AudioObjectGetPropertyData(devs[i], &addr, 0, NULL, &size, buflist);
if (result != noErr) {
free(buflist);
continue;
}
if (buflist->mNumberBuffers == 0) {
free(buflist);
continue;
}
size = sizeof(CFStringRef);
if (AudioObjectGetPropertyData(devs[i], &nameaddr, 0, NULL, &size, &cfstr) != kAudioHardwareNoError) {
free(buflist);
continue;
}
CFIndex len = CFStringGetMaximumSizeForEncoding(CFStringGetLength(cfstr), kCFStringEncodingUTF8);
const char *device_name_cstr = (const char *)malloc(len + 1);
CFStringGetCString(cfstr, (char *)device_name_cstr, len + 1, kCFStringEncodingUTF8);
if (device_name_cstr != NULL)
{
const wxString device_name(device_name_cstr, wxConvLibc);
if (device_name == name) {
dev = devs[i];
free(devs);
free(buflist);
free((void *)device_name_cstr);
CFRelease(cfstr);
return dev;
}
}
free(buflist);
free((void *)device_name_cstr);
CFRelease(cfstr);
}
free(devs);
return 0;
}
@@ -269,16 +260,37 @@ initialized(false)
void CoreAudioAudio::deinit() {
if (!initialized)
return;
AudioObjectRemovePropertyListener(device, &alive_address, DeviceAliveNotification, this);
for (int i = 0; i < OPTION(kSoundBuffers); i++) {
AudioQueueFreeBuffer(audioQueue, buffers[i]);
if (device != 0) {
AudioObjectRemovePropertyListener(device, &alive_address, DeviceAliveNotification, this);
}
if (buffers != NULL) {
for (int i = 0; i < OPTION(kSoundBuffers); i++) {
if (buffers[i] != NULL) {
AudioQueueFreeBuffer(audioQueue, buffers[i]);
}
}
free(buffers);
buffers = NULL;
}
AudioQueueStop(audioQueue, TRUE);
if (timeline != NULL) {
AudioQueueDisposeTimeline(audioQueue, timeline);
timeline = NULL;
}
if (audioQueue != NULL) {
AudioQueueDispose(audioQueue, TRUE);
audioQueue = NULL;
}
device = 0;
current_buffer = 0;
filled_buffers = 0;
initialized = false;
}
@@ -293,24 +305,20 @@ static bool AssignDeviceToAudioQueue(CoreAudioAudio *cadevice)
kAudioDevicePropertyScopeOutput,
kAudioObjectPropertyElementMain
};
OSStatus result;
CFStringRef devuid;
UInt32 devuidsize = sizeof(devuid);
result = AudioObjectGetPropertyData(cadevice->device, &prop, 0, NULL, &devuidsize, &devuid);
if (result != noErr) {
return false;
}
result = AudioQueueSetProperty(cadevice->audioQueue, kAudioQueueProperty_CurrentDevice, &devuid, devuidsize);
if (result != noErr) {
return false;
}
CFRelease(devuid); // Release devuid; we're done with it and AudioQueueSetProperty should have retained if it wants to keep it.
return (bool)(result == noErr);
}
@@ -362,12 +370,19 @@ bool CoreAudioAudio::init(long sampleRate) {
AudioChannelLayout layout;
memset(&layout, 0, sizeof(layout));
device = GetCoreAudioDevice(OPTION(kSoundAudioDevice));
if (device == 0) {
fprintf(stderr, "Couldn't get Core Audio device\n");
return false;
const wxString device_name = OPTION(kSoundAudioDevice);
const bool use_default_device = (device_name.IsEmpty() || device_name == _("Default device"));
if (!use_default_device) {
device = GetCoreAudioDevice(device_name);
if (device == 0) {
wxLogError(_("Could not get CoreAudio device"));
return false;
}
} else {
device = 0;
}
description.mFormatID = kAudioFormatLinearPCM;
@@ -382,16 +397,20 @@ bool CoreAudioAudio::init(long sampleRate) {
soundBufferLen = (sampleRate / 60) * description.mBytesPerPacket;
PrepareDevice(this);
if (!use_default_device) {
PrepareDevice(this);
AudioObjectAddPropertyListener(device, &alive_address, DeviceAliveNotification, this);
}
AudioObjectAddPropertyListener(device, &alive_address, DeviceAliveNotification, this);
result = AudioQueueNewOutput(&description, PlaybackBufferReadyCallback, this, NULL, NULL, 0, &audioQueue);
if (result != noErr) {
return false;
}
AssignDeviceToAudioQueue(this);
if (!use_default_device) {
AssignDeviceToAudioQueue(this);
}
layout.mChannelLayoutTag = kAudioChannelLayoutTag_Stereo;
result = AudioQueueSetProperty(audioQueue, kAudioQueueProperty_ChannelLayout, &layout, sizeof(layout));
@@ -466,10 +485,17 @@ void CoreAudioAudio::setBuffer(uint16_t* finalWave, int length) {
this_buf = buffers[current_buffer];
if (this_buf == NULL) {
fprintf(stderr, "Current buffer is NULL\n");
return;
}
// Bounds checking: ensure we don't write past buffer capacity
if (this_buf->mAudioDataByteSize + length > this_buf->mAudioDataBytesCapacity) {
length = this_buf->mAudioDataBytesCapacity - this_buf->mAudioDataByteSize;
if (length <= 0) {
return;
}
}
memcpy((uint8_t *)this_buf->mAudioData + this_buf->mAudioDataByteSize, finalWave, length);
this_buf->mAudioDataByteSize += (UInt32)length;
@@ -495,10 +521,11 @@ void CoreAudioAudio::write(uint16_t* finalWave, int length) {
{
setBuffer(finalWave, (avail * (description.mBitsPerChannel / 8)));
finalWave += (avail * (description.mBitsPerChannel / 8));
finalWave += avail;
samples -= avail;
if (buffers[current_buffer]->mAudioDataByteSize >= buffers[current_buffer]->mAudioDataBytesCapacity) {
std::lock_guard<std::mutex> lock(buffer_mutex);
current_buffer++;
filled_buffers++;
}
@@ -507,7 +534,13 @@ void CoreAudioAudio::write(uint16_t* finalWave, int length) {
current_buffer = 0;
}
while (filled_buffers >= OPTION(kSoundBuffers)) {
while (true) {
{
std::lock_guard<std::mutex> lock(buffer_mutex);
if (filled_buffers < OPTION(kSoundBuffers)) {
break;
}
}
wxMilliSleep(((soundGetSampleRate() / 60) * 4) / (soundGetSampleRate() >> 7));
}
}
@@ -515,6 +548,7 @@ void CoreAudioAudio::write(uint16_t* finalWave, int length) {
setBuffer(finalWave, samples * (description.mBitsPerChannel / 8));
if (buffers[current_buffer]->mAudioDataByteSize >= buffers[current_buffer]->mAudioDataBytesCapacity) {
std::lock_guard<std::mutex> lock(buffer_mutex);
current_buffer++;
filled_buffers++;
}
@@ -523,7 +557,13 @@ void CoreAudioAudio::write(uint16_t* finalWave, int length) {
current_buffer = 0;
}
while (filled_buffers >= OPTION(kSoundBuffers)) {
while (true) {
{
std::lock_guard<std::mutex> lock(buffer_mutex);
if (filled_buffers < OPTION(kSoundBuffers)) {
break;
}
}
wxMilliSleep(((soundGetSampleRate() / 60) * 4) / (soundGetSampleRate() >> 7));
}
}
@@ -592,6 +632,7 @@ std::vector<AudioDevice> GetCoreAudioDevices() {
free(buflist);
free((void *)name);
CFRelease(cfstr);
}
return devices;

View File

@@ -6,10 +6,10 @@
namespace audio {
namespace internal {
// Returns the set of OpenAL devices.
// Returns the set of CoreAudio devices.
std::vector<AudioDevice> GetCoreAudioDevices();
// Creates an OpenAL sound driver.
// Creates a CoreAudio sound driver.
std::unique_ptr<SoundDriver> CreateCoreAudioDriver();
} // namespace internal