[PATCH] audio: allow spice buffer_length to be adjusted

Geoffrey McRae posted 1 patch 2 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220109033332.402609-1-geoff@hostfission.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
audio/spiceaudio.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
[PATCH] audio: allow spice buffer_length to be adjusted
Posted by Geoffrey McRae 2 years, 3 months ago
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);
+    } 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);
+    } else {
+        hw->samples = LINE_IN_SAMPLES;
+    }
+
     in->active = 0;
 
     in->sin.base.sif = &record_sif.base;
-- 
2.30.2


Re: [PATCH] audio: allow spice buffer_length to be adjusted
Posted by Volker Rümelin 2 years, 3 months ago
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