1. Patchew
  2. QEMU
  3. [PATCH v3 0/2] coreaudio fixes
  4. View patch

[PATCH v3 2/2] coreaudio: Initialize the buffer for device change

Akihiko Odaki posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 2/2] coreaudio: Initialize the buffer for device change
Posted by Akihiko Odaki 2 months, 2 weeks ago 
Reallocate buffers when the active device change as the required buffer
size may differ.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 audio/audio_int.h |  2 ++
 audio/audio.c     | 24 ++++++++++++++++++------
 audio/coreaudio.m |  1 +
 3 files changed, 21 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,
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 v3 2/2] coreaudio: Initialize the buffer for device change
Posted by Christian Schoenebeck 2 months, 2 weeks ago 
On Wednesday, January 15, 2025 1:06:56 PM 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>
> ---
>  audio/audio_int.h |  2 ++
>  audio/audio.c     | 24 ++++++++++++++++++------
>  audio/coreaudio.m |  1 +
>  3 files changed, 21 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;
> +}
> +

Better something like "reinit" in the name maybe?

>  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,
> 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;

I would have probably separated this change into a separate patch, as changes
above were more or less just refactoring, whereas this one changes behaviour.

And like my comment in the previous patch, I wonder whether that call comes
too late. Keep in mind there are e.g. audio devices where you can't change
certain parameters. So not every error here is a fatal error.

>  
>      return 0;
Re: [PATCH v3 2/2] coreaudio: Initialize the buffer for device change
Posted by Akihiko Odaki 2 months, 2 weeks ago 
On 2025/01/16 0:31, Christian Schoenebeck wrote:
> On Wednesday, January 15, 2025 1:06:56 PM 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>
>> ---
>>   audio/audio_int.h |  2 ++
>>   audio/audio.c     | 24 ++++++++++++++++++------
>>   audio/coreaudio.m |  1 +
>>   3 files changed, 21 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;
>> +}
>> +
> 
> Better something like "reinit" in the name maybe?
> 
>>   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,
>> 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;
> 
> I would have probably separated this change into a separate patch, as changes
> above were more or less just refactoring, whereas this one changes behaviour.

That makes sense. I will do so if I have to respin this series.

> 
> And like my comment in the previous patch, I wonder whether that call comes
> too late. Keep in mind there are e.g. audio devices where you can't change
> certain parameters. So not every error here is a fatal error.

A function called late, AudioDeviceCreateIOProcID() is essential to make 
the output device functional so any error which triggers an early return 
before that is fatal.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.