[PATCH v8 0/6] coreaudio fixes

[PATCH v8 0/6] coreaudio fixes

[Akihiko Odaki]

This series contains two fixes for coreaudio. See each one for details.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Changes in v8:
- Rebased.
- Link to v7: https://lore.kernel.org/qemu-devel/20250124-coreaudio-v7-0-9d9a4d91db37@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-mixeng-be.c |  24 ++++--
 audio/coreaudio.m       | 206 +++++++++++++++++++++++++-----------------------
 3 files changed, 127 insertions(+), 105 deletions(-)
---
base-commit: ffcf1a7981793973ffbd8100a7c3c6042d02ae23
change-id: 20250109-coreaudio-c984607e1d8c

Best regards,
--  
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

[PATCH v8 1/6] coreaudio: Remove unnecessary explicit casts

[Akihiko Odaki]

coreaudio had unnecessary explicit casts and they had extra whitespaces
around them so remove them.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 audio/coreaudio.m | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index a28fa77d73ff..cbda4f143698 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -310,7 +310,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")) {
@@ -394,11 +394,11 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
     }
 
     if (frameRange.mMinimum > core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
+        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
         warn_report("coreaudio: Upsizing buffer frames to %f",
                     frameRange.mMinimum);
     } else if (frameRange.mMaximum < core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
+        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
         warn_report("coreaudio: Downsizing buffer frames to %f",
                     frameRange.mMaximum);
     } else {

-- 
2.53.0

Re: [PATCH v8 1/6] coreaudio: Remove unnecessary explicit casts

[Marc-André Lureau]

On Wed, Mar 4, 2026 at 7:17 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> coreaudio had unnecessary explicit casts and they had extra whitespaces
> around them so remove them.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  audio/coreaudio.m | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index a28fa77d73ff..cbda4f143698 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -310,7 +310,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")) {
> @@ -394,11 +394,11 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>      }
>
>      if (frameRange.mMinimum > core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>          warn_report("coreaudio: Upsizing buffer frames to %f",
>                      frameRange.mMinimum);
>      } else if (frameRange.mMaximum < core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>          warn_report("coreaudio: Downsizing buffer frames to %f",
>                      frameRange.mMaximum);
>      } else {
>
> --
> 2.53.0
>

[PATCH v8 2/6] coreaudio: Remove extra whitespaces

[Akihiko Odaki]

Remove extra whitespaces around parentheses.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 audio/coreaudio.m | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index cbda4f143698..bc9ab7477b68 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -272,7 +272,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)) {         \
@@ -313,13 +313,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;
     }
 
@@ -329,7 +329,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;
     }
 
@@ -349,7 +349,7 @@ static OSStatus audioDeviceIOProc(
         out += write_len;
     }
 
-    coreaudio_buf_unlock (core, "audioDeviceIOProc");
+    coreaudio_buf_unlock(core, "audioDeviceIOProc");
     return 0;
 }
 
@@ -563,7 +563,7 @@ static OSStatus handle_voice_change(
 static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
 {
     OSStatus status;
-    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
+    coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
     int err;
     Audiodev *dev = hw->s->dev;
     AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
@@ -579,7 +579,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
     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);
@@ -615,7 +615,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,
@@ -636,7 +636,7 @@ 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);

-- 
2.53.0

Re: [PATCH v8 2/6] coreaudio: Remove extra whitespaces

[Marc-André Lureau]

On Wed, Mar 4, 2026 at 7:17 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> Remove extra whitespaces around parentheses.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  audio/coreaudio.m | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index cbda4f143698..bc9ab7477b68 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -272,7 +272,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)) {         \
> @@ -313,13 +313,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;
>      }
>
> @@ -329,7 +329,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;
>      }
>
> @@ -349,7 +349,7 @@ static OSStatus audioDeviceIOProc(
>          out += write_len;
>      }
>
> -    coreaudio_buf_unlock (core, "audioDeviceIOProc");
> +    coreaudio_buf_unlock(core, "audioDeviceIOProc");
>      return 0;
>  }
>
> @@ -563,7 +563,7 @@ static OSStatus handle_voice_change(
>  static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
>  {
>      OSStatus status;
> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
> +    coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
>      int err;
>      Audiodev *dev = hw->s->dev;
>      AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
> @@ -579,7 +579,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
>      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);
> @@ -615,7 +615,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,
> @@ -636,7 +636,7 @@ 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);
>
> --
> 2.53.0
>

[PATCH v8 3/6] coreaudio: Improve naming

[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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 audio/coreaudio.m | 188 +++++++++++++++++++++++++++---------------------------
 1 file changed, 95 insertions(+), 93 deletions(-)

diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index bc9ab7477b68..736227eb2b7a 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -43,34 +43,34 @@
 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 = {
@@ -87,7 +87,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 = {
@@ -104,7 +104,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 = {
@@ -121,8 +121,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 = {
@@ -139,7 +139,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 = {
@@ -243,7 +243,8 @@ static void G_GNUC_PRINTF(3, 4) coreaudio_logerr2(OSStatus 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_voice_out_buf_lock(CoreaudioVoiceOut *core,
+                                        const char *fn_name)
 {
     int err;
 
@@ -256,7 +257,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;
 
@@ -269,20 +271,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),
@@ -298,7 +300,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,
@@ -307,33 +309,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;
 
@@ -349,16 +351,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)
 {
+    AudioValueRange value_range;
     OSStatus status;
-    AudioValueRange frameRange;
 
-    AudioStreamBasicDescription streamBasicDescription = {
+    AudioStreamBasicDescription stream_basic_description = {
         .mBitsPerChannel = audio_format_bits(core->hw.info.af),
         .mBytesPerFrame = core->hw.info.bytes_per_frame,
         .mBytesPerPacket = core->hw.info.bytes_per_frame,
@@ -369,21 +371,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");
         return status;
     }
-    if (core->outputDeviceID == kAudioDeviceUnknown) {
+    if (core->device_id == kAudioDeviceUnknown) {
         error_report("coreaudio: Could not initialize playback: "
                      "Unknown audio device");
         return status;
     }
 
     /* get minimum and maximum buffer frame sizes */
-    status = coreaudio_get_framesizerange(core->outputDeviceID,
-                                          &frameRange);
+    status = coreaudio_get_out_framesizerange(core->device_id, &value_range);
     if (status == kAudioHardwareBadObjectError) {
         return 0;
     }
@@ -393,34 +394,34 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
         return status;
     }
 
-    if (frameRange.mMinimum > core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
+    if (value_range.mMinimum > core->frame_size_setting) {
+        core->device_frame_size = value_range.mMinimum;
         warn_report("coreaudio: Upsizing buffer frames to %f",
-                    frameRange.mMinimum);
-    } else if (frameRange.mMaximum < core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
+                    value_range.mMinimum);
+    } else if (value_range.mMaximum < core->frame_size_setting) {
+        core->device_frame_size = value_range.mMaximum;
         warn_report("coreaudio: Downsizing buffer frames to %f",
-                    frameRange.mMaximum);
+                    value_range.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,
-                                  (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;
     }
@@ -429,19 +430,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
                                   "Could not get device buffer frame size");
         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",
-                                  streamBasicDescription.mSampleRate);
-        core->outputDeviceID = kAudioDeviceUnknown;
+                                  stream_basic_description.mSampleRate);
+        core->device_id = kAudioDeviceUnknown;
         return status;
     }
 
@@ -456,8 +457,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) {
@@ -465,20 +466,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
     }
     if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
         coreaudio_playback_logerr(status, "Could not set IOProc");
-        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,
@@ -486,7 +487,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");
             }
@@ -494,20 +495,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");
     }
-    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,
@@ -520,7 +521,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");
             }
@@ -528,7 +529,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");
@@ -538,22 +539,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();
@@ -563,7 +564,7 @@ static OSStatus handle_voice_change(
 static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
 {
     OSStatus status;
-    coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
+    CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
     int err;
     Audiodev *dev = hw->s->dev;
     AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
@@ -581,13 +582,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,
@@ -597,8 +599,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,
@@ -615,11 +617,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");
@@ -636,10 +638,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 audio_coreaudio_class_init(ObjectClass *klass, const void *data)

-- 
2.53.0

Re: [PATCH v8 3/6] coreaudio: Improve naming

[Marc-André Lureau]

On Wed, Mar 4, 2026 at 7:17 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  audio/coreaudio.m | 188 +++++++++++++++++++++++++++---------------------------
>  1 file changed, 95 insertions(+), 93 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index bc9ab7477b68..736227eb2b7a 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -43,34 +43,34 @@
>  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 = {
> @@ -87,7 +87,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 = {
> @@ -104,7 +104,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 = {
> @@ -121,8 +121,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 = {
> @@ -139,7 +139,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 = {
> @@ -243,7 +243,8 @@ static void G_GNUC_PRINTF(3, 4) coreaudio_logerr2(OSStatus 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_voice_out_buf_lock(CoreaudioVoiceOut *core,
> +                                        const char *fn_name)
>  {
>      int err;
>
> @@ -256,7 +257,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;
>
> @@ -269,20 +271,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),
> @@ -298,7 +300,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,
> @@ -307,33 +309,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;
>
> @@ -349,16 +351,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)
>  {
> +    AudioValueRange value_range;
>      OSStatus status;
> -    AudioValueRange frameRange;
>
> -    AudioStreamBasicDescription streamBasicDescription = {
> +    AudioStreamBasicDescription stream_basic_description = {
>          .mBitsPerChannel = audio_format_bits(core->hw.info.af),
>          .mBytesPerFrame = core->hw.info.bytes_per_frame,
>          .mBytesPerPacket = core->hw.info.bytes_per_frame,
> @@ -369,21 +371,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");
>          return status;
>      }
> -    if (core->outputDeviceID == kAudioDeviceUnknown) {
> +    if (core->device_id == kAudioDeviceUnknown) {
>          error_report("coreaudio: Could not initialize playback: "
>                       "Unknown audio device");
>          return status;
>      }
>
>      /* get minimum and maximum buffer frame sizes */
> -    status = coreaudio_get_framesizerange(core->outputDeviceID,
> -                                          &frameRange);
> +    status = coreaudio_get_out_framesizerange(core->device_id, &value_range);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
> @@ -393,34 +394,34 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>          return status;
>      }
>
> -    if (frameRange.mMinimum > core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
> +    if (value_range.mMinimum > core->frame_size_setting) {
> +        core->device_frame_size = value_range.mMinimum;
>          warn_report("coreaudio: Upsizing buffer frames to %f",
> -                    frameRange.mMinimum);
> -    } else if (frameRange.mMaximum < core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
> +                    value_range.mMinimum);
> +    } else if (value_range.mMaximum < core->frame_size_setting) {
> +        core->device_frame_size = value_range.mMaximum;
>          warn_report("coreaudio: Downsizing buffer frames to %f",
> -                    frameRange.mMaximum);
> +                    value_range.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,
> -                                  (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;
>      }
> @@ -429,19 +430,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>                                    "Could not get device buffer frame size");
>          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",
> -                                  streamBasicDescription.mSampleRate);
> -        core->outputDeviceID = kAudioDeviceUnknown;
> +                                  stream_basic_description.mSampleRate);
> +        core->device_id = kAudioDeviceUnknown;
>          return status;
>      }
>
> @@ -456,8 +457,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) {
> @@ -465,20 +466,20 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>      }
>      if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
>          coreaudio_playback_logerr(status, "Could not set IOProc");
> -        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,
> @@ -486,7 +487,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");
>              }
> @@ -494,20 +495,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");
>      }
> -    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,
> @@ -520,7 +521,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");
>              }
> @@ -528,7 +529,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");
> @@ -538,22 +539,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();
> @@ -563,7 +564,7 @@ static OSStatus handle_voice_change(
>  static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
>  {
>      OSStatus status;
> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
> +    CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>      int err;
>      Audiodev *dev = hw->s->dev;
>      AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
> @@ -581,13 +582,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,
> @@ -597,8 +599,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,
> @@ -615,11 +617,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");
> @@ -636,10 +638,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 audio_coreaudio_class_init(ObjectClass *klass, const void *data)
>
> --
> 2.53.0
>

Re: [PATCH v8 3/6] coreaudio: Improve naming

[Christian Schoenebeck]

On Wednesday, 4 March 2026 07:16:56 CET Akihiko Odaki wrote:
> coreaudio had names that are not conforming to QEMU codding style.

coding

> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  audio/coreaudio.m | 188
> +++++++++++++++++++++++++++--------------------------- 1 file changed, 95
> insertions(+), 93 deletions(-)
> 
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index bc9ab7477b68..736227eb2b7a 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -43,34 +43,34 @@
>  typedef struct coreaudioVoiceOut {

Leaving this lower case?

>      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 = {
> @@ -87,7 +87,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID
> id, framerange);
>  }

So here it is "framerange" and not "frame_range".

> -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 = {
> @@ -104,7 +104,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID
> id, UInt32 *framesize) framesize);
>  }

Here it is "framesize" and not "frame_size".

> -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 = {
> @@ -121,8 +121,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 = {
> @@ -139,7 +139,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 = {
> @@ -243,7 +243,8 @@ static void G_GNUC_PRINTF(3, 4)
> coreaudio_logerr2(OSStatus 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_voice_out_buf_lock(CoreaudioVoiceOut *core,
> +                                        const char *fn_name)
>  {
>      int err;
> 
> @@ -256,7 +257,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;
> 
> @@ -269,20 +271,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), @@ -298,7 +300,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,
> @@ -307,33 +309,33 @@ static OSStatus audioDeviceIOProc(
>      const AudioTimeStamp *inOutputTime,
>      void *hwptr)
>  {
> -    UInt32 frameCount, pending_frames;
> +    UInt32 frame_size, pending_frames;

And here it is "frame_size".

That's why I try to avoid looking at such kind of refactoring patches. Never
ending white noise added to the universe.

>      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;
> 
> @@ -349,16 +351,16 @@ static OSStatus audioDeviceIOProc(
>          out += write_len;
>      }
> 
> -    coreaudio_buf_unlock(core, "audioDeviceIOProc");
> +    coreaudio_voice_out_buf_unlock(core, "out_device_ioproc");

Behaviour change. Not pure refactoring. Should be split into a separate patch.

>      return 0;
>  }
> 
> -static OSStatus init_out_device(coreaudioVoiceOut *core)
> +static OSStatus init_out_device(CoreaudioVoiceOut *core)
>  {
> +    AudioValueRange value_range;
>      OSStatus status;
> -    AudioValueRange frameRange;
> 
> -    AudioStreamBasicDescription streamBasicDescription = {
> +    AudioStreamBasicDescription stream_basic_description = {
>          .mBitsPerChannel = audio_format_bits(core->hw.info.af),
>          .mBytesPerFrame = core->hw.info.bytes_per_frame,
>          .mBytesPerPacket = core->hw.info.bytes_per_frame,
> @@ -369,21 +371,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");
>          return status;
>      }
> -    if (core->outputDeviceID == kAudioDeviceUnknown) {
> +    if (core->device_id == kAudioDeviceUnknown) {
>          error_report("coreaudio: Could not initialize playback: "
>                       "Unknown audio device");
>          return status;
>      }
> 
>      /* get minimum and maximum buffer frame sizes */
> -    status = coreaudio_get_framesizerange(core->outputDeviceID,
> -                                          &frameRange);
> +    status = coreaudio_get_out_framesizerange(core->device_id,
> &value_range); if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
> @@ -393,34 +394,34 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core) return status;
>      }
> 
> -    if (frameRange.mMinimum > core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
> +    if (value_range.mMinimum > core->frame_size_setting) {
> +        core->device_frame_size = value_range.mMinimum;
>          warn_report("coreaudio: Upsizing buffer frames to %f",
> -                    frameRange.mMinimum);
> -    } else if (frameRange.mMaximum < core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
> +                    value_range.mMinimum);
> +    } else if (value_range.mMaximum < core->frame_size_setting) {
> +        core->device_frame_size = value_range.mMaximum;
>          warn_report("coreaudio: Downsizing buffer frames to %f",
> -                    frameRange.mMaximum);
> +                    value_range.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, -                                 
> (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;
>      }
> @@ -429,19 +430,19 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core) "Could not get device buffer frame size"); 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",
> -                                  streamBasicDescription.mSampleRate);
> -        core->outputDeviceID = kAudioDeviceUnknown;
> +                                  stream_basic_description.mSampleRate);
> +        core->device_id = kAudioDeviceUnknown;
>          return status;
>      }
> 
> @@ -456,8 +457,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) {
> @@ -465,20 +466,20 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core) }
>      if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
>          coreaudio_playback_logerr(status, "Could not set IOProc");
> -        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,
> @@ -486,7 +487,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"); }
> @@ -494,20 +495,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");
>      }
> -    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,
> @@ -520,7 +521,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"); }
> @@ -528,7 +529,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"); @@ -538,22 +539,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();
> @@ -563,7 +564,7 @@ static OSStatus handle_voice_change(
>  static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
>  {
>      OSStatus status;
> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
> +    CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>      int err;
>      Audiodev *dev = hw->s->dev;
>      AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
> @@ -581,13 +582,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,
> @@ -597,8 +599,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,
> @@ -615,11 +617,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"); @@ -636,10 +638,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 audio_coreaudio_class_init(ObjectClass *klass, const void
> *data)

Re: [PATCH v8 3/6] coreaudio: Improve naming

[Akihiko Odaki]

On 2026/03/04 18:25, Christian Schoenebeck wrote:
> On Wednesday, 4 March 2026 07:16:56 CET Akihiko Odaki wrote:
>> coreaudio had names that are not conforming to QEMU codding style.
> 
> coding
> 
>> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   audio/coreaudio.m | 188
>> +++++++++++++++++++++++++++--------------------------- 1 file changed, 95
>> insertions(+), 93 deletions(-)
>>
>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>> index bc9ab7477b68..736227eb2b7a 100644
>> --- a/audio/coreaudio.m
>> +++ b/audio/coreaudio.m
>> @@ -43,34 +43,34 @@
>>   typedef struct coreaudioVoiceOut {
> 
> Leaving this lower case?
> 
>>       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 = {
>> @@ -87,7 +87,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID
>> id, framerange);
>>   }
> 
> So here it is "framerange" and not "frame_range".
> 
>> -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 = {
>> @@ -104,7 +104,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID
>> id, UInt32 *framesize) framesize);
>>   }
> 
> Here it is "framesize" and not "frame_size".
> 
>> -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 = {
>> @@ -121,8 +121,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 = {
>> @@ -139,7 +139,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 = {
>> @@ -243,7 +243,8 @@ static void G_GNUC_PRINTF(3, 4)
>> coreaudio_logerr2(OSStatus 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_voice_out_buf_lock(CoreaudioVoiceOut *core,
>> +                                        const char *fn_name)
>>   {
>>       int err;
>>
>> @@ -256,7 +257,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;
>>
>> @@ -269,20 +271,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), @@ -298,7 +300,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,
>> @@ -307,33 +309,33 @@ static OSStatus audioDeviceIOProc(
>>       const AudioTimeStamp *inOutputTime,
>>       void *hwptr)
>>   {
>> -    UInt32 frameCount, pending_frames;
>> +    UInt32 frame_size, pending_frames;
> 
> And here it is "frame_size".
> 
> That's why I try to avoid looking at such kind of refactoring patches. Never
> ending white noise added to the universe.

This patch addresses naming concerns raised for an old version of this 
series. I will address the suggestions you made with the next version.

> 
>>       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;
>>
>> @@ -349,16 +351,16 @@ static OSStatus audioDeviceIOProc(
>>           out += write_len;
>>       }
>>
>> -    coreaudio_buf_unlock(core, "audioDeviceIOProc");
>> +    coreaudio_voice_out_buf_unlock(core, "out_device_ioproc");
> 
> Behaviour change. Not pure refactoring. Should be split into a separate patch.

Two things are renamed here.
- coreaudio_buf_unlock() is renamed to coreaudio_voice_out_buf_unlock().
- "audioDeviceIOProc" is the name of this function. This function is
   renamed to "out_device_ioproc".

Regards,
Akihiko Odaki

> 
>>       return 0;
>>   }
>>
>> -static OSStatus init_out_device(coreaudioVoiceOut *core)
>> +static OSStatus init_out_device(CoreaudioVoiceOut *core)
>>   {
>> +    AudioValueRange value_range;
>>       OSStatus status;
>> -    AudioValueRange frameRange;
>>
>> -    AudioStreamBasicDescription streamBasicDescription = {
>> +    AudioStreamBasicDescription stream_basic_description = {
>>           .mBitsPerChannel = audio_format_bits(core->hw.info.af),
>>           .mBytesPerFrame = core->hw.info.bytes_per_frame,
>>           .mBytesPerPacket = core->hw.info.bytes_per_frame,
>> @@ -369,21 +371,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");
>>           return status;
>>       }
>> -    if (core->outputDeviceID == kAudioDeviceUnknown) {
>> +    if (core->device_id == kAudioDeviceUnknown) {
>>           error_report("coreaudio: Could not initialize playback: "
>>                        "Unknown audio device");
>>           return status;
>>       }
>>
>>       /* get minimum and maximum buffer frame sizes */
>> -    status = coreaudio_get_framesizerange(core->outputDeviceID,
>> -                                          &frameRange);
>> +    status = coreaudio_get_out_framesizerange(core->device_id,
>> &value_range); if (status == kAudioHardwareBadObjectError) {
>>           return 0;
>>       }
>> @@ -393,34 +394,34 @@ static OSStatus init_out_device(coreaudioVoiceOut
>> *core) return status;
>>       }
>>
>> -    if (frameRange.mMinimum > core->frameSizeSetting) {
>> -        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>> +    if (value_range.mMinimum > core->frame_size_setting) {
>> +        core->device_frame_size = value_range.mMinimum;
>>           warn_report("coreaudio: Upsizing buffer frames to %f",
>> -                    frameRange.mMinimum);
>> -    } else if (frameRange.mMaximum < core->frameSizeSetting) {
>> -        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>> +                    value_range.mMinimum);
>> +    } else if (value_range.mMaximum < core->frame_size_setting) {
>> +        core->device_frame_size = value_range.mMaximum;
>>           warn_report("coreaudio: Downsizing buffer frames to %f",
>> -                    frameRange.mMaximum);
>> +                    value_range.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, -
>> (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;
>>       }
>> @@ -429,19 +430,19 @@ static OSStatus init_out_device(coreaudioVoiceOut
>> *core) "Could not get device buffer frame size"); 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",
>> -                                  streamBasicDescription.mSampleRate);
>> -        core->outputDeviceID = kAudioDeviceUnknown;
>> +                                  stream_basic_description.mSampleRate);
>> +        core->device_id = kAudioDeviceUnknown;
>>           return status;
>>       }
>>
>> @@ -456,8 +457,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) {
>> @@ -465,20 +466,20 @@ static OSStatus init_out_device(coreaudioVoiceOut
>> *core) }
>>       if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
>>           coreaudio_playback_logerr(status, "Could not set IOProc");
>> -        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,
>> @@ -486,7 +487,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"); }
>> @@ -494,20 +495,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");
>>       }
>> -    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,
>> @@ -520,7 +521,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"); }
>> @@ -528,7 +529,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"); @@ -538,22 +539,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();
>> @@ -563,7 +564,7 @@ static OSStatus handle_voice_change(
>>   static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as)
>>   {
>>       OSStatus status;
>> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *)hw;
>> +    CoreaudioVoiceOut *core = (CoreaudioVoiceOut *)hw;
>>       int err;
>>       Audiodev *dev = hw->s->dev;
>>       AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
>> @@ -581,13 +582,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,
>> @@ -597,8 +599,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,
>> @@ -615,11 +617,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"); @@ -636,10 +638,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 audio_coreaudio_class_init(ObjectClass *klass, const void
>> *data)
> 
> 
> 
> 

Re: [PATCH v8 3/6] coreaudio: Improve naming

[Christian Schoenebeck]

On Wednesday, 4 March 2026 11:42:41 CET Akihiko Odaki wrote:
> On 2026/03/04 18:25, Christian Schoenebeck wrote:
> > On Wednesday, 4 March 2026 07:16:56 CET Akihiko Odaki wrote:
> >> coreaudio had names that are not conforming to QEMU codding style.
> > 
> > coding
> > 
> >> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[...]
> >> @@ -349,16 +351,16 @@ static OSStatus audioDeviceIOProc(
> >> 
> >>           out += write_len;
> >>       
> >>       }
> >> 
> >> -    coreaudio_buf_unlock(core, "audioDeviceIOProc");
> >> +    coreaudio_voice_out_buf_unlock(core, "out_device_ioproc");
> > 
> > Behaviour change. Not pure refactoring. Should be split into a separate
> > patch.
> Two things are renamed here.
> - coreaudio_buf_unlock() is renamed to coreaudio_voice_out_buf_unlock().
> - "audioDeviceIOProc" is the name of this function. This function is
>    renamed to "out_device_ioproc".

Ah, these strings just end up being used in a log message. My bad, fine then!

/Christian

Re: [PATCH v8 3/6] coreaudio: Improve naming

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

On Wed, 4 Mar 2026, Christian Schoenebeck wrote:
> On Wednesday, 4 March 2026 07:16:56 CET Akihiko Odaki wrote:
>> coreaudio had names that are not conforming to QEMU codding style.
>
> coding
>
>> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>  audio/coreaudio.m | 188
>> +++++++++++++++++++++++++++--------------------------- 1 file changed, 95
>> insertions(+), 93 deletions(-)
>>
>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>> index bc9ab7477b68..736227eb2b7a 100644
>> --- a/audio/coreaudio.m
>> +++ b/audio/coreaudio.m
>> @@ -43,34 +43,34 @@
>>  typedef struct coreaudioVoiceOut {
>
> Leaving this lower case?

I think you can just drop it if you never refer to the type with struct 
only as the CoreaudioVoiceOut typedef.

Regards,
BALATON Zoltan

[PATCH v8 4/6] coreaudio: Commit the result of init in the end

[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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 audio/coreaudio.m | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 736227eb2b7a..23c3d1f80ac5 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -357,8 +357,11 @@ static OSStatus out_device_ioproc(
 
 static OSStatus init_out_device(CoreaudioVoiceOut *core)
 {
+    AudioDeviceID device_id;
+    AudioDeviceIOProcID ioprocid;
     AudioValueRange value_range;
     OSStatus status;
+    UInt32 device_frame_size;
 
     AudioStreamBasicDescription stream_basic_description = {
         .mBitsPerChannel = audio_format_bits(core->hw.info.af),
@@ -371,20 +374,20 @@ 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");
         return status;
     }
-    if (core->device_id == kAudioDeviceUnknown) {
+    if (device_id == kAudioDeviceUnknown) {
         error_report("coreaudio: Could not initialize playback: "
                      "Unknown audio device");
         return status;
     }
 
     /* get minimum and maximum buffer frame sizes */
-    status = coreaudio_get_out_framesizerange(core->device_id, &value_range);
+    status = coreaudio_get_out_framesizerange(device_id, &value_range);
     if (status == kAudioHardwareBadObjectError) {
         return 0;
     }
@@ -395,33 +398,31 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
     }
 
     if (value_range.mMinimum > core->frame_size_setting) {
-        core->device_frame_size = value_range.mMinimum;
+        device_frame_size = value_range.mMinimum;
         warn_report("coreaudio: Upsizing buffer frames to %f",
                     value_range.mMinimum);
     } else if (value_range.mMaximum < core->frame_size_setting) {
-        core->device_frame_size = value_range.mMaximum;
+        device_frame_size = value_range.mMaximum;
         warn_report("coreaudio: Downsizing buffer frames to %f",
                     value_range.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,
-                                  (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;
     }
@@ -430,10 +431,9 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
                                   "Could not get device buffer frame size");
         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;
@@ -442,7 +442,6 @@ static OSStatus init_out_device(CoreaudioVoiceOut *core)
         coreaudio_playback_logerr(status,
                                   "Could not set samplerate %lf",
                                   stream_basic_description.mSampleRate);
-        core->device_id = kAudioDeviceUnknown;
         return status;
     }
 
@@ -456,20 +455,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");
-        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;
 }
 
@@ -553,7 +556,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);
     }
 
@@ -650,7 +655,7 @@ static void audio_coreaudio_class_init(ObjectClass *klass, const void *data)
 
     k->max_voices_out = 1;
     k->max_voices_in = 0;
-    k->voice_size_out = sizeof(coreaudioVoiceOut);
+    k->voice_size_out = sizeof(CoreaudioVoiceOut);
     k->voice_size_in = 0;
 
     k->init_out = coreaudio_init_out;

-- 
2.53.0

Re: [PATCH v8 4/6] coreaudio: Commit the result of init in the end

[Christian Schoenebeck]

On Wednesday, 4 March 2026 07:16:57 CET Akihiko Odaki wrote:
> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>  audio/coreaudio.m | 47 ++++++++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index 736227eb2b7a..23c3d1f80ac5 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -357,8 +357,11 @@ static OSStatus out_device_ioproc(
> 
>  static OSStatus init_out_device(CoreaudioVoiceOut *core)
>  {
> +    AudioDeviceID device_id;
> +    AudioDeviceIOProcID ioprocid;
>      AudioValueRange value_range;
>      OSStatus status;
> +    UInt32 device_frame_size;

Shouldn't then be this here at the beginning?

  core->device_id = kAudioDeviceUnknown;
  core->ioprocid = NULL;
  ...

[...]
>      if (status != kAudioHardwareNoError) {
>          coreaudio_playback_logerr(status,
>                                    "Could not set device buffer frame size %" PRIu32,
> -                                  (uint32_t)core->device_frame_size);
> +                                  (uint32_t)device_frame_size);
>          return status;
>      }

Why the cast here? device_frame_size is declared as UInt32.

/Christian

Re: [PATCH v8 4/6] coreaudio: Commit the result of init in the end

[Akihiko Odaki]

On 2026/03/04 20:56, Christian Schoenebeck wrote:
> On Wednesday, 4 March 2026 07:16:57 CET Akihiko Odaki wrote:
>> 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 <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>>   audio/coreaudio.m | 47 ++++++++++++++++++++++++++---------------------
>>   1 file changed, 26 insertions(+), 21 deletions(-)
>>
>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>> index 736227eb2b7a..23c3d1f80ac5 100644
>> --- a/audio/coreaudio.m
>> +++ b/audio/coreaudio.m
>> @@ -357,8 +357,11 @@ static OSStatus out_device_ioproc(
>>
>>   static OSStatus init_out_device(CoreaudioVoiceOut *core)
>>   {
>> +    AudioDeviceID device_id;
>> +    AudioDeviceIOProcID ioprocid;
>>       AudioValueRange value_range;
>>       OSStatus status;
>> +    UInt32 device_frame_size;
> 
> Shouldn't then be this here at the beginning?
> 
>    core->device_id = kAudioDeviceUnknown;
>    core->ioprocid = NULL;
>    ...

Setting device members is done in correspondence with actual device 
operations. Concretely, fini_out_device() sets kAudioDeviceUnknown to 
device_id to express it finalized the device. This function initializes 
the device or does nothing if an error happens, so it sets device 
members or does not touch otherwise.

> 
> [...]
>>       if (status != kAudioHardwareNoError) {
>>           coreaudio_playback_logerr(status,
>>                                     "Could not set device buffer frame size %" PRIu32,
>> -                                  (uint32_t)core->device_frame_size);
>> +                                  (uint32_t)device_frame_size);
>>           return status;
>>       }
> 
> Why the cast here? device_frame_size is declared as UInt32.

The definition of uint32_t is not literally identical with UInt32, so 
the compiler complains it is different from what the format string 
(PRIu32) expects. I think I'm going to use uint32_t for the variable to 
avoid this.

Regards,
Akihiko Odaki

[PATCH v8 5/6] audio: Add functions to initialize buffers

[Akihiko Odaki]

These functions can be used to re-initialize buffers when hardware
parameters change due to device hotplug, for example.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 audio/audio_int.h       |  2 ++
 audio/audio-mixeng-be.c | 24 ++++++++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index 06f79ade6b0a..290cf373b49f 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -141,9 +141,11 @@ static inline void *advance(void *p, size_t incr)
 int wav_start_capture(AudioBackend *state, CaptureState *s, const char *path,
                       int freq, int bits, int nchannels);
 
+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-mixeng-be.c b/audio/audio-mixeng-be.c
index 370404505111..1419e7521503 100644
--- a/audio/audio-mixeng-be.c
+++ b/audio/audio-mixeng-be.c
@@ -1187,14 +1187,20 @@ void audio_run(AudioMixengBackend *s, const char *msg)
     }
 }
 
+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)
 {
     AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_GET_CLASS(hw->s);
 
     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) {
@@ -1227,6 +1233,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) {
@@ -1260,9 +1274,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.53.0

Re: [PATCH v8 5/6] audio: Add functions to initialize buffers

[Marc-André Lureau]

On Wed, Mar 4, 2026 at 7:17 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> These functions can be used to re-initialize buffers when hardware
> parameters change due to device hotplug, for example.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  audio/audio_int.h       |  2 ++
>  audio/audio-mixeng-be.c | 24 ++++++++++++++++++------
>  2 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 06f79ade6b0a..290cf373b49f 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -141,9 +141,11 @@ static inline void *advance(void *p, size_t incr)
>  int wav_start_capture(AudioBackend *state, CaptureState *s, const char *path,
>                        int freq, int bits, int nchannels);
>
> +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-mixeng-be.c b/audio/audio-mixeng-be.c
> index 370404505111..1419e7521503 100644
> --- a/audio/audio-mixeng-be.c
> +++ b/audio/audio-mixeng-be.c
> @@ -1187,14 +1187,20 @@ void audio_run(AudioMixengBackend *s, const char *msg)
>      }
>  }
>
> +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)
>  {
>      AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_GET_CLASS(hw->s);
>
>      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) {
> @@ -1227,6 +1233,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) {
> @@ -1260,9 +1274,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.53.0
>

Re: [PATCH v8 5/6] audio: Add functions to initialize buffers

[Christian Schoenebeck]

On Wednesday, 4 March 2026 07:16:58 CET Akihiko Odaki wrote:
> These functions can be used to re-initialize buffers when hardware
> parameters change due to device hotplug, for example.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  audio/audio_int.h       |  2 ++
>  audio/audio-mixeng-be.c | 24 ++++++++++++++++++------
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 06f79ade6b0a..290cf373b49f 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -141,9 +141,11 @@ static inline void *advance(void *p, size_t incr)
>  int wav_start_capture(AudioBackend *state, CaptureState *s, const char
> *path, int freq, int bits, int nchannels);
> 
> +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-mixeng-be.c b/audio/audio-mixeng-be.c
> index 370404505111..1419e7521503 100644
> --- a/audio/audio-mixeng-be.c
> +++ b/audio/audio-mixeng-be.c
> @@ -1187,14 +1187,20 @@ void audio_run(AudioMixengBackend *s, const char
> *msg) }
>  }
> 
> +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)
>  {
>      AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_GET_CLASS(hw->s);
> 
>      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);
>      }

It would be cleaner having split this patch over 2 patches. One for pure
refactoring (no behaviour change), and one that adds g_free(hw->buf_emul);

>      while (hw->pending_emul < hw->size_emul) {
> @@ -1227,6 +1233,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) {
> @@ -1260,9 +1274,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,

Re: [PATCH v8 5/6] audio: Add functions to initialize buffers

[Akihiko Odaki]

On 2026/03/04 17:36, Christian Schoenebeck wrote:
> On Wednesday, 4 March 2026 07:16:58 CET Akihiko Odaki wrote:
>> These functions can be used to re-initialize buffers when hardware
>> parameters change due to device hotplug, for example.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
>> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>> ---
>>   audio/audio_int.h       |  2 ++
>>   audio/audio-mixeng-be.c | 24 ++++++++++++++++++------
>>   2 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/audio/audio_int.h b/audio/audio_int.h
>> index 06f79ade6b0a..290cf373b49f 100644
>> --- a/audio/audio_int.h
>> +++ b/audio/audio_int.h
>> @@ -141,9 +141,11 @@ static inline void *advance(void *p, size_t incr)
>>   int wav_start_capture(AudioBackend *state, CaptureState *s, const char
>> *path, int freq, int bits, int nchannels);
>>
>> +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-mixeng-be.c b/audio/audio-mixeng-be.c
>> index 370404505111..1419e7521503 100644
>> --- a/audio/audio-mixeng-be.c
>> +++ b/audio/audio-mixeng-be.c
>> @@ -1187,14 +1187,20 @@ void audio_run(AudioMixengBackend *s, const char
>> *msg) }
>>   }
>>
>> +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)
>>   {
>>       AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_GET_CLASS(hw->s);
>>
>>       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);
>>       }
> 
> It would be cleaner having split this patch over 2 patches. One for pure
> refactoring (no behaviour change), and one that adds g_free(hw->buf_emul);

g_free() was added because the extracted function can be called with 
hw->buf_emul set. Extracting this into a function without adding 
g_free() makes the function dangerous with non-NULL hw->buf_emul. Adding 
g_free() without extracting it into a function doesn't make sense as it 
checks !hw->buf_emul immediate above.

Regards,
Akihiko Odaki

> 
>>       while (hw->pending_emul < hw->size_emul) {
>> @@ -1227,6 +1233,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) {
>> @@ -1260,9 +1274,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,
> 
> 
> 
> 

Re: [PATCH v8 5/6] audio: Add functions to initialize buffers

[Christian Schoenebeck]

On Wednesday, 4 March 2026 11:47:01 CET Akihiko Odaki wrote:
> On 2026/03/04 17:36, Christian Schoenebeck wrote:
> > On Wednesday, 4 March 2026 07:16:58 CET Akihiko Odaki wrote:
> >> These functions can be used to re-initialize buffers when hardware
> >> parameters change due to device hotplug, for example.
> >> 
> >> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
> >> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> >> ---
[...]
> >> 
> >> +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)
> >>   {
> >>   
> >>       AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_GET_CLASS(hw->s);
> >>       
> >>       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);
> >> 
> >>       }
> > 
> > It would be cleaner having split this patch over 2 patches. One for pure
> > refactoring (no behaviour change), and one that adds g_free(hw->buf_emul);
> 
> g_free() was added because the extracted function can be called with
> hw->buf_emul set. Extracting this into a function without adding
> g_free() makes the function dangerous with non-NULL hw->buf_emul. Adding
> g_free() without extracting it into a function doesn't make sense as it
> checks !hw->buf_emul immediate above.

You are adding the function. So at this point, nobody could have called the
function by accident except you. And as you would have added g_malloc()
immediately with the next patch, there would be no danger.

In general it does make sense splitting pure refactoring from behaviour
changes. And by adding g_free() you are changing existing behaviour. That's
not bike shedding, it's for the sake of being able to skip all those white
noise changes when investigating if something breaks.

And the potential danger argument contradicts itself, because if the function
was called with an uninitialized structure then g_free() would crash. Without
g_free() it was just a potential memory leak OTOH. Choose your poison.

/Christian

Re: [PATCH v8 5/6] audio: Add functions to initialize buffers

[Akihiko Odaki]

On 2026/03/04 20:17, Christian Schoenebeck wrote:
> On Wednesday, 4 March 2026 11:47:01 CET Akihiko Odaki wrote:
>> On 2026/03/04 17:36, Christian Schoenebeck wrote:
>>> On Wednesday, 4 March 2026 07:16:58 CET Akihiko Odaki wrote:
>>>> These functions can be used to re-initialize buffers when hardware
>>>> parameters change due to device hotplug, for example.
>>>>
>>>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
>>>> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>>> ---
> [...]
>>>>
>>>> +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)
>>>>    {
>>>>    
>>>>        AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_GET_CLASS(hw->s);
>>>>        
>>>>        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);
>>>>
>>>>        }
>>>
>>> It would be cleaner having split this patch over 2 patches. One for pure
>>> refactoring (no behaviour change), and one that adds g_free(hw->buf_emul);
>>
>> g_free() was added because the extracted function can be called with
>> hw->buf_emul set. Extracting this into a function without adding
>> g_free() makes the function dangerous with non-NULL hw->buf_emul. Adding
>> g_free() without extracting it into a function doesn't make sense as it
>> checks !hw->buf_emul immediate above.
> 
> You are adding the function. So at this point, nobody could have called the
> function by accident except you. And as you would have added g_malloc()
> immediately with the next patch, there would be no danger.
> 
> In general it does make sense splitting pure refactoring from behaviour
> changes. And by adding g_free() you are changing existing behaviour. That's
> not bike shedding, it's for the sake of being able to skip all those white
> noise changes when investigating if something breaks.

You will also certainly wonder why adding g_free() in that case.

> 
> And the potential danger argument contradicts itself, because if the function
> was called with an uninitialized structure then g_free() would crash. Without
> g_free() it was just a potential memory leak OTOH. Choose your poison.

In the state machine of AudioMixengBackend it is NULL or a valid 
pointer: it never has an invalid pointer.

Marc-André Lureau, I'd like to ask you for an opinion on these matters 
since you are recently working on this and I think you are eventually 
going to make a pull request for this series.

Regards,
Akihiko Odaki

[PATCH v8 6/6] coreaudio: Initialize the buffer for device change

[Akihiko Odaki]

Reallocate buffers when the active device change as the required buffer
size may differ.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
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 23c3d1f80ac5..e4ec1df971c8 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -471,6 +471,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.53.0

Re: [PATCH v8 6/6] coreaudio: Initialize the buffer for device change

[Marc-André Lureau]

On Wed, Mar 4, 2026 at 7:17 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> Reallocate buffers when the active device change as the required buffer
> size may differ.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 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 23c3d1f80ac5..e4ec1df971c8 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -471,6 +471,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);

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>      core->ioprocid = ioprocid;
>
>      return 0;
>
> --
> 2.53.0
>

Re: [PATCH v8 6/6] coreaudio: Initialize the buffer for device change

[Christian Schoenebeck]

On Wednesday, 4 March 2026 07:16:59 CET Akihiko Odaki wrote:
> Reallocate buffers when the active device change as the required buffer
> size may differ.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 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 23c3d1f80ac5..e4ec1df971c8 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -471,6 +471,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;

There is no specific reason to insert that call between the struct
initializers here, or is there? I mean the device is suspended at this point,
so it should not matter. Just looks a bit weird.

But in general, LGTM:

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

Re: [PATCH v8 6/6] coreaudio: Initialize the buffer for device change

[Akihiko Odaki]

On 2026/03/04 20:39, Christian Schoenebeck wrote:
> On Wednesday, 4 March 2026 07:16:59 CET Akihiko Odaki wrote:
>> Reallocate buffers when the active device change as the required buffer
>> size may differ.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> 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 23c3d1f80ac5..e4ec1df971c8 100644
>> --- a/audio/coreaudio.m
>> +++ b/audio/coreaudio.m
>> @@ -471,6 +471,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;
> 
> There is no specific reason to insert that call between the struct
> initializers here, or is there? I mean the device is suspended at this point,
> so it should not matter. Just looks a bit weird.

I will move it after core->ioprocid = ioprocid.

Regards,
Akihiko Odaki

> 
> But in general, LGTM:
> 
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> 
> 
> 

Re: [PATCH v8 0/6] coreaudio fixes

[Christian Schoenebeck]

On Wednesday, 4 March 2026 07:16:53 CET Akihiko Odaki wrote:
> This series contains two fixes for coreaudio. See each one for details.


Most of the patches in this series actually just do plain refactoring, which I
personally don't care for at all.

It would be therefore helpful to make it clear right from the patch commit log
whether a patch is just flipping names or really changing some behaviour.

/Christian


> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> Changes in v8:
> - Rebased.
> - Link to v7:
> https://lore.kernel.org/qemu-devel/20250124-coreaudio-v7-0-9d9a4d91db37@day
> nix.com
> 
> Changes in v7:
> - Added patch "coreaudio: Remove extra whitespaces".
> - Link to v6:
> https://lore.kernel.org/qemu-devel/20250124-coreaudio-v6-0-11fbcb6c47cf@day
> nix.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@day
> nix.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-mixeng-be.c |  24 ++++--
>  audio/coreaudio.m       | 206
> +++++++++++++++++++++++++----------------------- 3 files changed, 127
> insertions(+), 105 deletions(-)
> ---
> base-commit: ffcf1a7981793973ffbd8100a7c3c6042d02ae23
> change-id: 20250109-coreaudio-c984607e1d8c
> 
> Best regards,
> --
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>