[PATCH v4 0/3] coreaudio fixes

[PATCH v4 0/3] coreaudio fixes

[Akihiko Odaki]

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

Signed-off-by: Akihiko Odaki <akihiko.odaki@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 (3):
      coreaudio: Commit the result of init in the end
      audio: Add functions to initialize buffers
      coreaudio: Initialize the buffer for device change

 audio/audio_int.h |  2 ++
 audio/audio.c     | 24 ++++++++++++++++++------
 audio/coreaudio.m | 50 ++++++++++++++++++++++++++++----------------------
 3 files changed, 48 insertions(+), 28 deletions(-)
---
base-commit: 38d0939b86e2eef6f6a622c6f1f7befda0146595
change-id: 20250109-coreaudio-c984607e1d8c

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH v4 1/3] 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 <akihiko.odaki@daynix.com>
---
 audio/coreaudio.m | 49 +++++++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index cadd729d5053..b9e1a952ed37 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -355,7 +355,10 @@ static OSStatus audioDeviceIOProc(
 static OSStatus init_out_device(coreaudioVoiceOut *core)
 {
     OSStatus status;
+    AudioDeviceID deviceID;
     AudioValueRange frameRange;
+    UInt32 audioDevicePropertyBufferFrameSize;
+    AudioDeviceIOProcID ioprocid;
 
     AudioStreamBasicDescription streamBasicDescription = {
         .mBitsPerChannel = core->hw.info.bits,
@@ -368,20 +371,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
         .mSampleRate = core->hw.info.freq
     };
 
-    status = coreaudio_get_voice(&core->outputDeviceID);
+    status = coreaudio_get_voice(&deviceID);
     if (status != kAudioHardwareNoError) {
         coreaudio_playback_logerr (status,
                                    "Could not get default output Device\n");
         return status;
     }
-    if (core->outputDeviceID == kAudioDeviceUnknown) {
+    if (deviceID == kAudioDeviceUnknown) {
         dolog ("Could not initialize playback - Unknown Audiodevice\n");
         return status;
     }
 
     /* get minimum and maximum buffer frame sizes */
-    status = coreaudio_get_framesizerange(core->outputDeviceID,
-                                          &frameRange);
+    status = coreaudio_get_framesizerange(deviceID, &frameRange);
     if (status == kAudioHardwareBadObjectError) {
         return 0;
     }
@@ -392,31 +394,31 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
     }
 
     if (frameRange.mMinimum > core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
+        audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
         dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
     } else if (frameRange.mMaximum < core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
+        audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
         dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
     } else {
-        core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
+        audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
     }
 
     /* set Buffer Frame Size */
-    status = coreaudio_set_framesize(core->outputDeviceID,
-                                     &core->audioDevicePropertyBufferFrameSize);
+    status = coreaudio_set_framesize(deviceID,
+                                     &audioDevicePropertyBufferFrameSize);
     if (status == kAudioHardwareBadObjectError) {
         return 0;
     }
     if (status != kAudioHardwareNoError) {
         coreaudio_playback_logerr (status,
                                     "Could not set device buffer frame size %" PRIu32 "\n",
-                                    (uint32_t)core->audioDevicePropertyBufferFrameSize);
+                                    (uint32_t)audioDevicePropertyBufferFrameSize);
         return status;
     }
 
     /* get Buffer Frame Size */
-    status = coreaudio_get_framesize(core->outputDeviceID,
-                                     &core->audioDevicePropertyBufferFrameSize);
+    status = coreaudio_get_framesize(deviceID,
+                                     &audioDevicePropertyBufferFrameSize);
     if (status == kAudioHardwareBadObjectError) {
         return 0;
     }
@@ -425,11 +427,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
                                     "Could not get device buffer frame size\n");
         return status;
     }
-    core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
 
     /* set Samplerate */
-    status = coreaudio_set_streamformat(core->outputDeviceID,
-                                        &streamBasicDescription);
+    status = coreaudio_set_streamformat(deviceID, &streamBasicDescription);
     if (status == kAudioHardwareBadObjectError) {
         return 0;
     }
@@ -437,7 +437,6 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
         coreaudio_playback_logerr (status,
                                    "Could not set samplerate %lf\n",
                                    streamBasicDescription.mSampleRate);
-        core->outputDeviceID = kAudioDeviceUnknown;
         return status;
     }
 
@@ -451,20 +450,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->outputDeviceID,
+    ioprocid = NULL;
+    status = AudioDeviceCreateIOProcID(deviceID,
                                        audioDeviceIOProc,
                                        &core->hw,
-                                       &core->ioprocid);
+                                       &ioprocid);
     if (status == kAudioHardwareBadDeviceError) {
         return 0;
     }
-    if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
+    if (status != kAudioHardwareNoError || ioprocid == NULL) {
         coreaudio_playback_logerr (status, "Could not set IOProc\n");
-        core->outputDeviceID = kAudioDeviceUnknown;
         return status;
     }
 
+    core->outputDeviceID = deviceID;
+    core->audioDevicePropertyBufferFrameSize = audioDevicePropertyBufferFrameSize;
+    core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
+    core->ioprocid = ioprocid;
+
     return 0;
 }
 
@@ -548,7 +551,9 @@ static OSStatus handle_voice_change(
         fini_out_device(core);
     }
 
-    if (!init_out_device(core)) {
+    init_out_device(core);
+
+    if (core->outputDeviceID) {
         update_device_playback_state(core);
     }
 

-- 
2.47.1

[PATCH v4 2/3] 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 <akihiko.odaki@daynix.com>
---
 audio/audio_int.h |  2 ++
 audio/audio.c     | 24 ++++++++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index 2d079d00a259..9ba4a144d571 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -187,9 +187,11 @@ struct audio_pcm_ops {
     void   (*volume_in)(HWVoiceIn *hw, Volume *vol);
 };
 
+void audio_generic_initialize_buffer_in(HWVoiceIn *hw);
 void audio_generic_run_buffer_in(HWVoiceIn *hw);
 void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
 void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size);
+void audio_generic_initialize_buffer_out(HWVoiceOut *hw);
 void audio_generic_run_buffer_out(HWVoiceOut *hw);
 size_t audio_generic_buffer_get_free(HWVoiceOut *hw);
 void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size);
diff --git a/audio/audio.c b/audio/audio.c
index 87b4e9b6f2f3..17c6bbd0ae9e 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1407,12 +1407,18 @@ void audio_run(AudioState *s, const char *msg)
 #endif
 }
 
+void audio_generic_initialize_buffer_in(HWVoiceIn *hw)
+{
+    g_free(hw->buf_emul);
+    hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+    hw->buf_emul = g_malloc(hw->size_emul);
+    hw->pos_emul = hw->pending_emul = 0;
+}
+
 void audio_generic_run_buffer_in(HWVoiceIn *hw)
 {
     if (unlikely(!hw->buf_emul)) {
-        hw->size_emul = hw->samples * hw->info.bytes_per_frame;
-        hw->buf_emul = g_malloc(hw->size_emul);
-        hw->pos_emul = hw->pending_emul = 0;
+        audio_generic_initialize_buffer_in(hw);
     }
 
     while (hw->pending_emul < hw->size_emul) {
@@ -1446,6 +1452,14 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
     hw->pending_emul -= size;
 }
 
+void audio_generic_initialize_buffer_out(HWVoiceOut *hw)
+{
+    g_free(hw->buf_emul);
+    hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+    hw->buf_emul = g_malloc(hw->size_emul);
+    hw->pos_emul = hw->pending_emul = 0;
+}
+
 size_t audio_generic_buffer_get_free(HWVoiceOut *hw)
 {
     if (hw->buf_emul) {
@@ -1477,9 +1491,7 @@ void audio_generic_run_buffer_out(HWVoiceOut *hw)
 void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size)
 {
     if (unlikely(!hw->buf_emul)) {
-        hw->size_emul = hw->samples * hw->info.bytes_per_frame;
-        hw->buf_emul = g_malloc(hw->size_emul);
-        hw->pos_emul = hw->pending_emul = 0;
+        audio_generic_initialize_buffer_out(hw);
     }
 
     *size = MIN(hw->size_emul - hw->pending_emul,

-- 
2.47.1

[PATCH v4 3/3] 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 <akihiko.odaki@daynix.com>
---
 audio/coreaudio.m | 1 +
 1 file changed, 1 insertion(+)

diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index b9e1a952ed37..72a6df0f75ee 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -466,6 +466,7 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
     core->outputDeviceID = deviceID;
     core->audioDevicePropertyBufferFrameSize = audioDevicePropertyBufferFrameSize;
     core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
+    audio_generic_initialize_buffer_out(&core->hw);
     core->ioprocid = ioprocid;
 
     return 0;

-- 
2.47.1

Re: [PATCH v4 2/3] audio: Add functions to initialize buffers

[Phil Dennis-Jordan]

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

On Fri, 17 Jan 2025 at 07:48, Akihiko Odaki <akihiko.odaki@daynix.com>
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 <akihiko.odaki@daynix.com>
>

Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>


> ---
>  audio/audio_int.h |  2 ++
>  audio/audio.c     | 24 ++++++++++++++++++------
>  2 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 2d079d00a259..9ba4a144d571 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -187,9 +187,11 @@ struct audio_pcm_ops {
>      void   (*volume_in)(HWVoiceIn *hw, Volume *vol);
>  };
>
> +void audio_generic_initialize_buffer_in(HWVoiceIn *hw);
>  void audio_generic_run_buffer_in(HWVoiceIn *hw);
>  void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
>  void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size);
> +void audio_generic_initialize_buffer_out(HWVoiceOut *hw);
>  void audio_generic_run_buffer_out(HWVoiceOut *hw);
>  size_t audio_generic_buffer_get_free(HWVoiceOut *hw);
>  void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size);
> diff --git a/audio/audio.c b/audio/audio.c
> index 87b4e9b6f2f3..17c6bbd0ae9e 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1407,12 +1407,18 @@ void audio_run(AudioState *s, const char *msg)
>  #endif
>  }
>
> +void audio_generic_initialize_buffer_in(HWVoiceIn *hw)
> +{
> +    g_free(hw->buf_emul);
> +    hw->size_emul = hw->samples * hw->info.bytes_per_frame;
> +    hw->buf_emul = g_malloc(hw->size_emul);
> +    hw->pos_emul = hw->pending_emul = 0;
> +}
> +
>  void audio_generic_run_buffer_in(HWVoiceIn *hw)
>  {
>      if (unlikely(!hw->buf_emul)) {
> -        hw->size_emul = hw->samples * hw->info.bytes_per_frame;
> -        hw->buf_emul = g_malloc(hw->size_emul);
> -        hw->pos_emul = hw->pending_emul = 0;
> +        audio_generic_initialize_buffer_in(hw);
>      }
>
>      while (hw->pending_emul < hw->size_emul) {
> @@ -1446,6 +1452,14 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw,
> void *buf, size_t size)
>      hw->pending_emul -= size;
>  }
>
> +void audio_generic_initialize_buffer_out(HWVoiceOut *hw)
> +{
> +    g_free(hw->buf_emul);
> +    hw->size_emul = hw->samples * hw->info.bytes_per_frame;
> +    hw->buf_emul = g_malloc(hw->size_emul);
> +    hw->pos_emul = hw->pending_emul = 0;
> +}
> +
>  size_t audio_generic_buffer_get_free(HWVoiceOut *hw)
>  {
>      if (hw->buf_emul) {
> @@ -1477,9 +1491,7 @@ void audio_generic_run_buffer_out(HWVoiceOut *hw)
>  void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size)
>  {
>      if (unlikely(!hw->buf_emul)) {
> -        hw->size_emul = hw->samples * hw->info.bytes_per_frame;
> -        hw->buf_emul = g_malloc(hw->size_emul);
> -        hw->pos_emul = hw->pending_emul = 0;
> +        audio_generic_initialize_buffer_out(hw);
>      }
>
>      *size = MIN(hw->size_emul - hw->pending_emul,
>
> --
> 2.47.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 4120 bytes --]

Re: [PATCH v4 3/3] coreaudio: Initialize the buffer for device change

[Phil Dennis-Jordan]

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

On Fri, 17 Jan 2025 at 07:48, Akihiko Odaki <akihiko.odaki@daynix.com>
wrote:

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

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>

---
>  audio/coreaudio.m | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index b9e1a952ed37..72a6df0f75ee 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -466,6 +466,7 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core)
>      core->outputDeviceID = deviceID;
>      core->audioDevicePropertyBufferFrameSize =
> audioDevicePropertyBufferFrameSize;
>      core->hw.samples = core->bufferCount *
> core->audioDevicePropertyBufferFrameSize;
> +    audio_generic_initialize_buffer_out(&core->hw);
>      core->ioprocid = ioprocid;
>
>      return 0;
>
> --
> 2.47.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 1807 bytes --]

Re: [PATCH v4 1/3] coreaudio: Commit the result of init in the end

[Phil Dennis-Jordan]

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

On Fri, 17 Jan 2025 at 07:48, Akihiko Odaki <akihiko.odaki@daynix.com>
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.
>

Apologies if this is a basic question, but I'm seeing this code for the
first time: What if init_out_device() is called from handle_voice_change()
because the system audio device changed though, will outputDeviceID be
valid on those error conditions? I assume those
kAudioHardwareBadObjectError checks are defending against the case where
the device disappears after coreaudio_get_voice() returned a valid audio
device.

So for example, we start out with the audio device being one of two devices
exposed by a compound device. The compound device is unplugged, but the
system audio sub-device appears to the system to be removed first, so the
second sub-device briefly becomes the system audio device. We get the
handle_voice_change() call, handle_voice_change() returns the second
sub-device, but then that one disappears too before we reach the end of
init_out_device(). We error out, but in this case outputDeviceID would
remain the same as the original device which has actually disappeared.

Or have I misunderstood?



> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  audio/coreaudio.m | 49 +++++++++++++++++++++++++++----------------------
>  1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index cadd729d5053..b9e1a952ed37 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -355,7 +355,10 @@ static OSStatus audioDeviceIOProc(
>  static OSStatus init_out_device(coreaudioVoiceOut *core)
>  {
>      OSStatus status;
> +    AudioDeviceID deviceID;
>      AudioValueRange frameRange;
> +    UInt32 audioDevicePropertyBufferFrameSize;
> +    AudioDeviceIOProcID ioprocid;
>
>      AudioStreamBasicDescription streamBasicDescription = {
>          .mBitsPerChannel = core->hw.info.bits,
> @@ -368,20 +371,19 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core)
>          .mSampleRate = core->hw.info.freq
>      };
>
> -    status = coreaudio_get_voice(&core->outputDeviceID);
> +    status = coreaudio_get_voice(&deviceID);
>      if (status != kAudioHardwareNoError) {
>          coreaudio_playback_logerr (status,
>                                     "Could not get default output
> Device\n");
>          return status;
>      }
> -    if (core->outputDeviceID == kAudioDeviceUnknown) {
> +    if (deviceID == kAudioDeviceUnknown) {
>          dolog ("Could not initialize playback - Unknown Audiodevice\n");
>          return status;
>      }
>
>      /* get minimum and maximum buffer frame sizes */
> -    status = coreaudio_get_framesizerange(core->outputDeviceID,
> -                                          &frameRange);
> +    status = coreaudio_get_framesizerange(deviceID, &frameRange);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>
     }
> @@ -392,31 +394,31 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core)
>      }
>
>      if (frameRange.mMinimum > core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32)
> frameRange.mMinimum;
> +        audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
>          dolog ("warning: Upsizing Buffer Frames to %f\n",
> frameRange.mMinimum);
>      } else if (frameRange.mMaximum < core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32)
> frameRange.mMaximum;
> +        audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
>          dolog ("warning: Downsizing Buffer Frames to %f\n",
> frameRange.mMaximum);
>      } else {
> -        core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> +        audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
>      }
>
>      /* set Buffer Frame Size */
> -    status = coreaudio_set_framesize(core->outputDeviceID,
> -
>  &core->audioDevicePropertyBufferFrameSize);
> +    status = coreaudio_set_framesize(deviceID,
> +                                     &audioDevicePropertyBufferFrameSize);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
>      if (status != kAudioHardwareNoError) {
>          coreaudio_playback_logerr (status,
>                                      "Could not set device buffer frame
> size %" PRIu32 "\n",
> -
> (uint32_t)core->audioDevicePropertyBufferFrameSize);
> +
> (uint32_t)audioDevicePropertyBufferFrameSize);
>          return status;
>      }
>
>      /* get Buffer Frame Size */
> -    status = coreaudio_get_framesize(core->outputDeviceID,
> -
>  &core->audioDevicePropertyBufferFrameSize);
> +    status = coreaudio_get_framesize(deviceID,
> +                                     &audioDevicePropertyBufferFrameSize);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
> @@ -425,11 +427,9 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core)
>                                      "Could not get device buffer frame
> size\n");
>          return status;
>      }
> -    core->hw.samples = core->bufferCount *
> core->audioDevicePropertyBufferFrameSize;
>
>      /* set Samplerate */
> -    status = coreaudio_set_streamformat(core->outputDeviceID,
> -                                        &streamBasicDescription);
> +    status = coreaudio_set_streamformat(deviceID,
> &streamBasicDescription);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
> @@ -437,7 +437,6 @@ static OSStatus init_out_device(coreaudioVoiceOut
> *core)
>          coreaudio_playback_logerr (status,
>                                     "Could not set samplerate %lf\n",
>                                     streamBasicDescription.mSampleRate);
> -        core->outputDeviceID = kAudioDeviceUnknown;
>          return status;
>      }
>
> @@ -451,20 +450,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->outputDeviceID,
> +    ioprocid = NULL;
> +    status = AudioDeviceCreateIOProcID(deviceID,
>                                         audioDeviceIOProc,
>                                         &core->hw,
> -                                       &core->ioprocid);
> +                                       &ioprocid);
>      if (status == kAudioHardwareBadDeviceError) {
>          return 0;
>      }
> -    if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
> +    if (status != kAudioHardwareNoError || ioprocid == NULL) {
>          coreaudio_playback_logerr (status, "Could not set IOProc\n");
> -        core->outputDeviceID = kAudioDeviceUnknown;
>          return status;
>      }
>
> +    core->outputDeviceID = deviceID;
> +    core->audioDevicePropertyBufferFrameSize =
> audioDevicePropertyBufferFrameSize;
> +    core->hw.samples = core->bufferCount *
> core->audioDevicePropertyBufferFrameSize;
> +    core->ioprocid = ioprocid;
> +
>      return 0;
>  }
>
> @@ -548,7 +551,9 @@ static OSStatus handle_voice_change(
>          fini_out_device(core);
>      }
>
> -    if (!init_out_device(core)) {
> +    init_out_device(core);
> +
> +    if (core->outputDeviceID) {
>          update_device_playback_state(core);
>      }
>
>
> --
> 2.47.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 10227 bytes --]

Re: [PATCH v4 3/3] coreaudio: Initialize the buffer for device change

[Christian Schoenebeck]

On Friday, January 17, 2025 7:47:02 AM CET Akihiko Odaki wrote:
> Reallocate buffers when the active device change as the required buffer
> size may differ.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

OTOH it also re-allocates the buffer when the size did not change, but Ok.

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 b9e1a952ed37..72a6df0f75ee 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -466,6 +466,7 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>      core->outputDeviceID = deviceID;
>      core->audioDevicePropertyBufferFrameSize = audioDevicePropertyBufferFrameSize;
>      core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
> +    audio_generic_initialize_buffer_out(&core->hw);
>      core->ioprocid = ioprocid;
>  
>      return 0;
> 
> 

Re: [PATCH v4 1/3] coreaudio: Commit the result of init in the end

[Christian Schoenebeck]

On Friday, January 17, 2025 7:47:00 AM 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 <akihiko.odaki@daynix.com>
> ---
>  audio/coreaudio.m | 49 +++++++++++++++++++++++++++----------------------
>  1 file changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index cadd729d5053..b9e1a952ed37 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -355,7 +355,10 @@ static OSStatus audioDeviceIOProc(
>  static OSStatus init_out_device(coreaudioVoiceOut *core)
>  {
>      OSStatus status;
> +    AudioDeviceID deviceID;
>      AudioValueRange frameRange;
> +    UInt32 audioDevicePropertyBufferFrameSize;
> +    AudioDeviceIOProcID ioprocid;

So you said 'outDeviceID' was 'too verbose', ... but
'audioDevicePropertyBufferFrameSize' is not? :-)

/Christian

>      AudioStreamBasicDescription streamBasicDescription = {
>          .mBitsPerChannel = core->hw.info.bits,
> @@ -368,20 +371,19 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>          .mSampleRate = core->hw.info.freq
>      };
>  
> -    status = coreaudio_get_voice(&core->outputDeviceID);
> +    status = coreaudio_get_voice(&deviceID);
>      if (status != kAudioHardwareNoError) {
>          coreaudio_playback_logerr (status,
>                                     "Could not get default output Device\n");
>          return status;
>      }
> -    if (core->outputDeviceID == kAudioDeviceUnknown) {
> +    if (deviceID == kAudioDeviceUnknown) {
>          dolog ("Could not initialize playback - Unknown Audiodevice\n");
>          return status;
>      }
>  
>      /* get minimum and maximum buffer frame sizes */
> -    status = coreaudio_get_framesizerange(core->outputDeviceID,
> -                                          &frameRange);
> +    status = coreaudio_get_framesizerange(deviceID, &frameRange);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
> @@ -392,31 +394,31 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>      }
>  
>      if (frameRange.mMinimum > core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
> +        audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
>          dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
>      } else if (frameRange.mMaximum < core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
> +        audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
>          dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
>      } else {
> -        core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> +        audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
>      }
>  
>      /* set Buffer Frame Size */
> -    status = coreaudio_set_framesize(core->outputDeviceID,
> -                                     &core->audioDevicePropertyBufferFrameSize);
> +    status = coreaudio_set_framesize(deviceID,
> +                                     &audioDevicePropertyBufferFrameSize);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
>      if (status != kAudioHardwareNoError) {
>          coreaudio_playback_logerr (status,
>                                      "Could not set device buffer frame size %" PRIu32 "\n",
> -                                    (uint32_t)core->audioDevicePropertyBufferFrameSize);
> +                                    (uint32_t)audioDevicePropertyBufferFrameSize);
>          return status;
>      }
>  
>      /* get Buffer Frame Size */
> -    status = coreaudio_get_framesize(core->outputDeviceID,
> -                                     &core->audioDevicePropertyBufferFrameSize);
> +    status = coreaudio_get_framesize(deviceID,
> +                                     &audioDevicePropertyBufferFrameSize);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
> @@ -425,11 +427,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>                                      "Could not get device buffer frame size\n");
>          return status;
>      }
> -    core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
>  
>      /* set Samplerate */
> -    status = coreaudio_set_streamformat(core->outputDeviceID,
> -                                        &streamBasicDescription);
> +    status = coreaudio_set_streamformat(deviceID, &streamBasicDescription);
>      if (status == kAudioHardwareBadObjectError) {
>          return 0;
>      }
> @@ -437,7 +437,6 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>          coreaudio_playback_logerr (status,
>                                     "Could not set samplerate %lf\n",
>                                     streamBasicDescription.mSampleRate);
> -        core->outputDeviceID = kAudioDeviceUnknown;
>          return status;
>      }
>  
> @@ -451,20 +450,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->outputDeviceID,
> +    ioprocid = NULL;
> +    status = AudioDeviceCreateIOProcID(deviceID,
>                                         audioDeviceIOProc,
>                                         &core->hw,
> -                                       &core->ioprocid);
> +                                       &ioprocid);
>      if (status == kAudioHardwareBadDeviceError) {
>          return 0;
>      }
> -    if (status != kAudioHardwareNoError || core->ioprocid == NULL) {
> +    if (status != kAudioHardwareNoError || ioprocid == NULL) {
>          coreaudio_playback_logerr (status, "Could not set IOProc\n");
> -        core->outputDeviceID = kAudioDeviceUnknown;
>          return status;
>      }
>  
> +    core->outputDeviceID = deviceID;
> +    core->audioDevicePropertyBufferFrameSize = audioDevicePropertyBufferFrameSize;
> +    core->hw.samples = core->bufferCount * core->audioDevicePropertyBufferFrameSize;
> +    core->ioprocid = ioprocid;
> +
>      return 0;
>  }
>  
> @@ -548,7 +551,9 @@ static OSStatus handle_voice_change(
>          fini_out_device(core);
>      }
>  
> -    if (!init_out_device(core)) {
> +    init_out_device(core);
> +
> +    if (core->outputDeviceID) {
>          update_device_playback_state(core);
>      }