Re: [PATCH] tests/qtest/ac97-test: add up-/downsampling tests

[Volker Rümelin]

Am 25.10.22 um 09:44 schrieb Marc-André Lureau:
> Hi
>
> On Tue, Oct 25, 2022 at 12:31 AM Volker Rümelin<vr_qemu@t-online.de>  wrote:
> > Am 24.10.22 um 10:13 schrieb Marc-André Lureau:
> > > Hi
> > >
> > > On Mon, Oct 24, 2022 at 9:28 AM Volker Rümelin<vr_qemu@t-online.de>
> > > wrote:
> > >
> > >      Test if the audio subsystem can handle extreme up- and down-
> > >      sampling ratios like 44100/1 and 1/44100. For some time these
> > >      used to trigger QEMU aborts. The test was taken from
> > >      https://gitlab.com/qemu-project/qemu/-/issues/71  where it was
> > >      used to demonstrate a very different issue.
> > >
> > >      Suggested-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> > >      Signed-off-by: Volker Rümelin<vr_qemu@t-online.de>
> > >
> > >
> > > Thanks for working on this
> > >
> > > It seems to show something different though:
> > > "
> > > A bug was just triggered in audio_calloc
> > > Save all your work and restart without audio
> > > I am sorry
> > > "
> > >
> > > AUD_open_out() is called with audsettings: {freq = 1, nchannels = 2,
> > > fmt = AUDIO_FORMAT_S16, endianness = 0}
> > >
> > > And that's it. Any idea?
> > Hi,
> >
> > the scary message is expected and doesn't mean this qos-test failed.
> > This is the currently not so silent 'the audio subsystem should (...)
> > silently give up' case.
> Ok, but it's not silent. According to the AC97 spec, "if the value
> written to the register is supported that value will be echoed back
> when read, otherwise the closest (higher in case of a tie) sample rate
> supported is returned". We should probably pick a low sample rate,
> like 8000 (see Table 32 in spec 2.1) for anything below it.

Hi,

I don't think we should limit the lowest sample rate to 8000 Hz. The 
sample rates in AC97 revision 2.1 Table 32 are sample rates the codec 
should support at minimum. We are free to support the whole 1-65535 Hz 
sample rate range. This is a convenient way to test edge cases. If you 
think the audio_bug message is an issue, I'll improve the error handling 
in AUD_open_* first and then resend this qos test.

> > The noaudio backend uses a mixing-engine buffer size of 1024 audio
> > frames and AUD_open_* tries to allocate memory for 1024/44100 = 0.0232
> > audio frames for the resample buffer in audio_pcm_sw_alloc_resources_*.
> > This allocation fails and produces the scary message. The error is
> > handled correctly and AUD_open_* returns NULL. AUD_read and AUD_write
> > return early if this pointer is NULL and the audio frontend callback
> > functions will also not be called because the audio_frontend_frames_*
> > functions return 0 in this case.
> Thanks, it'd be nice to have such a description in the commit message.

I'll improve the commit message of patch version 2.

With best regards,
Volker