audio/audio.c | 3 + audio/audio_template.h | 4 + audio/meson.build | 1 + audio/pwaudio.c | 814 ++++++++++++++++++++++++++++++++++ audio/trace-events | 7 + meson.build | 8 + meson_options.txt | 4 +- qapi/audio.json | 45 ++ qemu-options.hx | 17 + scripts/meson-buildoptions.sh | 8 +- 10 files changed, 908 insertions(+), 3 deletions(-) create mode 100644 audio/pwaudio.c
This commit adds a new audiodev backend to allow QEMU to use Pipewire as
both an audio sink and source. This backend is available on most systems
Add Pipewire entry points for QEMU Pipewire audio backend
Add wrappers for QEMU Pipewire audio backend in qpw_pcm_ops()
qpw_write function returns the current state of the stream to pwaudio
and Writes some data to the server for playback streams using pipewire
spa_ringbuffer implementation.
qpw_read function returns the current state of the stream to pwaudio and
reads some data from the server for capture streams using pipewire
spa_ringbuffer implementation. These functions qpw_write and qpw_read
are called during playback and capture.
Added some functions that convert pw audio formats to QEMU audio format
and vice versa which would be needed in the pipewire audio sink and
source functions qpw_init_in() & qpw_init_out().
These methods that implement playback and recording will create streams
for playback and capture that will start processing and will result in
the on_process callbacks to be called.
Built a connection to the Pipewire sound system server in the
qpw_audio_init() method.
Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
---
v7:
use qemu tracing tool
audio/audio.c | 3 +
audio/audio_template.h | 4 +
audio/meson.build | 1 +
audio/pwaudio.c | 814 ++++++++++++++++++++++++++++++++++
audio/trace-events | 7 +
meson.build | 8 +
meson_options.txt | 4 +-
qapi/audio.json | 45 ++
qemu-options.hx | 17 +
scripts/meson-buildoptions.sh | 8 +-
10 files changed, 908 insertions(+), 3 deletions(-)
create mode 100644 audio/pwaudio.c
diff --git a/audio/audio.c b/audio/audio.c
index 4290309d18..aa55e41ad8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2069,6 +2069,9 @@ void audio_create_pdos(Audiodev *dev)
#ifdef CONFIG_AUDIO_PA
CASE(PA, pa, Pa);
#endif
+#ifdef CONFIG_AUDIO_PIPEWIRE
+ CASE(PIPEWIRE, pipewire, Pipewire);
+#endif
#ifdef CONFIG_AUDIO_SDL
CASE(SDL, sdl, Sdl);
#endif
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 42b4712acb..0f02afb921 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -355,6 +355,10 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
case AUDIODEV_DRIVER_PA:
return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
#endif
+#ifdef CONFIG_AUDIO_PIPEWIRE
+ case AUDIODEV_DRIVER_PIPEWIRE:
+ return qapi_AudiodevPipewirePerDirectionOptions_base(dev->u.pipewire.TYPE);
+#endif
#ifdef CONFIG_AUDIO_SDL
case AUDIODEV_DRIVER_SDL:
return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
diff --git a/audio/meson.build b/audio/meson.build
index 0722224ba9..65a49c1a10 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -19,6 +19,7 @@ foreach m : [
['sdl', sdl, files('sdlaudio.c')],
['jack', jack, files('jackaudio.c')],
['sndio', sndio, files('sndioaudio.c')],
+ ['pipewire', pipewire, files('pwaudio.c')],
['spice', spice, files('spiceaudio.c')]
]
if m[1].found()
diff --git a/audio/pwaudio.c b/audio/pwaudio.c
new file mode 100644
index 0000000000..d357761152
--- /dev/null
+++ b/audio/pwaudio.c
@@ -0,0 +1,814 @@
+/*
+ * QEMU Pipewire audio driver
+ *
+ * Copyright (c) 2023 Red Hat Inc.
+ *
+ * Author: Dorinda Bassey <dbassey@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "audio.h"
+#include <errno.h>
+#include "qemu/error-report.h"
+#include <spa/param/audio/format-utils.h>
+#include <spa/utils/ringbuffer.h>
+#include <spa/utils/result.h>
+
+#include <pipewire/pipewire.h>
+#include "trace.h"
+
+#define AUDIO_CAP "pipewire"
+#define RINGBUFFER_SIZE (1u << 22)
+#define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
+#define BUFFER_SAMPLES 512
+
+#include "audio_int.h"
+
+enum {
+ MODE_SINK,
+ MODE_SOURCE
+};
+
+typedef struct pwaudio {
+ Audiodev *dev;
+ struct pw_thread_loop *thread_loop;
+ struct pw_context *context;
+
+ struct pw_core *core;
+ struct spa_hook core_listener;
+ int seq;
+} pwaudio;
+
+typedef struct PWVoice {
+ pwaudio *g;
+ bool enabled;
+ struct pw_stream *stream;
+ struct spa_hook stream_listener;
+ struct spa_audio_info_raw info;
+ uint32_t frame_size;
+ struct spa_ringbuffer ring;
+ uint8_t buffer[RINGBUFFER_SIZE];
+
+ uint32_t mode;
+ struct pw_properties *props;
+} PWVoice;
+
+typedef struct PWVoiceOut {
+ HWVoiceOut hw;
+ PWVoice v;
+} PWVoiceOut;
+
+typedef struct PWVoiceIn {
+ HWVoiceIn hw;
+ PWVoice v;
+} PWVoiceIn;
+
+static void
+stream_destroy(void *data)
+{
+ PWVoice *v = (PWVoice *) data;
+ spa_hook_remove(&v->stream_listener);
+ v->stream = NULL;
+}
+
+/* output data processing function to read stuffs from the buffer */
+static void
+playback_on_process(void *data)
+{
+ PWVoice *v = (PWVoice *) data;
+ void *p;
+ struct pw_buffer *b;
+ struct spa_buffer *buf;
+ uint32_t n_frames, req, index, n_bytes;
+ int32_t avail;
+
+ if (!v->stream) {
+ return;
+ }
+
+ /* obtain a buffer to read from */
+ b = pw_stream_dequeue_buffer(v->stream);
+ if (b == NULL) {
+ error_report("out of buffers: %s", strerror(errno));
+ return;
+ }
+
+ buf = b->buffer;
+ p = buf->datas[0].data;
+ if (p == NULL) {
+ return;
+ }
+ req = b->requested * v->frame_size;
+ if (req == 0) {
+ req = 4096 * v->frame_size;
+ }
+ n_frames = SPA_MIN(req, buf->datas[0].maxsize);
+ n_bytes = n_frames * v->frame_size;
+
+ /* get no of available bytes to read data from buffer */
+
+ avail = spa_ringbuffer_get_read_index(&v->ring, &index);
+
+ if (!v->enabled) {
+ avail = 0;
+ }
+
+ if (avail == 0) {
+ memset(p, 0, n_bytes);
+ } else {
+ if (avail < (int32_t) n_bytes) {
+ n_bytes = avail;
+ }
+
+ spa_ringbuffer_read_data(&v->ring,
+ v->buffer, RINGBUFFER_SIZE,
+ index & RINGBUFFER_MASK, p, n_bytes);
+
+ index += n_bytes;
+ spa_ringbuffer_read_update(&v->ring, index);
+ }
+
+ buf->datas[0].chunk->offset = 0;
+ buf->datas[0].chunk->stride = v->frame_size;
+ buf->datas[0].chunk->size = n_bytes;
+
+ /* queue the buffer for playback */
+ pw_stream_queue_buffer(v->stream, b);
+}
+
+/* output data processing function to generate stuffs in the buffer */
+static void
+capture_on_process(void *data)
+{
+ PWVoice *v = (PWVoice *) data;
+ void *p;
+ struct pw_buffer *b;
+ struct spa_buffer *buf;
+ int32_t filled;
+ uint32_t index, offs, n_bytes;
+
+ if (!v->stream) {
+ return;
+ }
+
+ /* obtain a buffer */
+ b = pw_stream_dequeue_buffer(v->stream);
+ if (b == NULL) {
+ error_report("out of buffers: %s", strerror(errno));
+ return;
+ }
+
+ /* Write data into buffer */
+ buf = b->buffer;
+ p = buf->datas[0].data;
+ if (p == NULL) {
+ return;
+ }
+ offs = SPA_MIN(buf->datas[0].chunk->offset, buf->datas[0].maxsize);
+ n_bytes = SPA_MIN(buf->datas[0].chunk->size, buf->datas[0].maxsize - offs);
+
+ filled = spa_ringbuffer_get_write_index(&v->ring, &index);
+
+ if (!v->enabled) {
+ n_bytes = 0;
+ }
+
+ if (filled < 0) {
+ error_report("%p: underrun write:%u filled:%d", p, index, filled);
+ } else {
+ if ((uint32_t) filled + n_bytes > RINGBUFFER_SIZE) {
+ error_report("%p: overrun write:%u filled:%d + size:%u > max:%u",
+ p, index, filled, n_bytes, RINGBUFFER_SIZE);
+ }
+ }
+ spa_ringbuffer_write_data(&v->ring,
+ v->buffer, RINGBUFFER_SIZE,
+ index & RINGBUFFER_MASK,
+ SPA_PTROFF(p, offs, void), n_bytes);
+ index += n_bytes;
+ spa_ringbuffer_write_update(&v->ring, index);
+
+ /* queue the buffer for playback */
+ pw_stream_queue_buffer(v->stream, b);
+}
+
+static void
+on_stream_state_changed(void *_data, enum pw_stream_state old,
+ enum pw_stream_state state, const char *error)
+{
+ PWVoice *v = (PWVoice *) _data;
+
+ trace_pw_state_changed(pw_stream_state_as_string(state));
+
+ switch (state) {
+ case PW_STREAM_STATE_ERROR:
+ case PW_STREAM_STATE_UNCONNECTED:
+ {
+ break;
+ }
+ case PW_STREAM_STATE_PAUSED:
+ trace_pw_node(pw_stream_get_node_id(v->stream));
+ break;
+ case PW_STREAM_STATE_CONNECTING:
+ case PW_STREAM_STATE_STREAMING:
+ break;
+ }
+}
+
+static const struct pw_stream_events capture_stream_events = {
+ PW_VERSION_STREAM_EVENTS,
+ .destroy = stream_destroy,
+ .state_changed = on_stream_state_changed,
+ .process = capture_on_process
+};
+
+static const struct pw_stream_events playback_stream_events = {
+ PW_VERSION_STREAM_EVENTS,
+ .destroy = stream_destroy,
+ .state_changed = on_stream_state_changed,
+ .process = playback_on_process
+};
+
+static size_t
+qpw_read(HWVoiceIn *hw, void *data, size_t len)
+{
+ PWVoiceIn *pw = (PWVoiceIn *) hw;
+ PWVoice *v = &pw->v;
+ pwaudio *c = v->g;
+ const char *error = NULL;
+ size_t l;
+ int32_t avail;
+ uint32_t index;
+
+ pw_thread_loop_lock(c->thread_loop);
+ if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
+ /* wait for stream to become ready */
+ l = 0;
+ goto done_unlock;
+ }
+ /* get no of available bytes to read data from buffer */
+ avail = spa_ringbuffer_get_read_index(&v->ring, &index);
+
+ trace_pw_read(avail, index, len);
+
+ if (avail < (int32_t) len) {
+ len = avail;
+ }
+
+ spa_ringbuffer_read_data(&v->ring,
+ v->buffer, RINGBUFFER_SIZE,
+ index & RINGBUFFER_MASK, data, len);
+ index += len;
+ spa_ringbuffer_read_update(&v->ring, index);
+ l = len;
+
+done_unlock:
+ pw_thread_loop_unlock(c->thread_loop);
+ return l;
+}
+
+static size_t
+qpw_write(HWVoiceOut *hw, void *data, size_t len)
+{
+ PWVoiceOut *pw = (PWVoiceOut *) hw;
+ PWVoice *v = &pw->v;
+ pwaudio *c = v->g;
+ const char *error = NULL;
+ const int periods = 3;
+ size_t l;
+ int32_t filled, avail;
+ uint32_t index;
+
+ pw_thread_loop_lock(c->thread_loop);
+ if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
+ /* wait for stream to become ready */
+ l = 0;
+ goto done_unlock;
+ }
+ filled = spa_ringbuffer_get_write_index(&v->ring, &index);
+
+ avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
+
+ trace_pw_write(filled, avail, index, len);
+
+ if (len > avail) {
+ len = avail;
+ }
+
+ if (filled < 0) {
+ error_report("%p: underrun write:%u filled:%d", pw, index, filled);
+ } else {
+ if ((uint32_t) filled + len > RINGBUFFER_SIZE) {
+ error_report("%p: overrun write:%u filled:%d + size:%zu > max:%u",
+ pw, index, filled, len, RINGBUFFER_SIZE);
+ }
+ }
+
+ spa_ringbuffer_write_data(&v->ring,
+ v->buffer, RINGBUFFER_SIZE,
+ index & RINGBUFFER_MASK, data, len);
+ index += len;
+ spa_ringbuffer_write_update(&v->ring, index);
+ l = len;
+
+done_unlock:
+ pw_thread_loop_unlock(c->thread_loop);
+ return l;
+}
+
+static int
+audfmt_to_pw(AudioFormat fmt, int endianness)
+{
+ int format;
+
+ switch (fmt) {
+ case AUDIO_FORMAT_S8:
+ format = SPA_AUDIO_FORMAT_S8;
+ break;
+ case AUDIO_FORMAT_U8:
+ format = SPA_AUDIO_FORMAT_U8;
+ break;
+ case AUDIO_FORMAT_S16:
+ format = endianness ? SPA_AUDIO_FORMAT_S16_BE : SPA_AUDIO_FORMAT_S16_LE;
+ break;
+ case AUDIO_FORMAT_U16:
+ format = endianness ? SPA_AUDIO_FORMAT_U16_BE : SPA_AUDIO_FORMAT_U16_LE;
+ break;
+ case AUDIO_FORMAT_S32:
+ format = endianness ? SPA_AUDIO_FORMAT_S32_BE : SPA_AUDIO_FORMAT_S32_LE;
+ break;
+ case AUDIO_FORMAT_U32:
+ format = endianness ? SPA_AUDIO_FORMAT_U32_BE : SPA_AUDIO_FORMAT_U32_LE;
+ break;
+ case AUDIO_FORMAT_F32:
+ format = endianness ? SPA_AUDIO_FORMAT_F32_BE : SPA_AUDIO_FORMAT_F32_LE;
+ break;
+ default:
+ dolog("Internal logic error: Bad audio format %d\n", fmt);
+ format = SPA_AUDIO_FORMAT_U8;
+ break;
+ }
+ return format;
+}
+
+static AudioFormat
+pw_to_audfmt(enum spa_audio_format fmt, int *endianness,
+ uint32_t *frame_size)
+{
+ switch (fmt) {
+ case SPA_AUDIO_FORMAT_S8:
+ *frame_size = 1;
+ return AUDIO_FORMAT_S8;
+ case SPA_AUDIO_FORMAT_U8:
+ *frame_size = 1;
+ return AUDIO_FORMAT_U8;
+ case SPA_AUDIO_FORMAT_S16_BE:
+ *frame_size = 2;
+ *endianness = 1;
+ return AUDIO_FORMAT_S16;
+ case SPA_AUDIO_FORMAT_S16_LE:
+ *frame_size = 2;
+ *endianness = 0;
+ return AUDIO_FORMAT_S16;
+ case SPA_AUDIO_FORMAT_U16_BE:
+ *frame_size = 2;
+ *endianness = 1;
+ return AUDIO_FORMAT_U16;
+ case SPA_AUDIO_FORMAT_U16_LE:
+ *frame_size = 2;
+ *endianness = 0;
+ return AUDIO_FORMAT_U16;
+ case SPA_AUDIO_FORMAT_S32_BE:
+ *frame_size = 4;
+ *endianness = 1;
+ return AUDIO_FORMAT_S32;
+ case SPA_AUDIO_FORMAT_S32_LE:
+ *frame_size = 4;
+ *endianness = 0;
+ return AUDIO_FORMAT_S32;
+ case SPA_AUDIO_FORMAT_U32_BE:
+ *frame_size = 4;
+ *endianness = 1;
+ return AUDIO_FORMAT_U32;
+ case SPA_AUDIO_FORMAT_U32_LE:
+ *frame_size = 4;
+ *endianness = 0;
+ return AUDIO_FORMAT_U32;
+ case SPA_AUDIO_FORMAT_F32_BE:
+ *frame_size = 4;
+ *endianness = 1;
+ return AUDIO_FORMAT_F32;
+ case SPA_AUDIO_FORMAT_F32_LE:
+ *frame_size = 4;
+ *endianness = 0;
+ return AUDIO_FORMAT_F32;
+ default:
+ *frame_size = 1;
+ dolog("Internal logic error: Bad spa_audio_format %d\n", fmt);
+ return AUDIO_FORMAT_U8;
+ }
+}
+
+static int
+create_stream(pwaudio *c, PWVoice *v, const char *name)
+{
+ int res;
+ uint32_t n_params;
+ const struct spa_pod *params[2];
+ uint8_t buffer[1024];
+ struct spa_pod_builder b;
+
+ v->stream = pw_stream_new(c->core, name, NULL);
+
+ if (v->stream == NULL) {
+ goto error;
+ }
+
+ if (v->mode == MODE_SOURCE) {
+ pw_stream_add_listener(v->stream,
+ &v->stream_listener, &capture_stream_events, v);
+ } else {
+ pw_stream_add_listener(v->stream,
+ &v->stream_listener, &playback_stream_events, v);
+ }
+
+ n_params = 0;
+ spa_pod_builder_init(&b, buffer, sizeof(buffer));
+ params[n_params++] = spa_format_audio_raw_build(&b,
+ SPA_PARAM_EnumFormat,
+ &v->info);
+
+ /* connect the stream to a sink or source */
+ res = pw_stream_connect(v->stream,
+ v->mode ==
+ MODE_SOURCE ? PW_DIRECTION_INPUT :
+ PW_DIRECTION_OUTPUT, PW_ID_ANY,
+ PW_STREAM_FLAG_AUTOCONNECT |
+ PW_STREAM_FLAG_MAP_BUFFERS |
+ PW_STREAM_FLAG_RT_PROCESS, params, n_params);
+ if (res < 0) {
+ goto error;
+ }
+
+ return 0;
+error:
+ pw_stream_destroy(v->stream);
+ return -1;
+}
+
+static int
+qpw_stream_new(pwaudio *c, PWVoice *v, const char *name)
+{
+ int r;
+
+ switch (v->info.channels) {
+ case 8:
+ v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
+ v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
+ v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
+ v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
+ v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
+ v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
+ v->info.position[6] = SPA_AUDIO_CHANNEL_SL;
+ v->info.position[7] = SPA_AUDIO_CHANNEL_SR;
+ break;
+ case 6:
+ v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
+ v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
+ v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
+ v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
+ v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
+ v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
+ break;
+ case 5:
+ v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
+ v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
+ v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
+ v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
+ v->info.position[4] = SPA_AUDIO_CHANNEL_RC;
+ break;
+ case 4:
+ v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
+ v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
+ v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
+ v->info.position[3] = SPA_AUDIO_CHANNEL_RC;
+ break;
+ case 3:
+ v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
+ v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
+ v->info.position[2] = SPA_AUDIO_CHANNEL_LFE;
+ break;
+ case 2:
+ v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
+ v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
+ break;
+ case 1:
+ v->info.position[0] = SPA_AUDIO_CHANNEL_MONO;
+ break;
+ default:
+ for (size_t i = 0; i < v->info.channels; i++) {
+ v->info.position[i] = SPA_AUDIO_CHANNEL_UNKNOWN;
+ }
+ break;
+ }
+
+ /* create a new unconnected pwstream */
+ r = create_stream(c, v, name);
+ if (r < 0) {
+ goto error;
+ }
+
+ return r;
+
+error:
+ AUD_log(AUDIO_CAP, "Failed to create stream.");
+ return -1;
+}
+
+static int
+qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
+{
+ PWVoiceOut *pw = (PWVoiceOut *) hw;
+ PWVoice *v = &pw->v;
+ struct audsettings obt_as = *as;
+ pwaudio *c = v->g = drv_opaque;
+ AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
+ AudiodevPipewirePerDirectionOptions *ppdo = popts->out;
+ int r;
+ v->enabled = false;
+
+ v->mode = MODE_SINK;
+
+ pw_thread_loop_lock(c->thread_loop);
+
+ v->info.format = audfmt_to_pw(as->fmt, as->endianness);
+ v->info.channels = as->nchannels;
+ v->info.rate = as->freq;
+
+ obt_as.fmt =
+ pw_to_audfmt(v->info.format, &obt_as.endianness, &v->frame_size);
+ v->frame_size *= as->nchannels;
+
+ /* call the function that creates a new stream for playback */
+ r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
+ if (r < 0) {
+ error_report("qpw_stream_new for playback failed\n ");
+ goto fail;
+ }
+
+ /* report the audio format we support */
+ audio_pcm_init_info(&hw->info, &obt_as);
+
+ /* report the buffer size to qemu */
+ hw->samples = BUFFER_SAMPLES;
+
+ pw_thread_loop_unlock(c->thread_loop);
+ return 0;
+fail:
+ pw_thread_loop_unlock(c->thread_loop);
+ return -1;
+}
+
+static int
+qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
+{
+ PWVoiceIn *pw = (PWVoiceIn *) hw;
+ PWVoice *v = &pw->v;
+ struct audsettings obt_as = *as;
+ pwaudio *c = v->g = drv_opaque;
+ AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
+ AudiodevPipewirePerDirectionOptions *ppdo = popts->in;
+ int r;
+ v->enabled = false;
+
+ v->mode = MODE_SOURCE;
+ pw_thread_loop_lock(c->thread_loop);
+
+ v->info.format = audfmt_to_pw(as->fmt, as->endianness);
+ v->info.channels = as->nchannels;
+ v->info.rate = as->freq;
+
+ obt_as.fmt =
+ pw_to_audfmt(v->info.format, &obt_as.endianness, &v->frame_size);
+ v->frame_size *= as->nchannels;
+
+ /* call the function that creates a new stream for recording */
+ r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
+ if (r < 0) {
+ error_report("qpw_stream_new for recording failed\n ");
+ goto fail;
+ }
+
+ /* report the audio format we support */
+ audio_pcm_init_info(&hw->info, &obt_as);
+
+ /* report the buffer size to qemu */
+ hw->samples = BUFFER_SAMPLES;
+
+ pw_thread_loop_unlock(c->thread_loop);
+ return 0;
+fail:
+ pw_thread_loop_unlock(c->thread_loop);
+ return -1;
+}
+
+static void
+qpw_fini_out(HWVoiceOut *hw)
+{
+ PWVoiceOut *pw = (PWVoiceOut *) hw;
+ PWVoice *v = &pw->v;
+
+ if (v->stream) {
+ pwaudio *c = v->g;
+ pw_thread_loop_lock(c->thread_loop);
+ pw_stream_destroy(v->stream);
+ v->stream = NULL;
+ pw_thread_loop_unlock(c->thread_loop);
+ }
+}
+
+static void
+qpw_fini_in(HWVoiceIn *hw)
+{
+ PWVoiceIn *pw = (PWVoiceIn *) hw;
+ PWVoice *v = &pw->v;
+
+ if (v->stream) {
+ pwaudio *c = v->g;
+ pw_thread_loop_lock(c->thread_loop);
+ pw_stream_destroy(v->stream);
+ v->stream = NULL;
+ pw_thread_loop_unlock(c->thread_loop);
+ }
+}
+
+static void
+qpw_enable_out(HWVoiceOut *hw, bool enable)
+{
+ PWVoiceOut *po = (PWVoiceOut *) hw;
+ PWVoice *v = &po->v;
+ v->enabled = enable;
+}
+
+static void
+qpw_enable_in(HWVoiceIn *hw, bool enable)
+{
+ PWVoiceIn *pi = (PWVoiceIn *) hw;
+ PWVoice *v = &pi->v;
+ v->enabled = enable;
+}
+
+static void
+on_core_error(void *data, uint32_t id, int seq, int res, const char *message)
+{
+ pwaudio *pw = data;
+
+ error_report("error id:%u seq:%d res:%d (%s): %s",
+ id, seq, res, spa_strerror(res), message);
+
+ pw_thread_loop_signal(pw->thread_loop, FALSE);
+}
+
+static void
+on_core_done(void *data, uint32_t id, int seq)
+{
+ pwaudio *pw = data;
+ if (id == PW_ID_CORE) {
+ pw->seq = seq;
+ pw_thread_loop_signal(pw->thread_loop, FALSE);
+ }
+}
+
+static const struct pw_core_events core_events = {
+ PW_VERSION_CORE_EVENTS,
+ .done = on_core_done,
+ .error = on_core_error,
+};
+
+static void *
+qpw_audio_init(Audiodev *dev)
+{
+ pwaudio *pw;
+ pw = g_new0(pwaudio, 1);
+ pw_init(NULL, NULL);
+
+ AudiodevPipewireOptions *popts;
+ trace_pw_audio_init("Initialize Pipewire context\n");
+ assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
+ popts = &dev->u.pipewire;
+
+ if (!popts->has_latency) {
+ popts->has_latency = true;
+ popts->latency = 15000;
+ }
+
+ pw->dev = dev;
+ pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
+ if (pw->thread_loop == NULL) {
+ error_report("Could not create Pipewire loop");
+ goto fail_loop;
+ }
+
+ pw->context =
+ pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL, 0);
+ if (pw->context == NULL) {
+ error_report("Could not create Pipewire context");
+ goto fail_loop;
+ }
+
+ if (pw_thread_loop_start(pw->thread_loop) < 0) {
+ error_report("Could not start Pipewire loop");
+ goto fail_start;
+ }
+
+ pw_thread_loop_lock(pw->thread_loop);
+
+ pw->core = pw_context_connect(pw->context, NULL, 0);
+ if (pw->core == NULL) {
+ goto fail_conn;
+ }
+
+ pw_core_add_listener(pw->core, &pw->core_listener, &core_events, pw);
+
+ pw_thread_loop_unlock(pw->thread_loop);
+
+ return pw;
+
+fail_loop:
+ pw_thread_loop_destroy(pw->thread_loop);
+ g_free(pw);
+ return NULL;
+fail_start:
+ pw_context_destroy(pw->context);
+ g_free(pw);
+ return NULL;
+fail_conn:
+ AUD_log(AUDIO_CAP, "Failed to initialize PW context");
+ pw_thread_loop_unlock(pw->thread_loop);
+ pw_thread_loop_stop(pw->thread_loop);
+ pw_core_disconnect(pw->core);
+ pw_context_destroy(pw->context);
+ pw_thread_loop_destroy(pw->thread_loop);
+ g_free(pw);
+ return NULL;
+}
+
+static void
+qpw_audio_fini(void *opaque)
+{
+ pwaudio *pw = opaque;
+
+ pw_thread_loop_stop(pw->thread_loop);
+
+ if (pw->core) {
+ spa_hook_remove(&pw->core_listener);
+ spa_zero(pw->core_listener);
+ pw_core_disconnect(pw->core);
+ }
+
+ if (pw->context) {
+ pw_context_destroy(pw->context);
+ }
+ pw_thread_loop_destroy(pw->thread_loop);
+
+ g_free(pw);
+}
+
+static struct audio_pcm_ops qpw_pcm_ops = {
+ .init_out = qpw_init_out,
+ .fini_out = qpw_fini_out,
+ .write = qpw_write,
+ .buffer_get_free = audio_generic_buffer_get_free,
+ .run_buffer_out = audio_generic_run_buffer_out,
+ .enable_out = qpw_enable_out,
+
+ .init_in = qpw_init_in,
+ .fini_in = qpw_fini_in,
+ .read = qpw_read,
+ .run_buffer_in = audio_generic_run_buffer_in,
+ .enable_in = qpw_enable_in
+};
+
+static struct audio_driver pw_audio_driver = {
+ .name = "pipewire",
+ .descr = "http://www.pipewire.org/",
+ .init = qpw_audio_init,
+ .fini = qpw_audio_fini,
+ .pcm_ops = &qpw_pcm_ops,
+ .can_be_default = 1,
+ .max_voices_out = INT_MAX,
+ .max_voices_in = INT_MAX,
+ .voice_size_out = sizeof(PWVoiceOut),
+ .voice_size_in = sizeof(PWVoiceIn),
+};
+
+static void
+register_audio_pw(void)
+{
+ audio_driver_register(&pw_audio_driver);
+}
+
+type_init(register_audio_pw);
diff --git a/audio/trace-events b/audio/trace-events
index e1ab643add..2ecd8851e6 100644
--- a/audio/trace-events
+++ b/audio/trace-events
@@ -18,6 +18,13 @@ dbus_audio_register(const char *s, const char *dir) "sender = %s, dir = %s"
dbus_audio_put_buffer_out(size_t len) "len = %zu"
dbus_audio_read(size_t len) "len = %zu"
+# pwaudio.c
+pw_state_changed(const char *s) "stream state: %s"
+pw_node(int nodeid) "node id: %d"
+pw_read(int32_t avail, uint32_t index, size_t len) "avail=%u index=%u len=%zu"
+pw_write(int32_t filled, int32_t avail, uint32_t index, size_t len) "filled=%u avail=%u index=%u len=%zu"
+pw_audio_init(const char *msg) "pipewire: %s"
+
# audio.c
audio_timer_start(int interval) "interval %d ms"
audio_timer_stop(void) ""
diff --git a/meson.build b/meson.build
index 6bcab8bf0d..51ec2931e1 100644
--- a/meson.build
+++ b/meson.build
@@ -730,6 +730,12 @@ if not get_option('jack').auto() or have_system
jack = dependency('jack', required: get_option('jack'),
method: 'pkg-config', kwargs: static_kwargs)
endif
+pipewire = not_found
+if not get_option('pipewire').auto() or (targetos == 'linux' and have_system)
+ pipewire = dependency('libpipewire-0.3', version: '>=0.3.60',
+ required: get_option('pipewire'),
+ method: 'pkg-config', kwargs: static_kwargs)
+endif
sndio = not_found
if not get_option('sndio').auto() or have_system
sndio = dependency('sndio', required: get_option('sndio'),
@@ -1667,6 +1673,7 @@ if have_system
'jack': jack.found(),
'oss': oss.found(),
'pa': pulse.found(),
+ 'pipewire': pipewire.found(),
'sdl': sdl.found(),
'sndio': sndio.found(),
}
@@ -3980,6 +3987,7 @@ if targetos == 'linux'
summary_info += {'ALSA support': alsa}
summary_info += {'PulseAudio support': pulse}
endif
+summary_info += {'Pipewire support': pipewire}
summary_info += {'JACK support': jack}
summary_info += {'brlapi support': brlapi}
summary_info += {'vde support': vde}
diff --git a/meson_options.txt b/meson_options.txt
index fc9447d267..9ae1ec7f47 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -21,7 +21,7 @@ option('tls_priority', type : 'string', value : 'NORMAL',
option('default_devices', type : 'boolean', value : true,
description: 'Include a default selection of devices in emulators')
option('audio_drv_list', type: 'array', value: ['default'],
- choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'sdl', 'sndio'],
+ choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'pipewire', 'sdl', 'sndio'],
description: 'Set audio driver list')
option('block_drv_rw_whitelist', type : 'string', value : '',
description: 'set block driver read-write whitelist (by default affects only QEMU, not tools like qemu-img)')
@@ -255,6 +255,8 @@ option('oss', type: 'feature', value: 'auto',
description: 'OSS sound support')
option('pa', type: 'feature', value: 'auto',
description: 'PulseAudio sound support')
+option('pipewire', type: 'feature', value: 'auto',
+ description: 'Pipewire sound support')
option('sndio', type: 'feature', value: 'auto',
description: 'sndio sound support')
diff --git a/qapi/audio.json b/qapi/audio.json
index 4e54c00f51..9a0d7d9ece 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -324,6 +324,48 @@
'*out': 'AudiodevPaPerDirectionOptions',
'*server': 'str' } }
+##
+# @AudiodevPipewirePerDirectionOptions:
+#
+# Options of the Pipewire backend that are used for both playback and
+# recording.
+#
+# @name: name of the sink/source to use
+#
+# @stream-name: name of the Pipewire stream created by qemu. Can be
+# used to identify the stream in Pipewire when you
+# create multiple Pipewire devices or run multiple qemu
+# instances (default: audiodev's id, since 7.1)
+#
+#
+# Since: 8.0
+##
+{ 'struct': 'AudiodevPipewirePerDirectionOptions',
+ 'base': 'AudiodevPerDirectionOptions',
+ 'data': {
+ '*name': 'str',
+ '*stream-name': 'str' } }
+
+##
+# @AudiodevPipewireOptions:
+#
+# Options of the Pipewire audio backend.
+#
+# @in: options of the capture stream
+#
+# @out: options of the playback stream
+#
+# @latency: add latency to playback in microseconds
+# (default 15000)
+#
+# Since: 8.0
+##
+{ 'struct': 'AudiodevPipewireOptions',
+ 'data': {
+ '*in': 'AudiodevPipewirePerDirectionOptions',
+ '*out': 'AudiodevPipewirePerDirectionOptions',
+ '*latency': 'uint32' } }
+
##
# @AudiodevSdlPerDirectionOptions:
#
@@ -416,6 +458,7 @@
{ 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
{ 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
{ 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
+ { 'name': 'pipewire', 'if': 'CONFIG_AUDIO_PIPEWIRE' },
{ 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
{ 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
{ 'name': 'spice', 'if': 'CONFIG_SPICE' },
@@ -456,6 +499,8 @@
'if': 'CONFIG_AUDIO_OSS' },
'pa': { 'type': 'AudiodevPaOptions',
'if': 'CONFIG_AUDIO_PA' },
+ 'pipewire': { 'type': 'AudiodevPipewireOptions',
+ 'if': 'CONFIG_AUDIO_PIPEWIRE' },
'sdl': { 'type': 'AudiodevSdlOptions',
'if': 'CONFIG_AUDIO_SDL' },
'sndio': { 'type': 'AudiodevSndioOptions',
diff --git a/qemu-options.hx b/qemu-options.hx
index d42f60fb91..009d58bbf2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -779,6 +779,11 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
" in|out.name= source/sink device name\n"
" in|out.latency= desired latency in microseconds\n"
#endif
+#ifdef CONFIG_AUDIO_PIPEWIRE
+ "-audiodev pipewire,id=id[,prop[=value][,...]]\n"
+ " in|out.name= source/sink device name\n"
+ " latency= desired latency in microseconds\n"
+#endif
#ifdef CONFIG_AUDIO_SDL
"-audiodev sdl,id=id[,prop[=value][,...]]\n"
" in|out.buffer-count= number of buffers\n"
@@ -942,6 +947,18 @@ SRST
Desired latency in microseconds. The PulseAudio server will try
to honor this value but actual latencies may be lower or higher.
+``-audiodev pipewire,id=id[,prop[=value][,...]]``
+ Creates a backend using Pipewire. This backend is available on
+ most systems.
+
+ Pipewire specific options are:
+
+ ``latency=latency``
+ Add extra latency to playback in microseconds
+
+ ``in|out.name=sink``
+ Use the specified source/sink for recording/playback.
+
``-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
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 009fab1515..ba1057b62c 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -1,7 +1,8 @@
# This file is generated by meson-buildoptions.py, do not edit!
meson_options_help() {
- printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list [default] (choices: alsa/co'
- printf "%s\n" ' reaudio/default/dsound/jack/oss/pa/sdl/sndio)'
+ printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list [default] (choices: al'
+ printf "%s\n" ' sa/coreaudio/default/dsound/jack/oss/pa/'
+ printf "%s\n" ' pipewire/sdl/sndio)'
printf "%s\n" ' --block-drv-ro-whitelist=VALUE'
printf "%s\n" ' set block driver read-only whitelist (by default'
printf "%s\n" ' affects only QEMU, not tools like qemu-img)'
@@ -136,6 +137,7 @@ meson_options_help() {
printf "%s\n" ' oss OSS sound support'
printf "%s\n" ' pa PulseAudio sound support'
printf "%s\n" ' parallels parallels image format support'
+ printf "%s\n" ' pipewire Pipewire sound support'
printf "%s\n" ' png PNG support with libpng'
printf "%s\n" ' pvrdma Enable PVRDMA support'
printf "%s\n" ' qcow1 qcow1 image format support'
@@ -370,6 +372,8 @@ _meson_option_parse() {
--disable-pa) printf "%s" -Dpa=disabled ;;
--enable-parallels) printf "%s" -Dparallels=enabled ;;
--disable-parallels) printf "%s" -Dparallels=disabled ;;
+ --enable-pipewire) printf "%s" -Dpipewire=enabled ;;
+ --disable-pipewire) printf "%s" -Dpipewire=disabled ;;
--with-pkgversion=*) quote_sh "-Dpkgversion=$2" ;;
--enable-png) printf "%s" -Dpng=enabled ;;
--disable-png) printf "%s" -Dpng=disabled ;;
--
2.39.1> +/* output data processing function to read stuffs from the buffer */
> +static void
> +playback_on_process(void *data)
> +{
> + PWVoice *v = (PWVoice *) data;
> + void *p;
> + struct pw_buffer *b;
> + struct spa_buffer *buf;
> + uint32_t n_frames, req, index, n_bytes;
> + int32_t avail;
> +
> + if (!v->stream) {
> + return;
> + }
> +
> + /* obtain a buffer to read from */
> + b = pw_stream_dequeue_buffer(v->stream);
> + if (b == NULL) {
> + error_report("out of buffers: %s", strerror(errno));
> + return;
> + }
> +
> + buf = b->buffer;
> + p = buf->datas[0].data;
> + if (p == NULL) {
> + return;
> + }
> + req = b->requested * v->frame_size;
> + if (req == 0) {
> + req = 4096 * v->frame_size;
> + }
> + n_frames = SPA_MIN(req, buf->datas[0].maxsize);
> + n_bytes = n_frames * v->frame_size;
> +
> + /* get no of available bytes to read data from buffer */
> +
> + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> +
> + if (!v->enabled) {
> + avail = 0;
> + }
> +
> + if (avail == 0) {
> + memset(p, 0, n_bytes);
memset() doesn't work for unsigned samples. For unsigned samples, a
stream of zeros is silence with a DC offset. When Pipewire mixes this
stream with another, the result is a clipped audio stream. To hear this,
start QEMU with qemu-system-x86_64 -machine pcspk-audiodev=audio0
-device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
pipewire,id=audio0,out.mixing-engine=off ... and start playback with the
hda device.
With best regards,
Volker
> + } else {
> + if (avail < (int32_t) n_bytes) {
> + n_bytes = avail;
> + }
> +
> + spa_ringbuffer_read_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK, p, n_bytes);
> +
> + index += n_bytes;
> + spa_ringbuffer_read_update(&v->ring, index);
> + }
> +
> + buf->datas[0].chunk->offset = 0;
> + buf->datas[0].chunk->stride = v->frame_size;
> + buf->datas[0].chunk->size = n_bytes;
> +
> + /* queue the buffer for playback */
> + pw_stream_queue_buffer(v->stream, b);
> +}
> +
>
Hi Volker,
> To hear this,
> start QEMU with qemu-system-x86_64 -machine pcspk-audiodev=audio0
> -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> pipewire,id=audio0,out.mixing-engine=off ...
>
I hear the clipped audio stream with these options. IMO, I don't think
memset is responsible for that behaviour, I still hear the harsh sound with
"-audiodev pa". I also tried using an alternative like:
@@ -117,7 +118,7 @@ playback_on_process(void *data)
}
if (avail == 0) {
- memset(p, 0, n_bytes);
+ p = g_malloc0(sizeof(n_bytes));
} else {
The clipped audio issue is still persistent.
Thanks,
Dorinda.
On Sun, Mar 12, 2023 at 9:01 AM Volker Rümelin <vr_qemu@t-online.de> wrote:
> > +/* output data processing function to read stuffs from the buffer */
> > +static void
> > +playback_on_process(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + void *p;
> > + struct pw_buffer *b;
> > + struct spa_buffer *buf;
> > + uint32_t n_frames, req, index, n_bytes;
> > + int32_t avail;
> > +
> > + if (!v->stream) {
> > + return;
> > + }
> > +
> > + /* obtain a buffer to read from */
> > + b = pw_stream_dequeue_buffer(v->stream);
> > + if (b == NULL) {
> > + error_report("out of buffers: %s", strerror(errno));
> > + return;
> > + }
> > +
> > + buf = b->buffer;
> > + p = buf->datas[0].data;
> > + if (p == NULL) {
> > + return;
> > + }
> > + req = b->requested * v->frame_size;
> > + if (req == 0) {
> > + req = 4096 * v->frame_size;
> > + }
> > + n_frames = SPA_MIN(req, buf->datas[0].maxsize);
> > + n_bytes = n_frames * v->frame_size;
> > +
> > + /* get no of available bytes to read data from buffer */
> > +
> > + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> > +
> > + if (!v->enabled) {
> > + avail = 0;
> > + }
> > +
> > + if (avail == 0) {
> > + memset(p, 0, n_bytes);
>
> memset() doesn't work for unsigned samples. For unsigned samples, a
> stream of zeros is silence with a DC offset. When Pipewire mixes this
> stream with another, the result is a clipped audio stream. To hear this,
> start QEMU with qemu-system-x86_64 -machine pcspk-audiodev=audio0
> -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> pipewire,id=audio0,out.mixing-engine=off ... and start playback with the
> hda device.
>
> With best regards,
> Volker
>
> > + } else {
> > + if (avail < (int32_t) n_bytes) {
> > + n_bytes = avail;
> > + }
> > +
> > + spa_ringbuffer_read_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, p,
> n_bytes);
> > +
> > + index += n_bytes;
> > + spa_ringbuffer_read_update(&v->ring, index);
> > + }
> > +
> > + buf->datas[0].chunk->offset = 0;
> > + buf->datas[0].chunk->stride = v->frame_size;
> > + buf->datas[0].chunk->size = n_bytes;
> > +
> > + /* queue the buffer for playback */
> > + pw_stream_queue_buffer(v->stream, b);
> > +}
> > +
> >
>
>
Am 13.03.23 um 14:11 schrieb Dorinda Bassey:
> Hi Volker,
>
> To hear this,
> start QEMU with qemu-system-x86_64 -machine pcspk-audiodev=audio0
> -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> pipewire,id=audio0,out.mixing-engine=off ...
>
> I hear the clipped audio stream with these options. IMO, I don't think
> memset is responsible for that behaviour, I still hear the harsh sound
> with "-audiodev pa". I also tried using an alternative like:
Hi Dorinda,
when you test -audiodev pa with a pulseaudio server, audio playback is
fine. Audio playback with -audiodev pa with the pipewire-pulse server is
clipped. This is a pipewire bug.
With best regards,
Volker
>
> @@ -117,7 +118,7 @@ playback_on_process(void *data)
> }
>
> if (avail == 0) {
> - memset(p, 0, n_bytes);
> + p = g_malloc0(sizeof(n_bytes));
> } else {
>
> The clipped audio issue is still persistent.
>
> Thanks,
> Dorinda.
>
> On Sun, Mar 12, 2023 at 9:01 AM Volker Rümelin <vr_qemu@t-online.de>
> wrote:
>
> > +/* output data processing function to read stuffs from the
> buffer */
> > +static void
> > +playback_on_process(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + void *p;
> > + struct pw_buffer *b;
> > + struct spa_buffer *buf;
> > + uint32_t n_frames, req, index, n_bytes;
> > + int32_t avail;
> > +
> > + if (!v->stream) {
> > + return;
> > + }
> > +
> > + /* obtain a buffer to read from */
> > + b = pw_stream_dequeue_buffer(v->stream);
> > + if (b == NULL) {
> > + error_report("out of buffers: %s", strerror(errno));
> > + return;
> > + }
> > +
> > + buf = b->buffer;
> > + p = buf->datas[0].data;
> > + if (p == NULL) {
> > + return;
> > + }
> > + req = b->requested * v->frame_size;
> > + if (req == 0) {
> > + req = 4096 * v->frame_size;
> > + }
> > + n_frames = SPA_MIN(req, buf->datas[0].maxsize);
> > + n_bytes = n_frames * v->frame_size;
> > +
> > + /* get no of available bytes to read data from buffer */
> > +
> > + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> > +
> > + if (!v->enabled) {
> > + avail = 0;
> > + }
> > +
> > + if (avail == 0) {
> > + memset(p, 0, n_bytes);
>
> memset() doesn't work for unsigned samples. For unsigned samples, a
> stream of zeros is silence with a DC offset. When Pipewire mixes this
> stream with another, the result is a clipped audio stream. To hear
> this,
> start QEMU with qemu-system-x86_64 -machine pcspk-audiodev=audio0
> -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> pipewire,id=audio0,out.mixing-engine=off ... and start playback
> with the
> hda device.
>
> With best regards,
> Volker
>
> > + } else {
> > + if (avail < (int32_t) n_bytes) {
> > + n_bytes = avail;
> > + }
> > +
> > + spa_ringbuffer_read_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, p,
> n_bytes);
> > +
> > + index += n_bytes;
> > + spa_ringbuffer_read_update(&v->ring, index);
> > + }
> > +
> > + buf->datas[0].chunk->offset = 0;
> > + buf->datas[0].chunk->stride = v->frame_size;
> > + buf->datas[0].chunk->size = n_bytes;
> > +
> > + /* queue the buffer for playback */
> > + pw_stream_queue_buffer(v->stream, b);
> > +}
> > +
> >
>
On Monday, March 13, 2023 2:11:11 PM CET Dorinda Bassey wrote:
> Hi Volker,
>
>
> > To hear this,
> > start QEMU with qemu-system-x86_64 -machine pcspk-audiodev=audio0
> > -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> > pipewire,id=audio0,out.mixing-engine=off ...
> >
> I hear the clipped audio stream with these options. IMO, I don't think
> memset is responsible for that behaviour, I still hear the harsh sound with
> "-audiodev pa". I also tried using an alternative like:
>
> @@ -117,7 +118,7 @@ playback_on_process(void *data)
> }
>
> if (avail == 0) {
> - memset(p, 0, n_bytes);
> + p = g_malloc0(sizeof(n_bytes));
> } else {
>
> The clipped audio issue is still persistent.
Are you sure about sizeof(n_bytes) here? That's 4. ;-)
Volker's point was that "silence" is the center of the wave range. With signed
range that's zero, yes, but with unsigned range that's 2^(bitdepth) / 2.
So you need to memset() the correct value to generate "silence".
Best regards,
Christian Schoenebeck
>
> Are you sure about sizeof(n_bytes) here? That's 4. ;-)
>
my bad!
>
> Volker's point was that "silence" is the center of the wave range. With
> signed
> range that's zero, yes, but with unsigned range that's 2^(bitdepth) / 2.
>
> So you need to memset() the correct value to generate "silence".
>
I understand now, Thanks. I guess it should work for signed range, so I
would do:
@@ -117,7 +117,9 @@ playback_on_process(void *data)
}
if (avail == 0) {
- memset(p, 0, n_bytes);
+ memset(p, 0, (int32_t) n_bytes);
CMIIW
Thanks,
Dorinda.
On Mon, Mar 13, 2023 at 2:37 PM Christian Schoenebeck <
qemu_oss@crudebyte.com> wrote:
> On Monday, March 13, 2023 2:11:11 PM CET Dorinda Bassey wrote:
> > Hi Volker,
> >
> >
> > > To hear this,
> > > start QEMU with qemu-system-x86_64 -machine pcspk-audiodev=audio0
> > > -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> > > pipewire,id=audio0,out.mixing-engine=off ...
> > >
> > I hear the clipped audio stream with these options. IMO, I don't think
> > memset is responsible for that behaviour, I still hear the harsh sound
> with
> > "-audiodev pa". I also tried using an alternative like:
> >
> > @@ -117,7 +118,7 @@ playback_on_process(void *data)
> > }
> >
> > if (avail == 0) {
> > - memset(p, 0, n_bytes);
> > + p = g_malloc0(sizeof(n_bytes));
> > } else {
> >
> > The clipped audio issue is still persistent.
>
> Are you sure about sizeof(n_bytes) here? That's 4. ;-)
>
> Volker's point was that "silence" is the center of the wave range. With
> signed
> range that's zero, yes, but with unsigned range that's 2^(bitdepth) / 2.
>
> So you need to memset() the correct value to generate "silence".
>
> Best regards,
> Christian Schoenebeck
>
>
>
On Monday, March 13, 2023 8:06:15 PM CET Dorinda Bassey wrote:
> >
> > Are you sure about sizeof(n_bytes) here? That's 4. ;-)
> >
> my bad!
>
> >
> > Volker's point was that "silence" is the center of the wave range. With
> > signed
> > range that's zero, yes, but with unsigned range that's 2^(bitdepth) / 2.
> >
> > So you need to memset() the correct value to generate "silence".
> >
> I understand now, Thanks. I guess it should work for signed range, so I
> would do:
>
> @@ -117,7 +117,9 @@ playback_on_process(void *data)
> }
>
> if (avail == 0) {
> - memset(p, 0, n_bytes);
> + memset(p, 0, (int32_t) n_bytes);
No, that would not fix anything. You are actually making several false
assumptions here.
Anyway, in audio/audio.c there is a function which does it correctly:
audio_pcm_info_clear_buf(struct audio_pcm_info *info, void *buf, int len)
So you probably just want to call this function instead to generate silence
correctly. Keep in mind though that it's `len` argument is in sample points,
not in bytes.
Hi Dorinda, I've started to write down my suggestions and comments. After more than one page of text, I think that without sample code, the text is not very understandable. Therefore I will write three mails. In this mail I describe the problem with the QEMU Pipewire audio backend. My next mail is a patch for the v7 QEMU pipewire backend with my suggestions how I would change the code. I can then reply to my own mail with comments, explaining my changes. I tested the pipewire audio backend with the QEMU hda device. Audio playback doesn't work well on my computer. I hear dropouts every few seconds. > +#define BUFFER_SAMPLES 512 BUFFER_SAMPLES can't be a fixed size. To hear the problem please start QEMU with qemu-system-x86_64 -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev pipewire,id=audio0,out.frequency=96000,in.frequency=96000 ... . The pipewire backend tells QEMU to use a buffer size of 512 audio frames. The buffer can hold data for 512 frames / 96000 frames/s = 5.3ms. With a timer-period of 10ms there is no way to transport enough frames to the pipewire buffer. Just using a larger BUFFER_SAMPLES value also doesn't work. When I start QEMU with qemu-system-x86_64 -device ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev pipewire,id=audio0,timer-period=2000 ... the maximum audio frame throughput increases as when using a larger BUFFER_SAMPLES value. But the higher maximum throughput makes the dropout issue more audible. This has to do with how often pipewire calls playback_on_process() and capture_on_process(). On my system pipewire calls playback_on_process() and capture_on_process() every 21ms and sometimes the callback is delayed by 42ms. At a guest audio frame rate of 44100 frames/s the QEMU hda device drops its buffer if data wasn't read for longer than 23ms on average. With a timer period of 2ms, the PWVoice buffer fills quicker and the time to the next pipewire update is longer. This increases the chance that the hda device drops its buffer. With a timer period of 10ms, the PWVoice buffer fills slower. This is similar to a jitter buffer, but a good jitter buffer implementation would work better. My next mail with the sample code will give pipewire a hint to call the callbacks faster than timer-period and then remove BUFFER_SAMPLES. With best regards, Volker
Based-on: <20230306171020.381116-1-dbassey@redhat.com>
([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend for QEMU)
This is sample code for the review of the pipewire backed. The
code actually works.
An email with explanations for the changes will follow.
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
audio/pwaudio.c | 67 +++++++++++++++++++++++++++++++++----------------
qapi/audio.json | 10 +++-----
2 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/audio/pwaudio.c b/audio/pwaudio.c
index d357761152..8e2a38938f 100644
--- a/audio/pwaudio.c
+++ b/audio/pwaudio.c
@@ -23,7 +23,6 @@
#define AUDIO_CAP "pipewire"
#define RINGBUFFER_SIZE (1u << 22)
#define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
-#define BUFFER_SAMPLES 512
#include "audio_int.h"
@@ -48,6 +47,7 @@ typedef struct PWVoice {
struct pw_stream *stream;
struct spa_hook stream_listener;
struct spa_audio_info_raw info;
+ uint32_t highwater_mark;
uint32_t frame_size;
struct spa_ringbuffer ring;
uint8_t buffer[RINGBUFFER_SIZE];
@@ -82,7 +82,7 @@ playback_on_process(void *data)
void *p;
struct pw_buffer *b;
struct spa_buffer *buf;
- uint32_t n_frames, req, index, n_bytes;
+ uint32_t req, index, n_bytes;
int32_t avail;
if (!v->stream) {
@@ -105,8 +105,7 @@ playback_on_process(void *data)
if (req == 0) {
req = 4096 * v->frame_size;
}
- n_frames = SPA_MIN(req, buf->datas[0].maxsize);
- n_bytes = n_frames * v->frame_size;
+ n_bytes = SPA_MIN(req, buf->datas[0].maxsize);
/* get no of available bytes to read data from buffer */
@@ -270,6 +269,30 @@ done_unlock:
return l;
}
+static size_t qpw_buffer_get_free(HWVoiceOut *hw)
+{
+ PWVoiceOut *pw = (PWVoiceOut *)hw;
+ PWVoice *v = &pw->v;
+ pwaudio *c = v->g;
+ const char *error = NULL;
+ int32_t filled, avail;
+ uint32_t index;
+
+ pw_thread_loop_lock(c->thread_loop);
+ if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
+ /* wait for stream to become ready */
+ avail = 0;
+ goto done_unlock;
+ }
+
+ filled = spa_ringbuffer_get_write_index(&v->ring, &index);
+ avail = v->highwater_mark - filled;
+
+done_unlock:
+ pw_thread_loop_unlock(c->thread_loop);
+ return avail;
+}
+
static size_t
qpw_write(HWVoiceOut *hw, void *data, size_t len)
{
@@ -277,20 +300,18 @@ qpw_write(HWVoiceOut *hw, void *data, size_t len)
PWVoice *v = &pw->v;
pwaudio *c = v->g;
const char *error = NULL;
- const int periods = 3;
- size_t l;
int32_t filled, avail;
uint32_t index;
pw_thread_loop_lock(c->thread_loop);
if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
/* wait for stream to become ready */
- l = 0;
+ len = 0;
goto done_unlock;
}
- filled = spa_ringbuffer_get_write_index(&v->ring, &index);
- avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
+ filled = spa_ringbuffer_get_write_index(&v->ring, &index);
+ avail = v->highwater_mark - filled;
trace_pw_write(filled, avail, index, len);
@@ -312,11 +333,10 @@ qpw_write(HWVoiceOut *hw, void *data, size_t len)
index & RINGBUFFER_MASK, data, len);
index += len;
spa_ringbuffer_write_update(&v->ring, index);
- l = len;
done_unlock:
pw_thread_loop_unlock(c->thread_loop);
- return l;
+ return len;
}
static int
@@ -420,8 +440,13 @@ create_stream(pwaudio *c, PWVoice *v, const char *name)
const struct spa_pod *params[2];
uint8_t buffer[1024];
struct spa_pod_builder b;
+ struct pw_properties *props;
- v->stream = pw_stream_new(c->core, name, NULL);
+ props = pw_properties_new(NULL, NULL);
+ pw_properties_setf(props, PW_KEY_NODE_LATENCY, "%" PRIu64 "/%u",
+ (uint64_t)v->g->dev->timer_period * v->info.rate
+ * 3 / 4 / 1000000, v->info.rate);
+ v->stream = pw_stream_new(c->core, name, props);
if (v->stream == NULL) {
goto error;
@@ -563,7 +588,11 @@ qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
audio_pcm_init_info(&hw->info, &obt_as);
/* report the buffer size to qemu */
- hw->samples = BUFFER_SAMPLES;
+ hw->samples = audio_buffer_frames(
+ qapi_AudiodevPipewirePerDirectionOptions_base(ppdo), &obt_as, 46440);
+ v->highwater_mark = MIN(RINGBUFFER_SIZE,
+ (ppdo->has_latency ? ppdo->latency : 46440)
+ * (uint64_t)v->info.rate / 1000000 * v->frame_size);
pw_thread_loop_unlock(c->thread_loop);
return 0;
@@ -606,7 +635,8 @@ qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
audio_pcm_init_info(&hw->info, &obt_as);
/* report the buffer size to qemu */
- hw->samples = BUFFER_SAMPLES;
+ hw->samples = audio_buffer_frames(
+ qapi_AudiodevPipewirePerDirectionOptions_base(ppdo), &obt_as, 46440);
pw_thread_loop_unlock(c->thread_loop);
return 0;
@@ -695,15 +725,8 @@ qpw_audio_init(Audiodev *dev)
pw = g_new0(pwaudio, 1);
pw_init(NULL, NULL);
- AudiodevPipewireOptions *popts;
trace_pw_audio_init("Initialize Pipewire context\n");
assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
- popts = &dev->u.pipewire;
-
- if (!popts->has_latency) {
- popts->has_latency = true;
- popts->latency = 15000;
- }
pw->dev = dev;
pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
@@ -781,7 +804,7 @@ static struct audio_pcm_ops qpw_pcm_ops = {
.init_out = qpw_init_out,
.fini_out = qpw_fini_out,
.write = qpw_write,
- .buffer_get_free = audio_generic_buffer_get_free,
+ .buffer_get_free = qpw_buffer_get_free,
.run_buffer_out = audio_generic_run_buffer_out,
.enable_out = qpw_enable_out,
diff --git a/qapi/audio.json b/qapi/audio.json
index 9a0d7d9ece..d49a8a670b 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -337,6 +337,7 @@
# create multiple Pipewire devices or run multiple qemu
# instances (default: audiodev's id, since 7.1)
#
+# @latency: Pipewire backend buffer size in microseconds (default 46440)
#
# Since: 8.0
##
@@ -344,7 +345,8 @@
'base': 'AudiodevPerDirectionOptions',
'data': {
'*name': 'str',
- '*stream-name': 'str' } }
+ '*stream-name': 'str',
+ '*latency': 'uint32' } }
##
# @AudiodevPipewireOptions:
@@ -355,16 +357,12 @@
#
# @out: options of the playback stream
#
-# @latency: add latency to playback in microseconds
-# (default 15000)
-#
# Since: 8.0
##
{ 'struct': 'AudiodevPipewireOptions',
'data': {
'*in': 'AudiodevPipewirePerDirectionOptions',
- '*out': 'AudiodevPipewirePerDirectionOptions',
- '*latency': 'uint32' } }
+ '*out': 'AudiodevPipewirePerDirectionOptions' } }
##
# @AudiodevSdlPerDirectionOptions:
--
2.35.3
> Based-on:<20230306171020.381116-1-dbassey@redhat.com>
> ([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend for QEMU)
>
> This is sample code for the review of the pipewire backed. The
> code actually works.
>
> An email with explanations for the changes will follow.
>
> Signed-off-by: Volker Rümelin<vr_qemu@t-online.de>
> ---
> audio/pwaudio.c | 67 +++++++++++++++++++++++++++++++++----------------
> qapi/audio.json | 10 +++-----
> 2 files changed, 49 insertions(+), 28 deletions(-)
>
> diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> index d357761152..8e2a38938f 100644
> --- a/audio/pwaudio.c
> +++ b/audio/pwaudio.c
> @@ -23,7 +23,6 @@
> #define AUDIO_CAP "pipewire"
> #define RINGBUFFER_SIZE (1u << 22)
> #define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> -#define BUFFER_SAMPLES 512
>
> #include "audio_int.h"
>
> @@ -48,6 +47,7 @@ typedef struct PWVoice {
> struct pw_stream *stream;
> struct spa_hook stream_listener;
> struct spa_audio_info_raw info;
> + uint32_t highwater_mark;
> uint32_t frame_size;
> struct spa_ringbuffer ring;
> uint8_t buffer[RINGBUFFER_SIZE];
> @@ -82,7 +82,7 @@ playback_on_process(void *data)
> void *p;
> struct pw_buffer *b;
> struct spa_buffer *buf;
> - uint32_t n_frames, req, index, n_bytes;
> + uint32_t req, index, n_bytes;
> int32_t avail;
>
> if (!v->stream) {
> @@ -105,8 +105,7 @@ playback_on_process(void *data)
> if (req == 0) {
> req = 4096 * v->frame_size;
> }
I don't understand how the req == 0 case can work at all. The downstream
audio device is the thinnest point in the playback stream. We can't
write more audio frames than the audio device will consume.
> - n_frames = SPA_MIN(req, buf->datas[0].maxsize);
> - n_bytes = n_frames * v->frame_size;
> + n_bytes = SPA_MIN(req, buf->datas[0].maxsize);
See Marc-André's review.
>
> /* get no of available bytes to read data from buffer */
>
> @@ -270,6 +269,30 @@ done_unlock:
> return l;
> }
>
> +static size_t qpw_buffer_get_free(HWVoiceOut *hw)
> +{
> + PWVoiceOut *pw = (PWVoiceOut *)hw;
> + PWVoice *v = &pw->v;
> + pwaudio *c = v->g;
> + const char *error = NULL;
> + int32_t filled, avail;
> + uint32_t index;
> +
> + pw_thread_loop_lock(c->thread_loop);
> + if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
> + /* wait for stream to become ready */
> + avail = 0;
> + goto done_unlock;
> + }
> +
> + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> + avail = v->highwater_mark - filled;
> +
> +done_unlock:
> + pw_thread_loop_unlock(c->thread_loop);
> + return avail;
> +}
> +
A pcm_ops buffer_get_free function is necessary. Otherwise the gus and
via-ac97 audio devices will not work properly for the -audiodev
pipewire,id=audio0,out.mixing-engine=off case. They need to know in
advance how many bytes they can write.
Also, without the buffer_get_free function, the generic audio buffer
will increase the playback latency.
> static size_t
> qpw_write(HWVoiceOut *hw, void *data, size_t len)
> {
> @@ -277,20 +300,18 @@ qpw_write(HWVoiceOut *hw, void *data, size_t len)
> PWVoice *v = &pw->v;
> pwaudio *c = v->g;
> const char *error = NULL;
> - const int periods = 3;
> - size_t l;
> int32_t filled, avail;
> uint32_t index;
>
> pw_thread_loop_lock(c->thread_loop);
> if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
> /* wait for stream to become ready */
> - l = 0;
> + len = 0;
> goto done_unlock;
> }
> - filled = spa_ringbuffer_get_write_index(&v->ring, &index);
>
> - avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
> + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> + avail = v->highwater_mark - filled;
>
> trace_pw_write(filled, avail, index, len);
>
> @@ -312,11 +333,10 @@ qpw_write(HWVoiceOut *hw, void *data, size_t len)
> index & RINGBUFFER_MASK, data, len);
> index += len;
> spa_ringbuffer_write_update(&v->ring, index);
> - l = len;
>
> done_unlock:
> pw_thread_loop_unlock(c->thread_loop);
> - return l;
> + return len;
> }
>
> static int
> @@ -420,8 +440,13 @@ create_stream(pwaudio *c, PWVoice *v, const char *name)
> const struct spa_pod *params[2];
> uint8_t buffer[1024];
> struct spa_pod_builder b;
> + struct pw_properties *props;
>
> - v->stream = pw_stream_new(c->core, name, NULL);
> + props = pw_properties_new(NULL, NULL);
> + pw_properties_setf(props, PW_KEY_NODE_LATENCY, "%" PRIu64 "/%u",
> + (uint64_t)v->g->dev->timer_period * v->info.rate
> + * 3 / 4 / 1000000, v->info.rate);
> + v->stream = pw_stream_new(c->core, name, props);
The PW_KEY_NODE_LATENCY property is a hint for Pipewire that we need
updates faster than timer_period. I selected 75% of timer-period. Then
there's a good chance the audio frontends can write or read new audio
packets every timer-period. It doesn't matter that Pipewire calls the
callbacks faster in most cases.
If it turns out that Pipewire often can't even approximately fulfill
this hint, we will additionally need a jitter buffer implementation to
split the larger Pipewire audio packets into timer-period sized packets.
>
> if (v->stream == NULL) {
> goto error;
> @@ -563,7 +588,11 @@ qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
> audio_pcm_init_info(&hw->info, &obt_as);
>
> /* report the buffer size to qemu */
> - hw->samples = BUFFER_SAMPLES;
> + hw->samples = audio_buffer_frames(
> + qapi_AudiodevPipewirePerDirectionOptions_base(ppdo), &obt_as, 46440);
> + v->highwater_mark = MIN(RINGBUFFER_SIZE,
> + (ppdo->has_latency ? ppdo->latency : 46440)
> + * (uint64_t)v->info.rate / 1000000 * v->frame_size);
>
The reported buffer size should be much larger than BUFFER_SAMPLES. This
gives the audio frontends a chance to catch up if they missed
timer-periods or if they have to fill the pipewire backend buffer
quickly after playback starts. The exact size is not critical, but to be
command line compatible with the pulseaudio backend, I suggest to use
46ms. A large hw->samples value doesn't increase the playback latency.
v->highwater_mark is the effective pipewire backend buffer size. At a
audio frame rate of 44100 frames/s, the code without this patch uses a
buffer size of BUFFER_SAMPLES * periods / 44100 frames/s = 512 frames *
3 / 44100 frames/s = 35ms. On my computer the buffer size has to be 30ms
at minimum. I suggest to add a good margin and use a default of 46ms.
This buffer is a larger contributor to the playback latency.
> pw_thread_loop_unlock(c->thread_loop);
> return 0;
> @@ -606,7 +635,8 @@ qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
> audio_pcm_init_info(&hw->info, &obt_as);
>
> /* report the buffer size to qemu */
> - hw->samples = BUFFER_SAMPLES;
> + hw->samples = audio_buffer_frames(
> + qapi_AudiodevPipewirePerDirectionOptions_base(ppdo), &obt_as, 46440);
>
See qpw_init_out().
> pw_thread_loop_unlock(c->thread_loop);
> return 0;
> @@ -695,15 +725,8 @@ qpw_audio_init(Audiodev *dev)
> pw = g_new0(pwaudio, 1);
> pw_init(NULL, NULL);
>
> - AudiodevPipewireOptions *popts;
> trace_pw_audio_init("Initialize Pipewire context\n");
> assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> - popts = &dev->u.pipewire;
> -
> - if (!popts->has_latency) {
> - popts->has_latency = true;
> - popts->latency = 15000;
> - }
>
> pw->dev = dev;
> pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
> @@ -781,7 +804,7 @@ static struct audio_pcm_ops qpw_pcm_ops = {
> .init_out = qpw_init_out,
> .fini_out = qpw_fini_out,
> .write = qpw_write,
> - .buffer_get_free = audio_generic_buffer_get_free,
> + .buffer_get_free = qpw_buffer_get_free,
> .run_buffer_out = audio_generic_run_buffer_out,
> .enable_out = qpw_enable_out,
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 9a0d7d9ece..d49a8a670b 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -337,6 +337,7 @@
> # create multiple Pipewire devices or run multiple qemu
> # instances (default: audiodev's id, since 7.1)
> #
> +# @latency: Pipewire backend buffer size in microseconds (default 46440)
> #
> # Since: 8.0
> ##
> @@ -344,7 +345,8 @@
> 'base': 'AudiodevPerDirectionOptions',
> 'data': {
> '*name': 'str',
> - '*stream-name': 'str' } }
> + '*stream-name': 'str',
> + '*latency': 'uint32' } }
>
I suggest to use the same option names as the pulseaudio backend.
out.latency is the effective Pipewire buffer size.
With best regards,
Volker
> ##
> # @AudiodevPipewireOptions:
> @@ -355,16 +357,12 @@
> #
> # @out: options of the playback stream
> #
> -# @latency: add latency to playback in microseconds
> -# (default 15000)
> -#
> # Since: 8.0
> ##
> { 'struct': 'AudiodevPipewireOptions',
> 'data': {
> '*in': 'AudiodevPipewirePerDirectionOptions',
> - '*out': 'AudiodevPipewirePerDirectionOptions',
> - '*latency': 'uint32' } }
> + '*out': 'AudiodevPipewirePerDirectionOptions' } }
>
> ##
> # @AudiodevSdlPerDirectionOptions:
Hi Volker,
Thanks for the patch, I've tested the patch and it works. I don't hear the
choppy audio with this option "qemu-system-x86_64 -device ich9-intel-hda
-device hda-duplex,audiodev=audio0 -audiodev
pipewire,id=audio0,out.frequency=96000,in.frequency=96000 ...."
I don't understand how the req == 0 case can work at all.
>
how this works is that b->requested could be zero when no suggestion is
provided. For playback streams, this field contains the suggested amount of
data to provide. hence the reason for this check.
I suggest to use the same option names as the pulseaudio backend.
> out.latency is the effective Pipewire buffer size.
>
Ack.
Thanks,
Dorinda.
On Sat, Mar 11, 2023 at 5:19 PM Volker Rümelin <vr_qemu@t-online.de> wrote:
> > Based-on:<20230306171020.381116-1-dbassey@redhat.com>
> > ([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend for QEMU)
> >
> > This is sample code for the review of the pipewire backed. The
> > code actually works.
> >
> > An email with explanations for the changes will follow.
> >
> > Signed-off-by: Volker Rümelin<vr_qemu@t-online.de>
> > ---
> > audio/pwaudio.c | 67 +++++++++++++++++++++++++++++++++----------------
> > qapi/audio.json | 10 +++-----
> > 2 files changed, 49 insertions(+), 28 deletions(-)
> >
> > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> > index d357761152..8e2a38938f 100644
> > --- a/audio/pwaudio.c
> > +++ b/audio/pwaudio.c
> > @@ -23,7 +23,6 @@
> > #define AUDIO_CAP "pipewire"
> > #define RINGBUFFER_SIZE (1u << 22)
> > #define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> > -#define BUFFER_SAMPLES 512
> >
> > #include "audio_int.h"
> >
> > @@ -48,6 +47,7 @@ typedef struct PWVoice {
> > struct pw_stream *stream;
> > struct spa_hook stream_listener;
> > struct spa_audio_info_raw info;
> > + uint32_t highwater_mark;
> > uint32_t frame_size;
> > struct spa_ringbuffer ring;
> > uint8_t buffer[RINGBUFFER_SIZE];
> > @@ -82,7 +82,7 @@ playback_on_process(void *data)
> > void *p;
> > struct pw_buffer *b;
> > struct spa_buffer *buf;
> > - uint32_t n_frames, req, index, n_bytes;
> > + uint32_t req, index, n_bytes;
> > int32_t avail;
> >
> > if (!v->stream) {
> > @@ -105,8 +105,7 @@ playback_on_process(void *data)
> > if (req == 0) {
> > req = 4096 * v->frame_size;
> > }
>
> I don't understand how the req == 0 case can work at all. The downstream
> audio device is the thinnest point in the playback stream. We can't
> write more audio frames than the audio device will consume.
>
> > - n_frames = SPA_MIN(req, buf->datas[0].maxsize);
> > - n_bytes = n_frames * v->frame_size;
> > + n_bytes = SPA_MIN(req, buf->datas[0].maxsize);
>
> See Marc-André's review.
>
> >
> > /* get no of available bytes to read data from buffer */
> >
> > @@ -270,6 +269,30 @@ done_unlock:
> > return l;
> > }
> >
> > +static size_t qpw_buffer_get_free(HWVoiceOut *hw)
> > +{
> > + PWVoiceOut *pw = (PWVoiceOut *)hw;
> > + PWVoice *v = &pw->v;
> > + pwaudio *c = v->g;
> > + const char *error = NULL;
> > + int32_t filled, avail;
> > + uint32_t index;
> > +
> > + pw_thread_loop_lock(c->thread_loop);
> > + if (pw_stream_get_state(v->stream, &error) !=
> PW_STREAM_STATE_STREAMING) {
> > + /* wait for stream to become ready */
> > + avail = 0;
> > + goto done_unlock;
> > + }
> > +
> > + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> > + avail = v->highwater_mark - filled;
> > +
> > +done_unlock:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return avail;
> > +}
> > +
>
> A pcm_ops buffer_get_free function is necessary. Otherwise the gus and
> via-ac97 audio devices will not work properly for the -audiodev
> pipewire,id=audio0,out.mixing-engine=off case. They need to know in
> advance how many bytes they can write.
>
> Also, without the buffer_get_free function, the generic audio buffer
> will increase the playback latency.
>
> > static size_t
> > qpw_write(HWVoiceOut *hw, void *data, size_t len)
> > {
> > @@ -277,20 +300,18 @@ qpw_write(HWVoiceOut *hw, void *data, size_t len)
> > PWVoice *v = &pw->v;
> > pwaudio *c = v->g;
> > const char *error = NULL;
> > - const int periods = 3;
> > - size_t l;
> > int32_t filled, avail;
> > uint32_t index;
> >
> > pw_thread_loop_lock(c->thread_loop);
> > if (pw_stream_get_state(v->stream, &error) !=
> PW_STREAM_STATE_STREAMING) {
> > /* wait for stream to become ready */
> > - l = 0;
> > + len = 0;
> > goto done_unlock;
> > }
> > - filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> >
> > - avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
> > + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> > + avail = v->highwater_mark - filled;
> >
> > trace_pw_write(filled, avail, index, len);
> >
> > @@ -312,11 +333,10 @@ qpw_write(HWVoiceOut *hw, void *data, size_t len)
> > index & RINGBUFFER_MASK, data, len);
> > index += len;
> > spa_ringbuffer_write_update(&v->ring, index);
> > - l = len;
> >
> > done_unlock:
> > pw_thread_loop_unlock(c->thread_loop);
> > - return l;
> > + return len;
> > }
> >
> > static int
> > @@ -420,8 +440,13 @@ create_stream(pwaudio *c, PWVoice *v, const char
> *name)
> > const struct spa_pod *params[2];
> > uint8_t buffer[1024];
> > struct spa_pod_builder b;
> > + struct pw_properties *props;
> >
> > - v->stream = pw_stream_new(c->core, name, NULL);
> > + props = pw_properties_new(NULL, NULL);
> > + pw_properties_setf(props, PW_KEY_NODE_LATENCY, "%" PRIu64 "/%u",
> > + (uint64_t)v->g->dev->timer_period * v->info.rate
> > + * 3 / 4 / 1000000, v->info.rate);
> > + v->stream = pw_stream_new(c->core, name, props);
>
> The PW_KEY_NODE_LATENCY property is a hint for Pipewire that we need
> updates faster than timer_period. I selected 75% of timer-period. Then
> there's a good chance the audio frontends can write or read new audio
> packets every timer-period. It doesn't matter that Pipewire calls the
> callbacks faster in most cases.
>
> If it turns out that Pipewire often can't even approximately fulfill
> this hint, we will additionally need a jitter buffer implementation to
> split the larger Pipewire audio packets into timer-period sized packets.
>
> >
> > if (v->stream == NULL) {
> > goto error;
> > @@ -563,7 +588,11 @@ qpw_init_out(HWVoiceOut *hw, struct audsettings
> *as, void *drv_opaque)
> > audio_pcm_init_info(&hw->info, &obt_as);
> >
> > /* report the buffer size to qemu */
> > - hw->samples = BUFFER_SAMPLES;
> > + hw->samples = audio_buffer_frames(
> > + qapi_AudiodevPipewirePerDirectionOptions_base(ppdo), &obt_as,
> 46440);
> > + v->highwater_mark = MIN(RINGBUFFER_SIZE,
> > + (ppdo->has_latency ? ppdo->latency : 46440)
> > + * (uint64_t)v->info.rate / 1000000 *
> v->frame_size);
> >
>
> The reported buffer size should be much larger than BUFFER_SAMPLES. This
> gives the audio frontends a chance to catch up if they missed
> timer-periods or if they have to fill the pipewire backend buffer
> quickly after playback starts. The exact size is not critical, but to be
> command line compatible with the pulseaudio backend, I suggest to use
> 46ms. A large hw->samples value doesn't increase the playback latency.
>
> v->highwater_mark is the effective pipewire backend buffer size. At a
> audio frame rate of 44100 frames/s, the code without this patch uses a
> buffer size of BUFFER_SAMPLES * periods / 44100 frames/s = 512 frames *
> 3 / 44100 frames/s = 35ms. On my computer the buffer size has to be 30ms
> at minimum. I suggest to add a good margin and use a default of 46ms.
> This buffer is a larger contributor to the playback latency.
>
> > pw_thread_loop_unlock(c->thread_loop);
> > return 0;
> > @@ -606,7 +635,8 @@ qpw_init_in(HWVoiceIn *hw, struct audsettings *as,
> void *drv_opaque)
> > audio_pcm_init_info(&hw->info, &obt_as);
> >
> > /* report the buffer size to qemu */
> > - hw->samples = BUFFER_SAMPLES;
> > + hw->samples = audio_buffer_frames(
> > + qapi_AudiodevPipewirePerDirectionOptions_base(ppdo), &obt_as,
> 46440);
> >
>
> See qpw_init_out().
>
> > pw_thread_loop_unlock(c->thread_loop);
> > return 0;
> > @@ -695,15 +725,8 @@ qpw_audio_init(Audiodev *dev)
> > pw = g_new0(pwaudio, 1);
> > pw_init(NULL, NULL);
> >
> > - AudiodevPipewireOptions *popts;
> > trace_pw_audio_init("Initialize Pipewire context\n");
> > assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> > - popts = &dev->u.pipewire;
> > -
> > - if (!popts->has_latency) {
> > - popts->has_latency = true;
> > - popts->latency = 15000;
> > - }
> >
> > pw->dev = dev;
> > pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
> > @@ -781,7 +804,7 @@ static struct audio_pcm_ops qpw_pcm_ops = {
> > .init_out = qpw_init_out,
> > .fini_out = qpw_fini_out,
> > .write = qpw_write,
> > - .buffer_get_free = audio_generic_buffer_get_free,
> > + .buffer_get_free = qpw_buffer_get_free,
> > .run_buffer_out = audio_generic_run_buffer_out,
> > .enable_out = qpw_enable_out,
> >
> > diff --git a/qapi/audio.json b/qapi/audio.json
> > index 9a0d7d9ece..d49a8a670b 100644
> > --- a/qapi/audio.json
> > +++ b/qapi/audio.json
> > @@ -337,6 +337,7 @@
> > # create multiple Pipewire devices or run multiple qemu
> > # instances (default: audiodev's id, since 7.1)
> > #
> > +# @latency: Pipewire backend buffer size in microseconds (default 46440)
> > #
> > # Since: 8.0
> > ##
> > @@ -344,7 +345,8 @@
> > 'base': 'AudiodevPerDirectionOptions',
> > 'data': {
> > '*name': 'str',
> > - '*stream-name': 'str' } }
> > + '*stream-name': 'str',
> > + '*latency': 'uint32' } }
> >
>
> I suggest to use the same option names as the pulseaudio backend.
> out.latency is the effective Pipewire buffer size.
>
> With best regards,
> Volker
>
> > ##
> > # @AudiodevPipewireOptions:
> > @@ -355,16 +357,12 @@
> > #
> > # @out: options of the playback stream
> > #
> > -# @latency: add latency to playback in microseconds
> > -# (default 15000)
> > -#
> > # Since: 8.0
> > ##
> > { 'struct': 'AudiodevPipewireOptions',
> > 'data': {
> > '*in': 'AudiodevPipewirePerDirectionOptions',
> > - '*out': 'AudiodevPipewirePerDirectionOptions',
> > - '*latency': 'uint32' } }
> > + '*out': 'AudiodevPipewirePerDirectionOptions' } }
> >
> > ##
> > # @AudiodevSdlPerDirectionOptions:
>
>
Am 13.03.23 um 13:28 schrieb Dorinda Bassey:
> Hi Volker,
>
> Thanks for the patch, I've tested the patch and it works. I don't hear
> the choppy audio with this option "qemu-system-x86_64 -device
> ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> pipewire,id=audio0,out.frequency=96000,in.frequency=96000 ...."
>
> I don't understand how the req == 0 case can work at all.
>
> how this works is that b->requested could be zero when no suggestion
> is provided. For playback streams, this field contains the suggested
> amount of data to provide. hence the reason for this check.
Hi Dorinda,
there has to be a control mechanism that ensures that our write rate on
average is exactly the frame rate that the down stream audio device
writes to the DAC. My question was how can this work if we always write
4096 frames.
The answer is, that after a 4096 frames write, the callback is delayed
by 4096 frames / 44100 frames/s = 93ms. This ensures that our write rate
is exactly 44100 frames/s.
This means a fixed 4096 frames write is wrong for the req == 0 case. We
have to write 75% of timer-period frames.
If you want to test this yourself, just ignore req and assume it's 0.
With best regards,
Volker
>
> I suggest to use the same option names as the pulseaudio backend.
> out.latency is the effective Pipewire buffer size.
>
> Ack.
>
> Thanks,
> Dorinda.
>
>
> On Sat, Mar 11, 2023 at 5:19 PM Volker Rümelin <vr_qemu@t-online.de>
> wrote:
>
> > Based-on:<20230306171020.381116-1-dbassey@redhat.com>
> > ([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend for QEMU)
> >
> > This is sample code for the review of the pipewire backed. The
> > code actually works.
> >
> > An email with explanations for the changes will follow.
> >
> > Signed-off-by: Volker Rümelin<vr_qemu@t-online.de>
> > ---
> > audio/pwaudio.c | 67
> +++++++++++++++++++++++++++++++++----------------
> > qapi/audio.json | 10 +++-----
> > 2 files changed, 49 insertions(+), 28 deletions(-)
> >
> > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> > index d357761152..8e2a38938f 100644
> > --- a/audio/pwaudio.c
> > +++ b/audio/pwaudio.c
> > @@ -23,7 +23,6 @@
> > #define AUDIO_CAP "pipewire"
> > #define RINGBUFFER_SIZE (1u << 22)
> > #define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> > -#define BUFFER_SAMPLES 512
> >
> > #include "audio_int.h"
> >
> > @@ -48,6 +47,7 @@ typedef struct PWVoice {
> > struct pw_stream *stream;
> > struct spa_hook stream_listener;
> > struct spa_audio_info_raw info;
> > + uint32_t highwater_mark;
> > uint32_t frame_size;
> > struct spa_ringbuffer ring;
> > uint8_t buffer[RINGBUFFER_SIZE];
> > @@ -82,7 +82,7 @@ playback_on_process(void *data)
> > void *p;
> > struct pw_buffer *b;
> > struct spa_buffer *buf;
> > - uint32_t n_frames, req, index, n_bytes;
> > + uint32_t req, index, n_bytes;
> > int32_t avail;
> >
> > if (!v->stream) {
> > @@ -105,8 +105,7 @@ playback_on_process(void *data)
> > if (req == 0) {
> > req = 4096 * v->frame_size;
> > }
>
> I don't understand how the req == 0 case can work at all. The
> downstream
> audio device is the thinnest point in the playback stream. We can't
> write more audio frames than the audio device will consume.
>
Hi Volker,
Thank you for the clarification. I see the problem now.
So is it safe to say that:
@@ -104,8 +104,9 @@ playback_on_process(void *data)
/* calculate the total no of bytes to read data from buffer */
req = b->requested * v->frame_size;
if (req == 0) {
- req = 4096 * v->frame_size;
+ req = v->g->dev->timer_period * v->info.rate * v->frame_size * 1 /
2 / 1000000;
}
I used 50% of the timer-period and the frame_size which would give a close
margin to what the downstream audio device writes to the DAC.
Thanks,
Dorinda.
On Mon, Mar 13, 2023 at 9:06 PM Volker Rümelin <vr_qemu@t-online.de> wrote:
> Am 13.03.23 um 13:28 schrieb Dorinda Bassey:
> > Hi Volker,
> >
> > Thanks for the patch, I've tested the patch and it works. I don't hear
> > the choppy audio with this option "qemu-system-x86_64 -device
> > ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> > pipewire,id=audio0,out.frequency=96000,in.frequency=96000 ...."
> >
> > I don't understand how the req == 0 case can work at all.
> >
> > how this works is that b->requested could be zero when no suggestion
> > is provided. For playback streams, this field contains the suggested
> > amount of data to provide. hence the reason for this check.
>
> Hi Dorinda,
>
> there has to be a control mechanism that ensures that our write rate on
> average is exactly the frame rate that the down stream audio device
> writes to the DAC. My question was how can this work if we always write
> 4096 frames.
>
> The answer is, that after a 4096 frames write, the callback is delayed
> by 4096 frames / 44100 frames/s = 93ms. This ensures that our write rate
> is exactly 44100 frames/s.
>
> This means a fixed 4096 frames write is wrong for the req == 0 case. We
> have to write 75% of timer-period frames.
>
> If you want to test this yourself, just ignore req and assume it's 0.
>
> With best regards,
> Volker
>
> >
> > I suggest to use the same option names as the pulseaudio backend.
> > out.latency is the effective Pipewire buffer size.
> >
> > Ack.
> >
> > Thanks,
> > Dorinda.
> >
> >
> > On Sat, Mar 11, 2023 at 5:19 PM Volker Rümelin <vr_qemu@t-online.de>
> > wrote:
> >
> > > Based-on:<20230306171020.381116-1-dbassey@redhat.com>
> > > ([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend for QEMU)
> > >
> > > This is sample code for the review of the pipewire backed. The
> > > code actually works.
> > >
> > > An email with explanations for the changes will follow.
> > >
> > > Signed-off-by: Volker Rümelin<vr_qemu@t-online.de>
> > > ---
> > > audio/pwaudio.c | 67
> > +++++++++++++++++++++++++++++++++----------------
> > > qapi/audio.json | 10 +++-----
> > > 2 files changed, 49 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> > > index d357761152..8e2a38938f 100644
> > > --- a/audio/pwaudio.c
> > > +++ b/audio/pwaudio.c
> > > @@ -23,7 +23,6 @@
> > > #define AUDIO_CAP "pipewire"
> > > #define RINGBUFFER_SIZE (1u << 22)
> > > #define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> > > -#define BUFFER_SAMPLES 512
> > >
> > > #include "audio_int.h"
> > >
> > > @@ -48,6 +47,7 @@ typedef struct PWVoice {
> > > struct pw_stream *stream;
> > > struct spa_hook stream_listener;
> > > struct spa_audio_info_raw info;
> > > + uint32_t highwater_mark;
> > > uint32_t frame_size;
> > > struct spa_ringbuffer ring;
> > > uint8_t buffer[RINGBUFFER_SIZE];
> > > @@ -82,7 +82,7 @@ playback_on_process(void *data)
> > > void *p;
> > > struct pw_buffer *b;
> > > struct spa_buffer *buf;
> > > - uint32_t n_frames, req, index, n_bytes;
> > > + uint32_t req, index, n_bytes;
> > > int32_t avail;
> > >
> > > if (!v->stream) {
> > > @@ -105,8 +105,7 @@ playback_on_process(void *data)
> > > if (req == 0) {
> > > req = 4096 * v->frame_size;
> > > }
> >
> > I don't understand how the req == 0 case can work at all. The
> > downstream
> > audio device is the thinnest point in the playback stream. We can't
> > write more audio frames than the audio device will consume.
> >
>
>
Am 14.03.23 um 12:50 schrieb Dorinda Bassey:
> Hi Volker,
>
> Thank you for the clarification. I see the problem now.
> So is it safe to say that:
>
> @@ -104,8 +104,9 @@ playback_on_process(void *data)
> /* calculate the total no of bytes to read data from buffer */
> req = b->requested * v->frame_size;
> if (req == 0) {
> - req = 4096 * v->frame_size;
> + req = v->g->dev->timer_period * v->info.rate * v->frame_size
> * 1 / 2 / 1000000;
> }
>
> I used 50% of the timer-period and the frame_size which would give a
> close margin to what the downstream audio device writes to the DAC.
Hi,
50% is fine by me. But you should rearrange the term. The multiplication
by v->frame_size should come last, otherwise it's not guaranteed that
req is a multiple of v->frame_size. I would cast timer_period to
uint64_t. Then timer_period * info.rate has a 32bit * 32bit => 64bit
result. 20000 us * 256000 frames/s already has more than 32 bits.
+ req = (uint64_t)v->g->dev->timer_period * v->info.rate * 1 / 2
/ 1000000 * v->frame_size;
With best regards,
Volker
>
> Thanks,
> Dorinda.
>
> On Mon, Mar 13, 2023 at 9:06 PM Volker Rümelin <vr_qemu@t-online.de>
> wrote:
>
> Am 13.03.23 um 13:28 schrieb Dorinda Bassey:
> > Hi Volker,
> >
> > Thanks for the patch, I've tested the patch and it works. I
> don't hear
> > the choppy audio with this option "qemu-system-x86_64 -device
> > ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> > pipewire,id=audio0,out.frequency=96000,in.frequency=96000 ...."
> >
> > I don't understand how the req == 0 case can work at all.
> >
> > how this works is that b->requested could be zero when no
> suggestion
> > is provided. For playback streams, this field contains the
> suggested
> > amount of data to provide. hence the reason for this check.
>
> Hi Dorinda,
>
> there has to be a control mechanism that ensures that our write
> rate on
> average is exactly the frame rate that the down stream audio device
> writes to the DAC. My question was how can this work if we always
> write
> 4096 frames.
>
> The answer is, that after a 4096 frames write, the callback is
> delayed
> by 4096 frames / 44100 frames/s = 93ms. This ensures that our
> write rate
> is exactly 44100 frames/s.
>
> This means a fixed 4096 frames write is wrong for the req == 0
> case. We
> have to write 75% of timer-period frames.
>
> If you want to test this yourself, just ignore req and assume it's 0.
>
> With best regards,
> Volker
>
> >
> > I suggest to use the same option names as the pulseaudio
> backend.
> > out.latency is the effective Pipewire buffer size.
> >
> > Ack.
> >
> > Thanks,
> > Dorinda.
> >
> >
> > On Sat, Mar 11, 2023 at 5:19 PM Volker Rümelin
> <vr_qemu@t-online.de>
> > wrote:
> >
> > > Based-on:<20230306171020.381116-1-dbassey@redhat.com>
> > > ([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend
> for QEMU)
> > >
> > > This is sample code for the review of the pipewire backed. The
> > > code actually works.
> > >
> > > An email with explanations for the changes will follow.
> > >
> > > Signed-off-by: Volker Rümelin<vr_qemu@t-online.de>
> > > ---
> > > audio/pwaudio.c | 67
> > +++++++++++++++++++++++++++++++++----------------
> > > qapi/audio.json | 10 +++-----
> > > 2 files changed, 49 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> > > index d357761152..8e2a38938f 100644
> > > --- a/audio/pwaudio.c
> > > +++ b/audio/pwaudio.c
> > > @@ -23,7 +23,6 @@
> > > #define AUDIO_CAP "pipewire"
> > > #define RINGBUFFER_SIZE (1u << 22)
> > > #define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> > > -#define BUFFER_SAMPLES 512
> > >
> > > #include "audio_int.h"
> > >
> > > @@ -48,6 +47,7 @@ typedef struct PWVoice {
> > > struct pw_stream *stream;
> > > struct spa_hook stream_listener;
> > > struct spa_audio_info_raw info;
> > > + uint32_t highwater_mark;
> > > uint32_t frame_size;
> > > struct spa_ringbuffer ring;
> > > uint8_t buffer[RINGBUFFER_SIZE];
> > > @@ -82,7 +82,7 @@ playback_on_process(void *data)
> > > void *p;
> > > struct pw_buffer *b;
> > > struct spa_buffer *buf;
> > > - uint32_t n_frames, req, index, n_bytes;
> > > + uint32_t req, index, n_bytes;
> > > int32_t avail;
> > >
> > > if (!v->stream) {
> > > @@ -105,8 +105,7 @@ playback_on_process(void *data)
> > > if (req == 0) {
> > > req = 4096 * v->frame_size;
> > > }
> >
> > I don't understand how the req == 0 case can work at all. The
> > downstream
> > audio device is the thinnest point in the playback stream.
> We can't
> > write more audio frames than the audio device will consume.
> >
>
Hi
On Mon, Mar 6, 2023 at 9:11 PM Dorinda Bassey <dbassey@redhat.com> wrote:
>
> This commit adds a new audiodev backend to allow QEMU to use Pipewire as
> both an audio sink and source. This backend is available on most systems
>
> Add Pipewire entry points for QEMU Pipewire audio backend
> Add wrappers for QEMU Pipewire audio backend in qpw_pcm_ops()
> qpw_write function returns the current state of the stream to pwaudio
> and Writes some data to the server for playback streams using pipewire
> spa_ringbuffer implementation.
> qpw_read function returns the current state of the stream to pwaudio and
> reads some data from the server for capture streams using pipewire
> spa_ringbuffer implementation. These functions qpw_write and qpw_read
> are called during playback and capture.
> Added some functions that convert pw audio formats to QEMU audio format
> and vice versa which would be needed in the pipewire audio sink and
> source functions qpw_init_in() & qpw_init_out().
> These methods that implement playback and recording will create streams
> for playback and capture that will start processing and will result in
> the on_process callbacks to be called.
> Built a connection to the Pipewire sound system server in the
> qpw_audio_init() method.
>
> Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> ---
> v7:
> use qemu tracing tool
>
> audio/audio.c | 3 +
> audio/audio_template.h | 4 +
> audio/meson.build | 1 +
> audio/pwaudio.c | 814 ++++++++++++++++++++++++++++++++++
> audio/trace-events | 7 +
> meson.build | 8 +
> meson_options.txt | 4 +-
> qapi/audio.json | 45 ++
> qemu-options.hx | 17 +
> scripts/meson-buildoptions.sh | 8 +-
> 10 files changed, 908 insertions(+), 3 deletions(-)
> create mode 100644 audio/pwaudio.c
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 4290309d18..aa55e41ad8 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -2069,6 +2069,9 @@ void audio_create_pdos(Audiodev *dev)
> #ifdef CONFIG_AUDIO_PA
> CASE(PA, pa, Pa);
> #endif
> +#ifdef CONFIG_AUDIO_PIPEWIRE
> + CASE(PIPEWIRE, pipewire, Pipewire);
> +#endif
> #ifdef CONFIG_AUDIO_SDL
> CASE(SDL, sdl, Sdl);
> #endif
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 42b4712acb..0f02afb921 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -355,6 +355,10 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
> case AUDIODEV_DRIVER_PA:
> return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
> #endif
> +#ifdef CONFIG_AUDIO_PIPEWIRE
> + case AUDIODEV_DRIVER_PIPEWIRE:
> + return qapi_AudiodevPipewirePerDirectionOptions_base(dev->u.pipewire.TYPE);
> +#endif
> #ifdef CONFIG_AUDIO_SDL
> case AUDIODEV_DRIVER_SDL:
> return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
> diff --git a/audio/meson.build b/audio/meson.build
> index 0722224ba9..65a49c1a10 100644
> --- a/audio/meson.build
> +++ b/audio/meson.build
> @@ -19,6 +19,7 @@ foreach m : [
> ['sdl', sdl, files('sdlaudio.c')],
> ['jack', jack, files('jackaudio.c')],
> ['sndio', sndio, files('sndioaudio.c')],
> + ['pipewire', pipewire, files('pwaudio.c')],
> ['spice', spice, files('spiceaudio.c')]
> ]
> if m[1].found()
> diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> new file mode 100644
> index 0000000000..d357761152
> --- /dev/null
> +++ b/audio/pwaudio.c
> @@ -0,0 +1,814 @@
> +/*
> + * QEMU Pipewire audio driver
> + *
> + * Copyright (c) 2023 Red Hat Inc.
> + *
> + * Author: Dorinda Bassey <dbassey@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "audio.h"
> +#include <errno.h>
> +#include "qemu/error-report.h"
> +#include <spa/param/audio/format-utils.h>
> +#include <spa/utils/ringbuffer.h>
> +#include <spa/utils/result.h>
> +
> +#include <pipewire/pipewire.h>
> +#include "trace.h"
> +
> +#define AUDIO_CAP "pipewire"
> +#define RINGBUFFER_SIZE (1u << 22)
> +#define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> +#define BUFFER_SAMPLES 512
> +
> +#include "audio_int.h"
> +
> +enum {
> + MODE_SINK,
> + MODE_SOURCE
> +};
> +
> +typedef struct pwaudio {
> + Audiodev *dev;
> + struct pw_thread_loop *thread_loop;
> + struct pw_context *context;
> +
> + struct pw_core *core;
> + struct spa_hook core_listener;
> + int seq;
> +} pwaudio;
> +
> +typedef struct PWVoice {
> + pwaudio *g;
> + bool enabled;
> + struct pw_stream *stream;
> + struct spa_hook stream_listener;
> + struct spa_audio_info_raw info;
> + uint32_t frame_size;
> + struct spa_ringbuffer ring;
> + uint8_t buffer[RINGBUFFER_SIZE];
> +
> + uint32_t mode;
> + struct pw_properties *props;
> +} PWVoice;
> +
> +typedef struct PWVoiceOut {
> + HWVoiceOut hw;
> + PWVoice v;
> +} PWVoiceOut;
> +
> +typedef struct PWVoiceIn {
> + HWVoiceIn hw;
> + PWVoice v;
> +} PWVoiceIn;
> +
> +static void
> +stream_destroy(void *data)
> +{
> + PWVoice *v = (PWVoice *) data;
> + spa_hook_remove(&v->stream_listener);
> + v->stream = NULL;
> +}
That doesn't seem necessary. The doc doesn't say much, but none of the
examples or pw_stream_new_simple() implementation remove the hook on
destroy. I think you can remove that callback.
> +
> +/* output data processing function to read stuffs from the buffer */
> +static void
> +playback_on_process(void *data)
> +{
> + PWVoice *v = (PWVoice *) data;
> + void *p;
> + struct pw_buffer *b;
> + struct spa_buffer *buf;
> + uint32_t n_frames, req, index, n_bytes;
> + int32_t avail;
> +
> + if (!v->stream) {
> + return;
> + }
Can this happen? If you get an event callback for a stream, it should be alive.
> +
> + /* obtain a buffer to read from */
> + b = pw_stream_dequeue_buffer(v->stream);
> + if (b == NULL) {
> + error_report("out of buffers: %s", strerror(errno));
> + return;
> + }
> +
> + buf = b->buffer;
> + p = buf->datas[0].data;
> + if (p == NULL) {
> + return;
> + }
> + req = b->requested * v->frame_size;
> + if (req == 0) {
> + req = 4096 * v->frame_size;
What is this for? worth a comment at least.
> + }
> + n_frames = SPA_MIN(req, buf->datas[0].maxsize);
req is in bytes already, maxsize as well.
> + n_bytes = n_frames * v->frame_size;
So here, you multiply by frame_size^2, looking wrong to me.
> +
> + /* get no of available bytes to read data from buffer */
> +
> + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> +
> + if (!v->enabled) {
See below for state/enabled change.
> + avail = 0;
> + }
> +
> + if (avail == 0) {
> + memset(p, 0, n_bytes);
> + } else {
> + if (avail < (int32_t) n_bytes) {
> + n_bytes = avail;
> + }
> +
> + spa_ringbuffer_read_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK, p, n_bytes);
The ringbuffer maybe full, in which case playback may suffer a large
delay. I think we should maintain a small fixed-delay ringbuffer, or
find a different solution. See below
> +
> + index += n_bytes;
> + spa_ringbuffer_read_update(&v->ring, index);
> + }
> +
> + buf->datas[0].chunk->offset = 0;
> + buf->datas[0].chunk->stride = v->frame_size;
> + buf->datas[0].chunk->size = n_bytes;
> +
> + /* queue the buffer for playback */
> + pw_stream_queue_buffer(v->stream, b);
> +}
> +
> +/* output data processing function to generate stuffs in the buffer */
> +static void
> +capture_on_process(void *data)
> +{
> + PWVoice *v = (PWVoice *) data;
> + void *p;
> + struct pw_buffer *b;
> + struct spa_buffer *buf;
> + int32_t filled;
> + uint32_t index, offs, n_bytes;
> +
> + if (!v->stream) {
> + return;
> + }
> +
> + /* obtain a buffer */
> + b = pw_stream_dequeue_buffer(v->stream);
> + if (b == NULL) {
> + error_report("out of buffers: %s", strerror(errno));
> + return;
> + }
> +
> + /* Write data into buffer */
> + buf = b->buffer;
> + p = buf->datas[0].data;
> + if (p == NULL) {
> + return;
> + }
> + offs = SPA_MIN(buf->datas[0].chunk->offset, buf->datas[0].maxsize);
> + n_bytes = SPA_MIN(buf->datas[0].chunk->size, buf->datas[0].maxsize - offs);
> +
> + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> +
> + if (!v->enabled) {
> + n_bytes = 0;
> + }
> +
> + if (filled < 0) {
> + error_report("%p: underrun write:%u filled:%d", p, index, filled);
> + } else {
> + if ((uint32_t) filled + n_bytes > RINGBUFFER_SIZE) {
> + error_report("%p: overrun write:%u filled:%d + size:%u > max:%u",
> + p, index, filled, n_bytes, RINGBUFFER_SIZE);
> + }
> + }
> + spa_ringbuffer_write_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK,
> + SPA_PTROFF(p, offs, void), n_bytes);
> + index += n_bytes;
> + spa_ringbuffer_write_update(&v->ring, index);
> +
> + /* queue the buffer for playback */
> + pw_stream_queue_buffer(v->stream, b);
> +}
> +
> +static void
> +on_stream_state_changed(void *_data, enum pw_stream_state old,
> + enum pw_stream_state state, const char *error)
> +{
> + PWVoice *v = (PWVoice *) _data;
> +
> + trace_pw_state_changed(pw_stream_state_as_string(state));
> +
> + switch (state) {
> + case PW_STREAM_STATE_ERROR:
> + case PW_STREAM_STATE_UNCONNECTED:
> + {
> + break;
> + }
> + case PW_STREAM_STATE_PAUSED:
> + trace_pw_node(pw_stream_get_node_id(v->stream));
> + break;
> + case PW_STREAM_STATE_CONNECTING:
> + case PW_STREAM_STATE_STREAMING:
> + break;
> + }
> +}
I suggest removing that callback, it doesn't seem useful either.
> +
> +static const struct pw_stream_events capture_stream_events = {
> + PW_VERSION_STREAM_EVENTS,
> + .destroy = stream_destroy,
> + .state_changed = on_stream_state_changed,
> + .process = capture_on_process
> +};
> +
> +static const struct pw_stream_events playback_stream_events = {
> + PW_VERSION_STREAM_EVENTS,
> + .destroy = stream_destroy,
> + .state_changed = on_stream_state_changed,
> + .process = playback_on_process
> +};
> +
> +static size_t
> +qpw_read(HWVoiceIn *hw, void *data, size_t len)
> +{
> + PWVoiceIn *pw = (PWVoiceIn *) hw;
> + PWVoice *v = &pw->v;
> + pwaudio *c = v->g;
> + const char *error = NULL;
> + size_t l;
> + int32_t avail;
> + uint32_t index;
> +
> + pw_thread_loop_lock(c->thread_loop);
> + if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
> + /* wait for stream to become ready */
> + l = 0;
> + goto done_unlock;
> + }
> + /* get no of available bytes to read data from buffer */
> + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> +
> + trace_pw_read(avail, index, len);
> +
> + if (avail < (int32_t) len) {
> + len = avail;
> + }
> +
> + spa_ringbuffer_read_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK, data, len);
> + index += len;
> + spa_ringbuffer_read_update(&v->ring, index);
> + l = len;
> +
> +done_unlock:
> + pw_thread_loop_unlock(c->thread_loop);
> + return l;
> +}
> +
> +static size_t
> +qpw_write(HWVoiceOut *hw, void *data, size_t len)
> +{
> + PWVoiceOut *pw = (PWVoiceOut *) hw;
> + PWVoice *v = &pw->v;
> + pwaudio *c = v->g;
> + const char *error = NULL;
> + const int periods = 3;
(hmm..)
> + size_t l;
> + int32_t filled, avail;
> + uint32_t index;
> +
> + pw_thread_loop_lock(c->thread_loop);
> + if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
> + /* wait for stream to become ready */
> + l = 0;
> + goto done_unlock;
> + }
> + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> +
> + avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
Ok, here I think you tried to mimic jackaudio.c, by using a default of
3 * 512 frames ring/fifo (seems quite arbitrary but if it works...)
However, you don't actually maintain the ringbuffer size, it will keep
growing, just with smaller writes,? I think you could use this value
as the ringbuffer size. The potential additional delay would then be
3*512/samplerate (ex 32ms at 48khz).
> +
> + trace_pw_write(filled, avail, index, len);
> +
> + if (len > avail) {
> + len = avail;
> + }
> +
> + if (filled < 0) {
> + error_report("%p: underrun write:%u filled:%d", pw, index, filled);
> + } else {
> + if ((uint32_t) filled + len > RINGBUFFER_SIZE) {
> + error_report("%p: overrun write:%u filled:%d + size:%zu > max:%u",
> + pw, index, filled, len, RINGBUFFER_SIZE);
> + }
> + }
> +
> + spa_ringbuffer_write_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK, data, len);
> + index += len;
> + spa_ringbuffer_write_update(&v->ring, index);
> + l = len;
> +
> +done_unlock:
> + pw_thread_loop_unlock(c->thread_loop);
> + return l;
> +}
> +
> +static int
> +audfmt_to_pw(AudioFormat fmt, int endianness)
> +{
> + int format;
> +
> + switch (fmt) {
> + case AUDIO_FORMAT_S8:
> + format = SPA_AUDIO_FORMAT_S8;
> + break;
> + case AUDIO_FORMAT_U8:
> + format = SPA_AUDIO_FORMAT_U8;
> + break;
> + case AUDIO_FORMAT_S16:
> + format = endianness ? SPA_AUDIO_FORMAT_S16_BE : SPA_AUDIO_FORMAT_S16_LE;
> + break;
> + case AUDIO_FORMAT_U16:
> + format = endianness ? SPA_AUDIO_FORMAT_U16_BE : SPA_AUDIO_FORMAT_U16_LE;
> + break;
> + case AUDIO_FORMAT_S32:
> + format = endianness ? SPA_AUDIO_FORMAT_S32_BE : SPA_AUDIO_FORMAT_S32_LE;
> + break;
> + case AUDIO_FORMAT_U32:
> + format = endianness ? SPA_AUDIO_FORMAT_U32_BE : SPA_AUDIO_FORMAT_U32_LE;
> + break;
> + case AUDIO_FORMAT_F32:
> + format = endianness ? SPA_AUDIO_FORMAT_F32_BE : SPA_AUDIO_FORMAT_F32_LE;
> + break;
> + default:
> + dolog("Internal logic error: Bad audio format %d\n", fmt);
> + format = SPA_AUDIO_FORMAT_U8;
> + break;
> + }
> + return format;
> +}
> +
> +static AudioFormat
> +pw_to_audfmt(enum spa_audio_format fmt, int *endianness,
> + uint32_t *frame_size)
> +{
> + switch (fmt) {
> + case SPA_AUDIO_FORMAT_S8:
> + *frame_size = 1;
> + return AUDIO_FORMAT_S8;
> + case SPA_AUDIO_FORMAT_U8:
> + *frame_size = 1;
> + return AUDIO_FORMAT_U8;
> + case SPA_AUDIO_FORMAT_S16_BE:
> + *frame_size = 2;
> + *endianness = 1;
> + return AUDIO_FORMAT_S16;
> + case SPA_AUDIO_FORMAT_S16_LE:
> + *frame_size = 2;
> + *endianness = 0;
> + return AUDIO_FORMAT_S16;
> + case SPA_AUDIO_FORMAT_U16_BE:
> + *frame_size = 2;
> + *endianness = 1;
> + return AUDIO_FORMAT_U16;
> + case SPA_AUDIO_FORMAT_U16_LE:
> + *frame_size = 2;
> + *endianness = 0;
> + return AUDIO_FORMAT_U16;
> + case SPA_AUDIO_FORMAT_S32_BE:
> + *frame_size = 4;
> + *endianness = 1;
> + return AUDIO_FORMAT_S32;
> + case SPA_AUDIO_FORMAT_S32_LE:
> + *frame_size = 4;
> + *endianness = 0;
> + return AUDIO_FORMAT_S32;
> + case SPA_AUDIO_FORMAT_U32_BE:
> + *frame_size = 4;
> + *endianness = 1;
> + return AUDIO_FORMAT_U32;
> + case SPA_AUDIO_FORMAT_U32_LE:
> + *frame_size = 4;
> + *endianness = 0;
> + return AUDIO_FORMAT_U32;
> + case SPA_AUDIO_FORMAT_F32_BE:
> + *frame_size = 4;
> + *endianness = 1;
> + return AUDIO_FORMAT_F32;
> + case SPA_AUDIO_FORMAT_F32_LE:
> + *frame_size = 4;
> + *endianness = 0;
> + return AUDIO_FORMAT_F32;
> + default:
> + *frame_size = 1;
> + dolog("Internal logic error: Bad spa_audio_format %d\n", fmt);
> + return AUDIO_FORMAT_U8;
> + }
> +}
> +
> +static int
> +create_stream(pwaudio *c, PWVoice *v, const char *name)
> +{
> + int res;
> + uint32_t n_params;
> + const struct spa_pod *params[2];
> + uint8_t buffer[1024];
> + struct spa_pod_builder b;
> +
> + v->stream = pw_stream_new(c->core, name, NULL);
> +
> + if (v->stream == NULL) {
> + goto error;
> + }
> +
> + if (v->mode == MODE_SOURCE) {
> + pw_stream_add_listener(v->stream,
> + &v->stream_listener, &capture_stream_events, v);
> + } else {
> + pw_stream_add_listener(v->stream,
> + &v->stream_listener, &playback_stream_events, v);
> + }
> +
> + n_params = 0;
> + spa_pod_builder_init(&b, buffer, sizeof(buffer));
> + params[n_params++] = spa_format_audio_raw_build(&b,
> + SPA_PARAM_EnumFormat,
> + &v->info);
> +
> + /* connect the stream to a sink or source */
> + res = pw_stream_connect(v->stream,
> + v->mode ==
> + MODE_SOURCE ? PW_DIRECTION_INPUT :
> + PW_DIRECTION_OUTPUT, PW_ID_ANY,
> + PW_STREAM_FLAG_AUTOCONNECT |
> + PW_STREAM_FLAG_MAP_BUFFERS |
> + PW_STREAM_FLAG_RT_PROCESS, params, n_params);
> + if (res < 0) {
> + goto error;
> + }
> +
> + return 0;
> +error:
> + pw_stream_destroy(v->stream);
> + return -1;
> +}
> +
> +static int
> +qpw_stream_new(pwaudio *c, PWVoice *v, const char *name)
> +{
> + int r;
> +
> + switch (v->info.channels) {
> + case 8:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> + v->info.position[6] = SPA_AUDIO_CHANNEL_SL;
> + v->info.position[7] = SPA_AUDIO_CHANNEL_SR;
> + break;
> + case 6:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> + break;
> + case 5:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> + v->info.position[4] = SPA_AUDIO_CHANNEL_RC;
> + break;
> + case 4:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_RC;
> + break;
> + case 3:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_LFE;
> + break;
> + case 2:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + break;
> + case 1:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_MONO;
> + break;
> + default:
> + for (size_t i = 0; i < v->info.channels; i++) {
> + v->info.position[i] = SPA_AUDIO_CHANNEL_UNKNOWN;
> + }
> + break;
> + }
> +
> + /* create a new unconnected pwstream */
> + r = create_stream(c, v, name);
> + if (r < 0) {
> + goto error;
> + }
> +
> + return r;
> +
> +error:
> + AUD_log(AUDIO_CAP, "Failed to create stream.");
> + return -1;
> +}
> +
> +static int
> +qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
> +{
> + PWVoiceOut *pw = (PWVoiceOut *) hw;
> + PWVoice *v = &pw->v;
> + struct audsettings obt_as = *as;
> + pwaudio *c = v->g = drv_opaque;
> + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> + AudiodevPipewirePerDirectionOptions *ppdo = popts->out;
> + int r;
> + v->enabled = false;
> +
> + v->mode = MODE_SINK;
> +
> + pw_thread_loop_lock(c->thread_loop);
> +
> + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> + v->info.channels = as->nchannels;
> + v->info.rate = as->freq;
> +
> + obt_as.fmt =
> + pw_to_audfmt(v->info.format, &obt_as.endianness, &v->frame_size);
> + v->frame_size *= as->nchannels;
> +
> + /* call the function that creates a new stream for playback */
> + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> + if (r < 0) {
> + error_report("qpw_stream_new for playback failed\n ");
> + goto fail;
> + }
> +
> + /* report the audio format we support */
> + audio_pcm_init_info(&hw->info, &obt_as);
> +
> + /* report the buffer size to qemu */
> + hw->samples = BUFFER_SAMPLES;
> +
> + pw_thread_loop_unlock(c->thread_loop);
> + return 0;
> +fail:
> + pw_thread_loop_unlock(c->thread_loop);
> + return -1;
> +}
> +
> +static int
> +qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
> +{
> + PWVoiceIn *pw = (PWVoiceIn *) hw;
> + PWVoice *v = &pw->v;
> + struct audsettings obt_as = *as;
> + pwaudio *c = v->g = drv_opaque;
> + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> + AudiodevPipewirePerDirectionOptions *ppdo = popts->in;
> + int r;
> + v->enabled = false;
> +
> + v->mode = MODE_SOURCE;
> + pw_thread_loop_lock(c->thread_loop);
> +
> + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> + v->info.channels = as->nchannels;
> + v->info.rate = as->freq;
> +
> + obt_as.fmt =
> + pw_to_audfmt(v->info.format, &obt_as.endianness, &v->frame_size);
> + v->frame_size *= as->nchannels;
> +
> + /* call the function that creates a new stream for recording */
> + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> + if (r < 0) {
> + error_report("qpw_stream_new for recording failed\n ");
> + goto fail;
> + }
> +
> + /* report the audio format we support */
> + audio_pcm_init_info(&hw->info, &obt_as);
> +
> + /* report the buffer size to qemu */
> + hw->samples = BUFFER_SAMPLES;
> +
> + pw_thread_loop_unlock(c->thread_loop);
> + return 0;
> +fail:
> + pw_thread_loop_unlock(c->thread_loop);
> + return -1;
> +}
> +
> +static void
> +qpw_fini_out(HWVoiceOut *hw)
> +{
> + PWVoiceOut *pw = (PWVoiceOut *) hw;
> + PWVoice *v = &pw->v;
> +
> + if (v->stream) {
> + pwaudio *c = v->g;
> + pw_thread_loop_lock(c->thread_loop);
> + pw_stream_destroy(v->stream);
> + v->stream = NULL;
> + pw_thread_loop_unlock(c->thread_loop);
> + }
> +}
> +
> +static void
> +qpw_fini_in(HWVoiceIn *hw)
> +{
> + PWVoiceIn *pw = (PWVoiceIn *) hw;
> + PWVoice *v = &pw->v;
> +
> + if (v->stream) {
> + pwaudio *c = v->g;
> + pw_thread_loop_lock(c->thread_loop);
> + pw_stream_destroy(v->stream);
> + v->stream = NULL;
> + pw_thread_loop_unlock(c->thread_loop);
> + }
> +}
> +
> +static void
> +qpw_enable_out(HWVoiceOut *hw, bool enable)
> +{
> + PWVoiceOut *po = (PWVoiceOut *) hw;
> + PWVoice *v = &po->v;
> + v->enabled = enable;
Have you considered pw_stream_set_active() ?
> +}
> +
> +static void
> +qpw_enable_in(HWVoiceIn *hw, bool enable)
> +{
> + PWVoiceIn *pi = (PWVoiceIn *) hw;
> + PWVoice *v = &pi->v;
> + v->enabled = enable;
same
> +}
> +
> +static void
> +on_core_error(void *data, uint32_t id, int seq, int res, const char *message)
> +{
> + pwaudio *pw = data;
> +
> + error_report("error id:%u seq:%d res:%d (%s): %s",
> + id, seq, res, spa_strerror(res), message);
> +
> + pw_thread_loop_signal(pw->thread_loop, FALSE);
> +}
> +
> +static void
> +on_core_done(void *data, uint32_t id, int seq)
> +{
> + pwaudio *pw = data;
> + if (id == PW_ID_CORE) {
> + pw->seq = seq;
> + pw_thread_loop_signal(pw->thread_loop, FALSE);
> + }
> +}
> +
> +static const struct pw_core_events core_events = {
> + PW_VERSION_CORE_EVENTS,
> + .done = on_core_done,
> + .error = on_core_error,
> +};
> +
> +static void *
> +qpw_audio_init(Audiodev *dev)
> +{
> + pwaudio *pw;
> + pw = g_new0(pwaudio, 1);
> + pw_init(NULL, NULL);
> +
> + AudiodevPipewireOptions *popts;
> + trace_pw_audio_init("Initialize Pipewire context\n");
> + assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> + popts = &dev->u.pipewire;
> +
> + if (!popts->has_latency) {
> + popts->has_latency = true;
> + popts->latency = 15000;
> + }
This option is not taken into account. It seems you should set the
stream PW_KEY_NODE_LATENCY, which must be a fraction as a string. You
can either convert the microseconds latency to a fraction
("delay/1000000"), or change the argument to use fractions instead
(although this would be different from other latency properties in
QEMU...)
> +
> + pw->dev = dev;
> + pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
> + if (pw->thread_loop == NULL) {
> + error_report("Could not create Pipewire loop");
> + goto fail_loop;
> + }
> +
> + pw->context =
> + pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL, 0);
> + if (pw->context == NULL) {
> + error_report("Could not create Pipewire context");
> + goto fail_loop;
> + }
> +
> + if (pw_thread_loop_start(pw->thread_loop) < 0) {
> + error_report("Could not start Pipewire loop");
> + goto fail_start;
> + }
> +
> + pw_thread_loop_lock(pw->thread_loop);
> +
> + pw->core = pw_context_connect(pw->context, NULL, 0);
> + if (pw->core == NULL) {
> + goto fail_conn;
> + }
> +
> + pw_core_add_listener(pw->core, &pw->core_listener, &core_events, pw);
> +
> + pw_thread_loop_unlock(pw->thread_loop);
> +
> + return pw;
> +
> +fail_loop:
> + pw_thread_loop_destroy(pw->thread_loop);
> + g_free(pw);
> + return NULL;
> +fail_start:
> + pw_context_destroy(pw->context);
> + g_free(pw);
> + return NULL;
> +fail_conn:
> + AUD_log(AUDIO_CAP, "Failed to initialize PW context");
> + pw_thread_loop_unlock(pw->thread_loop);
> + pw_thread_loop_stop(pw->thread_loop);
> + pw_core_disconnect(pw->core);
> + pw_context_destroy(pw->context);
> + pw_thread_loop_destroy(pw->thread_loop);
> + g_free(pw);
> + return NULL;
> +}
> +
> +static void
> +qpw_audio_fini(void *opaque)
> +{
> + pwaudio *pw = opaque;
> +
> + pw_thread_loop_stop(pw->thread_loop);
> +
> + if (pw->core) {
> + spa_hook_remove(&pw->core_listener);
> + spa_zero(pw->core_listener);
> + pw_core_disconnect(pw->core);
> + }
> +
> + if (pw->context) {
> + pw_context_destroy(pw->context);
> + }
> + pw_thread_loop_destroy(pw->thread_loop);
> +
> + g_free(pw);
> +}
> +
> +static struct audio_pcm_ops qpw_pcm_ops = {
> + .init_out = qpw_init_out,
> + .fini_out = qpw_fini_out,
> + .write = qpw_write,
> + .buffer_get_free = audio_generic_buffer_get_free,
> + .run_buffer_out = audio_generic_run_buffer_out,
> + .enable_out = qpw_enable_out,
> +
> + .init_in = qpw_init_in,
> + .fini_in = qpw_fini_in,
> + .read = qpw_read,
> + .run_buffer_in = audio_generic_run_buffer_in,
> + .enable_in = qpw_enable_in
> +};
> +
> +static struct audio_driver pw_audio_driver = {
> + .name = "pipewire",
> + .descr = "http://www.pipewire.org/",
> + .init = qpw_audio_init,
> + .fini = qpw_audio_fini,
> + .pcm_ops = &qpw_pcm_ops,
> + .can_be_default = 1,
> + .max_voices_out = INT_MAX,
> + .max_voices_in = INT_MAX,
> + .voice_size_out = sizeof(PWVoiceOut),
> + .voice_size_in = sizeof(PWVoiceIn),
> +};
> +
> +static void
> +register_audio_pw(void)
> +{
> + audio_driver_register(&pw_audio_driver);
> +}
> +
> +type_init(register_audio_pw);
> diff --git a/audio/trace-events b/audio/trace-events
> index e1ab643add..2ecd8851e6 100644
> --- a/audio/trace-events
> +++ b/audio/trace-events
> @@ -18,6 +18,13 @@ dbus_audio_register(const char *s, const char *dir) "sender = %s, dir = %s"
> dbus_audio_put_buffer_out(size_t len) "len = %zu"
> dbus_audio_read(size_t len) "len = %zu"
>
> +# pwaudio.c
> +pw_state_changed(const char *s) "stream state: %s"
> +pw_node(int nodeid) "node id: %d"
> +pw_read(int32_t avail, uint32_t index, size_t len) "avail=%u index=%u len=%zu"
> +pw_write(int32_t filled, int32_t avail, uint32_t index, size_t len) "filled=%u avail=%u index=%u len=%zu"
> +pw_audio_init(const char *msg) "pipewire: %s"
> +
> # audio.c
> audio_timer_start(int interval) "interval %d ms"
> audio_timer_stop(void) ""
> diff --git a/meson.build b/meson.build
> index 6bcab8bf0d..51ec2931e1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -730,6 +730,12 @@ if not get_option('jack').auto() or have_system
> jack = dependency('jack', required: get_option('jack'),
> method: 'pkg-config', kwargs: static_kwargs)
> endif
> +pipewire = not_found
> +if not get_option('pipewire').auto() or (targetos == 'linux' and have_system)
> + pipewire = dependency('libpipewire-0.3', version: '>=0.3.60',
> + required: get_option('pipewire'),
> + method: 'pkg-config', kwargs: static_kwargs)
> +endif
> sndio = not_found
> if not get_option('sndio').auto() or have_system
> sndio = dependency('sndio', required: get_option('sndio'),
> @@ -1667,6 +1673,7 @@ if have_system
> 'jack': jack.found(),
> 'oss': oss.found(),
> 'pa': pulse.found(),
> + 'pipewire': pipewire.found(),
> 'sdl': sdl.found(),
> 'sndio': sndio.found(),
> }
> @@ -3980,6 +3987,7 @@ if targetos == 'linux'
> summary_info += {'ALSA support': alsa}
> summary_info += {'PulseAudio support': pulse}
> endif
> +summary_info += {'Pipewire support': pipewire}
> summary_info += {'JACK support': jack}
> summary_info += {'brlapi support': brlapi}
> summary_info += {'vde support': vde}
> diff --git a/meson_options.txt b/meson_options.txt
> index fc9447d267..9ae1ec7f47 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -21,7 +21,7 @@ option('tls_priority', type : 'string', value : 'NORMAL',
> option('default_devices', type : 'boolean', value : true,
> description: 'Include a default selection of devices in emulators')
> option('audio_drv_list', type: 'array', value: ['default'],
> - choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'sdl', 'sndio'],
> + choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'pipewire', 'sdl', 'sndio'],
> description: 'Set audio driver list')
> option('block_drv_rw_whitelist', type : 'string', value : '',
> description: 'set block driver read-write whitelist (by default affects only QEMU, not tools like qemu-img)')
> @@ -255,6 +255,8 @@ option('oss', type: 'feature', value: 'auto',
> description: 'OSS sound support')
> option('pa', type: 'feature', value: 'auto',
> description: 'PulseAudio sound support')
> +option('pipewire', type: 'feature', value: 'auto',
> + description: 'Pipewire sound support')
> option('sndio', type: 'feature', value: 'auto',
> description: 'sndio sound support')
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 4e54c00f51..9a0d7d9ece 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -324,6 +324,48 @@
> '*out': 'AudiodevPaPerDirectionOptions',
> '*server': 'str' } }
>
> +##
> +# @AudiodevPipewirePerDirectionOptions:
> +#
> +# Options of the Pipewire backend that are used for both playback and
> +# recording.
> +#
> +# @name: name of the sink/source to use
> +#
> +# @stream-name: name of the Pipewire stream created by qemu. Can be
> +# used to identify the stream in Pipewire when you
> +# create multiple Pipewire devices or run multiple qemu
> +# instances (default: audiodev's id, since 7.1)
not 7.1, but you don't need to say "since .." here, as we assume it
was added with the struct then.
> +#
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'AudiodevPipewirePerDirectionOptions',
> + 'base': 'AudiodevPerDirectionOptions',
> + 'data': {
> + '*name': 'str',
> + '*stream-name': 'str' } }
> +
> +##
> +# @AudiodevPipewireOptions:
> +#
> +# Options of the Pipewire audio backend.
> +#
> +# @in: options of the capture stream
> +#
> +# @out: options of the playback stream
> +#
> +# @latency: add latency to playback in microseconds
> +# (default 15000)
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'AudiodevPipewireOptions',
> + 'data': {
> + '*in': 'AudiodevPipewirePerDirectionOptions',
> + '*out': 'AudiodevPipewirePerDirectionOptions',
> + '*latency': 'uint32' } }
> +
> ##
> # @AudiodevSdlPerDirectionOptions:
> #
> @@ -416,6 +458,7 @@
> { 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
> { 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
> { 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
> + { 'name': 'pipewire', 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> { 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
> { 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
> { 'name': 'spice', 'if': 'CONFIG_SPICE' },
> @@ -456,6 +499,8 @@
> 'if': 'CONFIG_AUDIO_OSS' },
> 'pa': { 'type': 'AudiodevPaOptions',
> 'if': 'CONFIG_AUDIO_PA' },
> + 'pipewire': { 'type': 'AudiodevPipewireOptions',
> + 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> 'sdl': { 'type': 'AudiodevSdlOptions',
> 'if': 'CONFIG_AUDIO_SDL' },
> 'sndio': { 'type': 'AudiodevSndioOptions',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d42f60fb91..009d58bbf2 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -779,6 +779,11 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> " in|out.name= source/sink device name\n"
> " in|out.latency= desired latency in microseconds\n"
> #endif
> +#ifdef CONFIG_AUDIO_PIPEWIRE
> + "-audiodev pipewire,id=id[,prop[=value][,...]]\n"
> + " in|out.name= source/sink device name\n"
> + " latency= desired latency in microseconds\n"
> +#endif
> #ifdef CONFIG_AUDIO_SDL
> "-audiodev sdl,id=id[,prop[=value][,...]]\n"
> " in|out.buffer-count= number of buffers\n"
> @@ -942,6 +947,18 @@ SRST
> Desired latency in microseconds. The PulseAudio server will try
> to honor this value but actual latencies may be lower or higher.
>
> +``-audiodev pipewire,id=id[,prop[=value][,...]]``
> + Creates a backend using Pipewire. This backend is available on
> + most systems.
> +
> + Pipewire specific options are:
> +
> + ``latency=latency``
> + Add extra latency to playback in microseconds
> +
> + ``in|out.name=sink``
> + Use the specified source/sink for recording/playback.
> +
> ``-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
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 009fab1515..ba1057b62c 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -1,7 +1,8 @@
> # This file is generated by meson-buildoptions.py, do not edit!
> meson_options_help() {
> - printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list [default] (choices: alsa/co'
> - printf "%s\n" ' reaudio/default/dsound/jack/oss/pa/sdl/sndio)'
> + printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list [default] (choices: al'
> + printf "%s\n" ' sa/coreaudio/default/dsound/jack/oss/pa/'
> + printf "%s\n" ' pipewire/sdl/sndio)'
> printf "%s\n" ' --block-drv-ro-whitelist=VALUE'
> printf "%s\n" ' set block driver read-only whitelist (by default'
> printf "%s\n" ' affects only QEMU, not tools like qemu-img)'
> @@ -136,6 +137,7 @@ meson_options_help() {
> printf "%s\n" ' oss OSS sound support'
> printf "%s\n" ' pa PulseAudio sound support'
> printf "%s\n" ' parallels parallels image format support'
> + printf "%s\n" ' pipewire Pipewire sound support'
> printf "%s\n" ' png PNG support with libpng'
> printf "%s\n" ' pvrdma Enable PVRDMA support'
> printf "%s\n" ' qcow1 qcow1 image format support'
> @@ -370,6 +372,8 @@ _meson_option_parse() {
> --disable-pa) printf "%s" -Dpa=disabled ;;
> --enable-parallels) printf "%s" -Dparallels=enabled ;;
> --disable-parallels) printf "%s" -Dparallels=disabled ;;
> + --enable-pipewire) printf "%s" -Dpipewire=enabled ;;
> + --disable-pipewire) printf "%s" -Dpipewire=disabled ;;
> --with-pkgversion=*) quote_sh "-Dpkgversion=$2" ;;
> --enable-png) printf "%s" -Dpng=enabled ;;
> --disable-png) printf "%s" -Dpng=disabled ;;
> --
> 2.39.1
>
>
Volker, Wim, it would be nice if you could review/comment too!
thanks
--
Marc-André Lureau
Hi Marc-André,
Thank you for your feedback.
Given that we have only one loop, I would set the thread name to NULL,
> (which will use "pw-thread-loop" by default)
>
I think it's preferable to be explicit and clear about the thread name.
It's not clear to me the reason being that it's only one loop.
static void *
> qpw_audio_init(Audiodev *dev)
> {
> AudiodevPipewireOptions *popts;
> g_autofree pwaudio *pw = g_new0(pwaudio, 1);
>
> assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> trace_pw_audio_init();
> pw_init(NULL, NULL);
>
> popts = &dev->u.pipewire;
> if (!popts->has_latency) {
> popts->has_latency = true;
> popts->latency = 15000;
> }
>
> pw->dev = dev;
> pw->thread_loop = pw_thread_loop_new(NULL, NULL);
> if (!pw->thread_loop) {
> goto fail;
> }
> pw->context =
> pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL, 0);
> if (!pw->context) {
> goto fail;
> }
> if (pw_thread_loop_start(pw->thread_loop) < 0) {
> goto fail;
> }
> pw_thread_loop_lock(pw->thread_loop);
> pw->core = pw_context_connect(pw->context, NULL, 0);
> if (!pw->core) {
> pw_thread_loop_unlock(pw->thread_loop);
> goto fail;
> }
> pw_core_add_listener(pw->core, &pw->core_listener, &core_events, pw);
> pw_thread_loop_unlock(pw->thread_loop);
>
> return g_steal_pointer(&pw);
>
> fail:
> error_report("Failed to initialize Pipewire: %s", strerror(errno));
> if (pw->thread_loop) {
> /* can be called even when the loop is not running */
> pw_thread_loop_stop(pw->thread_loop);
> }
> g_clear_pointer(&pw->context, pw_context_destroy);
> g_clear_pointer(&pw->thread_loop, pw_thread_loop_destroy);
> return NULL;
> }
This looks much better, Thanks. however in the error handling I would do
something like
fail:
AUD_log(AUDIO_CAP, "Failed to initialize PW context");
if (pw->thread_loop) {
pw_thread_loop_stop(pw->thread_loop);
g_clear_pointer(&pw->thread_loop, pw_thread_loop_destroy);
}
if (pw->context) {
g_clear_pointer(&pw->context, pw_context_destroy);
}
return NULL;
}
The core is always NULL when reaching error conditions. Whether this
> function must be called with the loop lock is unclear to me. Anyway,
> it should not be necessary.
>
I agree it's not needed here, this could lead to unexpected behaviour.
That doesn't seem necessary. The doc doesn't say much, but none of the
> examples or pw_stream_new_simple() implementation remove the hook on
> destroy. I think you can remove that callback.
>
In this case removing the hook ensures that the listener's callback
functions are not called after the listener has been destroyed, this would
help avoid potential issues. I would leave it there.
Can this happen? If you get an event callback for a stream, it should be
> alive.
>
I think in general it's good to have this check before attempting to
manipulate the buffer so as to avoid pointer errors. Just in case it
happens.
What is this for? worth a comment at least.
>
This represents a common buffer size for audio processing, and to ensure
that the buffer is large enough to hold a significant amount of audio data.
I had explained earlier in V2 of the patch that "For playback streams, this
size allows for more efficient streaming of audio data." Indeed, I should
add a comment. Thanks.
req is in bytes already, maxsize as well.
>
Could you please clarify a bit what you mean here?
So here, you multiply by frame_size^2, looking wrong to me.
>
basically to ensure that the buffer is large enough to hold the maximum
amount of data processed in a single transfer operation.
The ringbuffer maybe full, in which case playback may suffer a large
> delay. I think we should maintain a small fixed-delay ringbuffer, or
> find a different solution. See below
>
I don't completely understand what you mean here. IMO a larger buffer may
be more appropriate in case we need to process large amounts of data
quickly and have sufficient memory available, CMIIW.
I suggest removing that callback, it doesn't seem useful either.
>
Please clarify, which callback?
Ok, here I think you tried to mimic jackaudio.c, by using a default of
> 3 * 512 frames ring/fifo (seems quite arbitrary but if it works...)
>
yes.
However, you don't actually maintain the ringbuffer size, it will keep
> growing, just with smaller writes,? I think you could use this value
> as the ringbuffer size. The potential additional delay would then be
> 3*512/samplerate (ex 32ms at 48khz).
>
Could you explain a bit more why this suggestion?
Have you considered pw_stream_set_active() ?
>
FWIU this is used to pause or activate a stream. I think that v->enabled =
enable; is sufficient but I'm open to your suggestion, I just need to know
why and how do you suggest i use it?
This option is not taken into account. It seems you should set the
> stream PW_KEY_NODE_LATENCY, which must be a fraction as a string. You
> can either convert the microseconds latency to a fraction
> ("delay/1000000"), or change the argument to use fractions instead
> (although this would be different from other latency properties in
> QEMU...)
>
I'm open to the idea of using it.
not 7.1, but you don't need to say "since .." here, as we assume it
> was added with the struct then.
>
ack, Thanks.
On Wed, Mar 8, 2023 at 11:39 AM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Mon, Mar 6, 2023 at 9:11 PM Dorinda Bassey <dbassey@redhat.com> wrote:
> >
> > This commit adds a new audiodev backend to allow QEMU to use Pipewire as
> > both an audio sink and source. This backend is available on most systems
> >
> > Add Pipewire entry points for QEMU Pipewire audio backend
> > Add wrappers for QEMU Pipewire audio backend in qpw_pcm_ops()
> > qpw_write function returns the current state of the stream to pwaudio
> > and Writes some data to the server for playback streams using pipewire
> > spa_ringbuffer implementation.
> > qpw_read function returns the current state of the stream to pwaudio and
> > reads some data from the server for capture streams using pipewire
> > spa_ringbuffer implementation. These functions qpw_write and qpw_read
> > are called during playback and capture.
> > Added some functions that convert pw audio formats to QEMU audio format
> > and vice versa which would be needed in the pipewire audio sink and
> > source functions qpw_init_in() & qpw_init_out().
> > These methods that implement playback and recording will create streams
> > for playback and capture that will start processing and will result in
> > the on_process callbacks to be called.
> > Built a connection to the Pipewire sound system server in the
> > qpw_audio_init() method.
> >
> > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> > ---
> > v7:
> > use qemu tracing tool
> >
> > audio/audio.c | 3 +
> > audio/audio_template.h | 4 +
> > audio/meson.build | 1 +
> > audio/pwaudio.c | 814 ++++++++++++++++++++++++++++++++++
> > audio/trace-events | 7 +
> > meson.build | 8 +
> > meson_options.txt | 4 +-
> > qapi/audio.json | 45 ++
> > qemu-options.hx | 17 +
> > scripts/meson-buildoptions.sh | 8 +-
> > 10 files changed, 908 insertions(+), 3 deletions(-)
> > create mode 100644 audio/pwaudio.c
> >
> > diff --git a/audio/audio.c b/audio/audio.c
> > index 4290309d18..aa55e41ad8 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -2069,6 +2069,9 @@ void audio_create_pdos(Audiodev *dev)
> > #ifdef CONFIG_AUDIO_PA
> > CASE(PA, pa, Pa);
> > #endif
> > +#ifdef CONFIG_AUDIO_PIPEWIRE
> > + CASE(PIPEWIRE, pipewire, Pipewire);
> > +#endif
> > #ifdef CONFIG_AUDIO_SDL
> > CASE(SDL, sdl, Sdl);
> > #endif
> > diff --git a/audio/audio_template.h b/audio/audio_template.h
> > index 42b4712acb..0f02afb921 100644
> > --- a/audio/audio_template.h
> > +++ b/audio/audio_template.h
> > @@ -355,6 +355,10 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
> TYPE)(Audiodev *dev)
> > case AUDIODEV_DRIVER_PA:
> > return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
> > #endif
> > +#ifdef CONFIG_AUDIO_PIPEWIRE
> > + case AUDIODEV_DRIVER_PIPEWIRE:
> > + return
> qapi_AudiodevPipewirePerDirectionOptions_base(dev->u.pipewire.TYPE);
> > +#endif
> > #ifdef CONFIG_AUDIO_SDL
> > case AUDIODEV_DRIVER_SDL:
> > return
> qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
> > diff --git a/audio/meson.build b/audio/meson.build
> > index 0722224ba9..65a49c1a10 100644
> > --- a/audio/meson.build
> > +++ b/audio/meson.build
> > @@ -19,6 +19,7 @@ foreach m : [
> > ['sdl', sdl, files('sdlaudio.c')],
> > ['jack', jack, files('jackaudio.c')],
> > ['sndio', sndio, files('sndioaudio.c')],
> > + ['pipewire', pipewire, files('pwaudio.c')],
> > ['spice', spice, files('spiceaudio.c')]
> > ]
> > if m[1].found()
> > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> > new file mode 100644
> > index 0000000000..d357761152
> > --- /dev/null
> > +++ b/audio/pwaudio.c
> > @@ -0,0 +1,814 @@
> > +/*
> > + * QEMU Pipewire audio driver
> > + *
> > + * Copyright (c) 2023 Red Hat Inc.
> > + *
> > + * Author: Dorinda Bassey <dbassey@redhat.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/module.h"
> > +#include "audio.h"
> > +#include <errno.h>
> > +#include "qemu/error-report.h"
> > +#include <spa/param/audio/format-utils.h>
> > +#include <spa/utils/ringbuffer.h>
> > +#include <spa/utils/result.h>
> > +
> > +#include <pipewire/pipewire.h>
> > +#include "trace.h"
> > +
> > +#define AUDIO_CAP "pipewire"
> > +#define RINGBUFFER_SIZE (1u << 22)
> > +#define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> > +#define BUFFER_SAMPLES 512
> > +
> > +#include "audio_int.h"
> > +
> > +enum {
> > + MODE_SINK,
> > + MODE_SOURCE
> > +};
> > +
> > +typedef struct pwaudio {
> > + Audiodev *dev;
> > + struct pw_thread_loop *thread_loop;
> > + struct pw_context *context;
> > +
> > + struct pw_core *core;
> > + struct spa_hook core_listener;
> > + int seq;
> > +} pwaudio;
> > +
> > +typedef struct PWVoice {
> > + pwaudio *g;
> > + bool enabled;
> > + struct pw_stream *stream;
> > + struct spa_hook stream_listener;
> > + struct spa_audio_info_raw info;
> > + uint32_t frame_size;
> > + struct spa_ringbuffer ring;
> > + uint8_t buffer[RINGBUFFER_SIZE];
> > +
> > + uint32_t mode;
> > + struct pw_properties *props;
> > +} PWVoice;
> > +
> > +typedef struct PWVoiceOut {
> > + HWVoiceOut hw;
> > + PWVoice v;
> > +} PWVoiceOut;
> > +
> > +typedef struct PWVoiceIn {
> > + HWVoiceIn hw;
> > + PWVoice v;
> > +} PWVoiceIn;
> > +
> > +static void
> > +stream_destroy(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + spa_hook_remove(&v->stream_listener);
> > + v->stream = NULL;
> > +}
>
> That doesn't seem necessary. The doc doesn't say much, but none of the
> examples or pw_stream_new_simple() implementation remove the hook on
> destroy. I think you can remove that callback.
>
> > +
> > +/* output data processing function to read stuffs from the buffer */
> > +static void
> > +playback_on_process(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + void *p;
> > + struct pw_buffer *b;
> > + struct spa_buffer *buf;
> > + uint32_t n_frames, req, index, n_bytes;
> > + int32_t avail;
> > +
> > + if (!v->stream) {
> > + return;
> > + }
>
> Can this happen? If you get an event callback for a stream, it should be
> alive.
>
> > +
> > + /* obtain a buffer to read from */
> > + b = pw_stream_dequeue_buffer(v->stream);
> > + if (b == NULL) {
> > + error_report("out of buffers: %s", strerror(errno));
> > + return;
> > + }
> > +
> > + buf = b->buffer;
> > + p = buf->datas[0].data;
> > + if (p == NULL) {
> > + return;
> > + }
> > + req = b->requested * v->frame_size;
> > + if (req == 0) {
> > + req = 4096 * v->frame_size;
>
> What is this for? worth a comment at least.
>
> > + }
> > + n_frames = SPA_MIN(req, buf->datas[0].maxsize);
>
> req is in bytes already, maxsize as well.
>
> > + n_bytes = n_frames * v->frame_size;
>
> So here, you multiply by frame_size^2, looking wrong to me.
>
> > +
> > + /* get no of available bytes to read data from buffer */
> > +
> > + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> > +
> > + if (!v->enabled) {
>
> See below for state/enabled change.
>
> > + avail = 0;
> > + }
> > +
> > + if (avail == 0) {
> > + memset(p, 0, n_bytes);
> > + } else {
> > + if (avail < (int32_t) n_bytes) {
> > + n_bytes = avail;
> > + }
> > +
> > + spa_ringbuffer_read_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, p,
> n_bytes);
>
> The ringbuffer maybe full, in which case playback may suffer a large
> delay. I think we should maintain a small fixed-delay ringbuffer, or
> find a different solution. See below
>
> > +
> > + index += n_bytes;
> > + spa_ringbuffer_read_update(&v->ring, index);
> > + }
> > +
> > + buf->datas[0].chunk->offset = 0;
> > + buf->datas[0].chunk->stride = v->frame_size;
> > + buf->datas[0].chunk->size = n_bytes;
> > +
> > + /* queue the buffer for playback */
> > + pw_stream_queue_buffer(v->stream, b);
> > +}
> > +
> > +/* output data processing function to generate stuffs in the buffer */
> > +static void
> > +capture_on_process(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + void *p;
> > + struct pw_buffer *b;
> > + struct spa_buffer *buf;
> > + int32_t filled;
> > + uint32_t index, offs, n_bytes;
> > +
> > + if (!v->stream) {
> > + return;
> > + }
> > +
> > + /* obtain a buffer */
> > + b = pw_stream_dequeue_buffer(v->stream);
> > + if (b == NULL) {
> > + error_report("out of buffers: %s", strerror(errno));
> > + return;
> > + }
> > +
> > + /* Write data into buffer */
> > + buf = b->buffer;
> > + p = buf->datas[0].data;
> > + if (p == NULL) {
> > + return;
> > + }
> > + offs = SPA_MIN(buf->datas[0].chunk->offset, buf->datas[0].maxsize);
> > + n_bytes = SPA_MIN(buf->datas[0].chunk->size, buf->datas[0].maxsize
> - offs);
> > +
> > + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> > +
> > + if (!v->enabled) {
> > + n_bytes = 0;
> > + }
> > +
> > + if (filled < 0) {
> > + error_report("%p: underrun write:%u filled:%d", p, index,
> filled);
> > + } else {
> > + if ((uint32_t) filled + n_bytes > RINGBUFFER_SIZE) {
> > + error_report("%p: overrun write:%u filled:%d + size:%u >
> max:%u",
> > + p, index, filled, n_bytes, RINGBUFFER_SIZE);
> > + }
> > + }
> > + spa_ringbuffer_write_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK,
> > + SPA_PTROFF(p, offs, void), n_bytes);
> > + index += n_bytes;
> > + spa_ringbuffer_write_update(&v->ring, index);
> > +
> > + /* queue the buffer for playback */
> > + pw_stream_queue_buffer(v->stream, b);
> > +}
> > +
> > +static void
> > +on_stream_state_changed(void *_data, enum pw_stream_state old,
> > + enum pw_stream_state state, const char *error)
> > +{
> > + PWVoice *v = (PWVoice *) _data;
> > +
> > + trace_pw_state_changed(pw_stream_state_as_string(state));
> > +
> > + switch (state) {
> > + case PW_STREAM_STATE_ERROR:
> > + case PW_STREAM_STATE_UNCONNECTED:
> > + {
> > + break;
> > + }
> > + case PW_STREAM_STATE_PAUSED:
> > + trace_pw_node(pw_stream_get_node_id(v->stream));
> > + break;
> > + case PW_STREAM_STATE_CONNECTING:
> > + case PW_STREAM_STATE_STREAMING:
> > + break;
> > + }
> > +}
>
> I suggest removing that callback, it doesn't seem useful either.
>
> > +
> > +static const struct pw_stream_events capture_stream_events = {
> > + PW_VERSION_STREAM_EVENTS,
> > + .destroy = stream_destroy,
> > + .state_changed = on_stream_state_changed,
> > + .process = capture_on_process
> > +};
> > +
> > +static const struct pw_stream_events playback_stream_events = {
> > + PW_VERSION_STREAM_EVENTS,
> > + .destroy = stream_destroy,
> > + .state_changed = on_stream_state_changed,
> > + .process = playback_on_process
> > +};
> > +
> > +static size_t
> > +qpw_read(HWVoiceIn *hw, void *data, size_t len)
> > +{
> > + PWVoiceIn *pw = (PWVoiceIn *) hw;
> > + PWVoice *v = &pw->v;
> > + pwaudio *c = v->g;
> > + const char *error = NULL;
> > + size_t l;
> > + int32_t avail;
> > + uint32_t index;
> > +
> > + pw_thread_loop_lock(c->thread_loop);
> > + if (pw_stream_get_state(v->stream, &error) !=
> PW_STREAM_STATE_STREAMING) {
> > + /* wait for stream to become ready */
> > + l = 0;
> > + goto done_unlock;
> > + }
> > + /* get no of available bytes to read data from buffer */
> > + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> > +
> > + trace_pw_read(avail, index, len);
> > +
> > + if (avail < (int32_t) len) {
> > + len = avail;
> > + }
> > +
> > + spa_ringbuffer_read_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, data, len);
> > + index += len;
> > + spa_ringbuffer_read_update(&v->ring, index);
> > + l = len;
> > +
> > +done_unlock:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return l;
> > +}
> > +
> > +static size_t
> > +qpw_write(HWVoiceOut *hw, void *data, size_t len)
> > +{
> > + PWVoiceOut *pw = (PWVoiceOut *) hw;
> > + PWVoice *v = &pw->v;
> > + pwaudio *c = v->g;
> > + const char *error = NULL;
> > + const int periods = 3;
>
> (hmm..)
>
> > + size_t l;
> > + int32_t filled, avail;
> > + uint32_t index;
> > +
> > + pw_thread_loop_lock(c->thread_loop);
> > + if (pw_stream_get_state(v->stream, &error) !=
> PW_STREAM_STATE_STREAMING) {
> > + /* wait for stream to become ready */
> > + l = 0;
> > + goto done_unlock;
> > + }
> > + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> > +
> > + avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
>
> Ok, here I think you tried to mimic jackaudio.c, by using a default of
> 3 * 512 frames ring/fifo (seems quite arbitrary but if it works...)
>
> However, you don't actually maintain the ringbuffer size, it will keep
> growing, just with smaller writes,? I think you could use this value
> as the ringbuffer size. The potential additional delay would then be
> 3*512/samplerate (ex 32ms at 48khz).
>
> > +
> > + trace_pw_write(filled, avail, index, len);
> > +
> > + if (len > avail) {
> > + len = avail;
> > + }
> > +
> > + if (filled < 0) {
> > + error_report("%p: underrun write:%u filled:%d", pw, index,
> filled);
> > + } else {
> > + if ((uint32_t) filled + len > RINGBUFFER_SIZE) {
> > + error_report("%p: overrun write:%u filled:%d + size:%zu >
> max:%u",
> > + pw, index, filled, len, RINGBUFFER_SIZE);
> > + }
> > + }
> > +
> > + spa_ringbuffer_write_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, data, len);
> > + index += len;
> > + spa_ringbuffer_write_update(&v->ring, index);
> > + l = len;
> > +
> > +done_unlock:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return l;
> > +}
> > +
> > +static int
> > +audfmt_to_pw(AudioFormat fmt, int endianness)
> > +{
> > + int format;
> > +
> > + switch (fmt) {
> > + case AUDIO_FORMAT_S8:
> > + format = SPA_AUDIO_FORMAT_S8;
> > + break;
> > + case AUDIO_FORMAT_U8:
> > + format = SPA_AUDIO_FORMAT_U8;
> > + break;
> > + case AUDIO_FORMAT_S16:
> > + format = endianness ? SPA_AUDIO_FORMAT_S16_BE :
> SPA_AUDIO_FORMAT_S16_LE;
> > + break;
> > + case AUDIO_FORMAT_U16:
> > + format = endianness ? SPA_AUDIO_FORMAT_U16_BE :
> SPA_AUDIO_FORMAT_U16_LE;
> > + break;
> > + case AUDIO_FORMAT_S32:
> > + format = endianness ? SPA_AUDIO_FORMAT_S32_BE :
> SPA_AUDIO_FORMAT_S32_LE;
> > + break;
> > + case AUDIO_FORMAT_U32:
> > + format = endianness ? SPA_AUDIO_FORMAT_U32_BE :
> SPA_AUDIO_FORMAT_U32_LE;
> > + break;
> > + case AUDIO_FORMAT_F32:
> > + format = endianness ? SPA_AUDIO_FORMAT_F32_BE :
> SPA_AUDIO_FORMAT_F32_LE;
> > + break;
> > + default:
> > + dolog("Internal logic error: Bad audio format %d\n", fmt);
> > + format = SPA_AUDIO_FORMAT_U8;
> > + break;
> > + }
> > + return format;
> > +}
> > +
> > +static AudioFormat
> > +pw_to_audfmt(enum spa_audio_format fmt, int *endianness,
> > + uint32_t *frame_size)
> > +{
> > + switch (fmt) {
> > + case SPA_AUDIO_FORMAT_S8:
> > + *frame_size = 1;
> > + return AUDIO_FORMAT_S8;
> > + case SPA_AUDIO_FORMAT_U8:
> > + *frame_size = 1;
> > + return AUDIO_FORMAT_U8;
> > + case SPA_AUDIO_FORMAT_S16_BE:
> > + *frame_size = 2;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_S16;
> > + case SPA_AUDIO_FORMAT_S16_LE:
> > + *frame_size = 2;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_S16;
> > + case SPA_AUDIO_FORMAT_U16_BE:
> > + *frame_size = 2;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_U16;
> > + case SPA_AUDIO_FORMAT_U16_LE:
> > + *frame_size = 2;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_U16;
> > + case SPA_AUDIO_FORMAT_S32_BE:
> > + *frame_size = 4;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_S32;
> > + case SPA_AUDIO_FORMAT_S32_LE:
> > + *frame_size = 4;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_S32;
> > + case SPA_AUDIO_FORMAT_U32_BE:
> > + *frame_size = 4;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_U32;
> > + case SPA_AUDIO_FORMAT_U32_LE:
> > + *frame_size = 4;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_U32;
> > + case SPA_AUDIO_FORMAT_F32_BE:
> > + *frame_size = 4;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_F32;
> > + case SPA_AUDIO_FORMAT_F32_LE:
> > + *frame_size = 4;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_F32;
> > + default:
> > + *frame_size = 1;
> > + dolog("Internal logic error: Bad spa_audio_format %d\n", fmt);
> > + return AUDIO_FORMAT_U8;
> > + }
> > +}
> > +
> > +static int
> > +create_stream(pwaudio *c, PWVoice *v, const char *name)
> > +{
> > + int res;
> > + uint32_t n_params;
> > + const struct spa_pod *params[2];
> > + uint8_t buffer[1024];
> > + struct spa_pod_builder b;
> > +
> > + v->stream = pw_stream_new(c->core, name, NULL);
> > +
> > + if (v->stream == NULL) {
> > + goto error;
> > + }
> > +
> > + if (v->mode == MODE_SOURCE) {
> > + pw_stream_add_listener(v->stream,
> > + &v->stream_listener,
> &capture_stream_events, v);
> > + } else {
> > + pw_stream_add_listener(v->stream,
> > + &v->stream_listener,
> &playback_stream_events, v);
> > + }
> > +
> > + n_params = 0;
> > + spa_pod_builder_init(&b, buffer, sizeof(buffer));
> > + params[n_params++] = spa_format_audio_raw_build(&b,
> > + SPA_PARAM_EnumFormat,
> > + &v->info);
> > +
> > + /* connect the stream to a sink or source */
> > + res = pw_stream_connect(v->stream,
> > + v->mode ==
> > + MODE_SOURCE ? PW_DIRECTION_INPUT :
> > + PW_DIRECTION_OUTPUT, PW_ID_ANY,
> > + PW_STREAM_FLAG_AUTOCONNECT |
> > + PW_STREAM_FLAG_MAP_BUFFERS |
> > + PW_STREAM_FLAG_RT_PROCESS, params,
> n_params);
> > + if (res < 0) {
> > + goto error;
> > + }
> > +
> > + return 0;
> > +error:
> > + pw_stream_destroy(v->stream);
> > + return -1;
> > +}
> > +
> > +static int
> > +qpw_stream_new(pwaudio *c, PWVoice *v, const char *name)
> > +{
> > + int r;
> > +
> > + switch (v->info.channels) {
> > + case 8:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> > + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> > + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> > + v->info.position[6] = SPA_AUDIO_CHANNEL_SL;
> > + v->info.position[7] = SPA_AUDIO_CHANNEL_SR;
> > + break;
> > + case 6:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> > + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> > + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> > + break;
> > + case 5:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> > + v->info.position[4] = SPA_AUDIO_CHANNEL_RC;
> > + break;
> > + case 4:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_RC;
> > + break;
> > + case 3:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_LFE;
> > + break;
> > + case 2:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + break;
> > + case 1:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_MONO;
> > + break;
> > + default:
> > + for (size_t i = 0; i < v->info.channels; i++) {
> > + v->info.position[i] = SPA_AUDIO_CHANNEL_UNKNOWN;
> > + }
> > + break;
> > + }
> > +
> > + /* create a new unconnected pwstream */
> > + r = create_stream(c, v, name);
> > + if (r < 0) {
> > + goto error;
> > + }
> > +
> > + return r;
> > +
> > +error:
> > + AUD_log(AUDIO_CAP, "Failed to create stream.");
> > + return -1;
> > +}
> > +
> > +static int
> > +qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
> > +{
> > + PWVoiceOut *pw = (PWVoiceOut *) hw;
> > + PWVoice *v = &pw->v;
> > + struct audsettings obt_as = *as;
> > + pwaudio *c = v->g = drv_opaque;
> > + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> > + AudiodevPipewirePerDirectionOptions *ppdo = popts->out;
> > + int r;
> > + v->enabled = false;
> > +
> > + v->mode = MODE_SINK;
> > +
> > + pw_thread_loop_lock(c->thread_loop);
> > +
> > + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> > + v->info.channels = as->nchannels;
> > + v->info.rate = as->freq;
> > +
> > + obt_as.fmt =
> > + pw_to_audfmt(v->info.format, &obt_as.endianness,
> &v->frame_size);
> > + v->frame_size *= as->nchannels;
> > +
> > + /* call the function that creates a new stream for playback */
> > + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> > + if (r < 0) {
> > + error_report("qpw_stream_new for playback failed\n ");
> > + goto fail;
> > + }
> > +
> > + /* report the audio format we support */
> > + audio_pcm_init_info(&hw->info, &obt_as);
> > +
> > + /* report the buffer size to qemu */
> > + hw->samples = BUFFER_SAMPLES;
> > +
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return 0;
> > +fail:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return -1;
> > +}
> > +
> > +static int
> > +qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
> > +{
> > + PWVoiceIn *pw = (PWVoiceIn *) hw;
> > + PWVoice *v = &pw->v;
> > + struct audsettings obt_as = *as;
> > + pwaudio *c = v->g = drv_opaque;
> > + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> > + AudiodevPipewirePerDirectionOptions *ppdo = popts->in;
> > + int r;
> > + v->enabled = false;
> > +
> > + v->mode = MODE_SOURCE;
> > + pw_thread_loop_lock(c->thread_loop);
> > +
> > + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> > + v->info.channels = as->nchannels;
> > + v->info.rate = as->freq;
> > +
> > + obt_as.fmt =
> > + pw_to_audfmt(v->info.format, &obt_as.endianness,
> &v->frame_size);
> > + v->frame_size *= as->nchannels;
> > +
> > + /* call the function that creates a new stream for recording */
> > + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> > + if (r < 0) {
> > + error_report("qpw_stream_new for recording failed\n ");
> > + goto fail;
> > + }
> > +
> > + /* report the audio format we support */
> > + audio_pcm_init_info(&hw->info, &obt_as);
> > +
> > + /* report the buffer size to qemu */
> > + hw->samples = BUFFER_SAMPLES;
> > +
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return 0;
> > +fail:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return -1;
> > +}
> > +
> > +static void
> > +qpw_fini_out(HWVoiceOut *hw)
> > +{
> > + PWVoiceOut *pw = (PWVoiceOut *) hw;
> > + PWVoice *v = &pw->v;
> > +
> > + if (v->stream) {
> > + pwaudio *c = v->g;
> > + pw_thread_loop_lock(c->thread_loop);
> > + pw_stream_destroy(v->stream);
> > + v->stream = NULL;
> > + pw_thread_loop_unlock(c->thread_loop);
> > + }
> > +}
> > +
> > +static void
> > +qpw_fini_in(HWVoiceIn *hw)
> > +{
> > + PWVoiceIn *pw = (PWVoiceIn *) hw;
> > + PWVoice *v = &pw->v;
> > +
> > + if (v->stream) {
> > + pwaudio *c = v->g;
> > + pw_thread_loop_lock(c->thread_loop);
> > + pw_stream_destroy(v->stream);
> > + v->stream = NULL;
> > + pw_thread_loop_unlock(c->thread_loop);
> > + }
> > +}
> > +
> > +static void
> > +qpw_enable_out(HWVoiceOut *hw, bool enable)
> > +{
> > + PWVoiceOut *po = (PWVoiceOut *) hw;
> > + PWVoice *v = &po->v;
> > + v->enabled = enable;
>
> Have you considered pw_stream_set_active() ?
>
>
> > +}
> > +
> > +static void
> > +qpw_enable_in(HWVoiceIn *hw, bool enable)
> > +{
> > + PWVoiceIn *pi = (PWVoiceIn *) hw;
> > + PWVoice *v = &pi->v;
> > + v->enabled = enable;
>
> same
>
> > +}
> > +
> > +static void
> > +on_core_error(void *data, uint32_t id, int seq, int res, const char
> *message)
> > +{
> > + pwaudio *pw = data;
> > +
> > + error_report("error id:%u seq:%d res:%d (%s): %s",
> > + id, seq, res, spa_strerror(res), message);
> > +
> > + pw_thread_loop_signal(pw->thread_loop, FALSE);
> > +}
> > +
> > +static void
> > +on_core_done(void *data, uint32_t id, int seq)
> > +{
> > + pwaudio *pw = data;
> > + if (id == PW_ID_CORE) {
> > + pw->seq = seq;
> > + pw_thread_loop_signal(pw->thread_loop, FALSE);
> > + }
> > +}
> > +
> > +static const struct pw_core_events core_events = {
> > + PW_VERSION_CORE_EVENTS,
> > + .done = on_core_done,
> > + .error = on_core_error,
> > +};
> > +
> > +static void *
> > +qpw_audio_init(Audiodev *dev)
> > +{
> > + pwaudio *pw;
> > + pw = g_new0(pwaudio, 1);
> > + pw_init(NULL, NULL);
> > +
> > + AudiodevPipewireOptions *popts;
> > + trace_pw_audio_init("Initialize Pipewire context\n");
> > + assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> > + popts = &dev->u.pipewire;
> > +
> > + if (!popts->has_latency) {
> > + popts->has_latency = true;
> > + popts->latency = 15000;
> > + }
>
> This option is not taken into account. It seems you should set the
> stream PW_KEY_NODE_LATENCY, which must be a fraction as a string. You
> can either convert the microseconds latency to a fraction
> ("delay/1000000"), or change the argument to use fractions instead
> (although this would be different from other latency properties in
> QEMU...)
>
> > +
> > + pw->dev = dev;
> > + pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
> > + if (pw->thread_loop == NULL) {
> > + error_report("Could not create Pipewire loop");
> > + goto fail_loop;
> > + }
> > +
> > + pw->context =
> > + pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL,
> 0);
> > + if (pw->context == NULL) {
> > + error_report("Could not create Pipewire context");
> > + goto fail_loop;
> > + }
> > +
> > + if (pw_thread_loop_start(pw->thread_loop) < 0) {
> > + error_report("Could not start Pipewire loop");
> > + goto fail_start;
> > + }
> > +
> > + pw_thread_loop_lock(pw->thread_loop);
> > +
> > + pw->core = pw_context_connect(pw->context, NULL, 0);
> > + if (pw->core == NULL) {
> > + goto fail_conn;
> > + }
> > +
> > + pw_core_add_listener(pw->core, &pw->core_listener, &core_events,
> pw);
> > +
> > + pw_thread_loop_unlock(pw->thread_loop);
> > +
> > + return pw;
> > +
> > +fail_loop:
> > + pw_thread_loop_destroy(pw->thread_loop);
> > + g_free(pw);
> > + return NULL;
> > +fail_start:
> > + pw_context_destroy(pw->context);
> > + g_free(pw);
> > + return NULL;
> > +fail_conn:
> > + AUD_log(AUDIO_CAP, "Failed to initialize PW context");
> > + pw_thread_loop_unlock(pw->thread_loop);
> > + pw_thread_loop_stop(pw->thread_loop);
> > + pw_core_disconnect(pw->core);
> > + pw_context_destroy(pw->context);
> > + pw_thread_loop_destroy(pw->thread_loop);
> > + g_free(pw);
> > + return NULL;
> > +}
> > +
> > +static void
> > +qpw_audio_fini(void *opaque)
> > +{
> > + pwaudio *pw = opaque;
> > +
> > + pw_thread_loop_stop(pw->thread_loop);
> > +
> > + if (pw->core) {
> > + spa_hook_remove(&pw->core_listener);
> > + spa_zero(pw->core_listener);
> > + pw_core_disconnect(pw->core);
> > + }
> > +
> > + if (pw->context) {
> > + pw_context_destroy(pw->context);
> > + }
> > + pw_thread_loop_destroy(pw->thread_loop);
> > +
> > + g_free(pw);
> > +}
> > +
> > +static struct audio_pcm_ops qpw_pcm_ops = {
> > + .init_out = qpw_init_out,
> > + .fini_out = qpw_fini_out,
> > + .write = qpw_write,
> > + .buffer_get_free = audio_generic_buffer_get_free,
> > + .run_buffer_out = audio_generic_run_buffer_out,
> > + .enable_out = qpw_enable_out,
> > +
> > + .init_in = qpw_init_in,
> > + .fini_in = qpw_fini_in,
> > + .read = qpw_read,
> > + .run_buffer_in = audio_generic_run_buffer_in,
> > + .enable_in = qpw_enable_in
> > +};
> > +
> > +static struct audio_driver pw_audio_driver = {
> > + .name = "pipewire",
> > + .descr = "http://www.pipewire.org/",
> > + .init = qpw_audio_init,
> > + .fini = qpw_audio_fini,
> > + .pcm_ops = &qpw_pcm_ops,
> > + .can_be_default = 1,
> > + .max_voices_out = INT_MAX,
> > + .max_voices_in = INT_MAX,
> > + .voice_size_out = sizeof(PWVoiceOut),
> > + .voice_size_in = sizeof(PWVoiceIn),
> > +};
> > +
> > +static void
> > +register_audio_pw(void)
> > +{
> > + audio_driver_register(&pw_audio_driver);
> > +}
> > +
> > +type_init(register_audio_pw);
> > diff --git a/audio/trace-events b/audio/trace-events
> > index e1ab643add..2ecd8851e6 100644
> > --- a/audio/trace-events
> > +++ b/audio/trace-events
> > @@ -18,6 +18,13 @@ dbus_audio_register(const char *s, const char *dir)
> "sender = %s, dir = %s"
> > dbus_audio_put_buffer_out(size_t len) "len = %zu"
> > dbus_audio_read(size_t len) "len = %zu"
> >
> > +# pwaudio.c
> > +pw_state_changed(const char *s) "stream state: %s"
> > +pw_node(int nodeid) "node id: %d"
> > +pw_read(int32_t avail, uint32_t index, size_t len) "avail=%u index=%u
> len=%zu"
> > +pw_write(int32_t filled, int32_t avail, uint32_t index, size_t len)
> "filled=%u avail=%u index=%u len=%zu"
> > +pw_audio_init(const char *msg) "pipewire: %s"
> > +
> > # audio.c
> > audio_timer_start(int interval) "interval %d ms"
> > audio_timer_stop(void) ""
> > diff --git a/meson.build b/meson.build
> > index 6bcab8bf0d..51ec2931e1 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -730,6 +730,12 @@ if not get_option('jack').auto() or have_system
> > jack = dependency('jack', required: get_option('jack'),
> > method: 'pkg-config', kwargs: static_kwargs)
> > endif
> > +pipewire = not_found
> > +if not get_option('pipewire').auto() or (targetos == 'linux' and
> have_system)
> > + pipewire = dependency('libpipewire-0.3', version: '>=0.3.60',
> > + required: get_option('pipewire'),
> > + method: 'pkg-config', kwargs: static_kwargs)
> > +endif
> > sndio = not_found
> > if not get_option('sndio').auto() or have_system
> > sndio = dependency('sndio', required: get_option('sndio'),
> > @@ -1667,6 +1673,7 @@ if have_system
> > 'jack': jack.found(),
> > 'oss': oss.found(),
> > 'pa': pulse.found(),
> > + 'pipewire': pipewire.found(),
> > 'sdl': sdl.found(),
> > 'sndio': sndio.found(),
> > }
> > @@ -3980,6 +3987,7 @@ if targetos == 'linux'
> > summary_info += {'ALSA support': alsa}
> > summary_info += {'PulseAudio support': pulse}
> > endif
> > +summary_info += {'Pipewire support': pipewire}
> > summary_info += {'JACK support': jack}
> > summary_info += {'brlapi support': brlapi}
> > summary_info += {'vde support': vde}
> > diff --git a/meson_options.txt b/meson_options.txt
> > index fc9447d267..9ae1ec7f47 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -21,7 +21,7 @@ option('tls_priority', type : 'string', value :
> 'NORMAL',
> > option('default_devices', type : 'boolean', value : true,
> > description: 'Include a default selection of devices in
> emulators')
> > option('audio_drv_list', type: 'array', value: ['default'],
> > - choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack',
> 'oss', 'pa', 'sdl', 'sndio'],
> > + choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack',
> 'oss', 'pa', 'pipewire', 'sdl', 'sndio'],
> > description: 'Set audio driver list')
> > option('block_drv_rw_whitelist', type : 'string', value : '',
> > description: 'set block driver read-write whitelist (by default
> affects only QEMU, not tools like qemu-img)')
> > @@ -255,6 +255,8 @@ option('oss', type: 'feature', value: 'auto',
> > description: 'OSS sound support')
> > option('pa', type: 'feature', value: 'auto',
> > description: 'PulseAudio sound support')
> > +option('pipewire', type: 'feature', value: 'auto',
> > + description: 'Pipewire sound support')
> > option('sndio', type: 'feature', value: 'auto',
> > description: 'sndio sound support')
> >
> > diff --git a/qapi/audio.json b/qapi/audio.json
> > index 4e54c00f51..9a0d7d9ece 100644
> > --- a/qapi/audio.json
> > +++ b/qapi/audio.json
> > @@ -324,6 +324,48 @@
> > '*out': 'AudiodevPaPerDirectionOptions',
> > '*server': 'str' } }
> >
> > +##
> > +# @AudiodevPipewirePerDirectionOptions:
> > +#
> > +# Options of the Pipewire backend that are used for both playback and
> > +# recording.
> > +#
> > +# @name: name of the sink/source to use
> > +#
> > +# @stream-name: name of the Pipewire stream created by qemu. Can be
> > +# used to identify the stream in Pipewire when you
> > +# create multiple Pipewire devices or run multiple qemu
> > +# instances (default: audiodev's id, since 7.1)
>
> not 7.1, but you don't need to say "since .." here, as we assume it
> was added with the struct then.
>
>
> > +#
> > +#
> > +# Since: 8.0
> > +##
> > +{ 'struct': 'AudiodevPipewirePerDirectionOptions',
> > + 'base': 'AudiodevPerDirectionOptions',
> > + 'data': {
> > + '*name': 'str',
> > + '*stream-name': 'str' } }
> > +
> > +##
> > +# @AudiodevPipewireOptions:
> > +#
> > +# Options of the Pipewire audio backend.
> > +#
> > +# @in: options of the capture stream
> > +#
> > +# @out: options of the playback stream
> > +#
> > +# @latency: add latency to playback in microseconds
> > +# (default 15000)
> > +#
> > +# Since: 8.0
> > +##
> > +{ 'struct': 'AudiodevPipewireOptions',
> > + 'data': {
> > + '*in': 'AudiodevPipewirePerDirectionOptions',
> > + '*out': 'AudiodevPipewirePerDirectionOptions',
> > + '*latency': 'uint32' } }
> > +
> > ##
> > # @AudiodevSdlPerDirectionOptions:
> > #
> > @@ -416,6 +458,7 @@
> > { 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
> > { 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
> > { 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
> > + { 'name': 'pipewire', 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> > { 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
> > { 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
> > { 'name': 'spice', 'if': 'CONFIG_SPICE' },
> > @@ -456,6 +499,8 @@
> > 'if': 'CONFIG_AUDIO_OSS' },
> > 'pa': { 'type': 'AudiodevPaOptions',
> > 'if': 'CONFIG_AUDIO_PA' },
> > + 'pipewire': { 'type': 'AudiodevPipewireOptions',
> > + 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> > 'sdl': { 'type': 'AudiodevSdlOptions',
> > 'if': 'CONFIG_AUDIO_SDL' },
> > 'sndio': { 'type': 'AudiodevSndioOptions',
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index d42f60fb91..009d58bbf2 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -779,6 +779,11 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> > " in|out.name= source/sink device name\n"
> > " in|out.latency= desired latency in microseconds\n"
> > #endif
> > +#ifdef CONFIG_AUDIO_PIPEWIRE
> > + "-audiodev pipewire,id=id[,prop[=value][,...]]\n"
> > + " in|out.name= source/sink device name\n"
> > + " latency= desired latency in microseconds\n"
> > +#endif
> > #ifdef CONFIG_AUDIO_SDL
> > "-audiodev sdl,id=id[,prop[=value][,...]]\n"
> > " in|out.buffer-count= number of buffers\n"
> > @@ -942,6 +947,18 @@ SRST
> > Desired latency in microseconds. The PulseAudio server will try
> > to honor this value but actual latencies may be lower or higher.
> >
> > +``-audiodev pipewire,id=id[,prop[=value][,...]]``
> > + Creates a backend using Pipewire. This backend is available on
> > + most systems.
> > +
> > + Pipewire specific options are:
> > +
> > + ``latency=latency``
> > + Add extra latency to playback in microseconds
> > +
> > + ``in|out.name=sink``
> > + Use the specified source/sink for recording/playback.
> > +
> > ``-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
> > diff --git a/scripts/meson-buildoptions.sh
> b/scripts/meson-buildoptions.sh
> > index 009fab1515..ba1057b62c 100644
> > --- a/scripts/meson-buildoptions.sh
> > +++ b/scripts/meson-buildoptions.sh
> > @@ -1,7 +1,8 @@
> > # This file is generated by meson-buildoptions.py, do not edit!
> > meson_options_help() {
> > - printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list
> [default] (choices: alsa/co'
> > - printf "%s\n" '
> reaudio/default/dsound/jack/oss/pa/sdl/sndio)'
> > + printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list
> [default] (choices: al'
> > + printf "%s\n" '
> sa/coreaudio/default/dsound/jack/oss/pa/'
> > + printf "%s\n" ' pipewire/sdl/sndio)'
> > printf "%s\n" ' --block-drv-ro-whitelist=VALUE'
> > printf "%s\n" ' set block driver read-only
> whitelist (by default'
> > printf "%s\n" ' affects only QEMU, not
> tools like qemu-img)'
> > @@ -136,6 +137,7 @@ meson_options_help() {
> > printf "%s\n" ' oss OSS sound support'
> > printf "%s\n" ' pa PulseAudio sound support'
> > printf "%s\n" ' parallels parallels image format support'
> > + printf "%s\n" ' pipewire Pipewire sound support'
> > printf "%s\n" ' png PNG support with libpng'
> > printf "%s\n" ' pvrdma Enable PVRDMA support'
> > printf "%s\n" ' qcow1 qcow1 image format support'
> > @@ -370,6 +372,8 @@ _meson_option_parse() {
> > --disable-pa) printf "%s" -Dpa=disabled ;;
> > --enable-parallels) printf "%s" -Dparallels=enabled ;;
> > --disable-parallels) printf "%s" -Dparallels=disabled ;;
> > + --enable-pipewire) printf "%s" -Dpipewire=enabled ;;
> > + --disable-pipewire) printf "%s" -Dpipewire=disabled ;;
> > --with-pkgversion=*) quote_sh "-Dpkgversion=$2" ;;
> > --enable-png) printf "%s" -Dpng=enabled ;;
> > --disable-png) printf "%s" -Dpng=disabled ;;
> > --
> > 2.39.1
> >
> >
>
> Volker, Wim, it would be nice if you could review/comment too!
>
> thanks
>
> --
> Marc-André Lureau
>
>
Am 08.03.23 um 11:39 schrieb Marc-André Lureau: > Volker, Wim, it would be nice if you could review/comment too! > > thanks Hi, last weekend I replaced pulseaudio with pipewire on my host computer and tested the QEMU pipewire backend. It doesn't work well on my computer, but with a few changes it becomes usable. I hope to have some time tomorrow evening to write down my suggestions and comments. With best regards, Volker > -- > Marc-André Lureau
Hi Dorinda
On Mon, Mar 6, 2023 at 9:11 PM Dorinda Bassey <dbassey@redhat.com> wrote:
>
> This commit adds a new audiodev backend to allow QEMU to use Pipewire as
> both an audio sink and source. This backend is available on most systems
>
> Add Pipewire entry points for QEMU Pipewire audio backend
> Add wrappers for QEMU Pipewire audio backend in qpw_pcm_ops()
> qpw_write function returns the current state of the stream to pwaudio
> and Writes some data to the server for playback streams using pipewire
> spa_ringbuffer implementation.
> qpw_read function returns the current state of the stream to pwaudio and
> reads some data from the server for capture streams using pipewire
> spa_ringbuffer implementation. These functions qpw_write and qpw_read
> are called during playback and capture.
> Added some functions that convert pw audio formats to QEMU audio format
> and vice versa which would be needed in the pipewire audio sink and
> source functions qpw_init_in() & qpw_init_out().
> These methods that implement playback and recording will create streams
> for playback and capture that will start processing and will result in
> the on_process callbacks to be called.
> Built a connection to the Pipewire sound system server in the
> qpw_audio_init() method.
>
> Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
Some more comments before I start looking more closely at buffering.
thanks
> ---
> v7:
> use qemu tracing tool
>
> audio/audio.c | 3 +
> audio/audio_template.h | 4 +
> audio/meson.build | 1 +
> audio/pwaudio.c | 814 ++++++++++++++++++++++++++++++++++
> audio/trace-events | 7 +
> meson.build | 8 +
> meson_options.txt | 4 +-
> qapi/audio.json | 45 ++
> qemu-options.hx | 17 +
> scripts/meson-buildoptions.sh | 8 +-
> 10 files changed, 908 insertions(+), 3 deletions(-)
> create mode 100644 audio/pwaudio.c
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 4290309d18..aa55e41ad8 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -2069,6 +2069,9 @@ void audio_create_pdos(Audiodev *dev)
> #ifdef CONFIG_AUDIO_PA
> CASE(PA, pa, Pa);
> #endif
> +#ifdef CONFIG_AUDIO_PIPEWIRE
> + CASE(PIPEWIRE, pipewire, Pipewire);
> +#endif
> #ifdef CONFIG_AUDIO_SDL
> CASE(SDL, sdl, Sdl);
> #endif
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 42b4712acb..0f02afb921 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -355,6 +355,10 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
> case AUDIODEV_DRIVER_PA:
> return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
> #endif
> +#ifdef CONFIG_AUDIO_PIPEWIRE
> + case AUDIODEV_DRIVER_PIPEWIRE:
> + return qapi_AudiodevPipewirePerDirectionOptions_base(dev->u.pipewire.TYPE);
> +#endif
> #ifdef CONFIG_AUDIO_SDL
> case AUDIODEV_DRIVER_SDL:
> return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
> diff --git a/audio/meson.build b/audio/meson.build
> index 0722224ba9..65a49c1a10 100644
> --- a/audio/meson.build
> +++ b/audio/meson.build
> @@ -19,6 +19,7 @@ foreach m : [
> ['sdl', sdl, files('sdlaudio.c')],
> ['jack', jack, files('jackaudio.c')],
> ['sndio', sndio, files('sndioaudio.c')],
> + ['pipewire', pipewire, files('pwaudio.c')],
> ['spice', spice, files('spiceaudio.c')]
> ]
> if m[1].found()
> diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> new file mode 100644
> index 0000000000..d357761152
> --- /dev/null
> +++ b/audio/pwaudio.c
> @@ -0,0 +1,814 @@
> +/*
> + * QEMU Pipewire audio driver
> + *
> + * Copyright (c) 2023 Red Hat Inc.
> + *
> + * Author: Dorinda Bassey <dbassey@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "audio.h"
> +#include <errno.h>
> +#include "qemu/error-report.h"
> +#include <spa/param/audio/format-utils.h>
> +#include <spa/utils/ringbuffer.h>
> +#include <spa/utils/result.h>
> +
> +#include <pipewire/pipewire.h>
> +#include "trace.h"
> +
> +#define AUDIO_CAP "pipewire"
> +#define RINGBUFFER_SIZE (1u << 22)
> +#define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> +#define BUFFER_SAMPLES 512
> +
> +#include "audio_int.h"
> +
> +enum {
> + MODE_SINK,
> + MODE_SOURCE
> +};
> +
> +typedef struct pwaudio {
> + Audiodev *dev;
> + struct pw_thread_loop *thread_loop;
> + struct pw_context *context;
> +
> + struct pw_core *core;
> + struct spa_hook core_listener;
> + int seq;
> +} pwaudio;
> +
> +typedef struct PWVoice {
> + pwaudio *g;
> + bool enabled;
> + struct pw_stream *stream;
> + struct spa_hook stream_listener;
> + struct spa_audio_info_raw info;
> + uint32_t frame_size;
> + struct spa_ringbuffer ring;
> + uint8_t buffer[RINGBUFFER_SIZE];
> +
> + uint32_t mode;
> + struct pw_properties *props;
> +} PWVoice;
> +
> +typedef struct PWVoiceOut {
> + HWVoiceOut hw;
> + PWVoice v;
> +} PWVoiceOut;
> +
> +typedef struct PWVoiceIn {
> + HWVoiceIn hw;
> + PWVoice v;
> +} PWVoiceIn;
> +
> +static void
> +stream_destroy(void *data)
> +{
> + PWVoice *v = (PWVoice *) data;
> + spa_hook_remove(&v->stream_listener);
> + v->stream = NULL;
> +}
> +
> +/* output data processing function to read stuffs from the buffer */
> +static void
> +playback_on_process(void *data)
> +{
> + PWVoice *v = (PWVoice *) data;
> + void *p;
> + struct pw_buffer *b;
> + struct spa_buffer *buf;
> + uint32_t n_frames, req, index, n_bytes;
> + int32_t avail;
> +
> + if (!v->stream) {
> + return;
> + }
> +
> + /* obtain a buffer to read from */
> + b = pw_stream_dequeue_buffer(v->stream);
> + if (b == NULL) {
> + error_report("out of buffers: %s", strerror(errno));
> + return;
> + }
> +
> + buf = b->buffer;
> + p = buf->datas[0].data;
> + if (p == NULL) {
> + return;
> + }
> + req = b->requested * v->frame_size;
> + if (req == 0) {
> + req = 4096 * v->frame_size;
> + }
> + n_frames = SPA_MIN(req, buf->datas[0].maxsize);
> + n_bytes = n_frames * v->frame_size;
> +
> + /* get no of available bytes to read data from buffer */
> +
> + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> +
> + if (!v->enabled) {
> + avail = 0;
> + }
> +
> + if (avail == 0) {
> + memset(p, 0, n_bytes);
> + } else {
> + if (avail < (int32_t) n_bytes) {
> + n_bytes = avail;
> + }
> +
> + spa_ringbuffer_read_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK, p, n_bytes);
> +
> + index += n_bytes;
> + spa_ringbuffer_read_update(&v->ring, index);
> + }
> +
> + buf->datas[0].chunk->offset = 0;
> + buf->datas[0].chunk->stride = v->frame_size;
> + buf->datas[0].chunk->size = n_bytes;
> +
> + /* queue the buffer for playback */
> + pw_stream_queue_buffer(v->stream, b);
> +}
> +
> +/* output data processing function to generate stuffs in the buffer */
> +static void
> +capture_on_process(void *data)
> +{
> + PWVoice *v = (PWVoice *) data;
> + void *p;
> + struct pw_buffer *b;
> + struct spa_buffer *buf;
> + int32_t filled;
> + uint32_t index, offs, n_bytes;
> +
> + if (!v->stream) {
> + return;
> + }
> +
> + /* obtain a buffer */
> + b = pw_stream_dequeue_buffer(v->stream);
> + if (b == NULL) {
> + error_report("out of buffers: %s", strerror(errno));
> + return;
> + }
> +
> + /* Write data into buffer */
> + buf = b->buffer;
> + p = buf->datas[0].data;
> + if (p == NULL) {
> + return;
> + }
> + offs = SPA_MIN(buf->datas[0].chunk->offset, buf->datas[0].maxsize);
> + n_bytes = SPA_MIN(buf->datas[0].chunk->size, buf->datas[0].maxsize - offs);
> +
> + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> +
> + if (!v->enabled) {
> + n_bytes = 0;
> + }
> +
> + if (filled < 0) {
> + error_report("%p: underrun write:%u filled:%d", p, index, filled);
> + } else {
> + if ((uint32_t) filled + n_bytes > RINGBUFFER_SIZE) {
> + error_report("%p: overrun write:%u filled:%d + size:%u > max:%u",
> + p, index, filled, n_bytes, RINGBUFFER_SIZE);
> + }
> + }
> + spa_ringbuffer_write_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK,
> + SPA_PTROFF(p, offs, void), n_bytes);
> + index += n_bytes;
> + spa_ringbuffer_write_update(&v->ring, index);
> +
> + /* queue the buffer for playback */
> + pw_stream_queue_buffer(v->stream, b);
> +}
> +
> +static void
> +on_stream_state_changed(void *_data, enum pw_stream_state old,
> + enum pw_stream_state state, const char *error)
> +{
> + PWVoice *v = (PWVoice *) _data;
> +
> + trace_pw_state_changed(pw_stream_state_as_string(state));
> +
> + switch (state) {
> + case PW_STREAM_STATE_ERROR:
> + case PW_STREAM_STATE_UNCONNECTED:
> + {
> + break;
> + }
> + case PW_STREAM_STATE_PAUSED:
> + trace_pw_node(pw_stream_get_node_id(v->stream));
> + break;
> + case PW_STREAM_STATE_CONNECTING:
> + case PW_STREAM_STATE_STREAMING:
> + break;
> + }
> +}
> +
> +static const struct pw_stream_events capture_stream_events = {
> + PW_VERSION_STREAM_EVENTS,
> + .destroy = stream_destroy,
> + .state_changed = on_stream_state_changed,
> + .process = capture_on_process
> +};
> +
> +static const struct pw_stream_events playback_stream_events = {
> + PW_VERSION_STREAM_EVENTS,
> + .destroy = stream_destroy,
> + .state_changed = on_stream_state_changed,
> + .process = playback_on_process
> +};
> +
> +static size_t
> +qpw_read(HWVoiceIn *hw, void *data, size_t len)
> +{
> + PWVoiceIn *pw = (PWVoiceIn *) hw;
> + PWVoice *v = &pw->v;
> + pwaudio *c = v->g;
> + const char *error = NULL;
> + size_t l;
> + int32_t avail;
> + uint32_t index;
> +
> + pw_thread_loop_lock(c->thread_loop);
> + if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
> + /* wait for stream to become ready */
> + l = 0;
> + goto done_unlock;
> + }
> + /* get no of available bytes to read data from buffer */
> + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> +
> + trace_pw_read(avail, index, len);
> +
> + if (avail < (int32_t) len) {
> + len = avail;
> + }
> +
> + spa_ringbuffer_read_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK, data, len);
> + index += len;
> + spa_ringbuffer_read_update(&v->ring, index);
> + l = len;
> +
> +done_unlock:
> + pw_thread_loop_unlock(c->thread_loop);
> + return l;
> +}
> +
> +static size_t
> +qpw_write(HWVoiceOut *hw, void *data, size_t len)
> +{
> + PWVoiceOut *pw = (PWVoiceOut *) hw;
> + PWVoice *v = &pw->v;
> + pwaudio *c = v->g;
> + const char *error = NULL;
> + const int periods = 3;
> + size_t l;
> + int32_t filled, avail;
> + uint32_t index;
> +
> + pw_thread_loop_lock(c->thread_loop);
> + if (pw_stream_get_state(v->stream, &error) != PW_STREAM_STATE_STREAMING) {
> + /* wait for stream to become ready */
> + l = 0;
> + goto done_unlock;
> + }
> + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> +
> + avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
> +
> + trace_pw_write(filled, avail, index, len);
> +
> + if (len > avail) {
> + len = avail;
> + }
> +
> + if (filled < 0) {
> + error_report("%p: underrun write:%u filled:%d", pw, index, filled);
> + } else {
> + if ((uint32_t) filled + len > RINGBUFFER_SIZE) {
> + error_report("%p: overrun write:%u filled:%d + size:%zu > max:%u",
> + pw, index, filled, len, RINGBUFFER_SIZE);
> + }
> + }
> +
> + spa_ringbuffer_write_data(&v->ring,
> + v->buffer, RINGBUFFER_SIZE,
> + index & RINGBUFFER_MASK, data, len);
> + index += len;
> + spa_ringbuffer_write_update(&v->ring, index);
> + l = len;
> +
> +done_unlock:
> + pw_thread_loop_unlock(c->thread_loop);
> + return l;
> +}
> +
> +static int
> +audfmt_to_pw(AudioFormat fmt, int endianness)
> +{
> + int format;
> +
> + switch (fmt) {
> + case AUDIO_FORMAT_S8:
> + format = SPA_AUDIO_FORMAT_S8;
> + break;
> + case AUDIO_FORMAT_U8:
> + format = SPA_AUDIO_FORMAT_U8;
> + break;
> + case AUDIO_FORMAT_S16:
> + format = endianness ? SPA_AUDIO_FORMAT_S16_BE : SPA_AUDIO_FORMAT_S16_LE;
> + break;
> + case AUDIO_FORMAT_U16:
> + format = endianness ? SPA_AUDIO_FORMAT_U16_BE : SPA_AUDIO_FORMAT_U16_LE;
> + break;
> + case AUDIO_FORMAT_S32:
> + format = endianness ? SPA_AUDIO_FORMAT_S32_BE : SPA_AUDIO_FORMAT_S32_LE;
> + break;
> + case AUDIO_FORMAT_U32:
> + format = endianness ? SPA_AUDIO_FORMAT_U32_BE : SPA_AUDIO_FORMAT_U32_LE;
> + break;
> + case AUDIO_FORMAT_F32:
> + format = endianness ? SPA_AUDIO_FORMAT_F32_BE : SPA_AUDIO_FORMAT_F32_LE;
> + break;
> + default:
> + dolog("Internal logic error: Bad audio format %d\n", fmt);
> + format = SPA_AUDIO_FORMAT_U8;
> + break;
> + }
> + return format;
> +}
> +
> +static AudioFormat
> +pw_to_audfmt(enum spa_audio_format fmt, int *endianness,
> + uint32_t *frame_size)
> +{
> + switch (fmt) {
> + case SPA_AUDIO_FORMAT_S8:
> + *frame_size = 1;
> + return AUDIO_FORMAT_S8;
> + case SPA_AUDIO_FORMAT_U8:
> + *frame_size = 1;
> + return AUDIO_FORMAT_U8;
> + case SPA_AUDIO_FORMAT_S16_BE:
> + *frame_size = 2;
> + *endianness = 1;
> + return AUDIO_FORMAT_S16;
> + case SPA_AUDIO_FORMAT_S16_LE:
> + *frame_size = 2;
> + *endianness = 0;
> + return AUDIO_FORMAT_S16;
> + case SPA_AUDIO_FORMAT_U16_BE:
> + *frame_size = 2;
> + *endianness = 1;
> + return AUDIO_FORMAT_U16;
> + case SPA_AUDIO_FORMAT_U16_LE:
> + *frame_size = 2;
> + *endianness = 0;
> + return AUDIO_FORMAT_U16;
> + case SPA_AUDIO_FORMAT_S32_BE:
> + *frame_size = 4;
> + *endianness = 1;
> + return AUDIO_FORMAT_S32;
> + case SPA_AUDIO_FORMAT_S32_LE:
> + *frame_size = 4;
> + *endianness = 0;
> + return AUDIO_FORMAT_S32;
> + case SPA_AUDIO_FORMAT_U32_BE:
> + *frame_size = 4;
> + *endianness = 1;
> + return AUDIO_FORMAT_U32;
> + case SPA_AUDIO_FORMAT_U32_LE:
> + *frame_size = 4;
> + *endianness = 0;
> + return AUDIO_FORMAT_U32;
> + case SPA_AUDIO_FORMAT_F32_BE:
> + *frame_size = 4;
> + *endianness = 1;
> + return AUDIO_FORMAT_F32;
> + case SPA_AUDIO_FORMAT_F32_LE:
> + *frame_size = 4;
> + *endianness = 0;
> + return AUDIO_FORMAT_F32;
> + default:
> + *frame_size = 1;
> + dolog("Internal logic error: Bad spa_audio_format %d\n", fmt);
> + return AUDIO_FORMAT_U8;
> + }
> +}
> +
> +static int
> +create_stream(pwaudio *c, PWVoice *v, const char *name)
> +{
> + int res;
> + uint32_t n_params;
> + const struct spa_pod *params[2];
> + uint8_t buffer[1024];
> + struct spa_pod_builder b;
> +
> + v->stream = pw_stream_new(c->core, name, NULL);
> +
> + if (v->stream == NULL) {
(ok, some error reporting is done by the caller)
> + goto error;
you should return directly, see below for related issue
> + }
> +
> + if (v->mode == MODE_SOURCE) {
> + pw_stream_add_listener(v->stream,
> + &v->stream_listener, &capture_stream_events, v);
> + } else {
> + pw_stream_add_listener(v->stream,
> + &v->stream_listener, &playback_stream_events, v);
> + }
> +
> + n_params = 0;
> + spa_pod_builder_init(&b, buffer, sizeof(buffer));
> + params[n_params++] = spa_format_audio_raw_build(&b,
> + SPA_PARAM_EnumFormat,
> + &v->info);
> +
> + /* connect the stream to a sink or source */
> + res = pw_stream_connect(v->stream,
> + v->mode ==
> + MODE_SOURCE ? PW_DIRECTION_INPUT :
> + PW_DIRECTION_OUTPUT, PW_ID_ANY,
> + PW_STREAM_FLAG_AUTOCONNECT |
> + PW_STREAM_FLAG_MAP_BUFFERS |
> + PW_STREAM_FLAG_RT_PROCESS, params, n_params);
> + if (res < 0) {
> + goto error;
then you have a single "goto error", so you can fold the code here instead.
> + }
> +
> + return 0;
> +error:
> + pw_stream_destroy(v->stream);
Incorrect if v->stream is NULL
> + return -1;
> +}
> +
> +static int
> +qpw_stream_new(pwaudio *c, PWVoice *v, const char *name)
> +{
> + int r;
> +
> + switch (v->info.channels) {
> + case 8:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> + v->info.position[6] = SPA_AUDIO_CHANNEL_SL;
> + v->info.position[7] = SPA_AUDIO_CHANNEL_SR;
> + break;
> + case 6:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> + break;
> + case 5:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> + v->info.position[4] = SPA_AUDIO_CHANNEL_RC;
> + break;
> + case 4:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> + v->info.position[3] = SPA_AUDIO_CHANNEL_RC;
> + break;
> + case 3:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + v->info.position[2] = SPA_AUDIO_CHANNEL_LFE;
> + break;
> + case 2:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> + break;
> + case 1:
> + v->info.position[0] = SPA_AUDIO_CHANNEL_MONO;
> + break;
> + default:
> + for (size_t i = 0; i < v->info.channels; i++) {
> + v->info.position[i] = SPA_AUDIO_CHANNEL_UNKNOWN;
> + }
> + break;
> + }
> +
> + /* create a new unconnected pwstream */
> + r = create_stream(c, v, name);
> + if (r < 0) {
> + goto error;
single fail point, just fold the code here please
> + }
> +
> + return r;
> +
> +error:
> + AUD_log(AUDIO_CAP, "Failed to create stream.");
> + return -1;
> +}
> +
> +static int
> +qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
> +{
> + PWVoiceOut *pw = (PWVoiceOut *) hw;
> + PWVoice *v = &pw->v;
> + struct audsettings obt_as = *as;
> + pwaudio *c = v->g = drv_opaque;
> + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> + AudiodevPipewirePerDirectionOptions *ppdo = popts->out;
> + int r;
> + v->enabled = false;
> +
> + v->mode = MODE_SINK;
> +
> + pw_thread_loop_lock(c->thread_loop);
> +
> + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> + v->info.channels = as->nchannels;
> + v->info.rate = as->freq;
> +
> + obt_as.fmt =
> + pw_to_audfmt(v->info.format, &obt_as.endianness, &v->frame_size);
> + v->frame_size *= as->nchannels;
> +
> + /* call the function that creates a new stream for playback */
> + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> + if (r < 0) {
> + error_report("qpw_stream_new for playback failed\n ");
extra "\n "
> + goto fail;
single fail point, just fold the code here please
> + }
> +
> + /* report the audio format we support */
> + audio_pcm_init_info(&hw->info, &obt_as);
> +
> + /* report the buffer size to qemu */
> + hw->samples = BUFFER_SAMPLES;
> +
> + pw_thread_loop_unlock(c->thread_loop);
> + return 0;
> +fail:
> + pw_thread_loop_unlock(c->thread_loop);
> + return -1;
> +}
> +
> +static int
> +qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
> +{
> + PWVoiceIn *pw = (PWVoiceIn *) hw;
> + PWVoice *v = &pw->v;
> + struct audsettings obt_as = *as;
> + pwaudio *c = v->g = drv_opaque;
> + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> + AudiodevPipewirePerDirectionOptions *ppdo = popts->in;
> + int r;
> + v->enabled = false;
> +
> + v->mode = MODE_SOURCE;
> + pw_thread_loop_lock(c->thread_loop);
> +
> + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> + v->info.channels = as->nchannels;
> + v->info.rate = as->freq;
> +
> + obt_as.fmt =
> + pw_to_audfmt(v->info.format, &obt_as.endianness, &v->frame_size);
> + v->frame_size *= as->nchannels;
> +
> + /* call the function that creates a new stream for recording */
> + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> + if (r < 0) {
> + error_report("qpw_stream_new for recording failed\n ");
extra "\n "
> + goto fail;
single fail point, just fold the code here please
> + }
> +
> + /* report the audio format we support */
> + audio_pcm_init_info(&hw->info, &obt_as);
> +
> + /* report the buffer size to qemu */
> + hw->samples = BUFFER_SAMPLES;
> +
> + pw_thread_loop_unlock(c->thread_loop);
> + return 0;
> +fail:
> + pw_thread_loop_unlock(c->thread_loop);
> + return -1;
> +}
> +
> +static void
> +qpw_fini_out(HWVoiceOut *hw)
> +{
> + PWVoiceOut *pw = (PWVoiceOut *) hw;
> + PWVoice *v = &pw->v;
> +
> + if (v->stream) {
> + pwaudio *c = v->g;
> + pw_thread_loop_lock(c->thread_loop);
> + pw_stream_destroy(v->stream);
> + v->stream = NULL;
> + pw_thread_loop_unlock(c->thread_loop);
> + }
> +}
> +
> +static void
> +qpw_fini_in(HWVoiceIn *hw)
> +{
> + PWVoiceIn *pw = (PWVoiceIn *) hw;
> + PWVoice *v = &pw->v;
> +
> + if (v->stream) {
> + pwaudio *c = v->g;
> + pw_thread_loop_lock(c->thread_loop);
> + pw_stream_destroy(v->stream);
> + v->stream = NULL;
> + pw_thread_loop_unlock(c->thread_loop);
> + }
> +}
> +
> +static void
> +qpw_enable_out(HWVoiceOut *hw, bool enable)
> +{
> + PWVoiceOut *po = (PWVoiceOut *) hw;
> + PWVoice *v = &po->v;
> + v->enabled = enable;
> +}
> +
> +static void
> +qpw_enable_in(HWVoiceIn *hw, bool enable)
> +{
> + PWVoiceIn *pi = (PWVoiceIn *) hw;
> + PWVoice *v = &pi->v;
> + v->enabled = enable;
> +}
> +
> +static void
> +on_core_error(void *data, uint32_t id, int seq, int res, const char *message)
> +{
> + pwaudio *pw = data;
> +
> + error_report("error id:%u seq:%d res:%d (%s): %s",
> + id, seq, res, spa_strerror(res), message);
> +
> + pw_thread_loop_signal(pw->thread_loop, FALSE);
> +}
> +
> +static void
> +on_core_done(void *data, uint32_t id, int seq)
> +{
> + pwaudio *pw = data;
> + if (id == PW_ID_CORE) {
> + pw->seq = seq;
> + pw_thread_loop_signal(pw->thread_loop, FALSE);
> + }
> +}
> +
> +static const struct pw_core_events core_events = {
> + PW_VERSION_CORE_EVENTS,
> + .done = on_core_done,
> + .error = on_core_error,
> +};
> +
> +static void *
> +qpw_audio_init(Audiodev *dev)
> +{
> + pwaudio *pw;
> + pw = g_new0(pwaudio, 1);
> + pw_init(NULL, NULL);
> +
> + AudiodevPipewireOptions *popts;
we usually avoid mixing declaration and code.
> + trace_pw_audio_init("Initialize Pipewire context\n");
extra \n
you can simply drop the string argument. You only call
trace_pw_audio_init() once.
> + assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> + popts = &dev->u.pipewire;
> +
> + if (!popts->has_latency) {
> + popts->has_latency = true;
> + popts->latency = 15000;
> + }
> +
> + pw->dev = dev;
> + pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
Given that we have only one loop, I would set the thread name to NULL,
(which will use "pw-thread-loop" by default)
> + if (pw->thread_loop == NULL) {
> + error_report("Could not create Pipewire loop");
> + goto fail_loop;
> + }
> +
> + pw->context =
> + pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL, 0);
> + if (pw->context == NULL) {
> + error_report("Could not create Pipewire context");
> + goto fail_loop;
> + }
> +
> + if (pw_thread_loop_start(pw->thread_loop) < 0) {
> + error_report("Could not start Pipewire loop");
> + goto fail_start;
> + }
> +
> + pw_thread_loop_lock(pw->thread_loop);
> +
> + pw->core = pw_context_connect(pw->context, NULL, 0);
> + if (pw->core == NULL) {
> + goto fail_conn;
> + }
> +
> + pw_core_add_listener(pw->core, &pw->core_listener, &core_events, pw);
> +
> + pw_thread_loop_unlock(pw->thread_loop);
> +
> + return pw;
> +
> +fail_loop:
> + pw_thread_loop_destroy(pw->thread_loop);
You go here when thread_loop == NULL too, likely incorrect
> + g_free(pw);
> + return NULL;
> +fail_start:
> + pw_context_destroy(pw->context);
> + g_free(pw);
But you don't free thread loop here
> + return NULL;
> +fail_conn:
> + AUD_log(AUDIO_CAP, "Failed to initialize PW context");
> + pw_thread_loop_unlock(pw->thread_loop);
> + pw_thread_loop_stop(pw->thread_loop);
> + pw_core_disconnect(pw->core);
The core is always NULL when reaching error conditions. Whether this
function must be called with the loop lock is unclear to me. Anyway,
it should not be necessary.
> + pw_context_destroy(pw->context);
> + pw_thread_loop_destroy(pw->thread_loop);
> + g_free(pw);
You could use goto chains instead of repeating yourself, or forgetting
to free stuff.
Even better imho, having a single exit point. (or even better, is to
use autocleanups, but that is not provided by pipewire API apprently
yet)
In the end, it could look something like that:
static void *
qpw_audio_init(Audiodev *dev)
{
AudiodevPipewireOptions *popts;
g_autofree pwaudio *pw = g_new0(pwaudio, 1);
assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
trace_pw_audio_init();
pw_init(NULL, NULL);
popts = &dev->u.pipewire;
if (!popts->has_latency) {
popts->has_latency = true;
popts->latency = 15000;
}
pw->dev = dev;
pw->thread_loop = pw_thread_loop_new(NULL, NULL);
if (!pw->thread_loop) {
goto fail;
}
pw->context =
pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL, 0);
if (!pw->context) {
goto fail;
}
if (pw_thread_loop_start(pw->thread_loop) < 0) {
goto fail;
}
pw_thread_loop_lock(pw->thread_loop);
pw->core = pw_context_connect(pw->context, NULL, 0);
if (!pw->core) {
pw_thread_loop_unlock(pw->thread_loop);
goto fail;
}
pw_core_add_listener(pw->core, &pw->core_listener, &core_events, pw);
pw_thread_loop_unlock(pw->thread_loop);
return g_steal_pointer(&pw);
fail:
error_report("Failed to initialize Pipewire: %s", strerror(errno));
if (pw->thread_loop) {
/* can be called even when the loop is not running */
pw_thread_loop_stop(pw->thread_loop);
}
g_clear_pointer(&pw->context, pw_context_destroy);
g_clear_pointer(&pw->thread_loop, pw_thread_loop_destroy);
return NULL;
}
Arguably, error reporting could be improved (making sure errno is saved etc)
> + return NULL;
> +}
> +
> +static void
> +qpw_audio_fini(void *opaque)
> +{
> + pwaudio *pw = opaque;
> +
> + pw_thread_loop_stop(pw->thread_loop);
> +
> + if (pw->core) {
> + spa_hook_remove(&pw->core_listener);
> + spa_zero(pw->core_listener);
> + pw_core_disconnect(pw->core);
> + }
> +
> + if (pw->context) {
> + pw_context_destroy(pw->context);
> + }
> + pw_thread_loop_destroy(pw->thread_loop);
> +
> + g_free(pw);
> +}
> +
> +static struct audio_pcm_ops qpw_pcm_ops = {
> + .init_out = qpw_init_out,
> + .fini_out = qpw_fini_out,
> + .write = qpw_write,
> + .buffer_get_free = audio_generic_buffer_get_free,
> + .run_buffer_out = audio_generic_run_buffer_out,
> + .enable_out = qpw_enable_out,
> +
> + .init_in = qpw_init_in,
> + .fini_in = qpw_fini_in,
> + .read = qpw_read,
> + .run_buffer_in = audio_generic_run_buffer_in,
> + .enable_in = qpw_enable_in
> +};
> +
> +static struct audio_driver pw_audio_driver = {
> + .name = "pipewire",
> + .descr = "http://www.pipewire.org/",
> + .init = qpw_audio_init,
> + .fini = qpw_audio_fini,
> + .pcm_ops = &qpw_pcm_ops,
> + .can_be_default = 1,
> + .max_voices_out = INT_MAX,
> + .max_voices_in = INT_MAX,
> + .voice_size_out = sizeof(PWVoiceOut),
> + .voice_size_in = sizeof(PWVoiceIn),
> +};
> +
> +static void
> +register_audio_pw(void)
> +{
> + audio_driver_register(&pw_audio_driver);
> +}
> +
> +type_init(register_audio_pw);
> diff --git a/audio/trace-events b/audio/trace-events
> index e1ab643add..2ecd8851e6 100644
> --- a/audio/trace-events
> +++ b/audio/trace-events
> @@ -18,6 +18,13 @@ dbus_audio_register(const char *s, const char *dir) "sender = %s, dir = %s"
> dbus_audio_put_buffer_out(size_t len) "len = %zu"
> dbus_audio_read(size_t len) "len = %zu"
>
> +# pwaudio.c
> +pw_state_changed(const char *s) "stream state: %s"
> +pw_node(int nodeid) "node id: %d"
> +pw_read(int32_t avail, uint32_t index, size_t len) "avail=%u index=%u len=%zu"
> +pw_write(int32_t filled, int32_t avail, uint32_t index, size_t len) "filled=%u avail=%u index=%u len=%zu"
> +pw_audio_init(const char *msg) "pipewire: %s"
> +
> # audio.c
> audio_timer_start(int interval) "interval %d ms"
> audio_timer_stop(void) ""
> diff --git a/meson.build b/meson.build
> index 6bcab8bf0d..51ec2931e1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -730,6 +730,12 @@ if not get_option('jack').auto() or have_system
> jack = dependency('jack', required: get_option('jack'),
> method: 'pkg-config', kwargs: static_kwargs)
> endif
> +pipewire = not_found
> +if not get_option('pipewire').auto() or (targetos == 'linux' and have_system)
> + pipewire = dependency('libpipewire-0.3', version: '>=0.3.60',
> + required: get_option('pipewire'),
> + method: 'pkg-config', kwargs: static_kwargs)
> +endif
> sndio = not_found
> if not get_option('sndio').auto() or have_system
> sndio = dependency('sndio', required: get_option('sndio'),
> @@ -1667,6 +1673,7 @@ if have_system
> 'jack': jack.found(),
> 'oss': oss.found(),
> 'pa': pulse.found(),
> + 'pipewire': pipewire.found(),
> 'sdl': sdl.found(),
> 'sndio': sndio.found(),
> }
> @@ -3980,6 +3987,7 @@ if targetos == 'linux'
> summary_info += {'ALSA support': alsa}
> summary_info += {'PulseAudio support': pulse}
> endif
> +summary_info += {'Pipewire support': pipewire}
> summary_info += {'JACK support': jack}
> summary_info += {'brlapi support': brlapi}
> summary_info += {'vde support': vde}
> diff --git a/meson_options.txt b/meson_options.txt
> index fc9447d267..9ae1ec7f47 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -21,7 +21,7 @@ option('tls_priority', type : 'string', value : 'NORMAL',
> option('default_devices', type : 'boolean', value : true,
> description: 'Include a default selection of devices in emulators')
> option('audio_drv_list', type: 'array', value: ['default'],
> - choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'sdl', 'sndio'],
> + choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'pipewire', 'sdl', 'sndio'],
> description: 'Set audio driver list')
> option('block_drv_rw_whitelist', type : 'string', value : '',
> description: 'set block driver read-write whitelist (by default affects only QEMU, not tools like qemu-img)')
> @@ -255,6 +255,8 @@ option('oss', type: 'feature', value: 'auto',
> description: 'OSS sound support')
> option('pa', type: 'feature', value: 'auto',
> description: 'PulseAudio sound support')
> +option('pipewire', type: 'feature', value: 'auto',
> + description: 'Pipewire sound support')
> option('sndio', type: 'feature', value: 'auto',
> description: 'sndio sound support')
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 4e54c00f51..9a0d7d9ece 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -324,6 +324,48 @@
> '*out': 'AudiodevPaPerDirectionOptions',
> '*server': 'str' } }
>
> +##
> +# @AudiodevPipewirePerDirectionOptions:
> +#
> +# Options of the Pipewire backend that are used for both playback and
> +# recording.
> +#
> +# @name: name of the sink/source to use
> +#
> +# @stream-name: name of the Pipewire stream created by qemu. Can be
> +# used to identify the stream in Pipewire when you
> +# create multiple Pipewire devices or run multiple qemu
> +# instances (default: audiodev's id, since 7.1)
> +#
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'AudiodevPipewirePerDirectionOptions',
> + 'base': 'AudiodevPerDirectionOptions',
> + 'data': {
> + '*name': 'str',
> + '*stream-name': 'str' } }
> +
> +##
> +# @AudiodevPipewireOptions:
> +#
> +# Options of the Pipewire audio backend.
> +#
> +# @in: options of the capture stream
> +#
> +# @out: options of the playback stream
> +#
> +# @latency: add latency to playback in microseconds
> +# (default 15000)
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'AudiodevPipewireOptions',
> + 'data': {
> + '*in': 'AudiodevPipewirePerDirectionOptions',
> + '*out': 'AudiodevPipewirePerDirectionOptions',
> + '*latency': 'uint32' } }
> +
> ##
> # @AudiodevSdlPerDirectionOptions:
> #
> @@ -416,6 +458,7 @@
> { 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
> { 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
> { 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
> + { 'name': 'pipewire', 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> { 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
> { 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
> { 'name': 'spice', 'if': 'CONFIG_SPICE' },
> @@ -456,6 +499,8 @@
> 'if': 'CONFIG_AUDIO_OSS' },
> 'pa': { 'type': 'AudiodevPaOptions',
> 'if': 'CONFIG_AUDIO_PA' },
> + 'pipewire': { 'type': 'AudiodevPipewireOptions',
> + 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> 'sdl': { 'type': 'AudiodevSdlOptions',
> 'if': 'CONFIG_AUDIO_SDL' },
> 'sndio': { 'type': 'AudiodevSndioOptions',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d42f60fb91..009d58bbf2 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -779,6 +779,11 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> " in|out.name= source/sink device name\n"
> " in|out.latency= desired latency in microseconds\n"
> #endif
> +#ifdef CONFIG_AUDIO_PIPEWIRE
> + "-audiodev pipewire,id=id[,prop[=value][,...]]\n"
> + " in|out.name= source/sink device name\n"
> + " latency= desired latency in microseconds\n"
> +#endif
> #ifdef CONFIG_AUDIO_SDL
> "-audiodev sdl,id=id[,prop[=value][,...]]\n"
> " in|out.buffer-count= number of buffers\n"
> @@ -942,6 +947,18 @@ SRST
> Desired latency in microseconds. The PulseAudio server will try
> to honor this value but actual latencies may be lower or higher.
>
> +``-audiodev pipewire,id=id[,prop[=value][,...]]``
> + Creates a backend using Pipewire. This backend is available on
> + most systems.
> +
> + Pipewire specific options are:
> +
> + ``latency=latency``
> + Add extra latency to playback in microseconds
> +
> + ``in|out.name=sink``
> + Use the specified source/sink for recording/playback.
> +
> ``-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
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 009fab1515..ba1057b62c 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -1,7 +1,8 @@
> # This file is generated by meson-buildoptions.py, do not edit!
> meson_options_help() {
> - printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list [default] (choices: alsa/co'
> - printf "%s\n" ' reaudio/default/dsound/jack/oss/pa/sdl/sndio)'
> + printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list [default] (choices: al'
> + printf "%s\n" ' sa/coreaudio/default/dsound/jack/oss/pa/'
> + printf "%s\n" ' pipewire/sdl/sndio)'
> printf "%s\n" ' --block-drv-ro-whitelist=VALUE'
> printf "%s\n" ' set block driver read-only whitelist (by default'
> printf "%s\n" ' affects only QEMU, not tools like qemu-img)'
> @@ -136,6 +137,7 @@ meson_options_help() {
> printf "%s\n" ' oss OSS sound support'
> printf "%s\n" ' pa PulseAudio sound support'
> printf "%s\n" ' parallels parallels image format support'
> + printf "%s\n" ' pipewire Pipewire sound support'
> printf "%s\n" ' png PNG support with libpng'
> printf "%s\n" ' pvrdma Enable PVRDMA support'
> printf "%s\n" ' qcow1 qcow1 image format support'
> @@ -370,6 +372,8 @@ _meson_option_parse() {
> --disable-pa) printf "%s" -Dpa=disabled ;;
> --enable-parallels) printf "%s" -Dparallels=enabled ;;
> --disable-parallels) printf "%s" -Dparallels=disabled ;;
> + --enable-pipewire) printf "%s" -Dpipewire=enabled ;;
> + --disable-pipewire) printf "%s" -Dpipewire=disabled ;;
> --with-pkgversion=*) quote_sh "-Dpkgversion=$2" ;;
> --enable-png) printf "%s" -Dpng=enabled ;;
> --disable-png) printf "%s" -Dpng=disabled ;;
> --
> 2.39.1
>
>
--
Marc-André Lureau
Hi Marc-André,
Do you mind summarizing your review on the buffering section, and leave
comments so i can address everything once and for all? This will help
reduce the number of versions that needs to be done.
Thanks,
Dorinda.
On Tue, Mar 7, 2023 at 3:41 PM Marc-André Lureau <marcandre.lureau@gmail.com>
wrote:
> Hi Dorinda
>
> On Mon, Mar 6, 2023 at 9:11 PM Dorinda Bassey <dbassey@redhat.com> wrote:
> >
> > This commit adds a new audiodev backend to allow QEMU to use Pipewire as
> > both an audio sink and source. This backend is available on most systems
> >
> > Add Pipewire entry points for QEMU Pipewire audio backend
> > Add wrappers for QEMU Pipewire audio backend in qpw_pcm_ops()
> > qpw_write function returns the current state of the stream to pwaudio
> > and Writes some data to the server for playback streams using pipewire
> > spa_ringbuffer implementation.
> > qpw_read function returns the current state of the stream to pwaudio and
> > reads some data from the server for capture streams using pipewire
> > spa_ringbuffer implementation. These functions qpw_write and qpw_read
> > are called during playback and capture.
> > Added some functions that convert pw audio formats to QEMU audio format
> > and vice versa which would be needed in the pipewire audio sink and
> > source functions qpw_init_in() & qpw_init_out().
> > These methods that implement playback and recording will create streams
> > for playback and capture that will start processing and will result in
> > the on_process callbacks to be called.
> > Built a connection to the Pipewire sound system server in the
> > qpw_audio_init() method.
> >
> > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
>
> Some more comments before I start looking more closely at buffering.
> thanks
>
> > ---
> > v7:
> > use qemu tracing tool
> >
> > audio/audio.c | 3 +
> > audio/audio_template.h | 4 +
> > audio/meson.build | 1 +
> > audio/pwaudio.c | 814 ++++++++++++++++++++++++++++++++++
> > audio/trace-events | 7 +
> > meson.build | 8 +
> > meson_options.txt | 4 +-
> > qapi/audio.json | 45 ++
> > qemu-options.hx | 17 +
> > scripts/meson-buildoptions.sh | 8 +-
> > 10 files changed, 908 insertions(+), 3 deletions(-)
> > create mode 100644 audio/pwaudio.c
> >
> > diff --git a/audio/audio.c b/audio/audio.c
> > index 4290309d18..aa55e41ad8 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -2069,6 +2069,9 @@ void audio_create_pdos(Audiodev *dev)
> > #ifdef CONFIG_AUDIO_PA
> > CASE(PA, pa, Pa);
> > #endif
> > +#ifdef CONFIG_AUDIO_PIPEWIRE
> > + CASE(PIPEWIRE, pipewire, Pipewire);
> > +#endif
> > #ifdef CONFIG_AUDIO_SDL
> > CASE(SDL, sdl, Sdl);
> > #endif
> > diff --git a/audio/audio_template.h b/audio/audio_template.h
> > index 42b4712acb..0f02afb921 100644
> > --- a/audio/audio_template.h
> > +++ b/audio/audio_template.h
> > @@ -355,6 +355,10 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
> TYPE)(Audiodev *dev)
> > case AUDIODEV_DRIVER_PA:
> > return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
> > #endif
> > +#ifdef CONFIG_AUDIO_PIPEWIRE
> > + case AUDIODEV_DRIVER_PIPEWIRE:
> > + return
> qapi_AudiodevPipewirePerDirectionOptions_base(dev->u.pipewire.TYPE);
> > +#endif
> > #ifdef CONFIG_AUDIO_SDL
> > case AUDIODEV_DRIVER_SDL:
> > return
> qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
> > diff --git a/audio/meson.build b/audio/meson.build
> > index 0722224ba9..65a49c1a10 100644
> > --- a/audio/meson.build
> > +++ b/audio/meson.build
> > @@ -19,6 +19,7 @@ foreach m : [
> > ['sdl', sdl, files('sdlaudio.c')],
> > ['jack', jack, files('jackaudio.c')],
> > ['sndio', sndio, files('sndioaudio.c')],
> > + ['pipewire', pipewire, files('pwaudio.c')],
> > ['spice', spice, files('spiceaudio.c')]
> > ]
> > if m[1].found()
> > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> > new file mode 100644
> > index 0000000000..d357761152
> > --- /dev/null
> > +++ b/audio/pwaudio.c
> > @@ -0,0 +1,814 @@
> > +/*
> > + * QEMU Pipewire audio driver
> > + *
> > + * Copyright (c) 2023 Red Hat Inc.
> > + *
> > + * Author: Dorinda Bassey <dbassey@redhat.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/module.h"
> > +#include "audio.h"
> > +#include <errno.h>
> > +#include "qemu/error-report.h"
> > +#include <spa/param/audio/format-utils.h>
> > +#include <spa/utils/ringbuffer.h>
> > +#include <spa/utils/result.h>
> > +
> > +#include <pipewire/pipewire.h>
> > +#include "trace.h"
> > +
> > +#define AUDIO_CAP "pipewire"
> > +#define RINGBUFFER_SIZE (1u << 22)
> > +#define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> > +#define BUFFER_SAMPLES 512
> > +
> > +#include "audio_int.h"
> > +
> > +enum {
> > + MODE_SINK,
> > + MODE_SOURCE
> > +};
> > +
> > +typedef struct pwaudio {
> > + Audiodev *dev;
> > + struct pw_thread_loop *thread_loop;
> > + struct pw_context *context;
> > +
> > + struct pw_core *core;
> > + struct spa_hook core_listener;
> > + int seq;
> > +} pwaudio;
> > +
> > +typedef struct PWVoice {
> > + pwaudio *g;
> > + bool enabled;
> > + struct pw_stream *stream;
> > + struct spa_hook stream_listener;
> > + struct spa_audio_info_raw info;
> > + uint32_t frame_size;
> > + struct spa_ringbuffer ring;
> > + uint8_t buffer[RINGBUFFER_SIZE];
> > +
> > + uint32_t mode;
> > + struct pw_properties *props;
> > +} PWVoice;
> > +
> > +typedef struct PWVoiceOut {
> > + HWVoiceOut hw;
> > + PWVoice v;
> > +} PWVoiceOut;
> > +
> > +typedef struct PWVoiceIn {
> > + HWVoiceIn hw;
> > + PWVoice v;
> > +} PWVoiceIn;
> > +
> > +static void
> > +stream_destroy(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + spa_hook_remove(&v->stream_listener);
> > + v->stream = NULL;
> > +}
> > +
> > +/* output data processing function to read stuffs from the buffer */
> > +static void
> > +playback_on_process(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + void *p;
> > + struct pw_buffer *b;
> > + struct spa_buffer *buf;
> > + uint32_t n_frames, req, index, n_bytes;
> > + int32_t avail;
> > +
> > + if (!v->stream) {
> > + return;
> > + }
> > +
> > + /* obtain a buffer to read from */
> > + b = pw_stream_dequeue_buffer(v->stream);
> > + if (b == NULL) {
> > + error_report("out of buffers: %s", strerror(errno));
> > + return;
> > + }
> > +
> > + buf = b->buffer;
> > + p = buf->datas[0].data;
> > + if (p == NULL) {
> > + return;
> > + }
> > + req = b->requested * v->frame_size;
> > + if (req == 0) {
> > + req = 4096 * v->frame_size;
> > + }
> > + n_frames = SPA_MIN(req, buf->datas[0].maxsize);
> > + n_bytes = n_frames * v->frame_size;
> > +
> > + /* get no of available bytes to read data from buffer */
> > +
> > + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> > +
> > + if (!v->enabled) {
> > + avail = 0;
> > + }
> > +
> > + if (avail == 0) {
> > + memset(p, 0, n_bytes);
> > + } else {
> > + if (avail < (int32_t) n_bytes) {
> > + n_bytes = avail;
> > + }
> > +
> > + spa_ringbuffer_read_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, p,
> n_bytes);
> > +
> > + index += n_bytes;
> > + spa_ringbuffer_read_update(&v->ring, index);
> > + }
> > +
> > + buf->datas[0].chunk->offset = 0;
> > + buf->datas[0].chunk->stride = v->frame_size;
> > + buf->datas[0].chunk->size = n_bytes;
> > +
> > + /* queue the buffer for playback */
> > + pw_stream_queue_buffer(v->stream, b);
> > +}
> > +
> > +/* output data processing function to generate stuffs in the buffer */
> > +static void
> > +capture_on_process(void *data)
> > +{
> > + PWVoice *v = (PWVoice *) data;
> > + void *p;
> > + struct pw_buffer *b;
> > + struct spa_buffer *buf;
> > + int32_t filled;
> > + uint32_t index, offs, n_bytes;
> > +
> > + if (!v->stream) {
> > + return;
> > + }
> > +
> > + /* obtain a buffer */
> > + b = pw_stream_dequeue_buffer(v->stream);
> > + if (b == NULL) {
> > + error_report("out of buffers: %s", strerror(errno));
> > + return;
> > + }
> > +
> > + /* Write data into buffer */
> > + buf = b->buffer;
> > + p = buf->datas[0].data;
> > + if (p == NULL) {
> > + return;
> > + }
> > + offs = SPA_MIN(buf->datas[0].chunk->offset, buf->datas[0].maxsize);
> > + n_bytes = SPA_MIN(buf->datas[0].chunk->size, buf->datas[0].maxsize
> - offs);
> > +
> > + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> > +
> > + if (!v->enabled) {
> > + n_bytes = 0;
> > + }
> > +
> > + if (filled < 0) {
> > + error_report("%p: underrun write:%u filled:%d", p, index,
> filled);
> > + } else {
> > + if ((uint32_t) filled + n_bytes > RINGBUFFER_SIZE) {
> > + error_report("%p: overrun write:%u filled:%d + size:%u >
> max:%u",
> > + p, index, filled, n_bytes, RINGBUFFER_SIZE);
> > + }
> > + }
> > + spa_ringbuffer_write_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK,
> > + SPA_PTROFF(p, offs, void), n_bytes);
> > + index += n_bytes;
> > + spa_ringbuffer_write_update(&v->ring, index);
> > +
> > + /* queue the buffer for playback */
> > + pw_stream_queue_buffer(v->stream, b);
> > +}
> > +
> > +static void
> > +on_stream_state_changed(void *_data, enum pw_stream_state old,
> > + enum pw_stream_state state, const char *error)
> > +{
> > + PWVoice *v = (PWVoice *) _data;
> > +
> > + trace_pw_state_changed(pw_stream_state_as_string(state));
> > +
> > + switch (state) {
> > + case PW_STREAM_STATE_ERROR:
> > + case PW_STREAM_STATE_UNCONNECTED:
> > + {
> > + break;
> > + }
> > + case PW_STREAM_STATE_PAUSED:
> > + trace_pw_node(pw_stream_get_node_id(v->stream));
> > + break;
> > + case PW_STREAM_STATE_CONNECTING:
> > + case PW_STREAM_STATE_STREAMING:
> > + break;
> > + }
> > +}
> > +
> > +static const struct pw_stream_events capture_stream_events = {
> > + PW_VERSION_STREAM_EVENTS,
> > + .destroy = stream_destroy,
> > + .state_changed = on_stream_state_changed,
> > + .process = capture_on_process
> > +};
> > +
> > +static const struct pw_stream_events playback_stream_events = {
> > + PW_VERSION_STREAM_EVENTS,
> > + .destroy = stream_destroy,
> > + .state_changed = on_stream_state_changed,
> > + .process = playback_on_process
> > +};
> > +
> > +static size_t
> > +qpw_read(HWVoiceIn *hw, void *data, size_t len)
> > +{
> > + PWVoiceIn *pw = (PWVoiceIn *) hw;
> > + PWVoice *v = &pw->v;
> > + pwaudio *c = v->g;
> > + const char *error = NULL;
> > + size_t l;
> > + int32_t avail;
> > + uint32_t index;
> > +
> > + pw_thread_loop_lock(c->thread_loop);
> > + if (pw_stream_get_state(v->stream, &error) !=
> PW_STREAM_STATE_STREAMING) {
> > + /* wait for stream to become ready */
> > + l = 0;
> > + goto done_unlock;
> > + }
> > + /* get no of available bytes to read data from buffer */
> > + avail = spa_ringbuffer_get_read_index(&v->ring, &index);
> > +
> > + trace_pw_read(avail, index, len);
> > +
> > + if (avail < (int32_t) len) {
> > + len = avail;
> > + }
> > +
> > + spa_ringbuffer_read_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, data, len);
> > + index += len;
> > + spa_ringbuffer_read_update(&v->ring, index);
> > + l = len;
> > +
> > +done_unlock:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return l;
> > +}
> > +
> > +static size_t
> > +qpw_write(HWVoiceOut *hw, void *data, size_t len)
> > +{
> > + PWVoiceOut *pw = (PWVoiceOut *) hw;
> > + PWVoice *v = &pw->v;
> > + pwaudio *c = v->g;
> > + const char *error = NULL;
> > + const int periods = 3;
> > + size_t l;
> > + int32_t filled, avail;
> > + uint32_t index;
> > +
> > + pw_thread_loop_lock(c->thread_loop);
> > + if (pw_stream_get_state(v->stream, &error) !=
> PW_STREAM_STATE_STREAMING) {
> > + /* wait for stream to become ready */
> > + l = 0;
> > + goto done_unlock;
> > + }
> > + filled = spa_ringbuffer_get_write_index(&v->ring, &index);
> > +
> > + avail = BUFFER_SAMPLES * v->frame_size * periods - filled;
> > +
> > + trace_pw_write(filled, avail, index, len);
> > +
> > + if (len > avail) {
> > + len = avail;
> > + }
> > +
> > + if (filled < 0) {
> > + error_report("%p: underrun write:%u filled:%d", pw, index,
> filled);
> > + } else {
> > + if ((uint32_t) filled + len > RINGBUFFER_SIZE) {
> > + error_report("%p: overrun write:%u filled:%d + size:%zu >
> max:%u",
> > + pw, index, filled, len, RINGBUFFER_SIZE);
> > + }
> > + }
> > +
> > + spa_ringbuffer_write_data(&v->ring,
> > + v->buffer, RINGBUFFER_SIZE,
> > + index & RINGBUFFER_MASK, data, len);
> > + index += len;
> > + spa_ringbuffer_write_update(&v->ring, index);
> > + l = len;
> > +
> > +done_unlock:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return l;
> > +}
> > +
> > +static int
> > +audfmt_to_pw(AudioFormat fmt, int endianness)
> > +{
> > + int format;
> > +
> > + switch (fmt) {
> > + case AUDIO_FORMAT_S8:
> > + format = SPA_AUDIO_FORMAT_S8;
> > + break;
> > + case AUDIO_FORMAT_U8:
> > + format = SPA_AUDIO_FORMAT_U8;
> > + break;
> > + case AUDIO_FORMAT_S16:
> > + format = endianness ? SPA_AUDIO_FORMAT_S16_BE :
> SPA_AUDIO_FORMAT_S16_LE;
> > + break;
> > + case AUDIO_FORMAT_U16:
> > + format = endianness ? SPA_AUDIO_FORMAT_U16_BE :
> SPA_AUDIO_FORMAT_U16_LE;
> > + break;
> > + case AUDIO_FORMAT_S32:
> > + format = endianness ? SPA_AUDIO_FORMAT_S32_BE :
> SPA_AUDIO_FORMAT_S32_LE;
> > + break;
> > + case AUDIO_FORMAT_U32:
> > + format = endianness ? SPA_AUDIO_FORMAT_U32_BE :
> SPA_AUDIO_FORMAT_U32_LE;
> > + break;
> > + case AUDIO_FORMAT_F32:
> > + format = endianness ? SPA_AUDIO_FORMAT_F32_BE :
> SPA_AUDIO_FORMAT_F32_LE;
> > + break;
> > + default:
> > + dolog("Internal logic error: Bad audio format %d\n", fmt);
> > + format = SPA_AUDIO_FORMAT_U8;
> > + break;
> > + }
> > + return format;
> > +}
> > +
> > +static AudioFormat
> > +pw_to_audfmt(enum spa_audio_format fmt, int *endianness,
> > + uint32_t *frame_size)
> > +{
> > + switch (fmt) {
> > + case SPA_AUDIO_FORMAT_S8:
> > + *frame_size = 1;
> > + return AUDIO_FORMAT_S8;
> > + case SPA_AUDIO_FORMAT_U8:
> > + *frame_size = 1;
> > + return AUDIO_FORMAT_U8;
> > + case SPA_AUDIO_FORMAT_S16_BE:
> > + *frame_size = 2;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_S16;
> > + case SPA_AUDIO_FORMAT_S16_LE:
> > + *frame_size = 2;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_S16;
> > + case SPA_AUDIO_FORMAT_U16_BE:
> > + *frame_size = 2;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_U16;
> > + case SPA_AUDIO_FORMAT_U16_LE:
> > + *frame_size = 2;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_U16;
> > + case SPA_AUDIO_FORMAT_S32_BE:
> > + *frame_size = 4;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_S32;
> > + case SPA_AUDIO_FORMAT_S32_LE:
> > + *frame_size = 4;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_S32;
> > + case SPA_AUDIO_FORMAT_U32_BE:
> > + *frame_size = 4;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_U32;
> > + case SPA_AUDIO_FORMAT_U32_LE:
> > + *frame_size = 4;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_U32;
> > + case SPA_AUDIO_FORMAT_F32_BE:
> > + *frame_size = 4;
> > + *endianness = 1;
> > + return AUDIO_FORMAT_F32;
> > + case SPA_AUDIO_FORMAT_F32_LE:
> > + *frame_size = 4;
> > + *endianness = 0;
> > + return AUDIO_FORMAT_F32;
> > + default:
> > + *frame_size = 1;
> > + dolog("Internal logic error: Bad spa_audio_format %d\n", fmt);
> > + return AUDIO_FORMAT_U8;
> > + }
> > +}
> > +
> > +static int
> > +create_stream(pwaudio *c, PWVoice *v, const char *name)
> > +{
> > + int res;
> > + uint32_t n_params;
> > + const struct spa_pod *params[2];
> > + uint8_t buffer[1024];
> > + struct spa_pod_builder b;
> > +
> > + v->stream = pw_stream_new(c->core, name, NULL);
> > +
> > + if (v->stream == NULL) {
>
> (ok, some error reporting is done by the caller)
>
> > + goto error;
>
> you should return directly, see below for related issue
>
> > + }
> > +
> > + if (v->mode == MODE_SOURCE) {
> > + pw_stream_add_listener(v->stream,
> > + &v->stream_listener,
> &capture_stream_events, v);
> > + } else {
> > + pw_stream_add_listener(v->stream,
> > + &v->stream_listener,
> &playback_stream_events, v);
> > + }
> > +
> > + n_params = 0;
> > + spa_pod_builder_init(&b, buffer, sizeof(buffer));
> > + params[n_params++] = spa_format_audio_raw_build(&b,
> > + SPA_PARAM_EnumFormat,
> > + &v->info);
> > +
> > + /* connect the stream to a sink or source */
> > + res = pw_stream_connect(v->stream,
> > + v->mode ==
> > + MODE_SOURCE ? PW_DIRECTION_INPUT :
> > + PW_DIRECTION_OUTPUT, PW_ID_ANY,
> > + PW_STREAM_FLAG_AUTOCONNECT |
> > + PW_STREAM_FLAG_MAP_BUFFERS |
> > + PW_STREAM_FLAG_RT_PROCESS, params,
> n_params);
> > + if (res < 0) {
> > + goto error;
>
> then you have a single "goto error", so you can fold the code here instead.
>
> > + }
> > +
> > + return 0;
> > +error:
> > + pw_stream_destroy(v->stream);
>
> Incorrect if v->stream is NULL
>
> > + return -1;
> > +}
> > +
> > +static int
> > +qpw_stream_new(pwaudio *c, PWVoice *v, const char *name)
> > +{
> > + int r;
> > +
> > + switch (v->info.channels) {
> > + case 8:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> > + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> > + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> > + v->info.position[6] = SPA_AUDIO_CHANNEL_SL;
> > + v->info.position[7] = SPA_AUDIO_CHANNEL_SR;
> > + break;
> > + case 6:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> > + v->info.position[4] = SPA_AUDIO_CHANNEL_RL;
> > + v->info.position[5] = SPA_AUDIO_CHANNEL_RR;
> > + break;
> > + case 5:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_LFE;
> > + v->info.position[4] = SPA_AUDIO_CHANNEL_RC;
> > + break;
> > + case 4:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_FC;
> > + v->info.position[3] = SPA_AUDIO_CHANNEL_RC;
> > + break;
> > + case 3:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + v->info.position[2] = SPA_AUDIO_CHANNEL_LFE;
> > + break;
> > + case 2:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_FL;
> > + v->info.position[1] = SPA_AUDIO_CHANNEL_FR;
> > + break;
> > + case 1:
> > + v->info.position[0] = SPA_AUDIO_CHANNEL_MONO;
> > + break;
> > + default:
> > + for (size_t i = 0; i < v->info.channels; i++) {
> > + v->info.position[i] = SPA_AUDIO_CHANNEL_UNKNOWN;
> > + }
> > + break;
> > + }
> > +
> > + /* create a new unconnected pwstream */
> > + r = create_stream(c, v, name);
> > + if (r < 0) {
> > + goto error;
>
> single fail point, just fold the code here please
>
> > + }
> > +
> > + return r;
> > +
> > +error:
> > + AUD_log(AUDIO_CAP, "Failed to create stream.");
> > + return -1;
> > +}
> > +
> > +static int
> > +qpw_init_out(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque)
> > +{
> > + PWVoiceOut *pw = (PWVoiceOut *) hw;
> > + PWVoice *v = &pw->v;
> > + struct audsettings obt_as = *as;
> > + pwaudio *c = v->g = drv_opaque;
> > + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> > + AudiodevPipewirePerDirectionOptions *ppdo = popts->out;
> > + int r;
> > + v->enabled = false;
> > +
> > + v->mode = MODE_SINK;
> > +
> > + pw_thread_loop_lock(c->thread_loop);
> > +
> > + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> > + v->info.channels = as->nchannels;
> > + v->info.rate = as->freq;
> > +
> > + obt_as.fmt =
> > + pw_to_audfmt(v->info.format, &obt_as.endianness,
> &v->frame_size);
> > + v->frame_size *= as->nchannels;
> > +
> > + /* call the function that creates a new stream for playback */
> > + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> > + if (r < 0) {
> > + error_report("qpw_stream_new for playback failed\n ");
>
> extra "\n "
>
> > + goto fail;
>
> single fail point, just fold the code here please
>
> > + }
> > +
> > + /* report the audio format we support */
> > + audio_pcm_init_info(&hw->info, &obt_as);
> > +
> > + /* report the buffer size to qemu */
> > + hw->samples = BUFFER_SAMPLES;
> > +
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return 0;
> > +fail:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return -1;
> > +}
> > +
> > +static int
> > +qpw_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
> > +{
> > + PWVoiceIn *pw = (PWVoiceIn *) hw;
> > + PWVoice *v = &pw->v;
> > + struct audsettings obt_as = *as;
> > + pwaudio *c = v->g = drv_opaque;
> > + AudiodevPipewireOptions *popts = &c->dev->u.pipewire;
> > + AudiodevPipewirePerDirectionOptions *ppdo = popts->in;
> > + int r;
> > + v->enabled = false;
> > +
> > + v->mode = MODE_SOURCE;
> > + pw_thread_loop_lock(c->thread_loop);
> > +
> > + v->info.format = audfmt_to_pw(as->fmt, as->endianness);
> > + v->info.channels = as->nchannels;
> > + v->info.rate = as->freq;
> > +
> > + obt_as.fmt =
> > + pw_to_audfmt(v->info.format, &obt_as.endianness,
> &v->frame_size);
> > + v->frame_size *= as->nchannels;
> > +
> > + /* call the function that creates a new stream for recording */
> > + r = qpw_stream_new(c, v, ppdo->stream_name ? : c->dev->id);
> > + if (r < 0) {
> > + error_report("qpw_stream_new for recording failed\n ");
>
> extra "\n "
>
> > + goto fail;
>
> single fail point, just fold the code here please
>
> > + }
> > +
> > + /* report the audio format we support */
> > + audio_pcm_init_info(&hw->info, &obt_as);
> > +
> > + /* report the buffer size to qemu */
> > + hw->samples = BUFFER_SAMPLES;
> > +
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return 0;
> > +fail:
> > + pw_thread_loop_unlock(c->thread_loop);
> > + return -1;
> > +}
> > +
> > +static void
> > +qpw_fini_out(HWVoiceOut *hw)
> > +{
> > + PWVoiceOut *pw = (PWVoiceOut *) hw;
> > + PWVoice *v = &pw->v;
> > +
> > + if (v->stream) {
> > + pwaudio *c = v->g;
> > + pw_thread_loop_lock(c->thread_loop);
> > + pw_stream_destroy(v->stream);
> > + v->stream = NULL;
> > + pw_thread_loop_unlock(c->thread_loop);
> > + }
> > +}
> > +
> > +static void
> > +qpw_fini_in(HWVoiceIn *hw)
> > +{
> > + PWVoiceIn *pw = (PWVoiceIn *) hw;
> > + PWVoice *v = &pw->v;
> > +
> > + if (v->stream) {
> > + pwaudio *c = v->g;
> > + pw_thread_loop_lock(c->thread_loop);
> > + pw_stream_destroy(v->stream);
> > + v->stream = NULL;
> > + pw_thread_loop_unlock(c->thread_loop);
> > + }
> > +}
> > +
> > +static void
> > +qpw_enable_out(HWVoiceOut *hw, bool enable)
> > +{
> > + PWVoiceOut *po = (PWVoiceOut *) hw;
> > + PWVoice *v = &po->v;
> > + v->enabled = enable;
> > +}
> > +
> > +static void
> > +qpw_enable_in(HWVoiceIn *hw, bool enable)
> > +{
> > + PWVoiceIn *pi = (PWVoiceIn *) hw;
> > + PWVoice *v = &pi->v;
> > + v->enabled = enable;
> > +}
> > +
> > +static void
> > +on_core_error(void *data, uint32_t id, int seq, int res, const char
> *message)
> > +{
> > + pwaudio *pw = data;
> > +
> > + error_report("error id:%u seq:%d res:%d (%s): %s",
> > + id, seq, res, spa_strerror(res), message);
> > +
> > + pw_thread_loop_signal(pw->thread_loop, FALSE);
> > +}
> > +
> > +static void
> > +on_core_done(void *data, uint32_t id, int seq)
> > +{
> > + pwaudio *pw = data;
> > + if (id == PW_ID_CORE) {
> > + pw->seq = seq;
> > + pw_thread_loop_signal(pw->thread_loop, FALSE);
> > + }
> > +}
> > +
> > +static const struct pw_core_events core_events = {
> > + PW_VERSION_CORE_EVENTS,
> > + .done = on_core_done,
> > + .error = on_core_error,
> > +};
> > +
> > +static void *
> > +qpw_audio_init(Audiodev *dev)
> > +{
> > + pwaudio *pw;
> > + pw = g_new0(pwaudio, 1);
> > + pw_init(NULL, NULL);
> > +
> > + AudiodevPipewireOptions *popts;
>
> we usually avoid mixing declaration and code.
>
> > + trace_pw_audio_init("Initialize Pipewire context\n");
>
> extra \n
>
> you can simply drop the string argument. You only call
> trace_pw_audio_init() once.
>
> > + assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> > + popts = &dev->u.pipewire;
> > +
> > + if (!popts->has_latency) {
> > + popts->has_latency = true;
> > + popts->latency = 15000;
> > + }
> > +
> > + pw->dev = dev;
> > + pw->thread_loop = pw_thread_loop_new("Pipewire thread loop", NULL);
>
> Given that we have only one loop, I would set the thread name to NULL,
> (which will use "pw-thread-loop" by default)
>
> > + if (pw->thread_loop == NULL) {
> > + error_report("Could not create Pipewire loop");
> > + goto fail_loop;
> > + }
> > +
> > + pw->context =
> > + pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL,
> 0);
> > + if (pw->context == NULL) {
> > + error_report("Could not create Pipewire context");
> > + goto fail_loop;
> > + }
> > +
> > + if (pw_thread_loop_start(pw->thread_loop) < 0) {
> > + error_report("Could not start Pipewire loop");
> > + goto fail_start;
> > + }
> > +
> > + pw_thread_loop_lock(pw->thread_loop);
> > +
> > + pw->core = pw_context_connect(pw->context, NULL, 0);
> > + if (pw->core == NULL) {
> > + goto fail_conn;
> > + }
> > +
> > + pw_core_add_listener(pw->core, &pw->core_listener, &core_events,
> pw);
> > +
> > + pw_thread_loop_unlock(pw->thread_loop);
> > +
> > + return pw;
> > +
> > +fail_loop:
> > + pw_thread_loop_destroy(pw->thread_loop);
>
> You go here when thread_loop == NULL too, likely incorrect
>
> > + g_free(pw);
> > + return NULL;
> > +fail_start:
> > + pw_context_destroy(pw->context);
> > + g_free(pw);
>
> But you don't free thread loop here
>
> > + return NULL;
> > +fail_conn:
> > + AUD_log(AUDIO_CAP, "Failed to initialize PW context");
> > + pw_thread_loop_unlock(pw->thread_loop);
> > + pw_thread_loop_stop(pw->thread_loop);
> > + pw_core_disconnect(pw->core);
>
> The core is always NULL when reaching error conditions. Whether this
> function must be called with the loop lock is unclear to me. Anyway,
> it should not be necessary.
>
> > + pw_context_destroy(pw->context);
> > + pw_thread_loop_destroy(pw->thread_loop);
> > + g_free(pw);
>
> You could use goto chains instead of repeating yourself, or forgetting
> to free stuff.
>
> Even better imho, having a single exit point. (or even better, is to
> use autocleanups, but that is not provided by pipewire API apprently
> yet)
>
> In the end, it could look something like that:
>
> static void *
> qpw_audio_init(Audiodev *dev)
> {
> AudiodevPipewireOptions *popts;
> g_autofree pwaudio *pw = g_new0(pwaudio, 1);
>
> assert(dev->driver == AUDIODEV_DRIVER_PIPEWIRE);
> trace_pw_audio_init();
> pw_init(NULL, NULL);
>
> popts = &dev->u.pipewire;
> if (!popts->has_latency) {
> popts->has_latency = true;
> popts->latency = 15000;
> }
>
> pw->dev = dev;
> pw->thread_loop = pw_thread_loop_new(NULL, NULL);
> if (!pw->thread_loop) {
> goto fail;
> }
> pw->context =
> pw_context_new(pw_thread_loop_get_loop(pw->thread_loop), NULL, 0);
> if (!pw->context) {
> goto fail;
> }
> if (pw_thread_loop_start(pw->thread_loop) < 0) {
> goto fail;
> }
> pw_thread_loop_lock(pw->thread_loop);
> pw->core = pw_context_connect(pw->context, NULL, 0);
> if (!pw->core) {
> pw_thread_loop_unlock(pw->thread_loop);
> goto fail;
> }
> pw_core_add_listener(pw->core, &pw->core_listener, &core_events, pw);
> pw_thread_loop_unlock(pw->thread_loop);
>
> return g_steal_pointer(&pw);
>
> fail:
> error_report("Failed to initialize Pipewire: %s", strerror(errno));
> if (pw->thread_loop) {
> /* can be called even when the loop is not running */
> pw_thread_loop_stop(pw->thread_loop);
> }
> g_clear_pointer(&pw->context, pw_context_destroy);
> g_clear_pointer(&pw->thread_loop, pw_thread_loop_destroy);
> return NULL;
> }
>
> Arguably, error reporting could be improved (making sure errno is saved
> etc)
>
>
> > + return NULL;
> > +}
> > +
> > +static void
> > +qpw_audio_fini(void *opaque)
> > +{
> > + pwaudio *pw = opaque;
> > +
> > + pw_thread_loop_stop(pw->thread_loop);
> > +
> > + if (pw->core) {
> > + spa_hook_remove(&pw->core_listener);
> > + spa_zero(pw->core_listener);
> > + pw_core_disconnect(pw->core);
> > + }
> > +
> > + if (pw->context) {
> > + pw_context_destroy(pw->context);
> > + }
> > + pw_thread_loop_destroy(pw->thread_loop);
> > +
> > + g_free(pw);
> > +}
> > +
> > +static struct audio_pcm_ops qpw_pcm_ops = {
> > + .init_out = qpw_init_out,
> > + .fini_out = qpw_fini_out,
> > + .write = qpw_write,
> > + .buffer_get_free = audio_generic_buffer_get_free,
> > + .run_buffer_out = audio_generic_run_buffer_out,
> > + .enable_out = qpw_enable_out,
> > +
> > + .init_in = qpw_init_in,
> > + .fini_in = qpw_fini_in,
> > + .read = qpw_read,
> > + .run_buffer_in = audio_generic_run_buffer_in,
> > + .enable_in = qpw_enable_in
> > +};
> > +
> > +static struct audio_driver pw_audio_driver = {
> > + .name = "pipewire",
> > + .descr = "http://www.pipewire.org/",
> > + .init = qpw_audio_init,
> > + .fini = qpw_audio_fini,
> > + .pcm_ops = &qpw_pcm_ops,
> > + .can_be_default = 1,
> > + .max_voices_out = INT_MAX,
> > + .max_voices_in = INT_MAX,
> > + .voice_size_out = sizeof(PWVoiceOut),
> > + .voice_size_in = sizeof(PWVoiceIn),
> > +};
> > +
> > +static void
> > +register_audio_pw(void)
> > +{
> > + audio_driver_register(&pw_audio_driver);
> > +}
> > +
> > +type_init(register_audio_pw);
> > diff --git a/audio/trace-events b/audio/trace-events
> > index e1ab643add..2ecd8851e6 100644
> > --- a/audio/trace-events
> > +++ b/audio/trace-events
> > @@ -18,6 +18,13 @@ dbus_audio_register(const char *s, const char *dir)
> "sender = %s, dir = %s"
> > dbus_audio_put_buffer_out(size_t len) "len = %zu"
> > dbus_audio_read(size_t len) "len = %zu"
> >
> > +# pwaudio.c
> > +pw_state_changed(const char *s) "stream state: %s"
> > +pw_node(int nodeid) "node id: %d"
> > +pw_read(int32_t avail, uint32_t index, size_t len) "avail=%u index=%u
> len=%zu"
> > +pw_write(int32_t filled, int32_t avail, uint32_t index, size_t len)
> "filled=%u avail=%u index=%u len=%zu"
> > +pw_audio_init(const char *msg) "pipewire: %s"
> > +
> > # audio.c
> > audio_timer_start(int interval) "interval %d ms"
> > audio_timer_stop(void) ""
> > diff --git a/meson.build b/meson.build
> > index 6bcab8bf0d..51ec2931e1 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -730,6 +730,12 @@ if not get_option('jack').auto() or have_system
> > jack = dependency('jack', required: get_option('jack'),
> > method: 'pkg-config', kwargs: static_kwargs)
> > endif
> > +pipewire = not_found
> > +if not get_option('pipewire').auto() or (targetos == 'linux' and
> have_system)
> > + pipewire = dependency('libpipewire-0.3', version: '>=0.3.60',
> > + required: get_option('pipewire'),
> > + method: 'pkg-config', kwargs: static_kwargs)
> > +endif
> > sndio = not_found
> > if not get_option('sndio').auto() or have_system
> > sndio = dependency('sndio', required: get_option('sndio'),
> > @@ -1667,6 +1673,7 @@ if have_system
> > 'jack': jack.found(),
> > 'oss': oss.found(),
> > 'pa': pulse.found(),
> > + 'pipewire': pipewire.found(),
> > 'sdl': sdl.found(),
> > 'sndio': sndio.found(),
> > }
> > @@ -3980,6 +3987,7 @@ if targetos == 'linux'
> > summary_info += {'ALSA support': alsa}
> > summary_info += {'PulseAudio support': pulse}
> > endif
> > +summary_info += {'Pipewire support': pipewire}
> > summary_info += {'JACK support': jack}
> > summary_info += {'brlapi support': brlapi}
> > summary_info += {'vde support': vde}
> > diff --git a/meson_options.txt b/meson_options.txt
> > index fc9447d267..9ae1ec7f47 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -21,7 +21,7 @@ option('tls_priority', type : 'string', value :
> 'NORMAL',
> > option('default_devices', type : 'boolean', value : true,
> > description: 'Include a default selection of devices in
> emulators')
> > option('audio_drv_list', type: 'array', value: ['default'],
> > - choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack',
> 'oss', 'pa', 'sdl', 'sndio'],
> > + choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack',
> 'oss', 'pa', 'pipewire', 'sdl', 'sndio'],
> > description: 'Set audio driver list')
> > option('block_drv_rw_whitelist', type : 'string', value : '',
> > description: 'set block driver read-write whitelist (by default
> affects only QEMU, not tools like qemu-img)')
> > @@ -255,6 +255,8 @@ option('oss', type: 'feature', value: 'auto',
> > description: 'OSS sound support')
> > option('pa', type: 'feature', value: 'auto',
> > description: 'PulseAudio sound support')
> > +option('pipewire', type: 'feature', value: 'auto',
> > + description: 'Pipewire sound support')
> > option('sndio', type: 'feature', value: 'auto',
> > description: 'sndio sound support')
> >
> > diff --git a/qapi/audio.json b/qapi/audio.json
> > index 4e54c00f51..9a0d7d9ece 100644
> > --- a/qapi/audio.json
> > +++ b/qapi/audio.json
> > @@ -324,6 +324,48 @@
> > '*out': 'AudiodevPaPerDirectionOptions',
> > '*server': 'str' } }
> >
> > +##
> > +# @AudiodevPipewirePerDirectionOptions:
> > +#
> > +# Options of the Pipewire backend that are used for both playback and
> > +# recording.
> > +#
> > +# @name: name of the sink/source to use
> > +#
> > +# @stream-name: name of the Pipewire stream created by qemu. Can be
> > +# used to identify the stream in Pipewire when you
> > +# create multiple Pipewire devices or run multiple qemu
> > +# instances (default: audiodev's id, since 7.1)
> > +#
> > +#
> > +# Since: 8.0
> > +##
> > +{ 'struct': 'AudiodevPipewirePerDirectionOptions',
> > + 'base': 'AudiodevPerDirectionOptions',
> > + 'data': {
> > + '*name': 'str',
> > + '*stream-name': 'str' } }
> > +
> > +##
> > +# @AudiodevPipewireOptions:
> > +#
> > +# Options of the Pipewire audio backend.
> > +#
> > +# @in: options of the capture stream
> > +#
> > +# @out: options of the playback stream
> > +#
> > +# @latency: add latency to playback in microseconds
> > +# (default 15000)
> > +#
> > +# Since: 8.0
> > +##
> > +{ 'struct': 'AudiodevPipewireOptions',
> > + 'data': {
> > + '*in': 'AudiodevPipewirePerDirectionOptions',
> > + '*out': 'AudiodevPipewirePerDirectionOptions',
> > + '*latency': 'uint32' } }
> > +
> > ##
> > # @AudiodevSdlPerDirectionOptions:
> > #
> > @@ -416,6 +458,7 @@
> > { 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
> > { 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
> > { 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
> > + { 'name': 'pipewire', 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> > { 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
> > { 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
> > { 'name': 'spice', 'if': 'CONFIG_SPICE' },
> > @@ -456,6 +499,8 @@
> > 'if': 'CONFIG_AUDIO_OSS' },
> > 'pa': { 'type': 'AudiodevPaOptions',
> > 'if': 'CONFIG_AUDIO_PA' },
> > + 'pipewire': { 'type': 'AudiodevPipewireOptions',
> > + 'if': 'CONFIG_AUDIO_PIPEWIRE' },
> > 'sdl': { 'type': 'AudiodevSdlOptions',
> > 'if': 'CONFIG_AUDIO_SDL' },
> > 'sndio': { 'type': 'AudiodevSndioOptions',
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index d42f60fb91..009d58bbf2 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -779,6 +779,11 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> > " in|out.name= source/sink device name\n"
> > " in|out.latency= desired latency in microseconds\n"
> > #endif
> > +#ifdef CONFIG_AUDIO_PIPEWIRE
> > + "-audiodev pipewire,id=id[,prop[=value][,...]]\n"
> > + " in|out.name= source/sink device name\n"
> > + " latency= desired latency in microseconds\n"
> > +#endif
> > #ifdef CONFIG_AUDIO_SDL
> > "-audiodev sdl,id=id[,prop[=value][,...]]\n"
> > " in|out.buffer-count= number of buffers\n"
> > @@ -942,6 +947,18 @@ SRST
> > Desired latency in microseconds. The PulseAudio server will try
> > to honor this value but actual latencies may be lower or higher.
> >
> > +``-audiodev pipewire,id=id[,prop[=value][,...]]``
> > + Creates a backend using Pipewire. This backend is available on
> > + most systems.
> > +
> > + Pipewire specific options are:
> > +
> > + ``latency=latency``
> > + Add extra latency to playback in microseconds
> > +
> > + ``in|out.name=sink``
> > + Use the specified source/sink for recording/playback.
> > +
> > ``-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
> > diff --git a/scripts/meson-buildoptions.sh
> b/scripts/meson-buildoptions.sh
> > index 009fab1515..ba1057b62c 100644
> > --- a/scripts/meson-buildoptions.sh
> > +++ b/scripts/meson-buildoptions.sh
> > @@ -1,7 +1,8 @@
> > # This file is generated by meson-buildoptions.py, do not edit!
> > meson_options_help() {
> > - printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list
> [default] (choices: alsa/co'
> > - printf "%s\n" '
> reaudio/default/dsound/jack/oss/pa/sdl/sndio)'
> > + printf "%s\n" ' --audio-drv-list=CHOICES Set audio driver list
> [default] (choices: al'
> > + printf "%s\n" '
> sa/coreaudio/default/dsound/jack/oss/pa/'
> > + printf "%s\n" ' pipewire/sdl/sndio)'
> > printf "%s\n" ' --block-drv-ro-whitelist=VALUE'
> > printf "%s\n" ' set block driver read-only
> whitelist (by default'
> > printf "%s\n" ' affects only QEMU, not
> tools like qemu-img)'
> > @@ -136,6 +137,7 @@ meson_options_help() {
> > printf "%s\n" ' oss OSS sound support'
> > printf "%s\n" ' pa PulseAudio sound support'
> > printf "%s\n" ' parallels parallels image format support'
> > + printf "%s\n" ' pipewire Pipewire sound support'
> > printf "%s\n" ' png PNG support with libpng'
> > printf "%s\n" ' pvrdma Enable PVRDMA support'
> > printf "%s\n" ' qcow1 qcow1 image format support'
> > @@ -370,6 +372,8 @@ _meson_option_parse() {
> > --disable-pa) printf "%s" -Dpa=disabled ;;
> > --enable-parallels) printf "%s" -Dparallels=enabled ;;
> > --disable-parallels) printf "%s" -Dparallels=disabled ;;
> > + --enable-pipewire) printf "%s" -Dpipewire=enabled ;;
> > + --disable-pipewire) printf "%s" -Dpipewire=disabled ;;
> > --with-pkgversion=*) quote_sh "-Dpkgversion=$2" ;;
> > --enable-png) printf "%s" -Dpng=enabled ;;
> > --disable-png) printf "%s" -Dpng=disabled ;;
> > --
> > 2.39.1
> >
> >
>
>
> --
> Marc-André Lureau
>
>
© 2016 - 2024 Red Hat, Inc.