Re: [PATCH 01/11] audio: log unimplemented audio device sample rates

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Sunday, December 18, 2022 6:15:29 PM CET Volker Rümelin wrote:
> Some emulated audio devices allow guests to select very low
> sample rates that the audio subsystem doesn't support. The lowest
> supported sample rate depends on the audio backend used and in
> most cases can be changed with various -audiodev arguments. Until
> now, the audio_bug function emits an error message similar to the
> following error message
> 
> A bug was just triggered in audio_calloc
> Save all your work and restart without audio
> I am sorry
> Context:
> audio_pcm_sw_alloc_resources_out passed invalid arguments to
>  audio_calloc
> nmemb=0 size=16 (len=0)
> audio: Could not allocate buffer for `ac97.po' (0 samples)
> 
> and the audio subsystem continues without sound for the affected
> device.
> 
> The fact that the selected sample rate is not supported is not a
> guest error. Instead of displaying an error message, the missing
> audio support is now logged. Simply continuing without sound is
> correct, since the audio stream won't transport anything
> reasonable at such high resample ratios anyway.
> 
> The AUD_open_* functions return NULL like before. The opened
> audio device will not be registered in the audio subsystem and
> consequently the audio frontend callback functions will not be
> called. The AUD_read and AUD_write functions return early in this
> case. This is necessary because, for example, the Sound Blaster 16
> emulation calls AUD_write from the DMA callback function.
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>  audio/audio.c          |  1 +
>  audio/audio_template.h | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index d849a94a81..f6b420688d 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -31,6 +31,7 @@
>  #include "qapi/qobject-input-visitor.h"
>  #include "qapi/qapi-visit-audio.h"
>  #include "qemu/cutils.h"
> +#include "qemu/log.h"
>  #include "qemu/module.h"
>  #include "qemu/help_option.h"
>  #include "sysemu/sysemu.h"
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 720a32e57e..bfa94b4d22 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -115,6 +115,19 @@ static int glue (audio_pcm_sw_alloc_resources_, TYPE) (SW *sw)
>  #else
>      samples = (int64_t)sw->HWBUF->size * sw->ratio >> 32;
>  #endif
> +    if (samples == 0) {
> +        HW *hw = sw->hw;
> +        size_t f_fe_min;
> +
> +        /* f_fe_min = ceil(1 [frames] * f_be [Hz] / size_be [frames]) */
> +        f_fe_min = (hw->info.freq + HWBUF->size - 1) / HWBUF->size;
> +        qemu_log_mask(LOG_UNIMP,
> +                      AUDIO_CAP ": The guest selected a " NAME " sample rate"
> +                      " of %d Hz for %s. Only sample rates >= %zu Hz are"
> +                      " supported.\n",
> +                      sw->info.freq, sw->name, f_fe_min);
> +        return -1;

You probably want to `sw->buf = NULL;` before returning here, or adjust the
condition for the error message below.

The other thing that puzzles me, in error case these template functions return
-1, which would then be feed to g_malloc*()?

> +    }
>  
>      sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample));
>      if (!sw->buf) {
>