Re: [PATCH] coreaudio: fix coreaudio playback

["Zoltán Kővágó" <dirty.ice.hu@gmail.com>]

On 2020-02-02 15:06, Volker Rümelin wrote:
> There are reports that since commit 2ceb8240fa "coreaudio: port
> to the new audio backend api" audio playback with CoreAudio is
> broken. This patch reverts some parts the commit.
> 
> Because of changes in the audio subsystem the audio clip
> function in v4.1.0 of coreaudio.c had to be moved to mixeng.c
> and the generic buffer management code needed a hint about the
> size of the float type.
> 
> This patch is based on a patch from Zoltán Kővágó found at
> https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg02142.html.
> 
> Fixes: 2ceb8240fa "coreaudio: port to the new audio backend api"
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   audio/audio_template.h |  7 +++++++
>   audio/coreaudio.c      | 32 +++++++++-----------------------
>   audio/mixeng.c         | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>   audio/mixeng.h         |  5 +++++
>   4 files changed, 69 insertions(+), 23 deletions(-)

Hi,

I started reviewing this patch, but then I realized that I don't really 
like that "if (s->dev->driver == AUDIODEV_DRIVER_COREAUDIO)" hack, so I 
went ahead and added (hopefully proper) float samples support to mixeng. 
  I've submitted that patch to the mailing list.  Unless it causes 
problems I think it would be the best to merge it into this patch.

Regards,
Zoltan

> 
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 3287d7075e..0336d2670c 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -276,6 +276,13 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
>           goto err1;
>       }
>   
> +    if (s->dev->driver == AUDIODEV_DRIVER_COREAUDIO) {
> +#ifdef DAC
> +        hw->clip = clip_natural_float_from_stereo;
> +#else
> +        hw->conv = conv_natural_float_to_stereo;
> +#endif
> +    } else
>   #ifdef DAC
>       hw->clip = mixeng_clip
>   #else
> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> index 66f0f459cf..0049db97fa 100644
> --- a/audio/coreaudio.c
> +++ b/audio/coreaudio.c
> @@ -471,20 +471,6 @@ static OSStatus audioDeviceIOProc(
>       return 0;
>   }
>   
> -static UInt32 coreaudio_get_flags(struct audio_pcm_info *info,
> -                                  struct audsettings *as)
> -{
> -    UInt32 flags = info->sign ? kAudioFormatFlagIsSignedInteger : 0;
> -    if (as->endianness) { /* 0 = little, 1 = big */
> -        flags |= kAudioFormatFlagIsBigEndian;
> -    }
> -
> -    if (flags == 0) { /* must not be 0 */
> -        flags = kAudioFormatFlagsAreAllClear;
> -    }
> -    return flags;
> -}
> -
>   static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>                                 void *drv_opaque)
>   {
> @@ -496,6 +482,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>       Audiodev *dev = drv_opaque;
>       AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
>       int frames;
> +    struct audsettings fake_as;
>   
>       /* create mutex */
>       err = pthread_mutex_init(&core->mutex, NULL);
> @@ -504,6 +491,14 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>           return -1;
>       }
>   
> +    /*
> +     * The canonical audio format for CoreAudio on macOS is float. Currently
> +     * there is no generic code for AUDIO_FORMAT_F32 in qemu. Here we select
> +     * AUDIO_FORMAT_S32 instead because only the sample size has to match.
> +     */
> +    fake_as = *as;
> +    as = &fake_as;
> +    as->fmt = AUDIO_FORMAT_S32;
>       audio_pcm_init_info (&hw->info, as);
>   
>       status = coreaudio_get_voice(&core->outputDeviceID);
> @@ -572,15 +567,6 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
>   
>       /* set Samplerate */
>       core->outputStreamBasicDescription.mSampleRate = (Float64) as->freq;
> -    core->outputStreamBasicDescription.mFormatID = kAudioFormatLinearPCM;
> -    core->outputStreamBasicDescription.mFormatFlags =
> -        coreaudio_get_flags(&hw->info, as);
> -    core->outputStreamBasicDescription.mBytesPerPacket =
> -        core->outputStreamBasicDescription.mBytesPerFrame =
> -        hw->info.nchannels * hw->info.bits / 8;
> -    core->outputStreamBasicDescription.mFramesPerPacket = 1;
> -    core->outputStreamBasicDescription.mChannelsPerFrame = hw->info.nchannels;
> -    core->outputStreamBasicDescription.mBitsPerChannel = hw->info.bits;
>   
>       status = coreaudio_set_streamformat(core->outputDeviceID,
>                                           &core->outputStreamBasicDescription);
> diff --git a/audio/mixeng.c b/audio/mixeng.c
> index 2f5ba71381..16b646d48c 100644
> --- a/audio/mixeng.c
> +++ b/audio/mixeng.c
> @@ -267,6 +267,54 @@ f_sample *mixeng_clip[2][2][2][3] = {
>       }
>   };
>   
> +void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
> +                                  int samples)
> +{
> +    float *in = (float *)src;
> +#ifndef FLOAT_MIXENG
> +    const float scale = UINT_MAX;
> +#endif
> +
> +    while (samples--) {
> +#ifdef FLOAT_MIXENG
> +        dst->l = *in++;
> +        dst->r = *in++;
> +#else
> +        dst->l = *in++ * scale;
> +        dst->r = *in++ * scale;
> +#endif
> +        dst++;
> +    }
> +}
> +
> +void clip_natural_float_from_stereo(void *dst, const struct st_sample *src,
> +                                    int samples)
> +{
> +    float *out = (float *)dst;
> +#ifndef FLOAT_MIXENG
> +#ifdef RECIPROCAL
> +    const float scale = 1.f / UINT_MAX;
> +#else
> +    const float scale = UINT_MAX;
> +#endif
> +#endif
> +
> +    while (samples--) {
> +#ifdef FLOAT_MIXENG
> +        *out++ = src->l;
> +        *out++ = src->r;
> +#else
> +#ifdef RECIPROCAL
> +        *out++ = src->l * scale;
> +        *out++ = src->r * scale;
> +#else
> +        *out++ = src->l / scale;
> +        *out++ = src->r / scale;
> +#endif
> +#endif
> +        src++;
> +    }
> +}
>   
>   void audio_sample_to_uint64(void *samples, int pos,
>                               uint64_t *left, uint64_t *right)
> diff --git a/audio/mixeng.h b/audio/mixeng.h
> index 18e62c7c49..7ef61763e8 100644
> --- a/audio/mixeng.h
> +++ b/audio/mixeng.h
> @@ -41,6 +41,11 @@ typedef void (f_sample) (void *dst, const struct st_sample *src, int samples);
>   extern t_sample *mixeng_conv[2][2][2][3];
>   extern f_sample *mixeng_clip[2][2][2][3];
>   
> +void conv_natural_float_to_stereo(struct st_sample *dst, const void *src,
> +                                  int samples);
> +void clip_natural_float_from_stereo(void *dst, const struct st_sample *src,
> +                                    int samples);
> +
>   void *st_rate_start (int inrate, int outrate);
>   void st_rate_flow(void *opaque, st_sample *ibuf, st_sample *obuf,
>                     size_t *isamp, size_t *osamp);
>