* [PATCH v7 0/6] coreaudio fixes
@ 2025-01-24 5:12 Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts Akihiko Odaki
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-24 5:12 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, Christian Schoenebeck,
BALATON Zoltan
Cc: qemu-devel, devel, Akihiko Odaki, Phil Dennis-Jordan
This series contains two fixes for coreaudio. See each one for details.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v7:
- Added patch "coreaudio: Remove extra whitespaces".
- Link to v6: https://lore.kernel.org/qemu-devel/20250124-coreaudio-v6-0-11fbcb6c47cf@daynix.com
Changes in v6:
- Added patch "coreaudio: Remove unnecessary explicit casts".
- Link to v5: https://lore.kernel.org/qemu-devel/20250123-coreaudio-v5-0-6873df4215a0@daynix.com
Changes in v5:
- Added patch "coreaudio: Improve naming", removing verbose names like
outputDeviceID and audioDevicePropertyBufferFrameSize altogether.
- Link to v4: https://lore.kernel.org/r/20250117-coreaudio-v4-0-f8d4fa4cb5f4@daynix.com
Changes in v4:
- Splitted patch "audio: Add functions to initialize buffers" from
patch "coreaudio: Initialize the buffer for device change".
- Changed the message of patch "coreaudio: Commit the result of init in
the end" to tell that early returns happen when there is a fatal error
or the device gets unplugged.
- Link to v3: https://lore.kernel.org/r/20250115-coreaudio-v3-0-bdb6bcb5bf9f@daynix.com
---
Akihiko Odaki (6):
coreaudio: Remove unnecessary explicit casts
coreaudio: Remove extra whitespaces
coreaudio: Improve naming
coreaudio: Commit the result of init in the end
audio: Add functions to initialize buffers
coreaudio: Initialize the buffer for device change
audio/audio_int.h | 2 +
audio/audio.c | 24 +++--
audio/coreaudio.m | 284 ++++++++++++++++++++++++++++--------------------------
3 files changed, 166 insertions(+), 144 deletions(-)
---
base-commit: 7433709a147706ad7d1956b15669279933d0f82b
change-id: 20250109-coreaudio-c984607e1d8c
Best regards,
--
Akihiko Odaki <akihiko.odaki@daynix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
2025-01-24 5:12 [PATCH v7 0/6] coreaudio fixes Akihiko Odaki
@ 2025-01-24 5:12 ` Akihiko Odaki
2025-01-24 9:39 ` Christian Schoenebeck
2025-01-24 5:12 ` [PATCH v7 2/6] coreaudio: Remove extra whitespaces Akihiko Odaki
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-24 5:12 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, Christian Schoenebeck,
BALATON Zoltan
Cc: qemu-devel, devel, Akihiko Odaki
coreaudio had unnecessary explicit casts and they had extra whitespaces
around them so remove them.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
audio/coreaudio.m | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
UInt32 frameCount, pending_frames;
void *out = outOutputData->mBuffers[0].mData;
HWVoiceOut *hw = hwptr;
- coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
+ coreaudioVoiceOut *core = hwptr;
size_t len;
if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
@@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
}
if (frameRange.mMinimum > core->frameSizeSetting) {
- core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
+ core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
} else if (frameRange.mMaximum < core->frameSizeSetting) {
- core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
+ core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
} else {
core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 2/6] coreaudio: Remove extra whitespaces
2025-01-24 5:12 [PATCH v7 0/6] coreaudio fixes Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts Akihiko Odaki
@ 2025-01-24 5:12 ` Akihiko Odaki
2025-01-24 9:44 ` Christian Schoenebeck
2025-01-24 5:12 ` [PATCH v7 3/6] coreaudio: Improve naming Akihiko Odaki
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-24 5:12 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, Christian Schoenebeck,
BALATON Zoltan
Cc: qemu-devel, devel, Akihiko Odaki
Remove extra whitespaces around parentheses.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
audio/coreaudio.m | 108 +++++++++++++++++++++++++++---------------------------
1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 0b67347ad7e8c43a77af308a1a3a654dd7084083..04e8ac59f4572c1e5fb7dc4f04f5e21520507ab5 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -149,7 +149,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
result);
}
-static void coreaudio_logstatus (OSStatus status)
+static void coreaudio_logstatus(OSStatus status)
{
const char *str = "BUG";
@@ -199,14 +199,14 @@ static void coreaudio_logstatus (OSStatus status)
break;
default:
- AUD_log (AUDIO_CAP, "Reason: status code %" PRId32 "\n", (int32_t)status);
+ AUD_log(AUDIO_CAP, "Reason: status code %" PRId32 "\n", (int32_t)status);
return;
}
- AUD_log (AUDIO_CAP, "Reason: %s\n", str);
+ AUD_log(AUDIO_CAP, "Reason: %s\n", str);
}
-static void G_GNUC_PRINTF (2, 3) coreaudio_logerr (
+static void G_GNUC_PRINTF(2, 3) coreaudio_logerr(
OSStatus status,
const char *fmt,
...
@@ -214,14 +214,14 @@ static void G_GNUC_PRINTF (2, 3) coreaudio_logerr (
{
va_list ap;
- va_start (ap, fmt);
- AUD_log (AUDIO_CAP, fmt, ap);
- va_end (ap);
+ va_start(ap, fmt);
+ AUD_log(AUDIO_CAP, fmt, ap);
+ va_end(ap);
- coreaudio_logstatus (status);
+ coreaudio_logstatus(status);
}
-static void G_GNUC_PRINTF (3, 4) coreaudio_logerr2 (
+static void G_GNUC_PRINTF(3, 4) coreaudio_logerr2(
OSStatus status,
const char *typ,
const char *fmt,
@@ -230,39 +230,39 @@ static void G_GNUC_PRINTF (3, 4) coreaudio_logerr2 (
{
va_list ap;
- AUD_log (AUDIO_CAP, "Could not initialize %s\n", typ);
+ AUD_log(AUDIO_CAP, "Could not initialize %s\n", typ);
- va_start (ap, fmt);
- AUD_vlog (AUDIO_CAP, fmt, ap);
- va_end (ap);
+ va_start(ap, fmt);
+ AUD_vlog(AUDIO_CAP, fmt, ap);
+ va_end(ap);
- coreaudio_logstatus (status);
+ coreaudio_logstatus(status);
}
#define coreaudio_playback_logerr(status, ...) \
coreaudio_logerr2(status, "playback", __VA_ARGS__)
-static int coreaudio_buf_lock (coreaudioVoiceOut *core, const char *fn_name)
+static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
{
int err;
- err = pthread_mutex_lock (&core->buf_mutex);
+ err = pthread_mutex_lock(&core->buf_mutex);
if (err) {
- dolog ("Could not lock voice for %s\nReason: %s\n",
- fn_name, strerror (err));
+ dolog("Could not lock voice for %s\nReason: %s\n",
+ fn_name, strerror(err));
return -1;
}
return 0;
}
-static int coreaudio_buf_unlock (coreaudioVoiceOut *core, const char *fn_name)
+static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
{
int err;
- err = pthread_mutex_unlock (&core->buf_mutex);
+ err = pthread_mutex_unlock(&core->buf_mutex);
if (err) {
- dolog ("Could not unlock voice for %s\nReason: %s\n",
- fn_name, strerror (err));
+ dolog("Could not unlock voice for %s\nReason: %s\n",
+ fn_name, strerror(err));
return -1;
}
return 0;
@@ -271,7 +271,7 @@ static int coreaudio_buf_unlock (coreaudioVoiceOut *core, const char *fn_name)
#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
static ret_type glue(coreaudio_, name)args_decl \
{ \
- coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw; \
+ coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw; \
ret_type ret; \
\
if (coreaudio_buf_lock(core, "coreaudio_" #name)) { \
@@ -312,13 +312,13 @@ static OSStatus audioDeviceIOProc(
coreaudioVoiceOut *core = hwptr;
size_t len;
- if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
+ if (coreaudio_buf_lock(core, "audioDeviceIOProc")) {
inInputTime = 0;
return 0;
}
if (inDevice != core->outputDeviceID) {
- coreaudio_buf_unlock (core, "audioDeviceIOProc(old device)");
+ coreaudio_buf_unlock(core, "audioDeviceIOProc(old device)");
return 0;
}
@@ -328,7 +328,7 @@ static OSStatus audioDeviceIOProc(
/* if there are not enough samples, set signal and return */
if (pending_frames < frameCount) {
inInputTime = 0;
- coreaudio_buf_unlock (core, "audioDeviceIOProc(empty)");
+ coreaudio_buf_unlock(core, "audioDeviceIOProc(empty)");
return 0;
}
@@ -348,7 +348,7 @@ static OSStatus audioDeviceIOProc(
out += write_len;
}
- coreaudio_buf_unlock (core, "audioDeviceIOProc");
+ coreaudio_buf_unlock(core, "audioDeviceIOProc");
return 0;
}
@@ -370,12 +370,12 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
status = coreaudio_get_voice(&core->outputDeviceID);
if (status != kAudioHardwareNoError) {
- coreaudio_playback_logerr (status,
- "Could not get default output Device\n");
+ coreaudio_playback_logerr(status,
+ "Could not get default output Device\n");
return status;
}
if (core->outputDeviceID == kAudioDeviceUnknown) {
- dolog ("Could not initialize playback - Unknown Audiodevice\n");
+ dolog("Could not initialize playback - Unknown Audiodevice\n");
return status;
}
@@ -386,17 +386,17 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
return 0;
}
if (status != kAudioHardwareNoError) {
- coreaudio_playback_logerr (status,
- "Could not get device buffer frame range\n");
+ coreaudio_playback_logerr(status,
+ "Could not get device buffer frame range\n");
return status;
}
if (frameRange.mMinimum > core->frameSizeSetting) {
core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
- dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
+ dolog("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
} else if (frameRange.mMaximum < core->frameSizeSetting) {
core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
- dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
+ dolog("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
} else {
core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
}
@@ -408,9 +408,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
return 0;
}
if (status != kAudioHardwareNoError) {
- coreaudio_playback_logerr (status,
- "Could not set device buffer frame size %" PRIu32 "\n",
- (uint32_t)core->audioDevicePropertyBufferFrameSize);
+ coreaudio_playback_logerr(status,
+ "Could not set device buffer frame size %" PRIu32 "\n",
+ (uint32_t)core->audioDevicePropertyBufferFrameSize);
return status;
}
@@ -421,8 +421,8 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
return 0;
}
if (status != kAudioHardwareNoError) {
- coreaudio_playback_logerr (status,
- "Could not get device buffer frame size\n");
+ coreaudio_playback_logerr(status,
+ "Could not get device buffer frame size\n");
return status;
}
core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
@@ -434,9 +434,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
return 0;
}
if (status != kAudioHardwareNoError) {
- coreaudio_playback_logerr (status,
- "Could not set samplerate %lf\n",
- streamBasicDescription.mSampleRate);
+ coreaudio_playback_logerr(status,
+ "Could not set samplerate %lf\n",
+ streamBasicDescription.mSampleRate);
core->outputDeviceID = kAudioDeviceUnknown;
return status;
}
@@ -460,7 +460,7 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
return 0;
}
if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
- coreaudio_playback_logerr (status, "Could not set IOProc\n");
+ coreaudio_playback_logerr(status, "Could not set IOProc\n");
core->outputDeviceID = kAudioDeviceUnknown;
return status;
}
@@ -518,7 +518,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
if (!isrunning) {
status = AudioDeviceStart(core->outputDeviceID, core->ioprocid);
if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
- coreaudio_logerr (status, "Could not resume playback\n");
+ coreaudio_logerr(status, "Could not resume playback\n");
}
}
} else {
@@ -560,7 +560,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
void *drv_opaque)
{
OSStatus status;
- coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+ coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
int err;
Audiodev *dev = drv_opaque;
AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
@@ -569,14 +569,14 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
/* create mutex */
err = pthread_mutex_init(&core->buf_mutex, NULL);
if (err) {
- dolog("Could not create mutex\nReason: %s\n", strerror (err));
+ dolog("Could not create mutex\nReason: %s\n", strerror(err));
return -1;
}
obt_as = *as;
as = &obt_as;
as->fmt = AUDIO_FORMAT_F32;
- audio_pcm_init_info (&hw->info, as);
+ audio_pcm_init_info(&hw->info, as);
core->frameSizeSetting = audio_buffer_frames(
qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
@@ -587,8 +587,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
&voice_addr, handle_voice_change,
core);
if (status != kAudioHardwareNoError) {
- coreaudio_playback_logerr (status,
- "Could not listen to voice property change\n");
+ coreaudio_playback_logerr(status,
+ "Could not listen to voice property change\n");
return -1;
}
@@ -612,7 +612,7 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
{
OSStatus status;
int err;
- coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+ coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
&voice_addr,
@@ -627,13 +627,13 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
/* destroy mutex */
err = pthread_mutex_destroy(&core->buf_mutex);
if (err) {
- dolog("Could not destroy mutex\nReason: %s\n", strerror (err));
+ dolog("Could not destroy mutex\nReason: %s\n", strerror(err));
}
}
static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
{
- coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+ coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
core->enabled = enable;
update_device_playback_state(core);
@@ -644,7 +644,7 @@ static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
return dev;
}
-static void coreaudio_audio_fini (void *opaque)
+static void coreaudio_audio_fini(void *opaque)
{
}
@@ -670,7 +670,7 @@ static void coreaudio_audio_fini (void *opaque)
.pcm_ops = &coreaudio_pcm_ops,
.max_voices_out = 1,
.max_voices_in = 0,
- .voice_size_out = sizeof (coreaudioVoiceOut),
+ .voice_size_out = sizeof(coreaudioVoiceOut),
.voice_size_in = 0
};
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 3/6] coreaudio: Improve naming
2025-01-24 5:12 [PATCH v7 0/6] coreaudio fixes Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 2/6] coreaudio: Remove extra whitespaces Akihiko Odaki
@ 2025-01-24 5:12 ` Akihiko Odaki
2025-01-24 10:01 ` Christian Schoenebeck
2025-01-24 5:12 ` [PATCH v7 4/6] coreaudio: Commit the result of init in the end Akihiko Odaki
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-24 5:12 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, Christian Schoenebeck,
BALATON Zoltan
Cc: qemu-devel, devel, Akihiko Odaki
coreaudio had names that are not conforming to QEMU codding style.
coreaudioVoiceOut also had some members that are prefixed with redundant
words like "output" or "audio".
Global names included "out" to tell they are specific to output devices,
but this rule was not completely enforced.
The frame size had three different names "frameSize", "bufferFrameSize",
and "frameCount".
Replace identifiers to fix these problems.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
audio/coreaudio.m | 193 +++++++++++++++++++++++++++---------------------------
1 file changed, 98 insertions(+), 95 deletions(-)
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 04e8ac59f4572c1e5fb7dc4f04f5e21520507ab5..6f170a909983b2a5c6abd6fc04c6c3f32828c10c 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -33,37 +33,37 @@
#define AUDIO_CAP "coreaudio"
#include "audio_int.h"
-typedef struct coreaudioVoiceOut {
+typedef struct CoreaudioVoiceOut {
HWVoiceOut hw;
pthread_mutex_t buf_mutex;
- AudioDeviceID outputDeviceID;
- int frameSizeSetting;
- uint32_t bufferCount;
- UInt32 audioDevicePropertyBufferFrameSize;
+ AudioDeviceID device_id;
+ int frame_size_setting;
+ uint32_t buffer_count;
+ UInt32 device_frame_size;
AudioDeviceIOProcID ioprocid;
bool enabled;
-} coreaudioVoiceOut;
+} CoreaudioVoiceOut;
-static const AudioObjectPropertyAddress voice_addr = {
+static const AudioObjectPropertyAddress voice_out_addr = {
kAudioHardwarePropertyDefaultOutputDevice,
kAudioObjectPropertyScopeGlobal,
kAudioObjectPropertyElementMain
};
-static OSStatus coreaudio_get_voice(AudioDeviceID *id)
+static OSStatus coreaudio_get_voice_out(AudioDeviceID *id)
{
UInt32 size = sizeof(*id);
return AudioObjectGetPropertyData(kAudioObjectSystemObject,
- &voice_addr,
+ &voice_out_addr,
0,
NULL,
&size,
id);
}
-static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
- AudioValueRange *framerange)
+static OSStatus coreaudio_get_out_framesizerange(AudioDeviceID id,
+ AudioValueRange *framerange)
{
UInt32 size = sizeof(*framerange);
AudioObjectPropertyAddress addr = {
@@ -80,7 +80,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
framerange);
}
-static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
+static OSStatus coreaudio_get_out_framesize(AudioDeviceID id, UInt32 *framesize)
{
UInt32 size = sizeof(*framesize);
AudioObjectPropertyAddress addr = {
@@ -97,7 +97,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
framesize);
}
-static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
+static OSStatus coreaudio_set_out_framesize(AudioDeviceID id, UInt32 *framesize)
{
UInt32 size = sizeof(*framesize);
AudioObjectPropertyAddress addr = {
@@ -114,8 +114,8 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
framesize);
}
-static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
- AudioStreamBasicDescription *d)
+static OSStatus coreaudio_set_out_streamformat(AudioDeviceID id,
+ AudioStreamBasicDescription *d)
{
UInt32 size = sizeof(*d);
AudioObjectPropertyAddress addr = {
@@ -132,7 +132,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
d);
}
-static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
+static OSStatus coreaudio_get_out_isrunning(AudioDeviceID id, UInt32 *result)
{
UInt32 size = sizeof(*result);
AudioObjectPropertyAddress addr = {
@@ -242,7 +242,8 @@ static void G_GNUC_PRINTF(3, 4) coreaudio_logerr2(
#define coreaudio_playback_logerr(status, ...) \
coreaudio_logerr2(status, "playback", __VA_ARGS__)
-static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
+static int coreaudio_voice_out_buf_lock(CoreaudioVoiceOut *core,
+ const char *fn_name)
{
int err;
@@ -255,7 +256,8 @@ static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
return 0;
}
-static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
+static int coreaudio_voice_out_buf_unlock(CoreaudioVoiceOut *core,
+ const char *fn_name)
{
int err;
@@ -268,20 +270,20 @@ static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
return 0;
}
-#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
- static ret_type glue(coreaudio_, name)args_decl \
- { \
- coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw; \
- ret_type ret; \
- \
- if (coreaudio_buf_lock(core, "coreaudio_" #name)) { \
- return 0; \
- } \
- \
- ret = glue(audio_generic_, name)args; \
- \
- coreaudio_buf_unlock(core, "coreaudio_" #name); \
- return ret; \
+#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
+ static ret_type glue(coreaudio_, name)args_decl \
+ { \
+ CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw; \
+ ret_type ret; \
+ \
+ if (coreaudio_voice_out_buf_lock(core, "coreaudio_" #name)) { \
+ return 0; \
+ } \
+ \
+ ret = glue(audio_generic_, name)args; \
+ \
+ coreaudio_voice_out_buf_unlock(core, "coreaudio_" #name); \
+ return ret; \
}
COREAUDIO_WRAPPER_FUNC(buffer_get_free, size_t, (HWVoiceOut *hw), (hw))
COREAUDIO_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
@@ -297,7 +299,7 @@ static ret_type glue(coreaudio_, name)args_decl \
* callback to feed audiooutput buffer. called without BQL.
* allowed to lock "buf_mutex", but disallowed to have any other locks.
*/
-static OSStatus audioDeviceIOProc(
+static OSStatus out_device_ioproc(
AudioDeviceID inDevice,
const AudioTimeStamp *inNow,
const AudioBufferList *inInputData,
@@ -306,33 +308,33 @@ static OSStatus audioDeviceIOProc(
const AudioTimeStamp *inOutputTime,
void *hwptr)
{
- UInt32 frameCount, pending_frames;
+ UInt32 frame_size, pending_frames;
void *out = outOutputData->mBuffers[0].mData;
HWVoiceOut *hw = hwptr;
- coreaudioVoiceOut *core = hwptr;
+ CoreaudioVoiceOut *core = hwptr;
size_t len;
- if (coreaudio_buf_lock(core, "audioDeviceIOProc")) {
+ if (coreaudio_voice_out_buf_lock(core, "out_device_ioproc")) {
inInputTime = 0;
return 0;
}
- if (inDevice != core->outputDeviceID) {
- coreaudio_buf_unlock(core, "audioDeviceIOProc(old device)");
+ if (inDevice != core->device_id) {
+ coreaudio_voice_out_buf_unlock(core, "out_device_ioproc(old device)");
return 0;
}
- frameCount = core->audioDevicePropertyBufferFrameSize;
+ frame_size = core->device_frame_size;
pending_frames = hw->pending_emul / hw->info.bytes_per_frame;
/* if there are not enough samples, set signal and return */
- if (pending_frames < frameCount) {
+ if (pending_frames < frame_size) {
inInputTime = 0;
- coreaudio_buf_unlock(core, "audioDeviceIOProc(empty)");
+ coreaudio_voice_out_buf_unlock(core, "out_device_ioproc(empty)");
return 0;
}
- len = frameCount * hw->info.bytes_per_frame;
+ len = frame_size * hw->info.bytes_per_frame;
while (len) {
size_t write_len, start;
@@ -348,16 +350,16 @@ static OSStatus audioDeviceIOProc(
out += write_len;
}
- coreaudio_buf_unlock(core, "audioDeviceIOProc");
+ coreaudio_voice_out_buf_unlock(core, "out_device_ioproc");
return 0;
}
-static OSStatus init_out_device(coreaudioVoiceOut *core)
+static OSStatus init_out_device(CoreaudioVoiceOut *core)
{
OSStatus status;
- AudioValueRange frameRange;
+ AudioValueRange framerange;
- AudioStreamBasicDescription streamBasicDescription = {
+ AudioStreamBasicDescription stream_basic_description = {
.mBitsPerChannel = core->hw.info.bits,
.mBytesPerFrame = core->hw.info.bytes_per_frame,
.mBytesPerPacket = core->hw.info.bytes_per_frame,
@@ -368,20 +370,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
.mSampleRate = core->hw.info.freq
};
- status = coreaudio_get_voice(&core->outputDeviceID);
+ status = coreaudio_get_voice_out(&core->device_id);
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr(status,
"Could not get default output Device\n");
return status;
}
- if (core->outputDeviceID == kAudioDeviceUnknown) {
+ if (core->device_id == kAudioDeviceUnknown) {
dolog("Could not initialize playback - Unknown Audiodevice\n");
return status;
}
/* get minimum and maximum buffer frame sizes */
- status = coreaudio_get_framesizerange(core->outputDeviceID,
- &frameRange);
+ status = coreaudio_get_out_framesizerange(core->device_id,
+ &framerange);
if (status == kAudioHardwareBadObjectError) {
return 0;
}
@@ -391,32 +393,32 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
return status;
}
- if (frameRange.mMinimum > core->frameSizeSetting) {
- core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
- dolog("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
- } else if (frameRange.mMaximum < core->frameSizeSetting) {
- core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
- dolog("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
+ if (framerange.mMinimum > core->frame_size_setting) {
+ core->device_frame_size = framerange.mMinimum;
+ dolog("warning: Upsizing Buffer Frames to %f\n", framerange.mMinimum);
+ } else if (framerange.mMaximum < core->frame_size_setting) {
+ core->device_frame_size = framerange.mMaximum;
+ dolog("warning: Downsizing Buffer Frames to %f\n", framerange.mMaximum);
} else {
- core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
+ core->device_frame_size = core->frame_size_setting;
}
/* set Buffer Frame Size */
- status = coreaudio_set_framesize(core->outputDeviceID,
- &core->audioDevicePropertyBufferFrameSize);
+ status = coreaudio_set_out_framesize(core->device_id,
+ &core->device_frame_size);
if (status == kAudioHardwareBadObjectError) {
return 0;
}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr(status,
"Could not set device buffer frame size %" PRIu32 "\n",
- (uint32_t)core->audioDevicePropertyBufferFrameSize);
+ (uint32_t)core->device_frame_size);
return status;
}
/* get Buffer Frame Size */
- status = coreaudio_get_framesize(core->outputDeviceID,
- &core->audioDevicePropertyBufferFrameSize);
+ status = coreaudio_get_out_framesize(core->device_id,
+ &core->device_frame_size);
if (status == kAudioHardwareBadObjectError) {
return 0;
}
@@ -425,19 +427,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
"Could not get device buffer frame size\n");
return status;
}
- core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
+ core->hw.samples = core->buffer_count * core->device_frame_size;
/* set Samplerate */
- status = coreaudio_set_streamformat(core->outputDeviceID,
- &streamBasicDescription);
+ status = coreaudio_set_out_streamformat(core->device_id,
+ &stream_basic_description);
if (status == kAudioHardwareBadObjectError) {
return 0;
}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr(status,
"Could not set samplerate %lf\n",
- streamBasicDescription.mSampleRate);
- core->outputDeviceID = kAudioDeviceUnknown;
+ stream_basic_description.mSampleRate);
+ core->device_id = kAudioDeviceUnknown;
return status;
}
@@ -452,8 +454,8 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
* with the callers of AudioObjectGetPropertyData.
*/
core->ioprocid = NULL;
- status = AudioDeviceCreateIOProcID(core->outputDeviceID,
- audioDeviceIOProc,
+ status = AudioDeviceCreateIOProcID(core->device_id,
+ out_device_ioproc,
&core->hw,
&core->ioprocid);
if (status == kAudioHardwareBadDeviceError) {
@@ -461,20 +463,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
}
if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
coreaudio_playback_logerr(status, "Could not set IOProc\n");
- core->outputDeviceID = kAudioDeviceUnknown;
+ core->device_id = kAudioDeviceUnknown;
return status;
}
return 0;
}
-static void fini_out_device(coreaudioVoiceOut *core)
+static void fini_out_device(CoreaudioVoiceOut *core)
{
OSStatus status;
UInt32 isrunning;
/* stop playback */
- status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
+ status = coreaudio_get_out_isrunning(core->device_id, &isrunning);
if (status != kAudioHardwareBadObjectError) {
if (status != kAudioHardwareNoError) {
coreaudio_logerr(status,
@@ -482,7 +484,7 @@ static void fini_out_device(coreaudioVoiceOut *core)
}
if (isrunning) {
- status = AudioDeviceStop(core->outputDeviceID, core->ioprocid);
+ status = AudioDeviceStop(core->device_id, core->ioprocid);
if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
coreaudio_logerr(status, "Could not stop playback\n");
}
@@ -490,20 +492,20 @@ static void fini_out_device(coreaudioVoiceOut *core)
}
/* remove callback */
- status = AudioDeviceDestroyIOProcID(core->outputDeviceID,
+ status = AudioDeviceDestroyIOProcID(core->device_id,
core->ioprocid);
if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
coreaudio_logerr(status, "Could not remove IOProc\n");
}
- core->outputDeviceID = kAudioDeviceUnknown;
+ core->device_id = kAudioDeviceUnknown;
}
-static void update_device_playback_state(coreaudioVoiceOut *core)
+static void update_out_device_playback_state(CoreaudioVoiceOut *core)
{
OSStatus status;
UInt32 isrunning;
- status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
+ status = coreaudio_get_out_isrunning(core->device_id, &isrunning);
if (status != kAudioHardwareNoError) {
if (status != kAudioHardwareBadObjectError) {
coreaudio_logerr(status,
@@ -516,7 +518,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
if (core->enabled) {
/* start playback */
if (!isrunning) {
- status = AudioDeviceStart(core->outputDeviceID, core->ioprocid);
+ status = AudioDeviceStart(core->device_id, core->ioprocid);
if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
coreaudio_logerr(status, "Could not resume playback\n");
}
@@ -524,7 +526,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
} else {
/* stop playback */
if (isrunning) {
- status = AudioDeviceStop(core->outputDeviceID,
+ status = AudioDeviceStop(core->device_id,
core->ioprocid);
if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
coreaudio_logerr(status, "Could not pause playback\n");
@@ -534,22 +536,22 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
}
/* called without BQL. */
-static OSStatus handle_voice_change(
+static OSStatus handle_voice_out_change(
AudioObjectID in_object_id,
UInt32 in_number_addresses,
const AudioObjectPropertyAddress *in_addresses,
void *in_client_data)
{
- coreaudioVoiceOut *core = in_client_data;
+ CoreaudioVoiceOut *core = in_client_data;
bql_lock();
- if (core->outputDeviceID) {
+ if (core->device_id) {
fini_out_device(core);
}
if (!init_out_device(core)) {
- update_device_playback_state(core);
+ update_out_device_playback_state(core);
}
bql_unlock();
@@ -560,7 +562,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
void *drv_opaque)
{
OSStatus status;
- coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
+ CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
int err;
Audiodev *dev = drv_opaque;
AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
@@ -578,13 +580,14 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
as->fmt = AUDIO_FORMAT_F32;
audio_pcm_init_info(&hw->info, as);
- core->frameSizeSetting = audio_buffer_frames(
+ core->frame_size_setting = audio_buffer_frames(
qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
- core->bufferCount = cpdo->has_buffer_count ? cpdo->buffer_count : 4;
+ core->buffer_count = cpdo->has_buffer_count ? cpdo->buffer_count : 4;
status = AudioObjectAddPropertyListener(kAudioObjectSystemObject,
- &voice_addr, handle_voice_change,
+ &voice_out_addr,
+ handle_voice_out_change,
core);
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr(status,
@@ -594,8 +597,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
if (init_out_device(core)) {
status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
- &voice_addr,
- handle_voice_change,
+ &voice_out_addr,
+ handle_voice_out_change,
core);
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr(status,
@@ -612,11 +615,11 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
{
OSStatus status;
int err;
- coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
+ CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
- &voice_addr,
- handle_voice_change,
+ &voice_out_addr,
+ handle_voice_out_change,
core);
if (status != kAudioHardwareNoError) {
coreaudio_logerr(status, "Could not remove voice property change listener\n");
@@ -633,10 +636,10 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
{
- coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
+ CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
core->enabled = enable;
- update_device_playback_state(core);
+ update_out_device_playback_state(core);
}
static void *coreaudio_audio_init(Audiodev *dev, Error **errp)
@@ -670,7 +673,7 @@ static void coreaudio_audio_fini(void *opaque)
.pcm_ops = &coreaudio_pcm_ops,
.max_voices_out = 1,
.max_voices_in = 0,
- .voice_size_out = sizeof(coreaudioVoiceOut),
+ .voice_size_out = sizeof(CoreaudioVoiceOut),
.voice_size_in = 0
};
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 4/6] coreaudio: Commit the result of init in the end
2025-01-24 5:12 [PATCH v7 0/6] coreaudio fixes Akihiko Odaki
` (2 preceding siblings ...)
2025-01-24 5:12 ` [PATCH v7 3/6] coreaudio: Improve naming Akihiko Odaki
@ 2025-01-24 5:12 ` Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 5/6] audio: Add functions to initialize buffers Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 6/6] coreaudio: Initialize the buffer for device change Akihiko Odaki
5 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-24 5:12 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, Christian Schoenebeck,
BALATON Zoltan
Cc: qemu-devel, devel, Akihiko Odaki
init_out_device may only commit some part of the result and leave the
state inconsistent when it encounters a fatal error or the device gets
unplugged during the operation, which is expressed by
kAudioHardwareBadObjectError or kAudioHardwareBadDeviceError. Commit the
result in the end of the function so that it commits the result iff it
sees no fatal error and the device remains plugged.
With this change, handle_voice_change can rely on core->outputDeviceID
to know whether the output device is initialized after calling
init_out_device.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
audio/coreaudio.m | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 6f170a909983b2a5c6abd6fc04c6c3f32828c10c..43a5f837ba4cfe4464eaab8f1693696638e14113 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -357,7 +357,10 @@ static OSStatus out_device_ioproc(
static OSStatus init_out_device(CoreaudioVoiceOut *core)
{
OSStatus status;
+ AudioDeviceID device_id;
AudioValueRange framerange;
+ UInt32 device_frame_size;
+ AudioDeviceIOProcID ioprocid;
AudioStreamBasicDescription stream_basic_description = {
.mBitsPerChannel = core->hw.info.bits,
@@ -370,20 +373,19 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
.mSampleRate = core->hw.info.freq
};
- status = coreaudio_get_voice_out(&core->device_id);
+ status = coreaudio_get_voice_out(&device_id);
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr(status,
"Could not get default output Device\n");
return status;
}
- if (core->device_id == kAudioDeviceUnknown) {
+ if (device_id == kAudioDeviceUnknown) {
dolog("Could not initialize playback - Unknown Audiodevice\n");
return status;
}
/* get minimum and maximum buffer frame sizes */
- status = coreaudio_get_out_framesizerange(core->device_id,
- &framerange);
+ status = coreaudio_get_out_framesizerange(device_id, &framerange);
if (status == kAudioHardwareBadObjectError) {
return 0;
}
@@ -394,31 +396,29 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
}
if (framerange.mMinimum > core->frame_size_setting) {
- core->device_frame_size = framerange.mMinimum;
+ device_frame_size = framerange.mMinimum;
dolog("warning: Upsizing Buffer Frames to %f\n", framerange.mMinimum);
} else if (framerange.mMaximum < core->frame_size_setting) {
- core->device_frame_size = framerange.mMaximum;
+ device_frame_size = framerange.mMaximum;
dolog("warning: Downsizing Buffer Frames to %f\n", framerange.mMaximum);
} else {
- core->device_frame_size = core->frame_size_setting;
+ device_frame_size = core->frame_size_setting;
}
/* set Buffer Frame Size */
- status = coreaudio_set_out_framesize(core->device_id,
- &core->device_frame_size);
+ status = coreaudio_set_out_framesize(device_id, &device_frame_size);
if (status == kAudioHardwareBadObjectError) {
return 0;
}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr(status,
"Could not set device buffer frame size %" PRIu32 "\n",
- (uint32_t)core->device_frame_size);
+ (uint32_t)device_frame_size);
return status;
}
/* get Buffer Frame Size */
- status = coreaudio_get_out_framesize(core->device_id,
- &core->device_frame_size);
+ status = coreaudio_get_out_framesize(device_id, &device_frame_size);
if (status == kAudioHardwareBadObjectError) {
return 0;
}
@@ -427,10 +427,9 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
"Could not get device buffer frame size\n");
return status;
}
- core->hw.samples = core->buffer_count * core->device_frame_size;
/* set Samplerate */
- status = coreaudio_set_out_streamformat(core->device_id,
+ status = coreaudio_set_out_streamformat(device_id,
&stream_basic_description);
if (status == kAudioHardwareBadObjectError) {
return 0;
@@ -439,7 +438,6 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
coreaudio_playback_logerr(status,
"Could not set samplerate %lf\n",
stream_basic_description.mSampleRate);
- core->device_id = kAudioDeviceUnknown;
return status;
}
@@ -453,20 +451,24 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
* Therefore, the specified callback must be designed to avoid a deadlock
* with the callers of AudioObjectGetPropertyData.
*/
- core->ioprocid = NULL;
- status = AudioDeviceCreateIOProcID(core->device_id,
+ ioprocid = NULL;
+ status = AudioDeviceCreateIOProcID(device_id,
out_device_ioproc,
&core->hw,
- &core->ioprocid);
+ &ioprocid);
if (status == kAudioHardwareBadDeviceError) {
return 0;
}
- if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
+ if (status != kAudioHardwareNoError || ioprocid == NULL) {
coreaudio_playback_logerr(status, "Could not set IOProc\n");
- core->device_id = kAudioDeviceUnknown;
return status;
}
+ core->device_id = device_id;
+ core->device_frame_size = device_frame_size;
+ core->hw.samples = core->buffer_count * core->device_frame_size;
+ core->ioprocid = ioprocid;
+
return 0;
}
@@ -550,7 +552,9 @@ static OSStatus handle_voice_out_change(
fini_out_device(core);
}
- if (!init_out_device(core)) {
+ init_out_device(core);
+
+ if (core->device_id) {
update_out_device_playback_state(core);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 5/6] audio: Add functions to initialize buffers
2025-01-24 5:12 [PATCH v7 0/6] coreaudio fixes Akihiko Odaki
` (3 preceding siblings ...)
2025-01-24 5:12 ` [PATCH v7 4/6] coreaudio: Commit the result of init in the end Akihiko Odaki
@ 2025-01-24 5:12 ` Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 6/6] coreaudio: Initialize the buffer for device change Akihiko Odaki
5 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-24 5:12 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, Christian Schoenebeck,
BALATON Zoltan
Cc: qemu-devel, devel, Akihiko Odaki, Phil Dennis-Jordan
These functions can be used to re-initialize buffers when hardware
parameters change due to device hotplug, for example.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
audio/audio_int.h | 2 ++
audio/audio.c | 24 ++++++++++++++++++------
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 2d079d00a2596e6bce20e1f52abf01a88c4e0012..9ba4a144d571659ad2d32a4d6b2919ad0db89e25 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -187,9 +187,11 @@ struct audio_pcm_ops {
void (*volume_in)(HWVoiceIn *hw, Volume *vol);
};
+void audio_generic_initialize_buffer_in(HWVoiceIn *hw);
void audio_generic_run_buffer_in(HWVoiceIn *hw);
void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size);
+void audio_generic_initialize_buffer_out(HWVoiceOut *hw);
void audio_generic_run_buffer_out(HWVoiceOut *hw);
size_t audio_generic_buffer_get_free(HWVoiceOut *hw);
void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size);
diff --git a/audio/audio.c b/audio/audio.c
index 87b4e9b6f2f356b6e5e972eabc100cf270fcbc29..17c6bbd0ae9e6ff810c0989dbfa1710ef674ff0a 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1407,12 +1407,18 @@ void audio_run(AudioState *s, const char *msg)
#endif
}
+void audio_generic_initialize_buffer_in(HWVoiceIn *hw)
+{
+ g_free(hw->buf_emul);
+ hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+ hw->buf_emul = g_malloc(hw->size_emul);
+ hw->pos_emul = hw->pending_emul = 0;
+}
+
void audio_generic_run_buffer_in(HWVoiceIn *hw)
{
if (unlikely(!hw->buf_emul)) {
- hw->size_emul = hw->samples * hw->info.bytes_per_frame;
- hw->buf_emul = g_malloc(hw->size_emul);
- hw->pos_emul = hw->pending_emul = 0;
+ audio_generic_initialize_buffer_in(hw);
}
while (hw->pending_emul < hw->size_emul) {
@@ -1446,6 +1452,14 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
hw->pending_emul -= size;
}
+void audio_generic_initialize_buffer_out(HWVoiceOut *hw)
+{
+ g_free(hw->buf_emul);
+ hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+ hw->buf_emul = g_malloc(hw->size_emul);
+ hw->pos_emul = hw->pending_emul = 0;
+}
+
size_t audio_generic_buffer_get_free(HWVoiceOut *hw)
{
if (hw->buf_emul) {
@@ -1477,9 +1491,7 @@ void audio_generic_run_buffer_out(HWVoiceOut *hw)
void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size)
{
if (unlikely(!hw->buf_emul)) {
- hw->size_emul = hw->samples * hw->info.bytes_per_frame;
- hw->buf_emul = g_malloc(hw->size_emul);
- hw->pos_emul = hw->pending_emul = 0;
+ audio_generic_initialize_buffer_out(hw);
}
*size = MIN(hw->size_emul - hw->pending_emul,
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v7 6/6] coreaudio: Initialize the buffer for device change
2025-01-24 5:12 [PATCH v7 0/6] coreaudio fixes Akihiko Odaki
` (4 preceding siblings ...)
2025-01-24 5:12 ` [PATCH v7 5/6] audio: Add functions to initialize buffers Akihiko Odaki
@ 2025-01-24 5:12 ` Akihiko Odaki
5 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-24 5:12 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, Christian Schoenebeck,
BALATON Zoltan
Cc: qemu-devel, devel, Akihiko Odaki, Phil Dennis-Jordan
Reallocate buffers when the active device change as the required buffer
size may differ.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Acked-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
audio/coreaudio.m | 1 +
1 file changed, 1 insertion(+)
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 43a5f837ba4cfe4464eaab8f1693696638e14113..6ca5c038c9fc176238a9d845e8705177c0d2341c 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -467,6 +467,7 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
core->device_id = device_id;
core->device_frame_size = device_frame_size;
core->hw.samples = core->buffer_count * core->device_frame_size;
+ audio_generic_initialize_buffer_out(&core->hw);
core->ioprocid = ioprocid;
return 0;
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
2025-01-24 5:12 ` [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts Akihiko Odaki
@ 2025-01-24 9:39 ` Christian Schoenebeck
2025-01-25 5:58 ` Akihiko Odaki
0 siblings, 1 reply; 14+ messages in thread
From: Christian Schoenebeck @ 2025-01-24 9:39 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, BALATON Zoltan,
qemu-devel
Cc: devel, Akihiko Odaki
On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
> coreaudio had unnecessary explicit casts and they had extra whitespaces
> around them so remove them.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> audio/coreaudio.m | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
> UInt32 frameCount, pending_frames;
> void *out = outOutputData->mBuffers[0].mData;
> HWVoiceOut *hw = hwptr;
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
> + coreaudioVoiceOut *core = hwptr;
Well, hwptr is void*, so both versions are fine.
struct name 'coreaudioVoiceOut' should start with upper case BTW.
> size_t len;
>
> if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> }
>
> if (frameRange.mMinimum > core->frameSizeSetting) {
> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
> + core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
> dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
> } else if (frameRange.mMaximum < core->frameSizeSetting) {
> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
> + core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
> dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
> } else {
> core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
Those casts are actually necessary, as AudioValueRange's members are Float64
(a.k.a. double) types.
/Christian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 2/6] coreaudio: Remove extra whitespaces
2025-01-24 5:12 ` [PATCH v7 2/6] coreaudio: Remove extra whitespaces Akihiko Odaki
@ 2025-01-24 9:44 ` Christian Schoenebeck
0 siblings, 0 replies; 14+ messages in thread
From: Christian Schoenebeck @ 2025-01-24 9:44 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, BALATON Zoltan,
qemu-devel
Cc: devel, Akihiko Odaki
On Friday, January 24, 2025 6:12:05 AM CET Akihiko Odaki wrote:
> Remove extra whitespaces around parentheses.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> audio/coreaudio.m | 108 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 54 insertions(+), 54 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index 0b67347ad7e8c43a77af308a1a3a654dd7084083..04e8ac59f4572c1e5fb7dc4f04f5e21520507ab5 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -149,7 +149,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
> result);
> }
>
> -static void coreaudio_logstatus (OSStatus status)
> +static void coreaudio_logstatus(OSStatus status)
> {
> const char *str = "BUG";
>
> @@ -199,14 +199,14 @@ static void coreaudio_logstatus (OSStatus status)
> break;
>
> default:
> - AUD_log (AUDIO_CAP, "Reason: status code %" PRId32 "\n", (int32_t)status);
> + AUD_log(AUDIO_CAP, "Reason: status code %" PRId32 "\n", (int32_t)status);
> return;
> }
>
> - AUD_log (AUDIO_CAP, "Reason: %s\n", str);
> + AUD_log(AUDIO_CAP, "Reason: %s\n", str);
> }
>
> -static void G_GNUC_PRINTF (2, 3) coreaudio_logerr (
> +static void G_GNUC_PRINTF(2, 3) coreaudio_logerr(
> OSStatus status,
> const char *fmt,
> ...
> @@ -214,14 +214,14 @@ static void G_GNUC_PRINTF (2, 3) coreaudio_logerr (
> {
> va_list ap;
>
> - va_start (ap, fmt);
> - AUD_log (AUDIO_CAP, fmt, ap);
> - va_end (ap);
> + va_start(ap, fmt);
> + AUD_log(AUDIO_CAP, fmt, ap);
> + va_end(ap);
>
> - coreaudio_logstatus (status);
> + coreaudio_logstatus(status);
> }
>
> -static void G_GNUC_PRINTF (3, 4) coreaudio_logerr2 (
> +static void G_GNUC_PRINTF(3, 4) coreaudio_logerr2(
> OSStatus status,
> const char *typ,
> const char *fmt,
> @@ -230,39 +230,39 @@ static void G_GNUC_PRINTF (3, 4) coreaudio_logerr2 (
> {
> va_list ap;
>
> - AUD_log (AUDIO_CAP, "Could not initialize %s\n", typ);
> + AUD_log(AUDIO_CAP, "Could not initialize %s\n", typ);
>
> - va_start (ap, fmt);
> - AUD_vlog (AUDIO_CAP, fmt, ap);
> - va_end (ap);
> + va_start(ap, fmt);
> + AUD_vlog(AUDIO_CAP, fmt, ap);
> + va_end(ap);
>
> - coreaudio_logstatus (status);
> + coreaudio_logstatus(status);
> }
>
> #define coreaudio_playback_logerr(status, ...) \
> coreaudio_logerr2(status, "playback", __VA_ARGS__)
>
> -static int coreaudio_buf_lock (coreaudioVoiceOut *core, const char *fn_name)
> +static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
> {
> int err;
>
> - err = pthread_mutex_lock (&core->buf_mutex);
> + err = pthread_mutex_lock(&core->buf_mutex);
> if (err) {
> - dolog ("Could not lock voice for %s\nReason: %s\n",
> - fn_name, strerror (err));
> + dolog("Could not lock voice for %s\nReason: %s\n",
> + fn_name, strerror(err));
> return -1;
> }
> return 0;
> }
>
> -static int coreaudio_buf_unlock (coreaudioVoiceOut *core, const char *fn_name)
> +static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
> {
> int err;
>
> - err = pthread_mutex_unlock (&core->buf_mutex);
> + err = pthread_mutex_unlock(&core->buf_mutex);
> if (err) {
> - dolog ("Could not unlock voice for %s\nReason: %s\n",
> - fn_name, strerror (err));
> + dolog("Could not unlock voice for %s\nReason: %s\n",
> + fn_name, strerror(err));
Nit: this last line should be unindented left by one character. Except of
that:
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
/Christian
> return -1;
> }
> return 0;
> @@ -271,7 +271,7 @@ static int coreaudio_buf_unlock (coreaudioVoiceOut *core, const char *fn_name)
> #define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
> static ret_type glue(coreaudio_, name)args_decl \
> { \
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw; \
> + coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw; \
> ret_type ret; \
> \
> if (coreaudio_buf_lock(core, "coreaudio_" #name)) { \
> @@ -312,13 +312,13 @@ static OSStatus audioDeviceIOProc(
> coreaudioVoiceOut *core = hwptr;
> size_t len;
>
> - if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
> + if (coreaudio_buf_lock(core, "audioDeviceIOProc")) {
> inInputTime = 0;
> return 0;
> }
>
> if (inDevice != core->outputDeviceID) {
> - coreaudio_buf_unlock (core, "audioDeviceIOProc(old device)");
> + coreaudio_buf_unlock(core, "audioDeviceIOProc(old device)");
> return 0;
> }
>
> @@ -328,7 +328,7 @@ static OSStatus audioDeviceIOProc(
> /* if there are not enough samples, set signal and return */
> if (pending_frames < frameCount) {
> inInputTime = 0;
> - coreaudio_buf_unlock (core, "audioDeviceIOProc(empty)");
> + coreaudio_buf_unlock(core, "audioDeviceIOProc(empty)");
> return 0;
> }
>
> @@ -348,7 +348,7 @@ static OSStatus audioDeviceIOProc(
> out += write_len;
> }
>
> - coreaudio_buf_unlock (core, "audioDeviceIOProc");
> + coreaudio_buf_unlock(core, "audioDeviceIOProc");
> return 0;
> }
>
> @@ -370,12 +370,12 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>
> status = coreaudio_get_voice(&core->outputDeviceID);
> if (status != kAudioHardwareNoError) {
> - coreaudio_playback_logerr (status,
> - "Could not get default output Device\n");
> + coreaudio_playback_logerr(status,
> + "Could not get default output Device\n");
> return status;
> }
> if (core->outputDeviceID == kAudioDeviceUnknown) {
> - dolog ("Could not initialize playback - Unknown Audiodevice\n");
> + dolog("Could not initialize playback - Unknown Audiodevice\n");
> return status;
> }
>
> @@ -386,17 +386,17 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> return 0;
> }
> if (status != kAudioHardwareNoError) {
> - coreaudio_playback_logerr (status,
> - "Could not get device buffer frame range\n");
> + coreaudio_playback_logerr(status,
> + "Could not get device buffer frame range\n");
> return status;
> }
>
> if (frameRange.mMinimum > core->frameSizeSetting) {
> core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
> - dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
> + dolog("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
> } else if (frameRange.mMaximum < core->frameSizeSetting) {
> core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
> - dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
> + dolog("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
> } else {
> core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> }
> @@ -408,9 +408,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> return 0;
> }
> if (status != kAudioHardwareNoError) {
> - coreaudio_playback_logerr (status,
> - "Could not set device buffer frame size %" PRIu32 "\n",
> - (uint32_t)core->audioDevicePropertyBufferFrameSize);
> + coreaudio_playback_logerr(status,
> + "Could not set device buffer frame size %" PRIu32 "\n",
> + (uint32_t)core->audioDevicePropertyBufferFrameSize);
> return status;
> }
>
> @@ -421,8 +421,8 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> return 0;
> }
> if (status != kAudioHardwareNoError) {
> - coreaudio_playback_logerr (status,
> - "Could not get device buffer frame size\n");
> + coreaudio_playback_logerr(status,
> + "Could not get device buffer frame size\n");
> return status;
> }
> core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
> @@ -434,9 +434,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> return 0;
> }
> if (status != kAudioHardwareNoError) {
> - coreaudio_playback_logerr (status,
> - "Could not set samplerate %lf\n",
> - streamBasicDescription.mSampleRate);
> + coreaudio_playback_logerr(status,
> + "Could not set samplerate %lf\n",
> + streamBasicDescription.mSampleRate);
> core->outputDeviceID = kAudioDeviceUnknown;
> return status;
> }
> @@ -460,7 +460,7 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> return 0;
> }
> if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
> - coreaudio_playback_logerr (status, "Could not set IOProc\n");
> + coreaudio_playback_logerr(status, "Could not set IOProc\n");
> core->outputDeviceID = kAudioDeviceUnknown;
> return status;
> }
> @@ -518,7 +518,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
> if (!isrunning) {
> status = AudioDeviceStart(core->outputDeviceID, core->ioprocid);
> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
> - coreaudio_logerr (status, "Could not resume playback\n");
> + coreaudio_logerr(status, "Could not resume playback\n");
> }
> }
> } else {
> @@ -560,7 +560,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
> void *drv_opaque)
> {
> OSStatus status;
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
> + coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
> int err;
> Audiodev *dev = drv_opaque;
> AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
> @@ -569,14 +569,14 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
> /* create mutex */
> err = pthread_mutex_init(&core->buf_mutex, NULL);
> if (err) {
> - dolog("Could not create mutex\nReason: %s\n", strerror (err));
> + dolog("Could not create mutex\nReason: %s\n", strerror(err));
> return -1;
> }
>
> obt_as = *as;
> as = &obt_as;
> as->fmt = AUDIO_FORMAT_F32;
> - audio_pcm_init_info (&hw->info, as);
> + audio_pcm_init_info(&hw->info, as);
>
> core->frameSizeSetting = audio_buffer_frames(
> qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
> @@ -587,8 +587,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
> &voice_addr, handle_voice_change,
> core);
> if (status != kAudioHardwareNoError) {
> - coreaudio_playback_logerr (status,
> - "Could not listen to voice property change\n");
> + coreaudio_playback_logerr(status,
> + "Could not listen to voice property change\n");
> return -1;
> }
>
> @@ -612,7 +612,7 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
> {
> OSStatus status;
> int err;
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
> + coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
>
> status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
> &voice_addr,
> @@ -627,13 +627,13 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
> /* destroy mutex */
> err = pthread_mutex_destroy(&core->buf_mutex);
> if (err) {
> - dolog("Could not destroy mutex\nReason: %s\n", strerror (err));
> + dolog("Could not destroy mutex\nReason: %s\n", strerror(err));
> }
> }
>
> static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
> {
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
> + coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
>
> core->enabled = enable;
> update_device_playback_state(core);
> @@ -644,7 +644,7 @@ static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
> return dev;
> }
>
> -static void coreaudio_audio_fini (void *opaque)
> +static void coreaudio_audio_fini(void *opaque)
> {
> }
>
> @@ -670,7 +670,7 @@ static void coreaudio_audio_fini (void *opaque)
> .pcm_ops = &coreaudio_pcm_ops,
> .max_voices_out = 1,
> .max_voices_in = 0,
> - .voice_size_out = sizeof (coreaudioVoiceOut),
> + .voice_size_out = sizeof(coreaudioVoiceOut),
> .voice_size_in = 0
> };
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 3/6] coreaudio: Improve naming
2025-01-24 5:12 ` [PATCH v7 3/6] coreaudio: Improve naming Akihiko Odaki
@ 2025-01-24 10:01 ` Christian Schoenebeck
2025-01-25 6:05 ` Akihiko Odaki
0 siblings, 1 reply; 14+ messages in thread
From: Christian Schoenebeck @ 2025-01-24 10:01 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, BALATON Zoltan,
qemu-devel
Cc: devel, Akihiko Odaki
On Friday, January 24, 2025 6:12:06 AM CET Akihiko Odaki wrote:
> coreaudio had names that are not conforming to QEMU codding style.
> coreaudioVoiceOut also had some members that are prefixed with redundant
> words like "output" or "audio".
> Global names included "out" to tell they are specific to output devices,
> but this rule was not completely enforced.
> The frame size had three different names "frameSize", "bufferFrameSize",
> and "frameCount".
>
> Replace identifiers to fix these problems.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> audio/coreaudio.m | 193 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 98 insertions(+), 95 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index 04e8ac59f4572c1e5fb7dc4f04f5e21520507ab5..6f170a909983b2a5c6abd6fc04c6c3f32828c10c 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -33,37 +33,37 @@
> #define AUDIO_CAP "coreaudio"
> #include "audio_int.h"
>
> -typedef struct coreaudioVoiceOut {
> +typedef struct CoreaudioVoiceOut {
> HWVoiceOut hw;
> pthread_mutex_t buf_mutex;
> - AudioDeviceID outputDeviceID;
> - int frameSizeSetting;
> - uint32_t bufferCount;
> - UInt32 audioDevicePropertyBufferFrameSize;
> + AudioDeviceID device_id;
> + int frame_size_setting;
> + uint32_t buffer_count;
> + UInt32 device_frame_size;
Actually weird that there are two frame size variables here. AFAICS the
device_frame_size member could be dropped and turned into a local variable.
Not really an issue for this patch, just saying.
> AudioDeviceIOProcID ioprocid;
> bool enabled;
> -} coreaudioVoiceOut;
> +} CoreaudioVoiceOut;
Ah, there's the upper case change. :)
> -static const AudioObjectPropertyAddress voice_addr = {
> +static const AudioObjectPropertyAddress voice_out_addr = {
> kAudioHardwarePropertyDefaultOutputDevice,
> kAudioObjectPropertyScopeGlobal,
> kAudioObjectPropertyElementMain
> };
>
> -static OSStatus coreaudio_get_voice(AudioDeviceID *id)
> +static OSStatus coreaudio_get_voice_out(AudioDeviceID *id)
> {
> UInt32 size = sizeof(*id);
>
> return AudioObjectGetPropertyData(kAudioObjectSystemObject,
> - &voice_addr,
> + &voice_out_addr,
> 0,
> NULL,
> &size,
> id);
> }
>
> -static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
> - AudioValueRange *framerange)
> +static OSStatus coreaudio_get_out_framesizerange(AudioDeviceID id,
> + AudioValueRange *framerange)
> {
> UInt32 size = sizeof(*framerange);
> AudioObjectPropertyAddress addr = {
> @@ -80,7 +80,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
> framerange);
> }
>
> -static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
> +static OSStatus coreaudio_get_out_framesize(AudioDeviceID id, UInt32 *framesize)
> {
> UInt32 size = sizeof(*framesize);
> AudioObjectPropertyAddress addr = {
> @@ -97,7 +97,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
> framesize);
> }
>
> -static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
> +static OSStatus coreaudio_set_out_framesize(AudioDeviceID id, UInt32 *framesize)
> {
> UInt32 size = sizeof(*framesize);
> AudioObjectPropertyAddress addr = {
> @@ -114,8 +114,8 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
> framesize);
> }
>
> -static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
> - AudioStreamBasicDescription *d)
> +static OSStatus coreaudio_set_out_streamformat(AudioDeviceID id,
> + AudioStreamBasicDescription *d)
> {
> UInt32 size = sizeof(*d);
> AudioObjectPropertyAddress addr = {
> @@ -132,7 +132,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
> d);
> }
>
> -static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
> +static OSStatus coreaudio_get_out_isrunning(AudioDeviceID id, UInt32 *result)
> {
> UInt32 size = sizeof(*result);
> AudioObjectPropertyAddress addr = {
> @@ -242,7 +242,8 @@ static void G_GNUC_PRINTF(3, 4) coreaudio_logerr2(
> #define coreaudio_playback_logerr(status, ...) \
> coreaudio_logerr2(status, "playback", __VA_ARGS__)
>
> -static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
> +static int coreaudio_voice_out_buf_lock(CoreaudioVoiceOut *core,
> + const char *fn_name)
> {
> int err;
>
> @@ -255,7 +256,8 @@ static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
> return 0;
> }
>
> -static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
> +static int coreaudio_voice_out_buf_unlock(CoreaudioVoiceOut *core,
> + const char *fn_name)
> {
> int err;
>
> @@ -268,20 +270,20 @@ static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
> return 0;
> }
>
> -#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
> - static ret_type glue(coreaudio_, name)args_decl \
> - { \
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw; \
> - ret_type ret; \
> - \
> - if (coreaudio_buf_lock(core, "coreaudio_" #name)) { \
> - return 0; \
> - } \
> - \
> - ret = glue(audio_generic_, name)args; \
> - \
> - coreaudio_buf_unlock(core, "coreaudio_" #name); \
> - return ret; \
> +#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
> + static ret_type glue(coreaudio_, name)args_decl \
> + { \
> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw; \
> + ret_type ret; \
> + \
> + if (coreaudio_voice_out_buf_lock(core, "coreaudio_" #name)) { \
> + return 0; \
> + } \
> + \
> + ret = glue(audio_generic_, name)args; \
> + \
> + coreaudio_voice_out_buf_unlock(core, "coreaudio_" #name); \
> + return ret; \
That doesn't really belong into this patch, but OK.
> }
> COREAUDIO_WRAPPER_FUNC(buffer_get_free, size_t, (HWVoiceOut *hw), (hw))
> COREAUDIO_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
> @@ -297,7 +299,7 @@ static ret_type glue(coreaudio_, name)args_decl \
> * callback to feed audiooutput buffer. called without BQL.
> * allowed to lock "buf_mutex", but disallowed to have any other locks.
> */
> -static OSStatus audioDeviceIOProc(
> +static OSStatus out_device_ioproc(
> AudioDeviceID inDevice,
> const AudioTimeStamp *inNow,
> const AudioBufferList *inInputData,
> @@ -306,33 +308,33 @@ static OSStatus audioDeviceIOProc(
> const AudioTimeStamp *inOutputTime,
> void *hwptr)
> {
> - UInt32 frameCount, pending_frames;
> + UInt32 frame_size, pending_frames;
> void *out = outOutputData->mBuffers[0].mData;
> HWVoiceOut *hw = hwptr;
> - coreaudioVoiceOut *core = hwptr;
> + CoreaudioVoiceOut *core = hwptr;
> size_t len;
>
> - if (coreaudio_buf_lock(core, "audioDeviceIOProc")) {
> + if (coreaudio_voice_out_buf_lock(core, "out_device_ioproc")) {
> inInputTime = 0;
> return 0;
> }
>
> - if (inDevice != core->outputDeviceID) {
> - coreaudio_buf_unlock(core, "audioDeviceIOProc(old device)");
> + if (inDevice != core->device_id) {
> + coreaudio_voice_out_buf_unlock(core, "out_device_ioproc(old device)");
> return 0;
> }
>
> - frameCount = core->audioDevicePropertyBufferFrameSize;
> + frame_size = core->device_frame_size;
> pending_frames = hw->pending_emul / hw->info.bytes_per_frame;
>
> /* if there are not enough samples, set signal and return */
> - if (pending_frames < frameCount) {
> + if (pending_frames < frame_size) {
> inInputTime = 0;
> - coreaudio_buf_unlock(core, "audioDeviceIOProc(empty)");
> + coreaudio_voice_out_buf_unlock(core, "out_device_ioproc(empty)");
> return 0;
> }
>
> - len = frameCount * hw->info.bytes_per_frame;
> + len = frame_size * hw->info.bytes_per_frame;
> while (len) {
> size_t write_len, start;
>
> @@ -348,16 +350,16 @@ static OSStatus audioDeviceIOProc(
> out += write_len;
> }
>
> - coreaudio_buf_unlock(core, "audioDeviceIOProc");
> + coreaudio_voice_out_buf_unlock(core, "out_device_ioproc");
> return 0;
> }
>
> -static OSStatus init_out_device(coreaudioVoiceOut *core)
> +static OSStatus init_out_device(CoreaudioVoiceOut *core)
> {
> OSStatus status;
> - AudioValueRange frameRange;
> + AudioValueRange framerange;
>
> - AudioStreamBasicDescription streamBasicDescription = {
> + AudioStreamBasicDescription stream_basic_description = {
> .mBitsPerChannel = core->hw.info.bits,
> .mBytesPerFrame = core->hw.info.bytes_per_frame,
> .mBytesPerPacket = core->hw.info.bytes_per_frame,
> @@ -368,20 +370,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> .mSampleRate = core->hw.info.freq
> };
>
> - status = coreaudio_get_voice(&core->outputDeviceID);
> + status = coreaudio_get_voice_out(&core->device_id);
As can be seen here, I still think "*out*_device_id" is useful for review
purposes. Except of that:
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
/Christian
> if (status != kAudioHardwareNoError) {
> coreaudio_playback_logerr(status,
> "Could not get default output Device\n");
> return status;
> }
> - if (core->outputDeviceID == kAudioDeviceUnknown) {
> + if (core->device_id == kAudioDeviceUnknown) {
> dolog("Could not initialize playback - Unknown Audiodevice\n");
> return status;
> }
>
> /* get minimum and maximum buffer frame sizes */
> - status = coreaudio_get_framesizerange(core->outputDeviceID,
> - &frameRange);
> + status = coreaudio_get_out_framesizerange(core->device_id,
> + &framerange);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> @@ -391,32 +393,32 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> return status;
> }
>
> - if (frameRange.mMinimum > core->frameSizeSetting) {
> - core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
> - dolog("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
> - } else if (frameRange.mMaximum < core->frameSizeSetting) {
> - core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
> - dolog("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
> + if (framerange.mMinimum > core->frame_size_setting) {
> + core->device_frame_size = framerange.mMinimum;
> + dolog("warning: Upsizing Buffer Frames to %f\n", framerange.mMinimum);
> + } else if (framerange.mMaximum < core->frame_size_setting) {
> + core->device_frame_size = framerange.mMaximum;
> + dolog("warning: Downsizing Buffer Frames to %f\n", framerange.mMaximum);
> } else {
> - core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> + core->device_frame_size = core->frame_size_setting;
> }
>
> /* set Buffer Frame Size */
> - status = coreaudio_set_framesize(core->outputDeviceID,
> - &core->audioDevicePropertyBufferFrameSize);
> + status = coreaudio_set_out_framesize(core->device_id,
> + &core->device_frame_size);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> if (status != kAudioHardwareNoError) {
> coreaudio_playback_logerr(status,
> "Could not set device buffer frame size %" PRIu32 "\n",
> - (uint32_t)core->audioDevicePropertyBufferFrameSize);
> + (uint32_t)core->device_frame_size);
> return status;
> }
>
> /* get Buffer Frame Size */
> - status = coreaudio_get_framesize(core->outputDeviceID,
> - &core->audioDevicePropertyBufferFrameSize);
> + status = coreaudio_get_out_framesize(core->device_id,
> + &core->device_frame_size);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> @@ -425,19 +427,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> "Could not get device buffer frame size\n");
> return status;
> }
> - core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
> + core->hw.samples = core->buffer_count * core->device_frame_size;
>
> /* set Samplerate */
> - status = coreaudio_set_streamformat(core->outputDeviceID,
> - &streamBasicDescription);
> + status = coreaudio_set_out_streamformat(core->device_id,
> + &stream_basic_description);
> if (status == kAudioHardwareBadObjectError) {
> return 0;
> }
> if (status != kAudioHardwareNoError) {
> coreaudio_playback_logerr(status,
> "Could not set samplerate %lf\n",
> - streamBasicDescription.mSampleRate);
> - core->outputDeviceID = kAudioDeviceUnknown;
> + stream_basic_description.mSampleRate);
> + core->device_id = kAudioDeviceUnknown;
> return status;
> }
>
> @@ -452,8 +454,8 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> * with the callers of AudioObjectGetPropertyData.
> */
> core->ioprocid = NULL;
> - status = AudioDeviceCreateIOProcID(core->outputDeviceID,
> - audioDeviceIOProc,
> + status = AudioDeviceCreateIOProcID(core->device_id,
> + out_device_ioproc,
> &core->hw,
> &core->ioprocid);
> if (status == kAudioHardwareBadDeviceError) {
> @@ -461,20 +463,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> }
> if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
> coreaudio_playback_logerr(status, "Could not set IOProc\n");
> - core->outputDeviceID = kAudioDeviceUnknown;
> + core->device_id = kAudioDeviceUnknown;
> return status;
> }
>
> return 0;
> }
>
> -static void fini_out_device(coreaudioVoiceOut *core)
> +static void fini_out_device(CoreaudioVoiceOut *core)
> {
> OSStatus status;
> UInt32 isrunning;
>
> /* stop playback */
> - status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
> + status = coreaudio_get_out_isrunning(core->device_id, &isrunning);
> if (status != kAudioHardwareBadObjectError) {
> if (status != kAudioHardwareNoError) {
> coreaudio_logerr(status,
> @@ -482,7 +484,7 @@ static void fini_out_device(coreaudioVoiceOut *core)
> }
>
> if (isrunning) {
> - status = AudioDeviceStop(core->outputDeviceID, core->ioprocid);
> + status = AudioDeviceStop(core->device_id, core->ioprocid);
> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
> coreaudio_logerr(status, "Could not stop playback\n");
> }
> @@ -490,20 +492,20 @@ static void fini_out_device(coreaudioVoiceOut *core)
> }
>
> /* remove callback */
> - status = AudioDeviceDestroyIOProcID(core->outputDeviceID,
> + status = AudioDeviceDestroyIOProcID(core->device_id,
> core->ioprocid);
> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
> coreaudio_logerr(status, "Could not remove IOProc\n");
> }
> - core->outputDeviceID = kAudioDeviceUnknown;
> + core->device_id = kAudioDeviceUnknown;
> }
>
> -static void update_device_playback_state(coreaudioVoiceOut *core)
> +static void update_out_device_playback_state(CoreaudioVoiceOut *core)
> {
> OSStatus status;
> UInt32 isrunning;
>
> - status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
> + status = coreaudio_get_out_isrunning(core->device_id, &isrunning);
> if (status != kAudioHardwareNoError) {
> if (status != kAudioHardwareBadObjectError) {
> coreaudio_logerr(status,
> @@ -516,7 +518,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
> if (core->enabled) {
> /* start playback */
> if (!isrunning) {
> - status = AudioDeviceStart(core->outputDeviceID, core->ioprocid);
> + status = AudioDeviceStart(core->device_id, core->ioprocid);
> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
> coreaudio_logerr(status, "Could not resume playback\n");
> }
> @@ -524,7 +526,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
> } else {
> /* stop playback */
> if (isrunning) {
> - status = AudioDeviceStop(core->outputDeviceID,
> + status = AudioDeviceStop(core->device_id,
> core->ioprocid);
> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
> coreaudio_logerr(status, "Could not pause playback\n");
> @@ -534,22 +536,22 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
> }
>
> /* called without BQL. */
> -static OSStatus handle_voice_change(
> +static OSStatus handle_voice_out_change(
> AudioObjectID in_object_id,
> UInt32 in_number_addresses,
> const AudioObjectPropertyAddress *in_addresses,
> void *in_client_data)
> {
> - coreaudioVoiceOut *core = in_client_data;
> + CoreaudioVoiceOut *core = in_client_data;
>
> bql_lock();
>
> - if (core->outputDeviceID) {
> + if (core->device_id) {
> fini_out_device(core);
> }
>
> if (!init_out_device(core)) {
> - update_device_playback_state(core);
> + update_out_device_playback_state(core);
> }
>
> bql_unlock();
> @@ -560,7 +562,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
> void *drv_opaque)
> {
> OSStatus status;
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
> int err;
> Audiodev *dev = drv_opaque;
> AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
> @@ -578,13 +580,14 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
> as->fmt = AUDIO_FORMAT_F32;
> audio_pcm_init_info(&hw->info, as);
>
> - core->frameSizeSetting = audio_buffer_frames(
> + core->frame_size_setting = audio_buffer_frames(
> qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
>
> - core->bufferCount = cpdo->has_buffer_count ? cpdo->buffer_count : 4;
> + core->buffer_count = cpdo->has_buffer_count ? cpdo->buffer_count : 4;
>
> status = AudioObjectAddPropertyListener(kAudioObjectSystemObject,
> - &voice_addr, handle_voice_change,
> + &voice_out_addr,
> + handle_voice_out_change,
> core);
> if (status != kAudioHardwareNoError) {
> coreaudio_playback_logerr(status,
> @@ -594,8 +597,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>
> if (init_out_device(core)) {
> status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
> - &voice_addr,
> - handle_voice_change,
> + &voice_out_addr,
> + handle_voice_out_change,
> core);
> if (status != kAudioHardwareNoError) {
> coreaudio_playback_logerr(status,
> @@ -612,11 +615,11 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
> {
> OSStatus status;
> int err;
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>
> status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
> - &voice_addr,
> - handle_voice_change,
> + &voice_out_addr,
> + handle_voice_out_change,
> core);
> if (status != kAudioHardwareNoError) {
> coreaudio_logerr(status, "Could not remove voice property change listener\n");
> @@ -633,10 +636,10 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
>
> static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
> {
> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>
> core->enabled = enable;
> - update_device_playback_state(core);
> + update_out_device_playback_state(core);
> }
>
> static void *coreaudio_audio_init(Audiodev *dev, Error **errp)
> @@ -670,7 +673,7 @@ static void coreaudio_audio_fini(void *opaque)
> .pcm_ops = &coreaudio_pcm_ops,
> .max_voices_out = 1,
> .max_voices_in = 0,
> - .voice_size_out = sizeof(coreaudioVoiceOut),
> + .voice_size_out = sizeof(CoreaudioVoiceOut),
> .voice_size_in = 0
> };
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
2025-01-24 9:39 ` Christian Schoenebeck
@ 2025-01-25 5:58 ` Akihiko Odaki
2025-01-25 10:41 ` Christian Schoenebeck
0 siblings, 1 reply; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-25 5:58 UTC (permalink / raw)
To: Christian Schoenebeck, Gerd Hoffmann, Philippe Mathieu-Daudé,
BALATON Zoltan, qemu-devel
Cc: devel
On 2025/01/24 18:39, Christian Schoenebeck wrote:
> On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
>> coreaudio had unnecessary explicit casts and they had extra whitespaces
>> around them so remove them.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>> audio/coreaudio.m | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
>> --- a/audio/coreaudio.m
>> +++ b/audio/coreaudio.m
>> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
>> UInt32 frameCount, pending_frames;
>> void *out = outOutputData->mBuffers[0].mData;
>> HWVoiceOut *hw = hwptr;
>> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
>> + coreaudioVoiceOut *core = hwptr;
>
> Well, hwptr is void*, so both versions are fine.
>
> struct name 'coreaudioVoiceOut' should start with upper case BTW.
>
>> size_t len;
>>
>> if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
>> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>> }
>>
>> if (frameRange.mMinimum > core->frameSizeSetting) {
>> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
>> + core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>> dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
>> } else if (frameRange.mMaximum < core->frameSizeSetting) {
>> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
>> + core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>> dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
>> } else {
>> core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
>
> Those casts are actually necessary, as AudioValueRange's members are Float64
> (a.k.a. double) types.
Explicit casts are unnecessary. Implicit casts still happen at every
line changed with this patch.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 3/6] coreaudio: Improve naming
2025-01-24 10:01 ` Christian Schoenebeck
@ 2025-01-25 6:05 ` Akihiko Odaki
0 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-25 6:05 UTC (permalink / raw)
To: Christian Schoenebeck, Gerd Hoffmann, Philippe Mathieu-Daudé,
BALATON Zoltan, qemu-devel
Cc: devel
On 2025/01/24 19:01, Christian Schoenebeck wrote:
> On Friday, January 24, 2025 6:12:06 AM CET Akihiko Odaki wrote:
>> coreaudio had names that are not conforming to QEMU codding style.
>> coreaudioVoiceOut also had some members that are prefixed with redundant
>> words like "output" or "audio".
>> Global names included "out" to tell they are specific to output devices,
>> but this rule was not completely enforced.
>> The frame size had three different names "frameSize", "bufferFrameSize",
>> and "frameCount".
>>
>> Replace identifiers to fix these problems.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> audio/coreaudio.m | 193 +++++++++++++++++++++++++++---------------------------
>> 1 file changed, 98 insertions(+), 95 deletions(-)
>>
>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>> index 04e8ac59f4572c1e5fb7dc4f04f5e21520507ab5..6f170a909983b2a5c6abd6fc04c6c3f32828c10c 100644
>> --- a/audio/coreaudio.m
>> +++ b/audio/coreaudio.m
>> @@ -33,37 +33,37 @@
>> #define AUDIO_CAP "coreaudio"
>> #include "audio_int.h"
>>
>> -typedef struct coreaudioVoiceOut {
>> +typedef struct CoreaudioVoiceOut {
>> HWVoiceOut hw;
>> pthread_mutex_t buf_mutex;
>> - AudioDeviceID outputDeviceID;
>> - int frameSizeSetting;
>> - uint32_t bufferCount;
>> - UInt32 audioDevicePropertyBufferFrameSize;
>> + AudioDeviceID device_id;
>> + int frame_size_setting;
>> + uint32_t buffer_count;
>> + UInt32 device_frame_size;
>
> Actually weird that there are two frame size variables here. AFAICS the
> device_frame_size member could be dropped and turned into a local variable.
They can have different values. frame_size_setting is the value the user
configured. device_frame_size is the value that is the value supported
by the device and closest to frame_size_setting.
device_frame_size cannot be turned into a local variable as it is
referred by two functions, out_device_ioproc() and init_out_device().
>
> Not really an issue for this patch, just saying.
>
>> AudioDeviceIOProcID ioprocid;
>> bool enabled;
>> -} coreaudioVoiceOut;
>> +} CoreaudioVoiceOut;
>
> Ah, there's the upper case change. :)
>
>> -static const AudioObjectPropertyAddress voice_addr = {
>> +static const AudioObjectPropertyAddress voice_out_addr = {
>> kAudioHardwarePropertyDefaultOutputDevice,
>> kAudioObjectPropertyScopeGlobal,
>> kAudioObjectPropertyElementMain
>> };
>>
>> -static OSStatus coreaudio_get_voice(AudioDeviceID *id)
>> +static OSStatus coreaudio_get_voice_out(AudioDeviceID *id)
>> {
>> UInt32 size = sizeof(*id);
>>
>> return AudioObjectGetPropertyData(kAudioObjectSystemObject,
>> - &voice_addr,
>> + &voice_out_addr,
>> 0,
>> NULL,
>> &size,
>> id);
>> }
>>
>> -static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
>> - AudioValueRange *framerange)
>> +static OSStatus coreaudio_get_out_framesizerange(AudioDeviceID id,
>> + AudioValueRange *framerange)
>> {
>> UInt32 size = sizeof(*framerange);
>> AudioObjectPropertyAddress addr = {
>> @@ -80,7 +80,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
>> framerange);
>> }
>>
>> -static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
>> +static OSStatus coreaudio_get_out_framesize(AudioDeviceID id, UInt32 *framesize)
>> {
>> UInt32 size = sizeof(*framesize);
>> AudioObjectPropertyAddress addr = {
>> @@ -97,7 +97,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
>> framesize);
>> }
>>
>> -static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
>> +static OSStatus coreaudio_set_out_framesize(AudioDeviceID id, UInt32 *framesize)
>> {
>> UInt32 size = sizeof(*framesize);
>> AudioObjectPropertyAddress addr = {
>> @@ -114,8 +114,8 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
>> framesize);
>> }
>>
>> -static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
>> - AudioStreamBasicDescription *d)
>> +static OSStatus coreaudio_set_out_streamformat(AudioDeviceID id,
>> + AudioStreamBasicDescription *d)
>> {
>> UInt32 size = sizeof(*d);
>> AudioObjectPropertyAddress addr = {
>> @@ -132,7 +132,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
>> d);
>> }
>>
>> -static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
>> +static OSStatus coreaudio_get_out_isrunning(AudioDeviceID id, UInt32 *result)
>> {
>> UInt32 size = sizeof(*result);
>> AudioObjectPropertyAddress addr = {
>> @@ -242,7 +242,8 @@ static void G_GNUC_PRINTF(3, 4) coreaudio_logerr2(
>> #define coreaudio_playback_logerr(status, ...) \
>> coreaudio_logerr2(status, "playback", __VA_ARGS__)
>>
>> -static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
>> +static int coreaudio_voice_out_buf_lock(CoreaudioVoiceOut *core,
>> + const char *fn_name)
>> {
>> int err;
>>
>> @@ -255,7 +256,8 @@ static int coreaudio_buf_lock(coreaudioVoiceOut *core, const char *fn_name)
>> return 0;
>> }
>>
>> -static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
>> +static int coreaudio_voice_out_buf_unlock(CoreaudioVoiceOut *core,
>> + const char *fn_name)
>> {
>> int err;
>>
>> @@ -268,20 +270,20 @@ static int coreaudio_buf_unlock(coreaudioVoiceOut *core, const char *fn_name)
>> return 0;
>> }
>>
>> -#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
>> - static ret_type glue(coreaudio_, name)args_decl \
>> - { \
>> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw; \
>> - ret_type ret; \
>> - \
>> - if (coreaudio_buf_lock(core, "coreaudio_" #name)) { \
>> - return 0; \
>> - } \
>> - \
>> - ret = glue(audio_generic_, name)args; \
>> - \
>> - coreaudio_buf_unlock(core, "coreaudio_" #name); \
>> - return ret; \
>> +#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
>> + static ret_type glue(coreaudio_, name)args_decl \
>> + { \
>> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw; \
>> + ret_type ret; \
>> + \
>> + if (coreaudio_voice_out_buf_lock(core, "coreaudio_" #name)) { \
>> + return 0; \
>> + } \
>> + \
>> + ret = glue(audio_generic_, name)args; \
>> + \
>> + coreaudio_voice_out_buf_unlock(core, "coreaudio_" #name); \
>> + return ret; \
>
> That doesn't really belong into this patch, but OK.
>
>> }
>> COREAUDIO_WRAPPER_FUNC(buffer_get_free, size_t, (HWVoiceOut *hw), (hw))
>> COREAUDIO_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
>> @@ -297,7 +299,7 @@ static ret_type glue(coreaudio_, name)args_decl \
>> * callback to feed audiooutput buffer. called without BQL.
>> * allowed to lock "buf_mutex", but disallowed to have any other locks.
>> */
>> -static OSStatus audioDeviceIOProc(
>> +static OSStatus out_device_ioproc(
>> AudioDeviceID inDevice,
>> const AudioTimeStamp *inNow,
>> const AudioBufferList *inInputData,
>> @@ -306,33 +308,33 @@ static OSStatus audioDeviceIOProc(
>> const AudioTimeStamp *inOutputTime,
>> void *hwptr)
>> {
>> - UInt32 frameCount, pending_frames;
>> + UInt32 frame_size, pending_frames;
>> void *out = outOutputData->mBuffers[0].mData;
>> HWVoiceOut *hw = hwptr;
>> - coreaudioVoiceOut *core = hwptr;
>> + CoreaudioVoiceOut *core = hwptr;
>> size_t len;
>>
>> - if (coreaudio_buf_lock(core, "audioDeviceIOProc")) {
>> + if (coreaudio_voice_out_buf_lock(core, "out_device_ioproc")) {
>> inInputTime = 0;
>> return 0;
>> }
>>
>> - if (inDevice != core->outputDeviceID) {
>> - coreaudio_buf_unlock(core, "audioDeviceIOProc(old device)");
>> + if (inDevice != core->device_id) {
>> + coreaudio_voice_out_buf_unlock(core, "out_device_ioproc(old device)");
>> return 0;
>> }
>>
>> - frameCount = core->audioDevicePropertyBufferFrameSize;
>> + frame_size = core->device_frame_size;
>> pending_frames = hw->pending_emul / hw->info.bytes_per_frame;
>>
>> /* if there are not enough samples, set signal and return */
>> - if (pending_frames < frameCount) {
>> + if (pending_frames < frame_size) {
>> inInputTime = 0;
>> - coreaudio_buf_unlock(core, "audioDeviceIOProc(empty)");
>> + coreaudio_voice_out_buf_unlock(core, "out_device_ioproc(empty)");
>> return 0;
>> }
>>
>> - len = frameCount * hw->info.bytes_per_frame;
>> + len = frame_size * hw->info.bytes_per_frame;
>> while (len) {
>> size_t write_len, start;
>>
>> @@ -348,16 +350,16 @@ static OSStatus audioDeviceIOProc(
>> out += write_len;
>> }
>>
>> - coreaudio_buf_unlock(core, "audioDeviceIOProc");
>> + coreaudio_voice_out_buf_unlock(core, "out_device_ioproc");
>> return 0;
>> }
>>
>> -static OSStatus init_out_device(coreaudioVoiceOut *core)
>> +static OSStatus init_out_device(CoreaudioVoiceOut *core)
>> {
>> OSStatus status;
>> - AudioValueRange frameRange;
>> + AudioValueRange framerange;
>>
>> - AudioStreamBasicDescription streamBasicDescription = {
>> + AudioStreamBasicDescription stream_basic_description = {
>> .mBitsPerChannel = core->hw.info.bits,
>> .mBytesPerFrame = core->hw.info.bytes_per_frame,
>> .mBytesPerPacket = core->hw.info.bytes_per_frame,
>> @@ -368,20 +370,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>> .mSampleRate = core->hw.info.freq
>> };
>>
>> - status = coreaudio_get_voice(&core->outputDeviceID);
>> + status = coreaudio_get_voice_out(&core->device_id);
>
> As can be seen here, I still think "*out*_device_id" is useful for review
> purposes. Except of that:
>
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
If we want to clarify it is for the output, "core" should be renamed to
"core_out" or something like that so that we can see all members that
belongs to that struct is for the output.
But I also like that the current name is concise. It is like the "this"
in C++; so I think keeping such a frequently-used local variable short
makes sense.
Regards,
Akihiko Odaki
>
> /Christian
>
>> if (status != kAudioHardwareNoError) {
>> coreaudio_playback_logerr(status,
>> "Could not get default output Device\n");
>> return status;
>> }
>> - if (core->outputDeviceID == kAudioDeviceUnknown) {
>> + if (core->device_id == kAudioDeviceUnknown) {
>> dolog("Could not initialize playback - Unknown Audiodevice\n");
>> return status;
>> }
>>
>> /* get minimum and maximum buffer frame sizes */
>> - status = coreaudio_get_framesizerange(core->outputDeviceID,
>> - &frameRange);
>> + status = coreaudio_get_out_framesizerange(core->device_id,
>> + &framerange);
>> if (status == kAudioHardwareBadObjectError) {
>> return 0;
>> }
>> @@ -391,32 +393,32 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>> return status;
>> }
>>
>> - if (frameRange.mMinimum > core->frameSizeSetting) {
>> - core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>> - dolog("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
>> - } else if (frameRange.mMaximum < core->frameSizeSetting) {
>> - core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>> - dolog("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
>> + if (framerange.mMinimum > core->frame_size_setting) {
>> + core->device_frame_size = framerange.mMinimum;
>> + dolog("warning: Upsizing Buffer Frames to %f\n", framerange.mMinimum);
>> + } else if (framerange.mMaximum < core->frame_size_setting) {
>> + core->device_frame_size = framerange.mMaximum;
>> + dolog("warning: Downsizing Buffer Frames to %f\n", framerange.mMaximum);
>> } else {
>> - core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
>> + core->device_frame_size = core->frame_size_setting;
>> }
>>
>> /* set Buffer Frame Size */
>> - status = coreaudio_set_framesize(core->outputDeviceID,
>> - &core->audioDevicePropertyBufferFrameSize);
>> + status = coreaudio_set_out_framesize(core->device_id,
>> + &core->device_frame_size);
>> if (status == kAudioHardwareBadObjectError) {
>> return 0;
>> }
>> if (status != kAudioHardwareNoError) {
>> coreaudio_playback_logerr(status,
>> "Could not set device buffer frame size %" PRIu32 "\n",
>> - (uint32_t)core->audioDevicePropertyBufferFrameSize);
>> + (uint32_t)core->device_frame_size);
>> return status;
>> }
>>
>> /* get Buffer Frame Size */
>> - status = coreaudio_get_framesize(core->outputDeviceID,
>> - &core->audioDevicePropertyBufferFrameSize);
>> + status = coreaudio_get_out_framesize(core->device_id,
>> + &core->device_frame_size);
>> if (status == kAudioHardwareBadObjectError) {
>> return 0;
>> }
>> @@ -425,19 +427,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>> "Could not get device buffer frame size\n");
>> return status;
>> }
>> - core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
>> + core->hw.samples = core->buffer_count * core->device_frame_size;
>>
>> /* set Samplerate */
>> - status = coreaudio_set_streamformat(core->outputDeviceID,
>> - &streamBasicDescription);
>> + status = coreaudio_set_out_streamformat(core->device_id,
>> + &stream_basic_description);
>> if (status == kAudioHardwareBadObjectError) {
>> return 0;
>> }
>> if (status != kAudioHardwareNoError) {
>> coreaudio_playback_logerr(status,
>> "Could not set samplerate %lf\n",
>> - streamBasicDescription.mSampleRate);
>> - core->outputDeviceID = kAudioDeviceUnknown;
>> + stream_basic_description.mSampleRate);
>> + core->device_id = kAudioDeviceUnknown;
>> return status;
>> }
>>
>> @@ -452,8 +454,8 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>> * with the callers of AudioObjectGetPropertyData.
>> */
>> core->ioprocid = NULL;
>> - status = AudioDeviceCreateIOProcID(core->outputDeviceID,
>> - audioDeviceIOProc,
>> + status = AudioDeviceCreateIOProcID(core->device_id,
>> + out_device_ioproc,
>> &core->hw,
>> &core->ioprocid);
>> if (status == kAudioHardwareBadDeviceError) {
>> @@ -461,20 +463,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>> }
>> if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
>> coreaudio_playback_logerr(status, "Could not set IOProc\n");
>> - core->outputDeviceID = kAudioDeviceUnknown;
>> + core->device_id = kAudioDeviceUnknown;
>> return status;
>> }
>>
>> return 0;
>> }
>>
>> -static void fini_out_device(coreaudioVoiceOut *core)
>> +static void fini_out_device(CoreaudioVoiceOut *core)
>> {
>> OSStatus status;
>> UInt32 isrunning;
>>
>> /* stop playback */
>> - status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
>> + status = coreaudio_get_out_isrunning(core->device_id, &isrunning);
>> if (status != kAudioHardwareBadObjectError) {
>> if (status != kAudioHardwareNoError) {
>> coreaudio_logerr(status,
>> @@ -482,7 +484,7 @@ static void fini_out_device(coreaudioVoiceOut *core)
>> }
>>
>> if (isrunning) {
>> - status = AudioDeviceStop(core->outputDeviceID, core->ioprocid);
>> + status = AudioDeviceStop(core->device_id, core->ioprocid);
>> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
>> coreaudio_logerr(status, "Could not stop playback\n");
>> }
>> @@ -490,20 +492,20 @@ static void fini_out_device(coreaudioVoiceOut *core)
>> }
>>
>> /* remove callback */
>> - status = AudioDeviceDestroyIOProcID(core->outputDeviceID,
>> + status = AudioDeviceDestroyIOProcID(core->device_id,
>> core->ioprocid);
>> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
>> coreaudio_logerr(status, "Could not remove IOProc\n");
>> }
>> - core->outputDeviceID = kAudioDeviceUnknown;
>> + core->device_id = kAudioDeviceUnknown;
>> }
>>
>> -static void update_device_playback_state(coreaudioVoiceOut *core)
>> +static void update_out_device_playback_state(CoreaudioVoiceOut *core)
>> {
>> OSStatus status;
>> UInt32 isrunning;
>>
>> - status = coreaudio_get_isrunning(core->outputDeviceID, &isrunning);
>> + status = coreaudio_get_out_isrunning(core->device_id, &isrunning);
>> if (status != kAudioHardwareNoError) {
>> if (status != kAudioHardwareBadObjectError) {
>> coreaudio_logerr(status,
>> @@ -516,7 +518,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
>> if (core->enabled) {
>> /* start playback */
>> if (!isrunning) {
>> - status = AudioDeviceStart(core->outputDeviceID, core->ioprocid);
>> + status = AudioDeviceStart(core->device_id, core->ioprocid);
>> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
>> coreaudio_logerr(status, "Could not resume playback\n");
>> }
>> @@ -524,7 +526,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
>> } else {
>> /* stop playback */
>> if (isrunning) {
>> - status = AudioDeviceStop(core->outputDeviceID,
>> + status = AudioDeviceStop(core->device_id,
>> core->ioprocid);
>> if (status != kAudioHardwareBadDeviceError && status != kAudioHardwareNoError) {
>> coreaudio_logerr(status, "Could not pause playback\n");
>> @@ -534,22 +536,22 @@ static void update_device_playback_state(coreaudioVoiceOut *core)
>> }
>>
>> /* called without BQL. */
>> -static OSStatus handle_voice_change(
>> +static OSStatus handle_voice_out_change(
>> AudioObjectID in_object_id,
>> UInt32 in_number_addresses,
>> const AudioObjectPropertyAddress *in_addresses,
>> void *in_client_data)
>> {
>> - coreaudioVoiceOut *core = in_client_data;
>> + CoreaudioVoiceOut *core = in_client_data;
>>
>> bql_lock();
>>
>> - if (core->outputDeviceID) {
>> + if (core->device_id) {
>> fini_out_device(core);
>> }
>>
>> if (!init_out_device(core)) {
>> - update_device_playback_state(core);
>> + update_out_device_playback_state(core);
>> }
>>
>> bql_unlock();
>> @@ -560,7 +562,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>> void *drv_opaque)
>> {
>> OSStatus status;
>> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
>> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>> int err;
>> Audiodev *dev = drv_opaque;
>> AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
>> @@ -578,13 +580,14 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>> as->fmt = AUDIO_FORMAT_F32;
>> audio_pcm_init_info(&hw->info, as);
>>
>> - core->frameSizeSetting = audio_buffer_frames(
>> + core->frame_size_setting = audio_buffer_frames(
>> qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
>>
>> - core->bufferCount = cpdo->has_buffer_count ? cpdo->buffer_count : 4;
>> + core->buffer_count = cpdo->has_buffer_count ? cpdo->buffer_count : 4;
>>
>> status = AudioObjectAddPropertyListener(kAudioObjectSystemObject,
>> - &voice_addr, handle_voice_change,
>> + &voice_out_addr,
>> + handle_voice_out_change,
>> core);
>> if (status != kAudioHardwareNoError) {
>> coreaudio_playback_logerr(status,
>> @@ -594,8 +597,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>>
>> if (init_out_device(core)) {
>> status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
>> - &voice_addr,
>> - handle_voice_change,
>> + &voice_out_addr,
>> + handle_voice_out_change,
>> core);
>> if (status != kAudioHardwareNoError) {
>> coreaudio_playback_logerr(status,
>> @@ -612,11 +615,11 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
>> {
>> OSStatus status;
>> int err;
>> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
>> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>>
>> status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject,
>> - &voice_addr,
>> - handle_voice_change,
>> + &voice_out_addr,
>> + handle_voice_out_change,
>> core);
>> if (status != kAudioHardwareNoError) {
>> coreaudio_logerr(status, "Could not remove voice property change listener\n");
>> @@ -633,10 +636,10 @@ static void coreaudio_fini_out (HWVoiceOut *hw)
>>
>> static void coreaudio_enable_out(HWVoiceOut *hw, bool enable)
>> {
>> - coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
>> + CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>>
>> core->enabled = enable;
>> - update_device_playback_state(core);
>> + update_out_device_playback_state(core);
>> }
>>
>> static void *coreaudio_audio_init(Audiodev *dev, Error **errp)
>> @@ -670,7 +673,7 @@ static void coreaudio_audio_fini(void *opaque)
>> .pcm_ops = &coreaudio_pcm_ops,
>> .max_voices_out = 1,
>> .max_voices_in = 0,
>> - .voice_size_out = sizeof(coreaudioVoiceOut),
>> + .voice_size_out = sizeof(CoreaudioVoiceOut),
>> .voice_size_in = 0
>> };
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
2025-01-25 5:58 ` Akihiko Odaki
@ 2025-01-25 10:41 ` Christian Schoenebeck
2025-01-26 4:00 ` Akihiko Odaki
0 siblings, 1 reply; 14+ messages in thread
From: Christian Schoenebeck @ 2025-01-25 10:41 UTC (permalink / raw)
To: Gerd Hoffmann, Philippe Mathieu-Daudé, BALATON Zoltan,
qemu-devel
Cc: devel, Akihiko Odaki
On Saturday, January 25, 2025 6:58:30 AM CET Akihiko Odaki wrote:
> On 2025/01/24 18:39, Christian Schoenebeck wrote:
> > On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
> >> coreaudio had unnecessary explicit casts and they had extra whitespaces
> >> around them so remove them.
> >>
> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> >> ---
> >> audio/coreaudio.m | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> >> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
> >> --- a/audio/coreaudio.m
> >> +++ b/audio/coreaudio.m
> >> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
> >> UInt32 frameCount, pending_frames;
> >> void *out = outOutputData->mBuffers[0].mData;
> >> HWVoiceOut *hw = hwptr;
> >> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
> >> + coreaudioVoiceOut *core = hwptr;
> >
> > Well, hwptr is void*, so both versions are fine.
> >
> > struct name 'coreaudioVoiceOut' should start with upper case BTW.
> >
> >> size_t len;
> >>
> >> if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
> >> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> >> }
> >>
> >> if (frameRange.mMinimum > core->frameSizeSetting) {
> >> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
> >> + core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
> >> dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
> >> } else if (frameRange.mMaximum < core->frameSizeSetting) {
> >> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
> >> + core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
> >> dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
> >> } else {
> >> core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> >
> > Those casts are actually necessary, as AudioValueRange's members are Float64
> > (a.k.a. double) types.
>
> Explicit casts are unnecessary. Implicit casts still happen at every
> line changed with this patch.
Wooo, I wasn't aware that QEMU doesn't use -Wconversion. I am not used to
that. To me it makes sense to warn especially for things like implicit casts
from floating point to integer, as it would be the case here.
/Christian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
2025-01-25 10:41 ` Christian Schoenebeck
@ 2025-01-26 4:00 ` Akihiko Odaki
0 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2025-01-26 4:00 UTC (permalink / raw)
To: Christian Schoenebeck, Gerd Hoffmann, Philippe Mathieu-Daudé,
BALATON Zoltan, qemu-devel
Cc: devel
On 2025/01/25 19:41, Christian Schoenebeck wrote:
> On Saturday, January 25, 2025 6:58:30 AM CET Akihiko Odaki wrote:
>> On 2025/01/24 18:39, Christian Schoenebeck wrote:
>>> On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
>>>> coreaudio had unnecessary explicit casts and they had extra whitespaces
>>>> around them so remove them.
>>>>
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>> audio/coreaudio.m | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>>>> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
>>>> --- a/audio/coreaudio.m
>>>> +++ b/audio/coreaudio.m
>>>> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
>>>> UInt32 frameCount, pending_frames;
>>>> void *out = outOutputData->mBuffers[0].mData;
>>>> HWVoiceOut *hw = hwptr;
>>>> - coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
>>>> + coreaudioVoiceOut *core = hwptr;
>>>
>>> Well, hwptr is void*, so both versions are fine.
>>>
>>> struct name 'coreaudioVoiceOut' should start with upper case BTW.
>>>
>>>> size_t len;
>>>>
>>>> if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
>>>> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>>>> }
>>>>
>>>> if (frameRange.mMinimum > core->frameSizeSetting) {
>>>> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
>>>> + core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>>>> dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
>>>> } else if (frameRange.mMaximum < core->frameSizeSetting) {
>>>> - core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
>>>> + core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>>>> dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
>>>> } else {
>>>> core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
>>>
>>> Those casts are actually necessary, as AudioValueRange's members are Float64
>>> (a.k.a. double) types.
>>
>> Explicit casts are unnecessary. Implicit casts still happen at every
>> line changed with this patch.
>
> Wooo, I wasn't aware that QEMU doesn't use -Wconversion. I am not used to
> that. To me it makes sense to warn especially for things like implicit casts
> from floating point to integer, as it would be the case here.
I tried '--extra-cflags=-Wconversion -Wno-sign-conversion -Wno-error'.
It may be actually spotting real bugs, there are just too many warnings.
For this particular case, the implicit casts will never change the
values because the actual values are integers.
AudioHardware.h says kAudioDevicePropertyBufferFrameSizeRange is "an
AudioValueRange indicating the minimum and maximum values, inclusive,
for kAudioDevicePropertyBufferFrameSize."
kAudioDevicePropertyBufferFrameSize is UInt32 so the values should
always be in the range of UInt32. The number of frames cannot be a
fractional value after all. They have Float64 values probably because
Apple were so lazy that they reused the AudioValueRange type that has
Float64 members.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-01-26 4:01 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-24 5:12 [PATCH v7 0/6] coreaudio fixes Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts Akihiko Odaki
2025-01-24 9:39 ` Christian Schoenebeck
2025-01-25 5:58 ` Akihiko Odaki
2025-01-25 10:41 ` Christian Schoenebeck
2025-01-26 4:00 ` Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 2/6] coreaudio: Remove extra whitespaces Akihiko Odaki
2025-01-24 9:44 ` Christian Schoenebeck
2025-01-24 5:12 ` [PATCH v7 3/6] coreaudio: Improve naming Akihiko Odaki
2025-01-24 10:01 ` Christian Schoenebeck
2025-01-25 6:05 ` Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 4/6] coreaudio: Commit the result of init in the end Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 5/6] audio: Add functions to initialize buffers Akihiko Odaki
2025-01-24 5:12 ` [PATCH v7 6/6] coreaudio: Initialize the buffer for device change Akihiko Odaki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.