| Message ID | 20210110100239.27588-3-vr_qemu@t-online.de |
|---|---|
| State | New |
| Headers | show |
| Series | next round of audio patches | expand |
Hi, > - hw->samples = obt.samples; > + hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) * > + obt.samples; > +# @buffer-count: number of buffers (default 4) Any specific reason for this default? In my testing I've needed much higher values. 8 still got me crackling sound, 16 worked ok. take care, Gerd
Volker Rümelin <vr_qemu@t-online.de> writes: > Currently there is a crackling noise with SDL2 audio playback. > Commit bcf19777df: "audio/sdlaudio: Allow audio playback with > SDL2" already mentioned the crackling noise. > > Add an out.buffer-count option to give users a chance to select > sane settings for glitch free audio playback. The idea was taken > from the coreaudio backend. > > The in.buffer-count option will be used with one of the next > patches. > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> > --- [...] > diff --git a/qapi/audio.json b/qapi/audio.json > index 072ed79def..9cba0df8a4 100644 > --- a/qapi/audio.json > +++ b/qapi/audio.json > @@ -301,6 +301,37 @@ > '*out': 'AudiodevPaPerDirectionOptions', > '*server': 'str' } } > > +## > +# @AudiodevSdlPerDirectionOptions: > +# > +# Options of the SDL audio backend that are used for both playback and > +# recording. > +# > +# @buffer-count: number of buffers (default 4) > +# > +# Since: 6.0 > +## > +{ 'struct': 'AudiodevSdlPerDirectionOptions', > + 'base': 'AudiodevPerDirectionOptions', > + 'data': { > + '*buffer-count': 'uint32' } } > + > +## > +# @AudiodevSdlOptions: > +# > +# Options of the SDL audio backend. > +# > +# @in: options of the recording stream > +# > +# @out: options of the playback stream > +# > +# Since: 6.0 > +## > +{ 'struct': 'AudiodevSdlOptions', > + 'data': { > + '*in': 'AudiodevSdlPerDirectionOptions', > + '*out': 'AudiodevSdlPerDirectionOptions' } } > + > ## > # @AudiodevWavOptions: > # > @@ -385,6 +416,6 @@ > 'jack': 'AudiodevJackOptions', > 'oss': 'AudiodevOssOptions', > 'pa': 'AudiodevPaOptions', > - 'sdl': 'AudiodevGenericOptions', > + 'sdl': 'AudiodevSdlOptions', > 'spice': 'AudiodevGenericOptions', > 'wav': 'AudiodevWavOptions' } } > diff --git a/qemu-options.hx b/qemu-options.hx > index 1698a0c751..4e02e9bd76 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -588,6 +588,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev, > #endif > #ifdef CONFIG_AUDIO_SDL > "-audiodev sdl,id=id[,prop[=value][,...]]\n" > + " in|out.buffer-count= number of buffers\n" > #endif > #ifdef CONFIG_SPICE > "-audiodev spice,id=id[,prop[=value][,...]]\n" > @@ -745,7 +746,12 @@ SRST > ``-audiodev sdl,id=id[,prop[=value][,...]]`` > Creates a backend using SDL. This backend is available on most > systems, but you should use your platform's native backend if > - possible. This backend has no backend specific properties. > + possible. > + > + SDL specific options are: > + > + ``in|out.buffer-count=count`` > + Sets the count of the buffers. > > ``-audiodev spice,id=id[,prop[=value][,...]]`` > Creates a backend that sends audio through SPICE. This backend These parts: Acked-by: Markus Armbruster <armbru@redhat.com>
> Hi, > >> - hw->samples = obt.samples; >> + hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) * >> + obt.samples; >> +# @buffer-count: number of buffers (default 4) > Any specific reason for this default? > > In my testing I've needed much higher values. > 8 still got me crackling sound, 16 worked ok. Hi Gerd, this was an attempt to come up with SDL audio settings which work for all SDL audio drivers. Unfortunately, the different SDL audio drivers have different timings and there are no default settings that work for all of them. Here are two examples where buffer-count=4 works. On my Linux system I use export SDL_AUDIODRIVER=pulse and start qemu with -device intel-hda -device hda-duplex,audiodev=audio0 -machine pcspk-audiodev=audio0 -audiodev sdl,id=audio0,out.buffer-length=3750 Due to the mix-up of samples and frames in audio/sdlaudio.c the callback buffer has a size of 2 * 3.75ms = 7.5ms and SDL calls the callback function every 7.5ms. With out.buffer-count=4 that's a 4 * 7.5ms = 30ms buffer on the qemu side. This is larger than the minimum size of timer-period. On Windows the timing is different. The time between SDL callback calls is a multiple of 10ms. I have to use export SDL_AUDIODRIVER=directsound and start qemu with -device intel-hda -device hda-duplex,audiodev=audio0 -machine pcspk-audiodev=audio0 -audiodev sdl,id=audio0,timer-period=1000,out.buffer-length=5500 With the above settings the playback stream sometimes will see 2*10ms + 1ms stalls. The qemu hda codec can barely handle this. On average it will drop playback data after 23.22ms. With best regards, Volker > take care, > Gerd >
diff --git a/audio/audio.c b/audio/audio.c index b48471bb3f..d048d26283 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -2003,7 +2003,7 @@ void audio_create_pdos(Audiodev *dev) CASE(JACK, jack, Jack); CASE(OSS, oss, Oss); CASE(PA, pa, Pa); - CASE(SDL, sdl, ); + CASE(SDL, sdl, Sdl); CASE(SPICE, spice, ); CASE(WAV, wav, ); diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c index ffdbd0bcce..0fe827b057 100644 --- a/audio/audio_legacy.c +++ b/audio/audio_legacy.c @@ -286,7 +286,8 @@ static void handle_sdl(Audiodev *dev) { /* SDL is output only */ get_samples_to_usecs("QEMU_SDL_SAMPLES", &dev->u.sdl.out->buffer_length, - &dev->u.sdl.out->has_buffer_length, dev->u.sdl.out); + &dev->u.sdl.out->has_buffer_length, + qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.out)); } /* wav */ diff --git a/audio/audio_template.h b/audio/audio_template.h index 8dd48ce14e..434df5d5e7 100644 --- a/audio/audio_template.h +++ b/audio/audio_template.h @@ -337,7 +337,7 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev) case AUDIODEV_DRIVER_PA: return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE); case AUDIODEV_DRIVER_SDL: - return dev->u.sdl.TYPE; + return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE); case AUDIODEV_DRIVER_SPICE: return dev->u.spice.TYPE; case AUDIODEV_DRIVER_WAV: diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c index 00cd12ba66..431bfcfddd 100644 --- a/audio/sdlaudio.c +++ b/audio/sdlaudio.c @@ -276,12 +276,18 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as, int endianness; int err; AudioFormat effective_fmt; + AudiodevSdlPerDirectionOptions *spdo = s->dev->u.sdl.out; struct audsettings obt_as; req.freq = as->freq; req.format = aud_to_sdlfmt (as->fmt); req.channels = as->nchannels; - req.samples = audio_buffer_samples(s->dev->u.sdl.out, as, 11610); + /* + * This is wrong. SDL samples are QEMU frames. The buffer size will be + * the requested buffer size multiplied by the number of channels. + */ + req.samples = audio_buffer_samples( + qapi_AudiodevSdlPerDirectionOptions_base(spdo), as, 11610); req.callback = sdl_callback; req.userdata = sdl; @@ -301,7 +307,8 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as, obt_as.endianness = endianness; audio_pcm_init_info (&hw->info, &obt_as); - hw->samples = obt.samples; + hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) * + obt.samples; s->initialized = 1; s->exit = 0; diff --git a/qapi/audio.json b/qapi/audio.json index 072ed79def..9cba0df8a4 100644 --- a/qapi/audio.json +++ b/qapi/audio.json @@ -301,6 +301,37 @@ '*out': 'AudiodevPaPerDirectionOptions', '*server': 'str' } } +## +# @AudiodevSdlPerDirectionOptions: +# +# Options of the SDL audio backend that are used for both playback and +# recording. +# +# @buffer-count: number of buffers (default 4) +# +# Since: 6.0 +## +{ 'struct': 'AudiodevSdlPerDirectionOptions', + 'base': 'AudiodevPerDirectionOptions', + 'data': { + '*buffer-count': 'uint32' } } + +## +# @AudiodevSdlOptions: +# +# Options of the SDL audio backend. +# +# @in: options of the recording stream +# +# @out: options of the playback stream +# +# Since: 6.0 +## +{ 'struct': 'AudiodevSdlOptions', + 'data': { + '*in': 'AudiodevSdlPerDirectionOptions', + '*out': 'AudiodevSdlPerDirectionOptions' } } + ## # @AudiodevWavOptions: # @@ -385,6 +416,6 @@ 'jack': 'AudiodevJackOptions', 'oss': 'AudiodevOssOptions', 'pa': 'AudiodevPaOptions', - 'sdl': 'AudiodevGenericOptions', + 'sdl': 'AudiodevSdlOptions', 'spice': 'AudiodevGenericOptions', 'wav': 'AudiodevWavOptions' } } diff --git a/qemu-options.hx b/qemu-options.hx index 1698a0c751..4e02e9bd76 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -588,6 +588,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev, #endif #ifdef CONFIG_AUDIO_SDL "-audiodev sdl,id=id[,prop[=value][,...]]\n" + " in|out.buffer-count= number of buffers\n" #endif #ifdef CONFIG_SPICE "-audiodev spice,id=id[,prop[=value][,...]]\n" @@ -745,7 +746,12 @@ SRST ``-audiodev sdl,id=id[,prop[=value][,...]]`` Creates a backend using SDL. This backend is available on most systems, but you should use your platform's native backend if - possible. This backend has no backend specific properties. + possible. + + SDL specific options are: + + ``in|out.buffer-count=count`` + Sets the count of the buffers. ``-audiodev spice,id=id[,prop[=value][,...]]`` Creates a backend that sends audio through SPICE. This backend
Currently there is a crackling noise with SDL2 audio playback. Commit bcf19777df: "audio/sdlaudio: Allow audio playback with SDL2" already mentioned the crackling noise. Add an out.buffer-count option to give users a chance to select sane settings for glitch free audio playback. The idea was taken from the coreaudio backend. The in.buffer-count option will be used with one of the next patches. Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> --- audio/audio.c | 2 +- audio/audio_legacy.c | 3 ++- audio/audio_template.h | 2 +- audio/sdlaudio.c | 11 +++++++++-- qapi/audio.json | 33 ++++++++++++++++++++++++++++++++- qemu-options.hx | 8 +++++++- 6 files changed, 52 insertions(+), 7 deletions(-)