On Monday, December 19, 2022 8:22:25 AM CET Volker Rümelin wrote:
> Am 18.12.22 um 21:26 schrieb Christian Schoenebeck:
> > 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_q...@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.
>
> sw->buf is always NULL here. In the audio_pcm_create_voice_pair_*()
> functions we have sw = audio_calloc(__func__, 1, sizeof(*sw)) (after
> patch 08/11 sw = g_new0(SW, 1)) and the audio_pcm_sw_free_resources_*()
> functions also set sw->buf = NULL after freeing sw->buf.
OK
> > The other thing that puzzles me, in error case these template functions
> > return
> > -1, which would then be feed to g_malloc*()?
>
> Sorry, I can't see where -1 would be fed to g_malloc*().
>
> On error the audio_pcm_sw_alloc_resources_*() functions return error
> code -1, and that error code propagates up to the AUD_open_*() functions
> or the audio_pcm_create_voice_pair_*() functions which return NULL.
I thought about patch 7 where you do:
- hw = audio_calloc(__func__, 1, glue(drv->voice_size_, TYPE));
+ hw = g_malloc0(glue(drv->voice_size_, TYPE));
But I just realized that it is using audio_pcm_hw_add_new_, which not returns
a negative value anywhere, so fine as well:
Acked-by: Christian Schoenebeck <qemu_...@crudebyte.com>
>
> >> + }
> >>
> >> sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample));
> >> if (!sw->buf) {
> >>
>
>
>
