Re: [PATCH] audio: allow spice buffer_length to be adjusted

[Volker Rümelin]

Hi,

> Spice clients that are running directly on the host system have
> pratcially unlimited bandwidth so to reduce latency allow the user to
> configure the buffer_length to a lower value if desired.
>
> While virt-viewer can not take advantage of this, the PureSpice [1]
> library used by Looking Glass [2] is able to produce and consume audio
> at these rates, combined with the merge request for spice-server [3]
> this allows for latencies close to realtime.
>
> [1]https://github.com/gnif/PureSpice
> [2]https://github.com/gnif/LookingGlass
> [3]https://gitlab.freedesktop.org/spice/spice/-/merge_requests/199
>
> Signed-off-by: Geoffrey McRae<geoff@hostfission.com>
> ---
>   audio/spiceaudio.c | 19 ++++++++++++++++---
>   1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
> index a8d370fe6f..0c44bbe836 100644
> --- a/audio/spiceaudio.c
> +++ b/audio/spiceaudio.c
> @@ -76,7 +76,7 @@ static void *spice_audio_init(Audiodev *dev)
>       if (!using_spice) {
>           return NULL;
>       }
> -    return &spice_audio_init;
> +    return dev;
>   }
>   
>   static void spice_audio_fini (void *opaque)
> @@ -90,6 +90,8 @@ static int line_out_init(HWVoiceOut *hw, struct audsettings 
> *as,
>                            void *drv_opaque)
>   {
>       SpiceVoiceOut *out = container_of (hw, SpiceVoiceOut, hw);
> +    Audiodev      *dev = (Audiodev *)drv_opaque;
> +
>       struct audsettings settings;
>   
>   #if SPICE_INTERFACE_PLAYBACK_MAJOR > 1 || SPICE_INTERFACE_PLAYBACK_MINOR >= 3
> @@ -102,7 +104,12 @@ static int line_out_init(HWVoiceOut *hw, struct 
> audsettings *as,
>       settings.endianness = AUDIO_HOST_ENDIANNESS;
>   
>       audio_pcm_init_info (&hw->info, &settings);
> -    hw->samples = LINE_OUT_SAMPLES;
> +    if (dev->u.none.out->has_buffer_length) {
> +        hw->samples = audio_buffer_samples(dev->u.none.out, &settings, 10000);

hw->samples counts in frames. The buffer is twice as large as expected.

+        hw->samples = audio_buffer_frames(dev->u.none.out, &settings, 
10000);

I'm aware the default size of 10000us will not be used, but it's a bad 
example because with a default timer-period of 10000us the buffer has to 
be a few percent larger than timer-period. Otherwise the emulated audio 
device will never catch up if a AUD_write() has been delayed.

> +    } else {
> +        hw->samples = LINE_OUT_SAMPLES;
> +    }
> +
>       out->active = 0;
>   
>       out->sin.base.sif = &playback_sif.base;
> @@ -199,6 +206,7 @@ static void line_out_volume(HWVoiceOut *hw, Volume *vol)
>   static int line_in_init(HWVoiceIn *hw, struct audsettings *as, void 
> *drv_opaque)
>   {
>       SpiceVoiceIn *in = container_of (hw, SpiceVoiceIn, hw);
> +    Audiodev     *dev = (Audiodev *)drv_opaque;
>       struct audsettings settings;
>   
>   #if SPICE_INTERFACE_RECORD_MAJOR > 2 || SPICE_INTERFACE_RECORD_MINOR >= 3
> @@ -211,7 +219,12 @@ static int line_in_init(HWVoiceIn *hw, struct audsettings 
> *as, void *drv_opaque)
>       settings.endianness = AUDIO_HOST_ENDIANNESS;
>   
>       audio_pcm_init_info (&hw->info, &settings);
> -    hw->samples = LINE_IN_SAMPLES;
> +    if (dev->u.none.out->has_buffer_length) {
> +        hw->samples = audio_buffer_samples(dev->u.none.in, &settings, 10000);

-        hw->samples = audio_buffer_samples(dev->u.none.in, &settings, 
10000);
+        hw->samples = audio_buffer_frames(dev->u.none.in, &settings, 
10000);

> +    } else {
> +        hw->samples = LINE_IN_SAMPLES;
> +    }
> +
>       in->active = 0;
>   
>       in->sin.base.sif = &record_sif.base;

Btw. have you seen my "[PATCH 00/15] reduce audio playback latency" 
patch series at 
https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg00780.html? 
I haven't tested, but I think it's possible to add a buffer_get_free 
function to audio/spiceaudio.c. That would eliminate the need to 
fine-tune the audio buffer length.

With best regards,
Volker