audio/audio.c | 1 + audio/audio_template.h | 2 + audio/meson.build | 1 + audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++ meson.build | 7 + meson_options.txt | 4 +- qapi/audio.json | 25 +- qemu-options.hx | 8 + tests/vm/freebsd | 3 + 9 files changed, 604 insertions(+), 2 deletions(-) create mode 100644 audio/sndioaudio.c
audio: Add sndio backend
Add a sndio backend.
sndio is the native API used by OpenBSD, although it has been ported to
other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.).
The C code is from Alexandre Ratchov <alex@caoua.org> and the rest of
the bits are from me.
---
audio/audio.c | 1 +
audio/audio_template.h | 2 +
audio/meson.build | 1 +
audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++
meson.build | 7 +
meson_options.txt | 4 +-
qapi/audio.json | 25 +-
qemu-options.hx | 8 +
tests/vm/freebsd | 3 +
9 files changed, 604 insertions(+), 2 deletions(-)
create mode 100644 audio/sndioaudio.c
diff --git a/audio/audio.c b/audio/audio.c
index 54a153c0ef..bad1ceb69e 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2005,6 +2005,7 @@ void audio_create_pdos(Audiodev *dev)
CASE(OSS, oss, Oss);
CASE(PA, pa, Pa);
CASE(SDL, sdl, Sdl);
+ CASE(SNDIO, sndio, );
CASE(SPICE, spice, );
CASE(WAV, wav, );
diff --git a/audio/audio_template.h b/audio/audio_template.h
index c6714946aa..ecc5a0bc6d 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -337,6 +337,8 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
case AUDIODEV_DRIVER_SDL:
return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
+ case AUDIODEV_DRIVER_SNDIO:
+ return dev->u.sndio.TYPE;
case AUDIODEV_DRIVER_SPICE:
return dev->u.spice.TYPE;
case AUDIODEV_DRIVER_WAV:
diff --git a/audio/meson.build b/audio/meson.build
index 462533bb8c..e24c86e7e6 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -17,6 +17,7 @@ foreach m : [
['pa', pulse, files('paaudio.c')],
['sdl', sdl, files('sdlaudio.c')],
['jack', jack, files('jackaudio.c')],
+ ['sndio', sndio, files('sndioaudio.c')],
['spice', spice, files('spiceaudio.c')]
]
if m[1].found()
diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
new file mode 100644
index 0000000000..204af07781
--- /dev/null
+++ b/audio/sndioaudio.c
@@ -0,0 +1,555 @@
+/*
+ * Copyright (c) 2019 Alexandre Ratchov <alex@caoua.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * TODO :
+ *
+ * Use a single device and open it in full-duplex rather than
+ * opening it twice (once for playback once for recording).
+ *
+ * This is the only way to ensure that playback doesn't drift with respect
+ * to recording, which is what guest systems expect.
+ */
+
+#include <poll.h>
+#include <sndio.h>
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/main-loop.h"
+#include "audio.h"
+#include "trace.h"
+
+#define AUDIO_CAP "sndio"
+#include "audio_int.h"
+
+/* default latency in ms if no option is set */
+#define SNDIO_LATENCY_US 50000
+
+typedef struct SndioVoice {
+ union {
+ HWVoiceOut out;
+ HWVoiceIn in;
+ } hw;
+ struct sio_par par;
+ struct sio_hdl *hdl;
+ struct pollfd *pfds;
+ struct pollindex {
+ struct SndioVoice *self;
+ int index;
+ } *pindexes;
+ unsigned char *buf;
+ size_t buf_size;
+ size_t sndio_pos;
+ size_t qemu_pos;
+ unsigned int mode;
+ unsigned int nfds;
+} SndioVoice;
+
+typedef struct SndioConf {
+ const char *devname;
+ unsigned int latency;
+} SndioConf;
+
+/* needed for forward reference */
+static void sndio_poll_in(void *arg);
+static void sndio_poll_out(void *arg);
+
+/*
+ * stop polling descriptors
+ */
+static void sndio_poll_clear(SndioVoice *self)
+{
+ struct pollfd *pfd;
+ int i;
+
+ for (i = 0; i < self->nfds; i++) {
+ pfd = &self->pfds[i];
+ qemu_set_fd_handler (pfd->fd, NULL, NULL, NULL);
+ }
+
+ self->nfds = 0;
+}
+
+/*
+ * write data to the device until it blocks or
+ * all of our buffered data is written
+ */
+static void sndio_write(SndioVoice *self)
+{
+ size_t todo, n;
+
+ todo = self->qemu_pos - self->sndio_pos;
+
+ /*
+ * transfer data to device, until it blocks
+ */
+ while (todo > 0) {
+ n = sio_write(self->hdl, self->buf + self->sndio_pos, todo);
+ if (n == 0) {
+ break;
+ }
+ self->sndio_pos += n;
+ todo -= n;
+ }
+
+ if (self->sndio_pos == self->buf_size) {
+ /*
+ * we complete the block
+ */
+ self->sndio_pos = 0;
+ self->qemu_pos = 0;
+ }
+}
+
+/*
+ * read data from the device until it blocks or
+ * there no room any longer
+ */
+static void sndio_read(SndioVoice *self)
+{
+ size_t todo, n;
+
+ todo = self->buf_size - self->sndio_pos;
+
+ /*
+ * transfer data from the device, until it blocks
+ */
+ while (todo > 0) {
+ n = sio_read(self->hdl, self->buf + self->sndio_pos, todo);
+ if (n == 0) {
+ break;
+ }
+ self->sndio_pos += n;
+ todo -= n;
+ }
+}
+
+/*
+ * Set handlers for all descriptors libsndio needs to
+ * poll
+ */
+static void sndio_poll_wait(SndioVoice *self)
+{
+ struct pollfd *pfd;
+ int events, i;
+
+ events = 0;
+ if (self->mode == SIO_PLAY) {
+ if (self->sndio_pos < self->qemu_pos) {
+ events |= POLLOUT;
+ }
+ } else {
+ if (self->sndio_pos < self->buf_size) {
+ events |= POLLIN;
+ }
+ }
+
+ /*
+ * fill the given array of descriptors with the events sndio
+ * wants, they are different from our 'event' variable because
+ * sndio may use descriptors internally.
+ */
+ self->nfds = sio_pollfd(self->hdl, self->pfds, events);
+
+ for (i = 0; i < self->nfds; i++) {
+ pfd = &self->pfds[i];
+ if (pfd->fd < 0) {
+ continue;
+ }
+ qemu_set_fd_handler(pfd->fd,
+ (pfd->events & POLLIN) ? sndio_poll_in : NULL,
+ (pfd->events & POLLOUT) ? sndio_poll_out : NULL,
+ &self->pindexes[i]);
+ pfd->revents = 0;
+ }
+}
+
+/*
+ * call-back called when one of the descriptors
+ * became readable or writable
+ */
+static void sndio_poll_event(SndioVoice *self, int index, int event)
+{
+ int revents;
+
+ /*
+ * ensure we're not called twice this cycle
+ */
+ sndio_poll_clear(self);
+
+ /*
+ * make self->pfds[] look as we're returning from poll syscal,
+ * this is how sio_revents expects events to be.
+ */
+ self->pfds[index].revents = event;
+
+ /*
+ * tell sndio to handle events and return whether we can read or
+ * write without blocking.
+ */
+ revents = sio_revents(self->hdl, self->pfds);
+ if (self->mode == SIO_PLAY) {
+ if (revents & POLLOUT) {
+ sndio_write(self);
+ }
+
+ if (self->qemu_pos < self->buf_size) {
+ audio_run(self->hw.out.s, "sndio_out");
+ }
+ } else {
+ if (revents & POLLIN) {
+ sndio_read(self);
+ }
+
+ if (self->qemu_pos < self->sndio_pos) {
+ audio_run(self->hw.in.s, "sndio_in");
+ }
+ }
+
+ sndio_poll_wait(self);
+}
+
+/*
+ * return a buffer where data to play can be stored,
+ * its size is stored in the location pointed by the size argument.
+ */
+static void *sndio_get_buffer_out(HWVoiceOut *hw, size_t *size)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ /* size is not set by the caller */
+ *size = self->buf_size - self->qemu_pos;
+ return self->buf + self->qemu_pos;
+}
+
+/*
+ * return a buffer where data to play can be stored
+ */
+static size_t sndio_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ self->qemu_pos += size;
+ sndio_poll_wait(self);
+ return size;
+}
+
+/*
+ * return a buffer from where recorded data is available,
+ * its size is stored in the location pointed by the size argument.
+ * it may not exceed the initial value of "*size".
+ */
+static void *sndio_get_buffer_in(HWVoiceIn *hw, size_t *size)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+ size_t todo, max_todo;
+
+ /*
+ * unlike the get_buffer_out() method, get_buffer_in()
+ * must return a buffer of at most the given size, see audio.c
+ */
+ max_todo = *size;
+
+ todo = self->sndio_pos - self->qemu_pos;
+ if (todo > max_todo) {
+ todo = max_todo;
+ }
+
+ *size = todo;
+ return self->buf + self->qemu_pos;
+}
+
+/*
+ * discard the given amount of recorded data
+ */
+static void sndio_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ self->qemu_pos += size;
+ if (self->qemu_pos == self->buf_size) {
+ self->qemu_pos = 0;
+ self->sndio_pos = 0;
+ }
+ sndio_poll_wait(self);
+}
+
+/*
+ * call-back called when one of our descriptors becomes writable
+ */
+static void sndio_poll_out(void *arg)
+{
+ struct pollindex *pindex = (struct pollindex *) arg;
+
+ sndio_poll_event(pindex->self, pindex->index, POLLOUT);
+}
+
+/*
+ * call-back called when one of our descriptors becomes readable
+ */
+static void sndio_poll_in(void *arg)
+{
+ struct pollindex *pindex = (struct pollindex *) arg;
+
+ sndio_poll_event(pindex->self, pindex->index, POLLIN);
+}
+
+static void sndio_fini(SndioVoice *self)
+{
+ if (self->hdl) {
+ sio_close(self->hdl);
+ self->hdl = NULL;
+ }
+
+ g_free(self->pfds);
+ g_free(self->pindexes);
+ g_free(self->buf);
+}
+
+static int sndio_init(SndioVoice *self,
+ struct audsettings *as, int mode, Audiodev *dev)
+{
+ AudiodevSndioOptions *opts = &dev->u.sndio;
+ unsigned long long latency;
+ const char *dev_name;
+ struct sio_par req;
+ unsigned int nch;
+ int i, nfds;
+
+ dev_name = opts->has_dev ? opts->dev : SIO_DEVANY;
+ latency = opts->has_latency ? opts->latency : SNDIO_LATENCY_US;
+
+ /* open the device in non-blocking mode */
+ self->hdl = sio_open(dev_name, mode, 1);
+ if (self->hdl == NULL) {
+ dolog("failed to open device\n");
+ return -1;
+ }
+
+ self->mode = mode;
+
+ sio_initpar(&req);
+
+ switch (as->fmt) {
+ case AUDIO_FORMAT_S8:
+ req.bits = 8;
+ req.sig = 1;
+ break;
+ case AUDIO_FORMAT_U8:
+ req.bits = 8;
+ req.sig = 0;
+ break;
+ case AUDIO_FORMAT_S16:
+ req.bits = 16;
+ req.sig = 1;
+ break;
+ case AUDIO_FORMAT_U16:
+ req.bits = 16;
+ req.sig = 0;
+ break;
+ case AUDIO_FORMAT_S32:
+ req.bits = 32;
+ req.sig = 1;
+ break;
+ case AUDIO_FORMAT_U32:
+ req.bits = 32;
+ req.sig = 0;
+ default:
+ dolog("unknown audio sample format\n");
+ return -1;
+ }
+
+ if (req.bits > 8) {
+ req.le = as->endianness ? 0 : 1;
+ }
+
+ req.rate = as->freq;
+ if (mode == SIO_PLAY) {
+ req.pchan = as->nchannels;
+ } else {
+ req.rchan = as->nchannels;
+ }
+
+ /* set on-device buffer size */
+ req.appbufsz = req.rate * latency / 1000000;
+
+ if (!sio_setpar(self->hdl, &req)) {
+ dolog("failed set audio params\n");
+ goto fail;
+ }
+
+ if (!sio_getpar(self->hdl, &self->par)) {
+ dolog("failed get audio params\n");
+ goto fail;
+ }
+
+ nch = (mode == SIO_PLAY) ? self->par.pchan : self->par.rchan;
+
+ /*
+ * With the default setup, sndio supports any combination of parameters
+ * so these checks are mostly to catch configuration errors.
+ */
+ if (self->par.bits != req.bits || self->par.bps != req.bits / 8 ||
+ self->par.sig != req.sig || (req.bits > 8 && self->par.le != req.le) ||
+ self->par.rate != as->freq || nch != as->nchannels) {
+ dolog("unsupported audio params\n");
+ goto fail;
+ }
+
+ /*
+ * we use one block as buffer size; this is how
+ * transfers get well aligned
+ */
+ self->buf_size = self->par.round * self->par.bps * nch;
+
+ self->buf = g_malloc(self->buf_size);
+ if (self->buf == NULL) {
+ dolog("failed to allocate audio buffer\n");
+ goto fail;
+ }
+
+ nfds = sio_nfds(self->hdl);
+
+ self->pfds = g_malloc_n(nfds, sizeof(struct pollfd));
+ if (self->pfds == NULL) {
+ dolog("failed to allocate pollfd structures\n");
+ goto fail;
+ }
+
+ self->pindexes = g_malloc_n(nfds, sizeof(struct pollindex));
+ if (self->pindexes == NULL) {
+ dolog("failed to allocate pollindex structures\n");
+ goto fail;
+ }
+
+ for (i = 0; i < nfds; i++) {
+ self->pindexes[i].self = self;
+ self->pindexes[i].index = i;
+ }
+
+ return 0;
+fail:
+ sndio_fini(self);
+ return -1;
+}
+
+static void sndio_enable(SndioVoice *self, bool enable)
+{
+ if (enable) {
+ sio_start(self->hdl);
+ sndio_poll_wait(self);
+ } else {
+ sndio_poll_clear(self);
+ sio_stop(self->hdl);
+ }
+}
+
+static void sndio_enable_out(HWVoiceOut *hw, bool enable)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ return sndio_enable(self, enable);
+}
+
+static void sndio_enable_in(HWVoiceIn *hw, bool enable)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ return sndio_enable(self, enable);
+}
+
+static int sndio_init_out(HWVoiceOut *hw, struct audsettings *as, void *opaque)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ if (sndio_init(self, as, SIO_PLAY, opaque) == -1) {
+ return -1;
+ }
+
+ audio_pcm_init_info(&hw->info, as);
+ hw->samples = self->par.round;
+ return 0;
+}
+
+static int sndio_init_in(HWVoiceIn *hw, struct audsettings *as, void *opaque)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ if (sndio_init(self, as, SIO_REC, opaque) == -1) {
+ return -1;
+ }
+
+ audio_pcm_init_info(&hw->info, as);
+ hw->samples = self->par.round;
+ return 0;
+}
+
+static void sndio_fini_out(HWVoiceOut *hw)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ return sndio_fini(self);
+}
+
+static void sndio_fini_in(HWVoiceIn *hw)
+{
+ SndioVoice *self = (SndioVoice *) hw;
+
+ return sndio_fini(self);
+}
+
+static void *sndio_audio_init(Audiodev *dev)
+{
+ assert(dev->driver == AUDIODEV_DRIVER_SNDIO);
+ return dev;
+}
+
+static void sndio_audio_fini(void *opaque)
+{
+}
+
+static struct audio_pcm_ops sndio_pcm_ops = {
+ .init_out = sndio_init_out,
+ .fini_out = sndio_fini_out,
+ .enable_out = sndio_enable_out,
+ .get_buffer_out = sndio_get_buffer_out,
+ .put_buffer_out = sndio_put_buffer_out,
+ .init_in = sndio_init_in,
+ .fini_in = sndio_fini_in,
+ .enable_in = sndio_enable_in,
+ .get_buffer_in = sndio_get_buffer_in,
+ .put_buffer_in = sndio_put_buffer_in,
+};
+
+static struct audio_driver sndio_audio_driver = {
+ .name = "sndio",
+ .descr = "https://man.openbsd.org/sndio",
+ .init = sndio_audio_init,
+ .fini = sndio_audio_fini,
+ .pcm_ops = &sndio_pcm_ops,
+ .can_be_default = 1,
+ .max_voices_out = INT_MAX,
+ .max_voices_in = INT_MAX,
+ .voice_size_out = sizeof(SndioVoice),
+ .voice_size_in = sizeof(SndioVoice)
+};
+
+static void register_audio_sndio(void)
+{
+ audio_driver_register(&sndio_audio_driver);
+}
+
+type_init(register_audio_sndio);
diff --git a/meson.build b/meson.build
index 47df10afc2..551e8e3549 100644
--- a/meson.build
+++ b/meson.build
@@ -546,6 +546,11 @@ if not get_option('jack').auto() or have_system
jack = dependency('jack', required: get_option('jack'),
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'),
+ method: 'pkg-config', kwargs: static_kwargs)
+endif
spice_protocol = not_found
if not get_option('spice_protocol').auto() or have_system
@@ -1301,6 +1306,7 @@ if have_system
'oss': oss.found(),
'pa': pulse.found(),
'sdl': sdl.found(),
+ 'sndio': sndio.found(),
}
foreach k, v: audio_drivers_available
config_host_data.set('CONFIG_AUDIO_' + k.to_upper(), v)
@@ -3367,6 +3373,7 @@ if vnc.found()
endif
if targetos not in ['darwin', 'haiku', 'windows']
summary_info += {'OSS support': oss}
+ summary_info += {'sndio support': sndio}
elif targetos == 'darwin'
summary_info += {'CoreAudio support': coreaudio}
elif targetos == 'windows'
diff --git a/meson_options.txt b/meson_options.txt
index e740dce2a5..0f67c0a27b 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -13,7 +13,7 @@ option('sphinx_build', type : 'string', value : '',
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'],
+ choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'sdl', 'sndio'],
description: 'Set audio driver list')
option('fuzzing_engine', type : 'string', value : '',
description: 'fuzzing engine library for OSS-Fuzz')
@@ -184,6 +184,8 @@ option('oss', type: 'feature', value: 'auto',
description: 'OSS sound support')
option('pa', type: 'feature', value: 'auto',
description: 'PulseAudio sound support')
+option('sndio', type: 'feature', value: 'auto',
+ description: 'sndio sound support')
option('vhost_user_blk_server', type: 'feature', value: 'auto',
description: 'build vhost-user-blk server')
diff --git a/qapi/audio.json b/qapi/audio.json
index 9cba0df8a4..99c5c68ba6 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -105,6 +105,28 @@
'*out': 'AudiodevAlsaPerDirectionOptions',
'*threshold': 'uint32' } }
+##
+# @AudiodevSndioOptions:
+#
+# Options of the sndio audio backend.
+#
+# @in: options of the capture stream
+#
+# @out: options of the playback stream
+#
+# @dev: the name of the sndio device to use (default 'default')
+#
+# @latency: play buffer size (in microseconds)
+#
+# Since: 6.2
+##
+{ 'struct': 'AudiodevSndioOptions',
+ 'data': {
+ '*in': 'AudiodevPerDirectionOptions',
+ '*out': 'AudiodevPerDirectionOptions',
+ '*dev': 'str',
+ '*latency': 'uint32'} }
+
##
# @AudiodevCoreaudioPerDirectionOptions:
#
@@ -387,7 +409,7 @@
##
{ 'enum': 'AudiodevDriver',
'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
- 'sdl', 'spice', 'wav' ] }
+ 'sdl', 'sndio', 'spice', 'wav' ] }
##
# @Audiodev:
@@ -417,5 +439,6 @@
'oss': 'AudiodevOssOptions',
'pa': 'AudiodevPaOptions',
'sdl': 'AudiodevSdlOptions',
+ 'sndio': 'AudiodevSndioOptions',
'spice': 'AudiodevGenericOptions',
'wav': 'AudiodevWavOptions' } }
diff --git a/qemu-options.hx b/qemu-options.hx
index f051536b63..4a027b1abc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -657,6 +657,9 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
"-audiodev sdl,id=id[,prop[=value][,...]]\n"
" in|out.buffer-count= number of buffers\n"
#endif
+#ifdef CONFIG_AUDIO_SNDIO
+ "-audiodev sndio,id=id[,prop[=value][,...]]\n"
+#endif
#ifdef CONFIG_SPICE
"-audiodev spice,id=id[,prop[=value][,...]]\n"
#endif
@@ -820,6 +823,11 @@ SRST
``in|out.buffer-count=count``
Sets the count of the buffers.
+``-audiodev sndio,id=id[,prop[=value][,...]]``
+ Creates a backend using SNDIO. This backend is available on
+ OpenBSD and most other Unix-like systems. This backend has no
+ backend specific properties.
+
``-audiodev spice,id=id[,prop[=value][,...]]``
Creates a backend that sends audio through SPICE. This backend
requires ``-spice`` and automatically selected in that case, so
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 6e20e84322..a387f5c9df 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -63,6 +63,9 @@ class FreeBSDVM(basevm.BaseVM):
# libs: migration
"zstd",
+
+ # libs: sndio
+ "sndio",
]
# TODO: Enable gnutls again once FreeBSD's libtasn1 got fixed
--
2.33.1
On Sonntag, 7. November 2021 06:19:26 CET Brad Smith wrote:
> audio: Add sndio backend
>
> Add a sndio backend.
Hi Brad!
> sndio is the native API used by OpenBSD, although it has been ported to
> other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.).
>
> The C code is from Alexandre Ratchov <alex@caoua.org> and the rest of
> the bits are from me.
A Signed-off-by: line is mandatory for all QEMU patches:
https://wiki.qemu.org/Contribute/SubmitAPatch
Also, it should be clear from the patches who did what exactly, either by
splitting the patches up and assigning the respective authors accordingly, or
by making the person with the most relevant work the patch author and
describing in the commit log additional authors and what they have added/
changed, along with their Signed-off-by: line:
Signed-off-by: Alexandre Ratchov <alex@caoua.org>
[Brad Smith: - Added foo
- Some other change]
Signed-off-by: Brad Smith <brad@comstyle.com>
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/
Documentation/SubmittingPatches?
id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297
Please CC those involved authors.
> ---
> audio/audio.c | 1 +
> audio/audio_template.h | 2 +
> audio/meson.build | 1 +
> audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++
> meson.build | 7 +
> meson_options.txt | 4 +-
> qapi/audio.json | 25 +-
> qemu-options.hx | 8 +
> tests/vm/freebsd | 3 +
> 9 files changed, 604 insertions(+), 2 deletions(-)
An additional subsection for this backend should be added to MAINTAINERS.
> create mode 100644 audio/sndioaudio.c
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 54a153c0ef..bad1ceb69e 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -2005,6 +2005,7 @@ void audio_create_pdos(Audiodev *dev)
> CASE(OSS, oss, Oss);
> CASE(PA, pa, Pa);
> CASE(SDL, sdl, Sdl);
> + CASE(SNDIO, sndio, );
> CASE(SPICE, spice, );
> CASE(WAV, wav, );
>
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index c6714946aa..ecc5a0bc6d 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -337,6 +337,8 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
> TYPE)(Audiodev *dev) return
> qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE); case
> AUDIODEV_DRIVER_SDL:
> return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
> + case AUDIODEV_DRIVER_SNDIO:
> + return dev->u.sndio.TYPE;
> case AUDIODEV_DRIVER_SPICE:
> return dev->u.spice.TYPE;
> case AUDIODEV_DRIVER_WAV:
> diff --git a/audio/meson.build b/audio/meson.build
> index 462533bb8c..e24c86e7e6 100644
> --- a/audio/meson.build
> +++ b/audio/meson.build
> @@ -17,6 +17,7 @@ foreach m : [
> ['pa', pulse, files('paaudio.c')],
> ['sdl', sdl, files('sdlaudio.c')],
> ['jack', jack, files('jackaudio.c')],
> + ['sndio', sndio, files('sndioaudio.c')],
> ['spice', spice, files('spiceaudio.c')]
> ]
> if m[1].found()
> diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
> new file mode 100644
> index 0000000000..204af07781
> --- /dev/null
> +++ b/audio/sndioaudio.c
> @@ -0,0 +1,555 @@
> +/*
> + * Copyright (c) 2019 Alexandre Ratchov <alex@caoua.org>
> + *
It is quite common for new source files in QEMU to have an authors list
section in the header here like:
* Autors:
* Alexandre Ratchov <alex@caoua.org>
That way scripts/get_maintainer.pl can suggest those people as well in case
they are not explicitly listed in MAINTAINERS. Does not seem to be mandatory
for QEMU though. Just saying.
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + *
> MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + *
> ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + *
> WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + *
> ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + *
> OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */
> +
> +/*
> + * TODO :
> + *
> + * Use a single device and open it in full-duplex rather than
> + * opening it twice (once for playback once for recording).
> + *
> + * This is the only way to ensure that playback doesn't drift with respect
> + * to recording, which is what guest systems expect.
> + */
> +
> +#include <poll.h>
> +#include <sndio.h>
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/main-loop.h"
> +#include "audio.h"
> +#include "trace.h"
> +
> +#define AUDIO_CAP "sndio"
> +#include "audio_int.h"
> +
> +/* default latency in ms if no option is set */
> +#define SNDIO_LATENCY_US 50000
> +
> +typedef struct SndioVoice {
> + union {
> + HWVoiceOut out;
> + HWVoiceIn in;
> + } hw;
> + struct sio_par par;
> + struct sio_hdl *hdl;
> + struct pollfd *pfds;
> + struct pollindex {
> + struct SndioVoice *self;
> + int index;
> + } *pindexes;
> + unsigned char *buf;
> + size_t buf_size;
> + size_t sndio_pos;
> + size_t qemu_pos;
> + unsigned int mode;
> + unsigned int nfds;
> +} SndioVoice;
> +
> +typedef struct SndioConf {
> + const char *devname;
> + unsigned int latency;
> +} SndioConf;
> +
> +/* needed for forward reference */
> +static void sndio_poll_in(void *arg);
> +static void sndio_poll_out(void *arg);
> +
> +/*
> + * stop polling descriptors
> + */
> +static void sndio_poll_clear(SndioVoice *self)
> +{
> + struct pollfd *pfd;
> + int i;
> +
> + for (i = 0; i < self->nfds; i++) {
> + pfd = &self->pfds[i];
> + qemu_set_fd_handler (pfd->fd, NULL, NULL, NULL);
> + }
> +
> + self->nfds = 0;
> +}
> +
> +/*
> + * write data to the device until it blocks or
> + * all of our buffered data is written
> + */
> +static void sndio_write(SndioVoice *self)
> +{
> + size_t todo, n;
> +
> + todo = self->qemu_pos - self->sndio_pos;
> +
> + /*
> + * transfer data to device, until it blocks
> + */
> + while (todo > 0) {
> + n = sio_write(self->hdl, self->buf + self->sndio_pos, todo);
> + if (n == 0) {
> + break;
> + }
> + self->sndio_pos += n;
> + todo -= n;
> + }
> +
> + if (self->sndio_pos == self->buf_size) {
> + /*
> + * we complete the block
> + */
> + self->sndio_pos = 0;
> + self->qemu_pos = 0;
> + }
> +}
> +
> +/*
> + * read data from the device until it blocks or
> + * there no room any longer
> + */
> +static void sndio_read(SndioVoice *self)
> +{
> + size_t todo, n;
> +
> + todo = self->buf_size - self->sndio_pos;
> +
> + /*
> + * transfer data from the device, until it blocks
> + */
> + while (todo > 0) {
> + n = sio_read(self->hdl, self->buf + self->sndio_pos, todo);
> + if (n == 0) {
> + break;
> + }
> + self->sndio_pos += n;
> + todo -= n;
> + }
> +}
> +
> +/*
> + * Set handlers for all descriptors libsndio needs to
> + * poll
> + */
> +static void sndio_poll_wait(SndioVoice *self)
> +{
> + struct pollfd *pfd;
> + int events, i;
> +
> + events = 0;
> + if (self->mode == SIO_PLAY) {
> + if (self->sndio_pos < self->qemu_pos) {
> + events |= POLLOUT;
> + }
> + } else {
> + if (self->sndio_pos < self->buf_size) {
> + events |= POLLIN;
> + }
> + }
> +
> + /*
> + * fill the given array of descriptors with the events sndio
> + * wants, they are different from our 'event' variable because
> + * sndio may use descriptors internally.
> + */
> + self->nfds = sio_pollfd(self->hdl, self->pfds, events);
> +
> + for (i = 0; i < self->nfds; i++) {
> + pfd = &self->pfds[i];
> + if (pfd->fd < 0) {
> + continue;
> + }
> + qemu_set_fd_handler(pfd->fd,
> + (pfd->events & POLLIN) ? sndio_poll_in : NULL,
> + (pfd->events & POLLOUT) ? sndio_poll_out : NULL,
> + &self->pindexes[i]);
> + pfd->revents = 0;
> + }
> +}
> +
> +/*
> + * call-back called when one of the descriptors
> + * became readable or writable
> + */
> +static void sndio_poll_event(SndioVoice *self, int index, int event)
> +{
> + int revents;
> +
> + /*
> + * ensure we're not called twice this cycle
> + */
> + sndio_poll_clear(self);
> +
> + /*
> + * make self->pfds[] look as we're returning from poll syscal,
> + * this is how sio_revents expects events to be.
> + */
> + self->pfds[index].revents = event;
> +
> + /*
> + * tell sndio to handle events and return whether we can read or
> + * write without blocking.
> + */
> + revents = sio_revents(self->hdl, self->pfds);
> + if (self->mode == SIO_PLAY) {
> + if (revents & POLLOUT) {
> + sndio_write(self);
> + }
> +
> + if (self->qemu_pos < self->buf_size) {
> + audio_run(self->hw.out.s, "sndio_out");
> + }
> + } else {
> + if (revents & POLLIN) {
> + sndio_read(self);
> + }
> +
> + if (self->qemu_pos < self->sndio_pos) {
> + audio_run(self->hw.in.s, "sndio_in");
> + }
> + }
> +
> + sndio_poll_wait(self);
> +}
> +
> +/*
> + * return a buffer where data to play can be stored,
> + * its size is stored in the location pointed by the size argument.
> + */
> +static void *sndio_get_buffer_out(HWVoiceOut *hw, size_t *size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + /* size is not set by the caller */
> + *size = self->buf_size - self->qemu_pos;
> + return self->buf + self->qemu_pos;
> +}
> +
> +/*
> + * return a buffer where data to play can be stored
> + */
> +static size_t sndio_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + self->qemu_pos += size;
> + sndio_poll_wait(self);
> + return size;
> +}
> +
> +/*
> + * return a buffer from where recorded data is available,
> + * its size is stored in the location pointed by the size argument.
> + * it may not exceed the initial value of "*size".
> + */
> +static void *sndio_get_buffer_in(HWVoiceIn *hw, size_t *size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> + size_t todo, max_todo;
> +
> + /*
> + * unlike the get_buffer_out() method, get_buffer_in()
> + * must return a buffer of at most the given size, see audio.c
> + */
> + max_todo = *size;
> +
> + todo = self->sndio_pos - self->qemu_pos;
> + if (todo > max_todo) {
> + todo = max_todo;
> + }
> +
> + *size = todo;
> + return self->buf + self->qemu_pos;
> +}
> +
> +/*
> + * discard the given amount of recorded data
> + */
> +static void sndio_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + self->qemu_pos += size;
> + if (self->qemu_pos == self->buf_size) {
> + self->qemu_pos = 0;
> + self->sndio_pos = 0;
> + }
> + sndio_poll_wait(self);
> +}
> +
> +/*
> + * call-back called when one of our descriptors becomes writable
> + */
> +static void sndio_poll_out(void *arg)
> +{
> + struct pollindex *pindex = (struct pollindex *) arg;
> +
> + sndio_poll_event(pindex->self, pindex->index, POLLOUT);
> +}
> +
> +/*
> + * call-back called when one of our descriptors becomes readable
> + */
> +static void sndio_poll_in(void *arg)
> +{
> + struct pollindex *pindex = (struct pollindex *) arg;
> +
> + sndio_poll_event(pindex->self, pindex->index, POLLIN);
> +}
> +
> +static void sndio_fini(SndioVoice *self)
> +{
> + if (self->hdl) {
> + sio_close(self->hdl);
> + self->hdl = NULL;
> + }
> +
> + g_free(self->pfds);
> + g_free(self->pindexes);
> + g_free(self->buf);
> +}
> +
> +static int sndio_init(SndioVoice *self,
> + struct audsettings *as, int mode, Audiodev *dev)
> +{
> + AudiodevSndioOptions *opts = &dev->u.sndio;
> + unsigned long long latency;
> + const char *dev_name;
> + struct sio_par req;
> + unsigned int nch;
> + int i, nfds;
> +
> + dev_name = opts->has_dev ? opts->dev : SIO_DEVANY;
> + latency = opts->has_latency ? opts->latency : SNDIO_LATENCY_US;
> +
> + /* open the device in non-blocking mode */
> + self->hdl = sio_open(dev_name, mode, 1);
> + if (self->hdl == NULL) {
> + dolog("failed to open device\n");
> + return -1;
> + }
> +
> + self->mode = mode;
> +
> + sio_initpar(&req);
> +
> + switch (as->fmt) {
> + case AUDIO_FORMAT_S8:
> + req.bits = 8;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U8:
> + req.bits = 8;
> + req.sig = 0;
> + break;
> + case AUDIO_FORMAT_S16:
> + req.bits = 16;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U16:
> + req.bits = 16;
> + req.sig = 0;
> + break;
> + case AUDIO_FORMAT_S32:
> + req.bits = 32;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U32:
> + req.bits = 32;
> + req.sig = 0;
> + default:
> + dolog("unknown audio sample format\n");
> + return -1;
> + }
> +
> + if (req.bits > 8) {
> + req.le = as->endianness ? 0 : 1;
> + }
> +
> + req.rate = as->freq;
> + if (mode == SIO_PLAY) {
> + req.pchan = as->nchannels;
> + } else {
> + req.rchan = as->nchannels;
> + }
> +
> + /* set on-device buffer size */
> + req.appbufsz = req.rate * latency / 1000000;
> +
> + if (!sio_setpar(self->hdl, &req)) {
> + dolog("failed set audio params\n");
> + goto fail;
> + }
> +
> + if (!sio_getpar(self->hdl, &self->par)) {
> + dolog("failed get audio params\n");
> + goto fail;
> + }
> +
> + nch = (mode == SIO_PLAY) ? self->par.pchan : self->par.rchan;
> +
> + /*
> + * With the default setup, sndio supports any combination of parameters
> + * so these checks are mostly to catch configuration errors.
> + */
> + if (self->par.bits != req.bits || self->par.bps != req.bits / 8 ||
> + self->par.sig != req.sig || (req.bits > 8 && self->par.le !=
> req.le) || + self->par.rate != as->freq || nch != as->nchannels) {
> + dolog("unsupported audio params\n");
> + goto fail;
> + }
> +
> + /*
> + * we use one block as buffer size; this is how
> + * transfers get well aligned
> + */
> + self->buf_size = self->par.round * self->par.bps * nch;
> +
> + self->buf = g_malloc(self->buf_size);
> + if (self->buf == NULL) {
> + dolog("failed to allocate audio buffer\n");
> + goto fail;
> + }
> +
> + nfds = sio_nfds(self->hdl);
> +
> + self->pfds = g_malloc_n(nfds, sizeof(struct pollfd));
> + if (self->pfds == NULL) {
> + dolog("failed to allocate pollfd structures\n");
> + goto fail;
> + }
> +
> + self->pindexes = g_malloc_n(nfds, sizeof(struct pollindex));
> + if (self->pindexes == NULL) {
> + dolog("failed to allocate pollindex structures\n");
> + goto fail;
> + }
> +
> + for (i = 0; i < nfds; i++) {
> + self->pindexes[i].self = self;
> + self->pindexes[i].index = i;
> + }
> +
> + return 0;
> +fail:
> + sndio_fini(self);
> + return -1;
> +}
> +
> +static void sndio_enable(SndioVoice *self, bool enable)
> +{
> + if (enable) {
> + sio_start(self->hdl);
> + sndio_poll_wait(self);
> + } else {
> + sndio_poll_clear(self);
> + sio_stop(self->hdl);
> + }
> +}
> +
> +static void sndio_enable_out(HWVoiceOut *hw, bool enable)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_enable(self, enable);
> +}
> +
> +static void sndio_enable_in(HWVoiceIn *hw, bool enable)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_enable(self, enable);
> +}
> +
> +static int sndio_init_out(HWVoiceOut *hw, struct audsettings *as, void
> *opaque) +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + if (sndio_init(self, as, SIO_PLAY, opaque) == -1) {
> + return -1;
> + }
> +
> + audio_pcm_init_info(&hw->info, as);
> + hw->samples = self->par.round;
> + return 0;
> +}
> +
> +static int sndio_init_in(HWVoiceIn *hw, struct audsettings *as, void
> *opaque) +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + if (sndio_init(self, as, SIO_REC, opaque) == -1) {
> + return -1;
> + }
> +
> + audio_pcm_init_info(&hw->info, as);
> + hw->samples = self->par.round;
> + return 0;
> +}
> +
> +static void sndio_fini_out(HWVoiceOut *hw)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_fini(self);
> +}
> +
> +static void sndio_fini_in(HWVoiceIn *hw)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_fini(self);
> +}
> +
> +static void *sndio_audio_init(Audiodev *dev)
> +{
> + assert(dev->driver == AUDIODEV_DRIVER_SNDIO);
> + return dev;
> +}
> +
> +static void sndio_audio_fini(void *opaque)
> +{
> +}
> +
> +static struct audio_pcm_ops sndio_pcm_ops = {
> + .init_out = sndio_init_out,
> + .fini_out = sndio_fini_out,
> + .enable_out = sndio_enable_out,
> + .get_buffer_out = sndio_get_buffer_out,
> + .put_buffer_out = sndio_put_buffer_out,
> + .init_in = sndio_init_in,
> + .fini_in = sndio_fini_in,
> + .enable_in = sndio_enable_in,
> + .get_buffer_in = sndio_get_buffer_in,
> + .put_buffer_in = sndio_put_buffer_in,
> +};
> +
> +static struct audio_driver sndio_audio_driver = {
> + .name = "sndio",
> + .descr = "https://man.openbsd.org/sndio",
> + .init = sndio_audio_init,
> + .fini = sndio_audio_fini,
> + .pcm_ops = &sndio_pcm_ops,
> + .can_be_default = 1,
> + .max_voices_out = INT_MAX,
> + .max_voices_in = INT_MAX,
> + .voice_size_out = sizeof(SndioVoice),
> + .voice_size_in = sizeof(SndioVoice)
> +};
> +
> +static void register_audio_sndio(void)
> +{
> + audio_driver_register(&sndio_audio_driver);
> +}
> +
> +type_init(register_audio_sndio);
> diff --git a/meson.build b/meson.build
> index 47df10afc2..551e8e3549 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -546,6 +546,11 @@ if not get_option('jack').auto() or have_system
> jack = dependency('jack', required: get_option('jack'),
> 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'),
> + method: 'pkg-config', kwargs: static_kwargs)
> +endif
>
> spice_protocol = not_found
> if not get_option('spice_protocol').auto() or have_system
> @@ -1301,6 +1306,7 @@ if have_system
> 'oss': oss.found(),
> 'pa': pulse.found(),
> 'sdl': sdl.found(),
> + 'sndio': sndio.found(),
> }
> foreach k, v: audio_drivers_available
> config_host_data.set('CONFIG_AUDIO_' + k.to_upper(), v)
> @@ -3367,6 +3373,7 @@ if vnc.found()
> endif
> if targetos not in ['darwin', 'haiku', 'windows']
> summary_info += {'OSS support': oss}
> + summary_info += {'sndio support': sndio}
> elif targetos == 'darwin'
> summary_info += {'CoreAudio support': coreaudio}
> elif targetos == 'windows'
> diff --git a/meson_options.txt b/meson_options.txt
> index e740dce2a5..0f67c0a27b 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -13,7 +13,7 @@ option('sphinx_build', type : 'string', value : '',
> 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'], + choices: ['alsa', 'coreaudio', 'default', 'dsound',
> 'jack', 'oss', 'pa', 'sdl', 'sndio'], description: 'Set audio driver list')
> option('fuzzing_engine', type : 'string', value : '',
> description: 'fuzzing engine library for OSS-Fuzz')
> @@ -184,6 +184,8 @@ option('oss', type: 'feature', value: 'auto',
> description: 'OSS sound support')
> option('pa', type: 'feature', value: 'auto',
> description: 'PulseAudio sound support')
> +option('sndio', type: 'feature', value: 'auto',
> + description: 'sndio sound support')
>
> option('vhost_user_blk_server', type: 'feature', value: 'auto',
> description: 'build vhost-user-blk server')
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 9cba0df8a4..99c5c68ba6 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -105,6 +105,28 @@
> '*out': 'AudiodevAlsaPerDirectionOptions',
> '*threshold': 'uint32' } }
>
> +##
> +# @AudiodevSndioOptions:
> +#
> +# Options of the sndio audio backend.
> +#
> +# @in: options of the capture stream
> +#
> +# @out: options of the playback stream
> +#
> +# @dev: the name of the sndio device to use (default 'default')
> +#
> +# @latency: play buffer size (in microseconds)
> +#
> +# Since: 6.2
> +##
> +{ 'struct': 'AudiodevSndioOptions',
> + 'data': {
> + '*in': 'AudiodevPerDirectionOptions',
> + '*out': 'AudiodevPerDirectionOptions',
> + '*dev': 'str',
> + '*latency': 'uint32'} }
> +
> ##
> # @AudiodevCoreaudioPerDirectionOptions:
> #
> @@ -387,7 +409,7 @@
> ##
> { 'enum': 'AudiodevDriver',
> 'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
> - 'sdl', 'spice', 'wav' ] }
> + 'sdl', 'sndio', 'spice', 'wav' ] }
>
> ##
> # @Audiodev:
> @@ -417,5 +439,6 @@
> 'oss': 'AudiodevOssOptions',
> 'pa': 'AudiodevPaOptions',
> 'sdl': 'AudiodevSdlOptions',
> + 'sndio': 'AudiodevSndioOptions',
> 'spice': 'AudiodevGenericOptions',
> 'wav': 'AudiodevWavOptions' } }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f051536b63..4a027b1abc 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -657,6 +657,9 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> "-audiodev sdl,id=id[,prop[=value][,...]]\n"
> " in|out.buffer-count= number of buffers\n"
> #endif
> +#ifdef CONFIG_AUDIO_SNDIO
> + "-audiodev sndio,id=id[,prop[=value][,...]]\n"
> +#endif
> #ifdef CONFIG_SPICE
> "-audiodev spice,id=id[,prop[=value][,...]]\n"
> #endif
> @@ -820,6 +823,11 @@ SRST
> ``in|out.buffer-count=count``
> Sets the count of the buffers.
>
> +``-audiodev sndio,id=id[,prop[=value][,...]]``
> + Creates a backend using SNDIO. This backend is available on
> + OpenBSD and most other Unix-like systems. This backend has no
> + backend specific properties.
> +
> ``-audiodev spice,id=id[,prop[=value][,...]]``
> Creates a backend that sends audio through SPICE. This backend
> requires ``-spice`` and automatically selected in that case, so
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> index 6e20e84322..a387f5c9df 100755
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -63,6 +63,9 @@ class FreeBSDVM(basevm.BaseVM):
>
> # libs: migration
> "zstd",
> +
> + # libs: sndio
> + "sndio",
> ]
>
> # TODO: Enable gnutls again once FreeBSD's libtasn1 got fixed
On 11/8/2021 8:03 AM, Christian Schoenebeck wrote:
> On Sonntag, 7. November 2021 06:19:26 CET Brad Smith wrote:
>> audio: Add sndio backend
>>
>> Add a sndio backend.
> Hi Brad!
>
>> sndio is the native API used by OpenBSD, although it has been ported to
>> other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.).
>>
>> The C code is from Alexandre Ratchov <alex@caoua.org> and the rest of
>> the bits are from me.
> A Signed-off-by: line is mandatory for all QEMU patches:
> https://wiki.qemu.org/Contribute/SubmitAPatch
Ah, I was not aware of that. I usually include it but it was an
oversight this time.
> Also, it should be clear from the patches who did what exactly, either by
> splitting the patches up and assigning the respective authors accordingly, or
> by making the person with the most relevant work the patch author and
> describing in the commit log additional authors and what they have added/
> changed, along with their Signed-off-by: line:
>
> Signed-off-by: Alexandre Ratchov <alex@caoua.org>
> [Brad Smith: - Added foo
> - Some other change]
> Signed-off-by: Brad Smith <brad@comstyle.com>
I think I'll go with this.
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/
> Documentation/SubmittingPatches?
> id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297
>
> Please CC those involved authors.
Will do.
>> ---
>> audio/audio.c | 1 +
>> audio/audio_template.h | 2 +
>> audio/meson.build | 1 +
>> audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++
>> meson.build | 7 +
>> meson_options.txt | 4 +-
>> qapi/audio.json | 25 +-
>> qemu-options.hx | 8 +
>> tests/vm/freebsd | 3 +
>> 9 files changed, 604 insertions(+), 2 deletions(-)
> An additional subsection for this backend should be added to MAINTAINERS.
I did not add anything here as I figured it implies a certain level of
obligation. His time
available varies quite a bit (especially at the current time) and I
wasn't sure if it's
appropriate listing him.
>> create mode 100644 audio/sndioaudio.c
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 54a153c0ef..bad1ceb69e 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -2005,6 +2005,7 @@ void audio_create_pdos(Audiodev *dev)
>> CASE(OSS, oss, Oss);
>> CASE(PA, pa, Pa);
>> CASE(SDL, sdl, Sdl);
>> + CASE(SNDIO, sndio, );
>> CASE(SPICE, spice, );
>> CASE(WAV, wav, );
>>
>> diff --git a/audio/audio_template.h b/audio/audio_template.h
>> index c6714946aa..ecc5a0bc6d 100644
>> --- a/audio/audio_template.h
>> +++ b/audio/audio_template.h
>> @@ -337,6 +337,8 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
>> TYPE)(Audiodev *dev) return
>> qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE); case
>> AUDIODEV_DRIVER_SDL:
>> return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
>> + case AUDIODEV_DRIVER_SNDIO:
>> + return dev->u.sndio.TYPE;
>> case AUDIODEV_DRIVER_SPICE:
>> return dev->u.spice.TYPE;
>> case AUDIODEV_DRIVER_WAV:
>> diff --git a/audio/meson.build b/audio/meson.build
>> index 462533bb8c..e24c86e7e6 100644
>> --- a/audio/meson.build
>> +++ b/audio/meson.build
>> @@ -17,6 +17,7 @@ foreach m : [
>> ['pa', pulse, files('paaudio.c')],
>> ['sdl', sdl, files('sdlaudio.c')],
>> ['jack', jack, files('jackaudio.c')],
>> + ['sndio', sndio, files('sndioaudio.c')],
>> ['spice', spice, files('spiceaudio.c')]
>> ]
>> if m[1].found()
>> diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
>> new file mode 100644
>> index 0000000000..204af07781
>> --- /dev/null
>> +++ b/audio/sndioaudio.c
>> @@ -0,0 +1,555 @@
>> +/*
>> + * Copyright (c) 2019 Alexandre Ratchov <alex@caoua.org>
>> + *
> It is quite common for new source files in QEMU to have an authors list
> section in the header here like:
>
> * Autors:
> * Alexandre Ratchov <alex@caoua.org>
I was looking through the tree and all of the examples I came across
were using this
with a Copyright for a company as opposed to an individual. What would
be the
format?
> That way scripts/get_maintainer.pl can suggest those people as well in case
> they are not explicitly listed in MAINTAINERS. Does not seem to be mandatory
> for QEMU though. Just saying.
On Samstag, 13. November 2021 21:40:39 CET Brad Smith wrote:
> On 11/8/2021 8:03 AM, Christian Schoenebeck wrote:
> > On Sonntag, 7. November 2021 06:19:26 CET Brad Smith wrote:
> >> audio: Add sndio backend
> >>
> >> Add a sndio backend.
> >
> > Hi Brad!
> >
> >> sndio is the native API used by OpenBSD, although it has been ported to
> >> other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.).
> >>
> >> The C code is from Alexandre Ratchov <alex@caoua.org> and the rest of
> >> the bits are from me.
> >
> > A Signed-off-by: line is mandatory for all QEMU patches:
> > https://wiki.qemu.org/Contribute/SubmitAPatch
>
> Ah, I was not aware of that. I usually include it but it was an
> oversight this time.
>
> > Also, it should be clear from the patches who did what exactly, either by
> > splitting the patches up and assigning the respective authors accordingly,
> > or by making the person with the most relevant work the patch author and
> > describing in the commit log additional authors and what they have added/
> > changed, along with their Signed-off-by: line:
> >
> > Signed-off-by: Alexandre Ratchov <alex@caoua.org>
> > [Brad Smith: - Added foo
> >
> > - Some other change]
> >
> > Signed-off-by: Brad Smith <brad@comstyle.com>
>
> I think I'll go with this.
>
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/
> > Documentation/SubmittingPatches?
> > id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297
> >
> > Please CC those involved authors.
>
> Will do.
I added Alexandre Ratchov on CC as he seems to be the primary author of this
patch series.
> >> ---
> >>
> >> audio/audio.c | 1 +
> >> audio/audio_template.h | 2 +
> >> audio/meson.build | 1 +
> >> audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++
> >> meson.build | 7 +
> >> meson_options.txt | 4 +-
> >> qapi/audio.json | 25 +-
> >> qemu-options.hx | 8 +
> >> tests/vm/freebsd | 3 +
> >> 9 files changed, 604 insertions(+), 2 deletions(-)
> >
> > An additional subsection for this backend should be added to MAINTAINERS.
>
> I did not add anything here as I figured it implies a certain level of
> obligation. His time
> available varies quite a bit (especially at the current time) and I
> wasn't sure if it's
> appropriate listing him.
Yes, that's an unpleasant but legitimate question: will there be anybody
caring for this sndio backend in QEMU or would it go orphaned right from the
start?
It would be good to have at least somebody familiar with this code to
volunteer as reviewer(s) ("R:" line(s) in MAINTAINERS file). Reviewers are
automatically CCed, so that they can (optionally) give their feedback on
future changes to the sndio backend, i.e. when somebody sends sndio patches to
qemu-devel. This is voluntary and can be revoked at any time, and I do not
expect that you would frequently get emailed for this either.
As this is a BSD-specific audio backend, it is not likely that an active QEMU
developer would be able to care for it.
> >> create mode 100644 audio/sndioaudio.c
> >>
> >> diff --git a/audio/audio.c b/audio/audio.c
> >> index 54a153c0ef..bad1ceb69e 100644
> >> --- a/audio/audio.c
> >> +++ b/audio/audio.c
> >> @@ -2005,6 +2005,7 @@ void audio_create_pdos(Audiodev *dev)
> >>
> >> CASE(OSS, oss, Oss);
> >> CASE(PA, pa, Pa);
> >> CASE(SDL, sdl, Sdl);
> >>
> >> + CASE(SNDIO, sndio, );
> >>
> >> CASE(SPICE, spice, );
> >> CASE(WAV, wav, );
> >>
> >> diff --git a/audio/audio_template.h b/audio/audio_template.h
> >> index c6714946aa..ecc5a0bc6d 100644
> >> --- a/audio/audio_template.h
> >> +++ b/audio/audio_template.h
> >> @@ -337,6 +337,8 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
> >> TYPE)(Audiodev *dev) return
> >> qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE); case
> >>
> >> AUDIODEV_DRIVER_SDL:
> >> return
> >> qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
> >>
> >> + case AUDIODEV_DRIVER_SNDIO:
> >> + return dev->u.sndio.TYPE;
> >>
> >> case AUDIODEV_DRIVER_SPICE:
> >> return dev->u.spice.TYPE;
> >>
> >> case AUDIODEV_DRIVER_WAV:
> >> diff --git a/audio/meson.build b/audio/meson.build
> >> index 462533bb8c..e24c86e7e6 100644
> >> --- a/audio/meson.build
> >> +++ b/audio/meson.build
> >> @@ -17,6 +17,7 @@ foreach m : [
> >>
> >> ['pa', pulse, files('paaudio.c')],
> >> ['sdl', sdl, files('sdlaudio.c')],
> >> ['jack', jack, files('jackaudio.c')],
> >>
> >> + ['sndio', sndio, files('sndioaudio.c')],
> >>
> >> ['spice', spice, files('spiceaudio.c')]
> >>
> >> ]
> >>
> >> if m[1].found()
> >>
> >> diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
> >> new file mode 100644
> >> index 0000000000..204af07781
> >> --- /dev/null
> >> +++ b/audio/sndioaudio.c
> >> @@ -0,0 +1,555 @@
> >> +/*
> >> + * Copyright (c) 2019 Alexandre Ratchov <alex@caoua.org>
> >> + *
> >
> > It is quite common for new source files in QEMU to have an authors list
> >
> > section in the header here like:
> > * Autors:
> > * Alexandre Ratchov <alex@caoua.org>
>
> I was looking through the tree and all of the examples I came across
> were using this
> with a Copyright for a company as opposed to an individual. What would
> be the
> format?
There was nothing wrong with the copyright line. If it was an individual, then
the copyright line is an individual. And like I said, it does not seem to be
required in QEMU to have an "Authors:" block in the file header at all. So on
doubt just ignore this.
Deflating the file header by using SPDX license identifier as suggested would
make sense though.
> > That way scripts/get_maintainer.pl can suggest those people as well in
> > case
> > they are not explicitly listed in MAINTAINERS. Does not seem to be
> > mandatory for QEMU though. Just saying.
Best regards,
Christian Schoenebeck
On 11/14/2021 8:18 AM, Christian Schoenebeck wrote:
> On Samstag, 13. November 2021 21:40:39 CET Brad Smith wrote:
>> On 11/8/2021 8:03 AM, Christian Schoenebeck wrote:
>>> On Sonntag, 7. November 2021 06:19:26 CET Brad Smith wrote:
>>>> audio: Add sndio backend
>>>>
>>>> Add a sndio backend.
>>> Hi Brad!
>>>
>>>> sndio is the native API used by OpenBSD, although it has been ported to
>>>> other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.).
>>>>
>>>> The C code is from Alexandre Ratchov<alex@caoua.org> and the rest of
>>>> the bits are from me.
>>> A Signed-off-by: line is mandatory for all QEMU patches:
>>> https://wiki.qemu.org/Contribute/SubmitAPatch
>> Ah, I was not aware of that. I usually include it but it was an
>> oversight this time.
>>
>>> Also, it should be clear from the patches who did what exactly, either by
>>> splitting the patches up and assigning the respective authors accordingly,
>>> or by making the person with the most relevant work the patch author and
>>> describing in the commit log additional authors and what they have added/
>>> changed, along with their Signed-off-by: line:
>>>
>>> Signed-off-by: Alexandre Ratchov<alex@caoua.org>
>>> [Brad Smith: - Added foo
>>>
>>> - Some other change]
>>>
>>> Signed-off-by: Brad Smith<brad@comstyle.com>
>> I think I'll go with this.
>>
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/
>>> Documentation/SubmittingPatches?
>>> id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297
>>>
>>> Please CC those involved authors.
>> Will do.
> I added Alexandre Ratchov on CC as he seems to be the primary author of this
> patch series.
>
>>>> ---
>>>>
>>>> audio/audio.c | 1 +
>>>> audio/audio_template.h | 2 +
>>>> audio/meson.build | 1 +
>>>> audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++
>>>> meson.build | 7 +
>>>> meson_options.txt | 4 +-
>>>> qapi/audio.json | 25 +-
>>>> qemu-options.hx | 8 +
>>>> tests/vm/freebsd | 3 +
>>>> 9 files changed, 604 insertions(+), 2 deletions(-)
>>> An additional subsection for this backend should be added to MAINTAINERS.
>> I did not add anything here as I figured it implies a certain level of
>> obligation. His time
>> available varies quite a bit (especially at the current time) and I
>> wasn't sure if it's
>> appropriate listing him.
> Yes, that's an unpleasant but legitimate question: will there be anybody
> caring for this sndio backend in QEMU or would it go orphaned right from the
> start?
Orphaned from the start makes it sound like we're dumping code and
walking away.
When I say obligation I mean say responding withing say 3 - 4 days for
some sort of
an issue vs say maybe taking 1.5 - 2 weeks to respond.
> It would be good to have at least somebody familiar with this code to
> volunteer as reviewer(s) ("R:" line(s) in MAINTAINERS file). Reviewers are
> automatically CCed, so that they can (optionally) give their feedback on
> future changes to the sndio backend, i.e. when somebody sends sndio patches to
> qemu-devel. This is voluntary and can be revoked at any time, and I do not
> expect that you would frequently get emailed for this either.
That sounds reasonable. I'll prod Alexandre further about responding.
> As this is a BSD-specific audio backend, it is not likely that an active QEMU
> developer would be able to care for it.
I would not say it is BSD-specific. sndio is also packaged and available
on a good
number of Linux OS's (Alpine, Arch, Gentoo, Magia, Manjaro and some
others). I
know I have seen some Gentoo users around testing and contributing to sndio
backends. Some use it as their default sound API (Void Linux for
example). There
are older packages for Debian / Ubuntu that unfortunately don't have the
pkg-config file.
>>>> create mode 100644 audio/sndioaudio.c
>>>>
>>>> diff --git a/audio/audio.c b/audio/audio.c
>>>> index 54a153c0ef..bad1ceb69e 100644
>>>> --- a/audio/audio.c
>>>> +++ b/audio/audio.c
>>>> @@ -2005,6 +2005,7 @@ void audio_create_pdos(Audiodev *dev)
>>>>
>>>> CASE(OSS, oss, Oss);
>>>> CASE(PA, pa, Pa);
>>>> CASE(SDL, sdl, Sdl);
>>>>
>>>> + CASE(SNDIO, sndio, );
>>>>
>>>> CASE(SPICE, spice, );
>>>> CASE(WAV, wav, );
>>>>
>>>> diff --git a/audio/audio_template.h b/audio/audio_template.h
>>>> index c6714946aa..ecc5a0bc6d 100644
>>>> --- a/audio/audio_template.h
>>>> +++ b/audio/audio_template.h
>>>> @@ -337,6 +337,8 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
>>>> TYPE)(Audiodev *dev) return
>>>> qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE); case
>>>>
>>>> AUDIODEV_DRIVER_SDL:
>>>> return
>>>> qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
>>>>
>>>> + case AUDIODEV_DRIVER_SNDIO:
>>>> + return dev->u.sndio.TYPE;
>>>>
>>>> case AUDIODEV_DRIVER_SPICE:
>>>> return dev->u.spice.TYPE;
>>>>
>>>> case AUDIODEV_DRIVER_WAV:
>>>> diff --git a/audio/meson.build b/audio/meson.build
>>>> index 462533bb8c..e24c86e7e6 100644
>>>> --- a/audio/meson.build
>>>> +++ b/audio/meson.build
>>>> @@ -17,6 +17,7 @@ foreach m : [
>>>>
>>>> ['pa', pulse, files('paaudio.c')],
>>>> ['sdl', sdl, files('sdlaudio.c')],
>>>> ['jack', jack, files('jackaudio.c')],
>>>>
>>>> + ['sndio', sndio, files('sndioaudio.c')],
>>>>
>>>> ['spice', spice, files('spiceaudio.c')]
>>>>
>>>> ]
>>>>
>>>> if m[1].found()
>>>>
>>>> diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
>>>> new file mode 100644
>>>> index 0000000000..204af07781
>>>> --- /dev/null
>>>> +++ b/audio/sndioaudio.c
>>>> @@ -0,0 +1,555 @@
>>>> +/*
>>>> + * Copyright (c) 2019 Alexandre Ratchov<alex@caoua.org>
>>>> + *
>>> It is quite common for new source files in QEMU to have an authors list
>>>
>>> section in the header here like:
>>> * Autors:
>>> * Alexandre Ratchov<alex@caoua.org>
>> I was looking through the tree and all of the examples I came across
>> were using this
>> with a Copyright for a company as opposed to an individual. What would
>> be the
>> format?
> There was nothing wrong with the copyright line. If it was an individual, then
> the copyright line is an individual. And like I said, it does not seem to be
> required in QEMU to have an "Authors:" block in the file header at all. So on
> doubt just ignore this.
>
> Deflating the file header by using SPDX license identifier as suggested would
> make sense though.
I have substituted the ISC license with the SPDX identifier.
>>> That way scripts/get_maintainer.pl can suggest those people as well in
>>> case
>>> they are not explicitly listed in MAINTAINERS. Does not seem to be
>>> mandatory for QEMU though. Just saying.
> Best regards,
> Christian Schoenebeck
>
>
On 11/7/21 06:19, Brad Smith wrote:
> if not get_option('spice_protocol').auto() or have_system
> @@ -1301,6 +1306,7 @@ if have_system
> 'oss': oss.found(),
> 'pa': pulse.found(),
> 'sdl': sdl.found(),
> + 'sndio': sndio.found(),
> }
> foreach k, v: audio_drivers_available
> config_host_data.set('CONFIG_AUDIO_' + k.to_upper(), v)
Maybe you want to add sndio to the audio_drivers_priority array if
targetos == 'openbsd'?
Paolo
On 11/8/2021 9:58 AM, Paolo Bonzini wrote:
> On 11/7/21 06:19, Brad Smith wrote:
>> if not get_option('spice_protocol').auto() or have_system
>> @@ -1301,6 +1306,7 @@ if have_system
>> 'oss': oss.found(),
>> 'pa': pulse.found(),
>> 'sdl': sdl.found(),
>> + 'sndio': sndio.found(),
>> }
>> foreach k, v: audio_drivers_available
>> config_host_data.set('CONFIG_AUDIO_' + k.to_upper(), v)
>
> Maybe you want to add sndio to the audio_drivers_priority array if
> targetos == 'openbsd'?
That part I was not 100% sure of.
Am I to understand with the current Meson code it will try to use
PulseAudio instead of
ALSA on a Linux system unless overrriden?
El mar., 9 nov. 2021 22:53, Brad Smith <brad@comstyle.com> escribió: > On 11/8/2021 9:58 AM, Paolo Bonzini wrote: > > > Maybe you want to add sndio to the audio_drivers_priority array if > > targetos == 'openbsd'? > > That part I was not 100% sure of. > > Am I to understand with the current Meson code it will try to use > PulseAudio instead of > ALSA on a Linux system unless overrriden? > Yes, correct. Paolo > >
On 11/8/2021 9:58 AM, Paolo Bonzini wrote:
> On 11/7/21 06:19, Brad Smith wrote:
>> if not get_option('spice_protocol').auto() or have_system
>> @@ -1301,6 +1306,7 @@ if have_system
>> 'oss': oss.found(),
>> 'pa': pulse.found(),
>> 'sdl': sdl.found(),
>> + 'sndio': sndio.found(),
>> }
>> foreach k, v: audio_drivers_available
>> config_host_data.set('CONFIG_AUDIO_' + k.to_upper(), v)
>
> Maybe you want to add sndio to the audio_drivers_priority array if
> targetos == 'openbsd'?
>
> Paolo
I see what to do there now. When I first came across it I wasn't sure.
Hi Brad,
> audio: Add sndio backend
>
> Add a sndio backend.
>
> sndio is the native API used by OpenBSD, although it has been ported to
> other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.).
>
> The C code is from Alexandre Ratchov<alex@caoua.org> and the rest of
> the bits are from me.
> ---
> audio/audio.c | 1 +
> audio/audio_template.h | 2 +
> audio/meson.build | 1 +
> audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++
> meson.build | 7 +
> meson_options.txt | 4 +-
> qapi/audio.json | 25 +-
> qemu-options.hx | 8 +
> tests/vm/freebsd | 3 +
> 9 files changed, 604 insertions(+), 2 deletions(-)
> create mode 100644 audio/sndioaudio.c
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 54a153c0ef..bad1ceb69e 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -2005,6 +2005,7 @@ void audio_create_pdos(Audiodev *dev)
> CASE(OSS, oss, Oss);
> CASE(PA, pa, Pa);
> CASE(SDL, sdl, Sdl);
> + CASE(SNDIO, sndio, );
> CASE(SPICE, spice, );
> CASE(WAV, wav, );
>
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index c6714946aa..ecc5a0bc6d 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -337,6 +337,8 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
> return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
> case AUDIODEV_DRIVER_SDL:
> return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
> + case AUDIODEV_DRIVER_SNDIO:
> + return dev->u.sndio.TYPE;
> case AUDIODEV_DRIVER_SPICE:
> return dev->u.spice.TYPE;
> case AUDIODEV_DRIVER_WAV:
> diff --git a/audio/meson.build b/audio/meson.build
> index 462533bb8c..e24c86e7e6 100644
> --- a/audio/meson.build
> +++ b/audio/meson.build
> @@ -17,6 +17,7 @@ foreach m : [
> ['pa', pulse, files('paaudio.c')],
> ['sdl', sdl, files('sdlaudio.c')],
> ['jack', jack, files('jackaudio.c')],
> + ['sndio', sndio, files('sndioaudio.c')],
> ['spice', spice, files('spiceaudio.c')]
> ]
> if m[1].found()
> diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
> new file mode 100644
> index 0000000000..204af07781
> --- /dev/null
> +++ b/audio/sndioaudio.c
> @@ -0,0 +1,555 @@
> +/*
> + * Copyright (c) 2019 Alexandre Ratchov<alex@caoua.org>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +/*
> + * TODO :
> + *
> + * Use a single device and open it in full-duplex rather than
> + * opening it twice (once for playback once for recording).
> + *
> + * This is the only way to ensure that playback doesn't drift with respect
> + * to recording, which is what guest systems expect.
> + */
> +
> +#include <poll.h>
> +#include <sndio.h>
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/main-loop.h"
> +#include "audio.h"
> +#include "trace.h"
> +
> +#define AUDIO_CAP "sndio"
> +#include "audio_int.h"
> +
> +/* default latency in ms if no option is set */
> +#define SNDIO_LATENCY_US 50000
> +
> +typedef struct SndioVoice {
> + union {
> + HWVoiceOut out;
> + HWVoiceIn in;
> + } hw;
> + struct sio_par par;
> + struct sio_hdl *hdl;
> + struct pollfd *pfds;
> + struct pollindex {
> + struct SndioVoice *self;
> + int index;
> + } *pindexes;
> + unsigned char *buf;
> + size_t buf_size;
> + size_t sndio_pos;
> + size_t qemu_pos;
> + unsigned int mode;
> + unsigned int nfds;
> +} SndioVoice;
> +
> +typedef struct SndioConf {
> + const char *devname;
> + unsigned int latency;
> +} SndioConf;
> +
> +/* needed for forward reference */
> +static void sndio_poll_in(void *arg);
> +static void sndio_poll_out(void *arg);
> +
> +/*
> + * stop polling descriptors
> + */
> +static void sndio_poll_clear(SndioVoice *self)
> +{
> + struct pollfd *pfd;
> + int i;
> +
> + for (i = 0; i < self->nfds; i++) {
> + pfd = &self->pfds[i];
> + qemu_set_fd_handler (pfd->fd, NULL, NULL, NULL);
> + }
> +
> + self->nfds = 0;
> +}
> +
> +/*
> + * write data to the device until it blocks or
> + * all of our buffered data is written
> + */
> +static void sndio_write(SndioVoice *self)
> +{
> + size_t todo, n;
> +
> + todo = self->qemu_pos - self->sndio_pos;
> +
> + /*
> + * transfer data to device, until it blocks
> + */
> + while (todo > 0) {
> + n = sio_write(self->hdl, self->buf + self->sndio_pos, todo);
> + if (n == 0) {
> + break;
> + }
> + self->sndio_pos += n;
> + todo -= n;
> + }
> +
> + if (self->sndio_pos == self->buf_size) {
> + /*
> + * we complete the block
> + */
> + self->sndio_pos = 0;
> + self->qemu_pos = 0;
> + }
> +}
> +
> +/*
> + * read data from the device until it blocks or
> + * there no room any longer
> + */
> +static void sndio_read(SndioVoice *self)
> +{
> + size_t todo, n;
> +
> + todo = self->buf_size - self->sndio_pos;
> +
> + /*
> + * transfer data from the device, until it blocks
> + */
> + while (todo > 0) {
> + n = sio_read(self->hdl, self->buf + self->sndio_pos, todo);
> + if (n == 0) {
> + break;
> + }
> + self->sndio_pos += n;
> + todo -= n;
> + }
> +}
> +
> +/*
> + * Set handlers for all descriptors libsndio needs to
> + * poll
> + */
> +static void sndio_poll_wait(SndioVoice *self)
> +{
> + struct pollfd *pfd;
> + int events, i;
> +
> + events = 0;
> + if (self->mode == SIO_PLAY) {
> + if (self->sndio_pos < self->qemu_pos) {
> + events |= POLLOUT;
> + }
> + } else {
> + if (self->sndio_pos < self->buf_size) {
> + events |= POLLIN;
> + }
> + }
> +
> + /*
> + * fill the given array of descriptors with the events sndio
> + * wants, they are different from our 'event' variable because
> + * sndio may use descriptors internally.
> + */
> + self->nfds = sio_pollfd(self->hdl, self->pfds, events);
> +
> + for (i = 0; i < self->nfds; i++) {
> + pfd = &self->pfds[i];
> + if (pfd->fd < 0) {
> + continue;
> + }
> + qemu_set_fd_handler(pfd->fd,
> + (pfd->events & POLLIN) ? sndio_poll_in : NULL,
> + (pfd->events & POLLOUT) ? sndio_poll_out : NULL,
> + &self->pindexes[i]);
> + pfd->revents = 0;
> + }
> +}
> +
> +/*
> + * call-back called when one of the descriptors
> + * became readable or writable
> + */
> +static void sndio_poll_event(SndioVoice *self, int index, int event)
> +{
> + int revents;
> +
> + /*
> + * ensure we're not called twice this cycle
> + */
> + sndio_poll_clear(self);
> +
> + /*
> + * make self->pfds[] look as we're returning from poll syscal,
> + * this is how sio_revents expects events to be.
> + */
> + self->pfds[index].revents = event;
> +
> + /*
> + * tell sndio to handle events and return whether we can read or
> + * write without blocking.
> + */
> + revents = sio_revents(self->hdl, self->pfds);
> + if (self->mode == SIO_PLAY) {
> + if (revents & POLLOUT) {
> + sndio_write(self);
> + }
> +
> + if (self->qemu_pos < self->buf_size) {
> + audio_run(self->hw.out.s, "sndio_out");
> + }
> + } else {
> + if (revents & POLLIN) {
> + sndio_read(self);
> + }
> +
> + if (self->qemu_pos < self->sndio_pos) {
> + audio_run(self->hw.in.s, "sndio_in");
> + }
> + }
> +
> + sndio_poll_wait(self);
> +}
> +
> +/*
> + * return a buffer where data to play can be stored,
> + * its size is stored in the location pointed by the size argument.
> + */
> +static void *sndio_get_buffer_out(HWVoiceOut *hw, size_t *size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + /* size is not set by the caller */
This is an outdated comment. Size is set by the caller, but the backend
is still free to ignore the size.
> + *size = self->buf_size - self->qemu_pos;
> + return self->buf + self->qemu_pos;
> +}
> +
> +/*
> + * return a buffer where data to play can be stored
> + */
> +static size_t sndio_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + self->qemu_pos += size;
> + sndio_poll_wait(self);
> + return size;
> +}
> +
> +/*
> + * return a buffer from where recorded data is available,
> + * its size is stored in the location pointed by the size argument.
> + * it may not exceed the initial value of "*size".
> + */
> +static void *sndio_get_buffer_in(HWVoiceIn *hw, size_t *size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> + size_t todo, max_todo;
> +
> + /*
> + * unlike the get_buffer_out() method, get_buffer_in()
> + * must return a buffer of at most the given size, see audio.c
> + */
> + max_todo = *size;
> +
> + todo = self->sndio_pos - self->qemu_pos;
> + if (todo > max_todo) {
> + todo = max_todo;
> + }
> +
> + *size = todo;
> + return self->buf + self->qemu_pos;
> +}
> +
> +/*
> + * discard the given amount of recorded data
> + */
> +static void sndio_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + self->qemu_pos += size;
> + if (self->qemu_pos == self->buf_size) {
> + self->qemu_pos = 0;
> + self->sndio_pos = 0;
> + }
> + sndio_poll_wait(self);
> +}
> +
> +/*
> + * call-back called when one of our descriptors becomes writable
> + */
> +static void sndio_poll_out(void *arg)
> +{
> + struct pollindex *pindex = (struct pollindex *) arg;
> +
> + sndio_poll_event(pindex->self, pindex->index, POLLOUT);
> +}
> +
> +/*
> + * call-back called when one of our descriptors becomes readable
> + */
> +static void sndio_poll_in(void *arg)
> +{
> + struct pollindex *pindex = (struct pollindex *) arg;
> +
> + sndio_poll_event(pindex->self, pindex->index, POLLIN);
> +}
> +
> +static void sndio_fini(SndioVoice *self)
> +{
> + if (self->hdl) {
> + sio_close(self->hdl);
> + self->hdl = NULL;
> + }
> +
> + g_free(self->pfds);
> + g_free(self->pindexes);
> + g_free(self->buf);
> +}
> +
> +static int sndio_init(SndioVoice *self,
> + struct audsettings *as, int mode, Audiodev *dev)
> +{
> + AudiodevSndioOptions *opts = &dev->u.sndio;
> + unsigned long long latency;
> + const char *dev_name;
> + struct sio_par req;
> + unsigned int nch;
> + int i, nfds;
> +
> + dev_name = opts->has_dev ? opts->dev : SIO_DEVANY;
> + latency = opts->has_latency ? opts->latency : SNDIO_LATENCY_US;
> +
> + /* open the device in non-blocking mode */
> + self->hdl = sio_open(dev_name, mode, 1);
> + if (self->hdl == NULL) {
> + dolog("failed to open device\n");
> + return -1;
> + }
> +
> + self->mode = mode;
> +
> + sio_initpar(&req);
> +
> + switch (as->fmt) {
> + case AUDIO_FORMAT_S8:
> + req.bits = 8;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U8:
> + req.bits = 8;
> + req.sig = 0;
> + break;
> + case AUDIO_FORMAT_S16:
> + req.bits = 16;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U16:
> + req.bits = 16;
> + req.sig = 0;
> + break;
> + case AUDIO_FORMAT_S32:
> + req.bits = 32;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U32:
> + req.bits = 32;
> + req.sig = 0;
../qemu-master/audio/sndioaudio.c: In function ‘sndio_init’:
../qemu-master/audio/sndioaudio.c:369:17: error: this statement may fall
through [-Werror=implicit-fallthrough=]
req.sig = 0;
~~~~~~~~^~~
../qemu-master/audio/sndioaudio.c:370:5: note: here
default:
^~~~~~~
cc1: all warnings being treated as errors
> + default:
> + dolog("unknown audio sample format\n");
> + return -1;
> + }
> +
> + if (req.bits > 8) {
> + req.le = as->endianness ? 0 : 1;
> + }
> +
> + req.rate = as->freq;
> + if (mode == SIO_PLAY) {
> + req.pchan = as->nchannels;
> + } else {
> + req.rchan = as->nchannels;
> + }
> +
> + /* set on-device buffer size */
> + req.appbufsz = req.rate * latency / 1000000;
> +
> + if (!sio_setpar(self->hdl, &req)) {
> + dolog("failed set audio params\n");
> + goto fail;
> + }
> +
> + if (!sio_getpar(self->hdl, &self->par)) {
> + dolog("failed get audio params\n");
> + goto fail;
> + }
> +
> + nch = (mode == SIO_PLAY) ? self->par.pchan : self->par.rchan;
> +
> + /*
> + * With the default setup, sndio supports any combination of parameters
> + * so these checks are mostly to catch configuration errors.
> + */
> + if (self->par.bits != req.bits || self->par.bps != req.bits / 8 ||
> + self->par.sig != req.sig || (req.bits > 8 && self->par.le != req.le) ||
> + self->par.rate != as->freq || nch != as->nchannels) {
> + dolog("unsupported audio params\n");
> + goto fail;
> + }
> +
> + /*
> + * we use one block as buffer size; this is how
> + * transfers get well aligned
> + */
> + self->buf_size = self->par.round * self->par.bps * nch;
> +
> + self->buf = g_malloc(self->buf_size);
> + if (self->buf == NULL) {
> + dolog("failed to allocate audio buffer\n");
> + goto fail;
> + }
> +
> + nfds = sio_nfds(self->hdl);
> +
> + self->pfds = g_malloc_n(nfds, sizeof(struct pollfd));
> + if (self->pfds == NULL) {
> + dolog("failed to allocate pollfd structures\n");
> + goto fail;
> + }
> +
> + self->pindexes = g_malloc_n(nfds, sizeof(struct pollindex));
> + if (self->pindexes == NULL) {
> + dolog("failed to allocate pollindex structures\n");
> + goto fail;
> + }
> +
> + for (i = 0; i < nfds; i++) {
> + self->pindexes[i].self = self;
> + self->pindexes[i].index = i;
> + }
> +
> + return 0;
> +fail:
> + sndio_fini(self);
> + return -1;
> +}
> +
> +static void sndio_enable(SndioVoice *self, bool enable)
> +{
> + if (enable) {
> + sio_start(self->hdl);
> + sndio_poll_wait(self);
> + } else {
> + sndio_poll_clear(self);
> + sio_stop(self->hdl);
> + }
> +}
> +
> +static void sndio_enable_out(HWVoiceOut *hw, bool enable)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_enable(self, enable);
> +}
> +
> +static void sndio_enable_in(HWVoiceIn *hw, bool enable)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_enable(self, enable);
> +}
> +
> +static int sndio_init_out(HWVoiceOut *hw, struct audsettings *as, void *opaque)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + if (sndio_init(self, as, SIO_PLAY, opaque) == -1) {
> + return -1;
> + }
> +
> + audio_pcm_init_info(&hw->info, as);
> + hw->samples = self->par.round;
> + return 0;
> +}
> +
> +static int sndio_init_in(HWVoiceIn *hw, struct audsettings *as, void *opaque)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + if (sndio_init(self, as, SIO_REC, opaque) == -1) {
> + return -1;
> + }
> +
> + audio_pcm_init_info(&hw->info, as);
> + hw->samples = self->par.round;
> + return 0;
> +}
> +
> +static void sndio_fini_out(HWVoiceOut *hw)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_fini(self);
> +}
> +
> +static void sndio_fini_in(HWVoiceIn *hw)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_fini(self);
> +}
> +
> +static void *sndio_audio_init(Audiodev *dev)
> +{
> + assert(dev->driver == AUDIODEV_DRIVER_SNDIO);
> + return dev;
> +}
> +
> +static void sndio_audio_fini(void *opaque)
> +{
> +}
> +
> +static struct audio_pcm_ops sndio_pcm_ops = {
> + .init_out = sndio_init_out,
> + .fini_out = sndio_fini_out,
+ .write = audio_generic_write,
> + .enable_out = sndio_enable_out,
> + .get_buffer_out = sndio_get_buffer_out,
> + .put_buffer_out = sndio_put_buffer_out,
> + .init_in = sndio_init_in,
> + .fini_in = sndio_fini_in,
+ .read = audio_generic_read,
With . write = NULL and .read = NULL and -audiodev
sndio,id=audio0,in.mixing-engine=off,out.mixing-engine=off you will see
Program terminated with signal SIGSEGV, Segmentation fault.
> + .enable_in = sndio_enable_in,
> + .get_buffer_in = sndio_get_buffer_in,
> + .put_buffer_in = sndio_put_buffer_in,
> +};
> +
> +static struct audio_driver sndio_audio_driver = {
> + .name = "sndio",
> + .descr ="https://man.openbsd.org/sndio",
> + .init = sndio_audio_init,
> + .fini = sndio_audio_fini,
> + .pcm_ops = &sndio_pcm_ops,
> + .can_be_default = 1,
> + .max_voices_out = INT_MAX,
> + .max_voices_in = INT_MAX,
> + .voice_size_out = sizeof(SndioVoice),
> + .voice_size_in = sizeof(SndioVoice)
> +};
> +
> +static void register_audio_sndio(void)
> +{
> + audio_driver_register(&sndio_audio_driver);
> +}
> +
> +type_init(register_audio_sndio);
> diff --git a/meson.build b/meson.build
> index 47df10afc2..551e8e3549 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -546,6 +546,11 @@ if not get_option('jack').auto() or have_system
> jack = dependency('jack', required: get_option('jack'),
> 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'),
> + method: 'pkg-config', kwargs: static_kwargs)
> +endif
>
> spice_protocol = not_found
> if not get_option('spice_protocol').auto() or have_system
> @@ -1301,6 +1306,7 @@ if have_system
> 'oss': oss.found(),
> 'pa': pulse.found(),
> 'sdl': sdl.found(),
> + 'sndio': sndio.found(),
> }
> foreach k, v: audio_drivers_available
> config_host_data.set('CONFIG_AUDIO_' + k.to_upper(), v)
> @@ -3367,6 +3373,7 @@ if vnc.found()
> endif
> if targetos not in ['darwin', 'haiku', 'windows']
> summary_info += {'OSS support': oss}
> + summary_info += {'sndio support': sndio}
> elif targetos == 'darwin'
> summary_info += {'CoreAudio support': coreaudio}
> elif targetos == 'windows'
> diff --git a/meson_options.txt b/meson_options.txt
> index e740dce2a5..0f67c0a27b 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -13,7 +13,7 @@ option('sphinx_build', type : 'string', value : '',
> 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'],
> + choices: ['alsa', 'coreaudio', 'default', 'dsound', 'jack', 'oss', 'pa', 'sdl', 'sndio'],
> description: 'Set audio driver list')
> option('fuzzing_engine', type : 'string', value : '',
> description: 'fuzzing engine library for OSS-Fuzz')
> @@ -184,6 +184,8 @@ option('oss', type: 'feature', value: 'auto',
> description: 'OSS sound support')
> option('pa', type: 'feature', value: 'auto',
> description: 'PulseAudio sound support')
> +option('sndio', type: 'feature', value: 'auto',
> + description: 'sndio sound support')
>
> option('vhost_user_blk_server', type: 'feature', value: 'auto',
> description: 'build vhost-user-blk server')
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 9cba0df8a4..99c5c68ba6 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -105,6 +105,28 @@
> '*out': 'AudiodevAlsaPerDirectionOptions',
> '*threshold': 'uint32' } }
>
> +##
> +# @AudiodevSndioOptions:
> +#
> +# Options of the sndio audio backend.
> +#
> +# @in: options of the capture stream
> +#
> +# @out: options of the playback stream
> +#
> +# @dev: the name of the sndio device to use (default 'default')
> +#
> +# @latency: play buffer size (in microseconds)
> +#
> +# Since: 6.2
I don't think this patch will be accepted for 6.2. See
https://wiki.qemu.org/Planning/6.2
Since: 7.0 is probably correct.
> +##
> +{ 'struct': 'AudiodevSndioOptions',
> + 'data': {
> + '*in': 'AudiodevPerDirectionOptions',
> + '*out': 'AudiodevPerDirectionOptions',
> + '*dev': 'str',
> + '*latency': 'uint32'} }
> +
> ##
> # @AudiodevCoreaudioPerDirectionOptions:
> #
> @@ -387,7 +409,7 @@
> ##
> { 'enum': 'AudiodevDriver',
> 'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
> - 'sdl', 'spice', 'wav' ] }
> + 'sdl', 'sndio', 'spice', 'wav' ] }
>
> ##
> # @Audiodev:
> @@ -417,5 +439,6 @@
> 'oss': 'AudiodevOssOptions',
> 'pa': 'AudiodevPaOptions',
> 'sdl': 'AudiodevSdlOptions',
> + 'sndio': 'AudiodevSndioOptions',
> 'spice': 'AudiodevGenericOptions',
> 'wav': 'AudiodevWavOptions' } }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f051536b63..4a027b1abc 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -657,6 +657,9 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> "-audiodev sdl,id=id[,prop[=value][,...]]\n"
> " in|out.buffer-count= number of buffers\n"
> #endif
> +#ifdef CONFIG_AUDIO_SNDIO
> + "-audiodev sndio,id=id[,prop[=value][,...]]\n"
> +#endif
> #ifdef CONFIG_SPICE
> "-audiodev spice,id=id[,prop[=value][,...]]\n"
> #endif
> @@ -820,6 +823,11 @@ SRST
> ``in|out.buffer-count=count``
> Sets the count of the buffers.
>
> +``-audiodev sndio,id=id[,prop[=value][,...]]``
> + Creates a backend using SNDIO. This backend is available on
> + OpenBSD and most other Unix-like systems. This backend has no
> + backend specific properties.
The properties latency and dev are backend specific properties.
> +
> ``-audiodev spice,id=id[,prop[=value][,...]]``
> Creates a backend that sends audio through SPICE. This backend
> requires ``-spice`` and automatically selected in that case, so
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> index 6e20e84322..a387f5c9df 100755
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -63,6 +63,9 @@ class FreeBSDVM(basevm.BaseVM):
>
> # libs: migration
> "zstd",
> +
> + # libs: sndio
> + "sndio",
> ]
>
> # TODO: Enable gnutls again once FreeBSD's libtasn1 got fixed
I think the rest of the code looks good. Don't forget to use
./scripts/get_maintainer.pl 0001-audio-Add-sndio-backend.patch to cc all
maintainers.
With best regards,
Volker
Hi Brad,
just a few white space and coding style issues.
> +/*
> + * stop polling descriptors
> + */
> +static void sndio_poll_clear(SndioVoice *self)
> +{
> + struct pollfd *pfd;
> + int i;
> +
> + for (i = 0; i < self->nfds; i++) {
> + pfd = &self->pfds[i];
> + qemu_set_fd_handler (pfd->fd, NULL, NULL, NULL);
No space between function name and opening brace.
> +/*
> + * Set handlers for all descriptors libsndio needs to
> + * poll
> + */
> +static void sndio_poll_wait(SndioVoice *self)
> +{
> + struct pollfd *pfd;
> + int events, i;
> +
> + events = 0;
> + if (self->mode == SIO_PLAY) {
> + if (self->sndio_pos < self->qemu_pos) {
> + events |= POLLOUT;
> + }
> + } else {
> + if (self->sndio_pos < self->buf_size) {
> + events |= POLLIN;
> + }
> + }
> +
> + /*
> + * fill the given array of descriptors with the events sndio
> + * wants, they are different from our 'event' variable because
> + * sndio may use descriptors internally.
> + */
> + self->nfds = sio_pollfd(self->hdl, self->pfds, events);
> +
> + for (i = 0; i < self->nfds; i++) {
> + pfd = &self->pfds[i];
> + if (pfd->fd < 0) {
> + continue;
Indent with 4 spaces.
> + }
> + qemu_set_fd_handler(pfd->fd,
> + (pfd->events & POLLIN) ? sndio_poll_in : NULL,
> + (pfd->events & POLLOUT) ? sndio_poll_out : NULL,
> + &self->pindexes[i]);
Same here.
> +static int sndio_init(SndioVoice *self,
> + struct audsettings *as, int mode, Audiodev *dev)
> +{
> + AudiodevSndioOptions *opts = &dev->u.sndio;
> + unsigned long long latency;
> + const char *dev_name;
> + struct sio_par req;
> + unsigned int nch;
> + int i, nfds;
> +
> + dev_name = opts->has_dev ? opts->dev : SIO_DEVANY;
> + latency = opts->has_latency ? opts->latency : SNDIO_LATENCY_US;
> +
> + /* open the device in non-blocking mode */
> + self->hdl = sio_open(dev_name, mode, 1);
> + if (self->hdl == NULL) {
> + dolog("failed to open device\n");
> + return -1;
> + }
> +
> + self->mode = mode;
> +
> + sio_initpar(&req);
> +
> + switch (as->fmt) {
> + case AUDIO_FORMAT_S8:
> + req.bits = 8;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U8:
> + req.bits = 8;
> + req.sig = 0;
> + break;
> + case AUDIO_FORMAT_S16:
> + req.bits = 16;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U16:
> + req.bits = 16;
> + req.sig = 0;
> + break;
> + case AUDIO_FORMAT_S32:
> + req.bits = 32;
> + req.sig = 1;
> + break;
> + case AUDIO_FORMAT_U32:
> + req.bits = 32;
> + req.sig = 0;
> + default:
> + dolog("unknown audio sample format\n");
> + return -1;
> + }
> +
> + if (req.bits > 8) {
> + req.le = as->endianness ? 0 : 1;
> + }
> +
> + req.rate = as->freq;
> + if (mode == SIO_PLAY) {
> + req.pchan = as->nchannels;
Indent with 4 spaces.
> + } else {
> + req.rchan = as->nchannels;
Here too.
> +static void sndio_enable_out(HWVoiceOut *hw, bool enable)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_enable(self, enable);
> +}
Unnecessary return statement.
> +
> +static void sndio_enable_in(HWVoiceIn *hw, bool enable)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_enable(self, enable);
> +}
The same here
> +static void sndio_fini_out(HWVoiceOut *hw)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_fini(self);
> +}
and here
> +
> +static void sndio_fini_in(HWVoiceIn *hw)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + return sndio_fini(self);
> +}
and here.
With best regards,
Volker
Hi Brad,
I tested the sndio backend on my Linux system and I found a bug in the
sndio backend. The problem is that the function audio_run() can call the
function sndio_enable_out() to disable audio playback.
In the sndio_poll_event() function, audio_run() is called, which removes
the poll handlers when the playback stream is stopped by calling
sndio_enable_out(). Next, sndio_poll_wait() is called in
sndio_poll_event(), which can reinstall the poll handlers of the stopped
stream again. After a subsequent call to sndio_fini_out(), the pindex
pointer of the still installed poll handlers points to a memory area
that has already been freed. This can lead to a segmentation fault or a
QEMU lockup because of a blocking read.
I suggest to use a flag to prevent that sndio_poll_event() reinstalls
the poll handlers.
> +typedef struct SndioVoice {
> + union {
> + HWVoiceOut out;
> + HWVoiceIn in;
> + } hw;
> + struct sio_par par;
> + struct sio_hdl *hdl;
> + struct pollfd *pfds;
> + struct pollindex {
> + struct SndioVoice *self;
> + int index;
> + } *pindexes;
> + unsigned char *buf;
> + size_t buf_size;
> + size_t sndio_pos;
> + size_t qemu_pos;
> + unsigned int mode;
> + unsigned int nfds;
+ bool enabled;
> +} SndioVoice;
> +/*
> + * call-back called when one of the descriptors
> + * became readable or writable
> + */
> +static void sndio_poll_event(SndioVoice *self, int index, int event)
> +{
> + int revents;
> +
> + /*
> + * ensure we're not called twice this cycle
> + */
> + sndio_poll_clear(self);
> +
> + /*
> + * make self->pfds[] look as we're returning from poll syscal,
> + * this is how sio_revents expects events to be.
> + */
> + self->pfds[index].revents = event;
> +
> + /*
> + * tell sndio to handle events and return whether we can read or
> + * write without blocking.
> + */
> + revents = sio_revents(self->hdl, self->pfds);
> + if (self->mode == SIO_PLAY) {
> + if (revents & POLLOUT) {
> + sndio_write(self);
> + }
> +
> + if (self->qemu_pos < self->buf_size) {
> + audio_run(self->hw.out.s, "sndio_out");
> + }
> + } else {
> + if (revents & POLLIN) {
> + sndio_read(self);
> + }
> +
> + if (self->qemu_pos < self->sndio_pos) {
> + audio_run(self->hw.in.s, "sndio_in");
> + }
> + }
> +
> + sndio_poll_wait(self);
- sndio_poll_wait(self);
+ if (self->enabled) {
+ sndio_poll_wait(self);
+ }
> +}
> +/*
> + * return a buffer where data to play can be stored
> + */
> +static size_t sndio_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + self->qemu_pos += size;
> + sndio_poll_wait(self);
> + return size;
> +}
> +/*
> + * discard the given amount of recorded data
> + */
> +static void sndio_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
> +{
> + SndioVoice *self = (SndioVoice *) hw;
> +
> + self->qemu_pos += size;
> + if (self->qemu_pos == self->buf_size) {
> + self->qemu_pos = 0;
> + self->sndio_pos = 0;
> + }
> + sndio_poll_wait(self);
> +}
It's not necessary to guard sndio_poll_wait() in sndio_put_buffer_out()
and sndio_put_buffer_in() because audio_run() will never call those
functions for a disabled stream.
> +static void sndio_enable(SndioVoice *self, bool enable)
> +{
> + if (enable) {
> + sio_start(self->hdl);
+ self->enabled = true;
> + sndio_poll_wait(self);
> + } else {
+ self->enabled = false;
> + sndio_poll_clear(self);
> + sio_stop(self->hdl);
> + }
> +}
With best regards,
Volker
On 2021/11/7 13:19, Brad Smith wrote: > audio: Add sndio backend > > Add a sndio backend. > > sndio is the native API used by OpenBSD, although it has been ported to > other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.). > > The C code is from Alexandre Ratchov <alex@caoua.org> and the rest of > the bits are from me. As pointed out by others, this is lacking Signed-off-by lines; IIUC you may contact Alexandre to get theirs, and then add yours. I'm not familiar with this part of qemu, so what follows is only a somewhat brief review. That said... > --- > audio/audio.c | 1 + > audio/audio_template.h | 2 + > audio/meson.build | 1 + > audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++ > meson.build | 7 + > meson_options.txt | 4 +- > qapi/audio.json | 25 +- > qemu-options.hx | 8 + > tests/vm/freebsd | 3 + > 9 files changed, 604 insertions(+), 2 deletions(-) > create mode 100644 audio/sndioaudio.c > > diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c > new file mode 100644 > index 0000000000..204af07781 > --- /dev/null > +++ b/audio/sndioaudio.c > @@ -0,0 +1,555 @@ > +/* > + * Copyright (c) 2019 Alexandre Ratchov <alex@caoua.org> > + * > + * Permission to use, copy, modify, and distribute this software for any > + * purpose with or without fee is hereby granted, provided that the above > + * copyright notice and this permission notice appear in all copies. > + * > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > + */ Perhaps using an SPDX license identifier would be better? > + > +/* > + * TODO : > + * > + * Use a single device and open it in full-duplex rather than > + * opening it twice (once for playback once for recording). > + * > + * This is the only way to ensure that playback doesn't drift with respect > + * to recording, which is what guest systems expect. > + */ > + > +#include <poll.h> > +#include <sndio.h> > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "qemu/main-loop.h" > +#include "audio.h" > +#include "trace.h" > + > +#define AUDIO_CAP "sndio" > +#include "audio_int.h" > + > +/* default latency in ms if no option is set */ > +#define SNDIO_LATENCY_US 50000 Maybe you mean "microseconds" in the comment? 50 *seconds* seems a bit long ;)
On 11/10/2021 1:22 AM, WANG Xuerui wrote: > On 2021/11/7 13:19, Brad Smith wrote: >> audio: Add sndio backend >> >> Add a sndio backend. >> >> sndio is the native API used by OpenBSD, although it has been ported to >> other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.). >> >> The C code is from Alexandre Ratchov <alex@caoua.org> and the rest of >> the bits are from me. > As pointed out by others, this is lacking Signed-off-by lines; IIUC you > may contact Alexandre to get theirs, and then add yours. > > I'm not familiar with this part of qemu, so what follows is only a > somewhat brief review. That said... > >> --- >> audio/audio.c | 1 + >> audio/audio_template.h | 2 + >> audio/meson.build | 1 + >> audio/sndioaudio.c | 555 +++++++++++++++++++++++++++++++++++++++++ >> meson.build | 7 + >> meson_options.txt | 4 +- >> qapi/audio.json | 25 +- >> qemu-options.hx | 8 + >> tests/vm/freebsd | 3 + >> 9 files changed, 604 insertions(+), 2 deletions(-) >> create mode 100644 audio/sndioaudio.c >> >> diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c >> new file mode 100644 >> index 0000000000..204af07781 >> --- /dev/null >> +++ b/audio/sndioaudio.c >> @@ -0,0 +1,555 @@ >> +/* >> + * Copyright (c) 2019 Alexandre Ratchov <alex@caoua.org> >> + * >> + * Permission to use, copy, modify, and distribute this software for any >> + * purpose with or without fee is hereby granted, provided that the above >> + * copyright notice and this permission notice appear in all copies. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES >> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF >> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR >> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES >> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN >> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF >> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. >> + */ > Perhaps using an SPDX license identifier would be better? Have done so. >> + >> +/* >> + * TODO : >> + * >> + * Use a single device and open it in full-duplex rather than >> + * opening it twice (once for playback once for recording). >> + * >> + * This is the only way to ensure that playback doesn't drift with respect >> + * to recording, which is what guest systems expect. >> + */ >> + >> +#include <poll.h> >> +#include <sndio.h> >> +#include "qemu/osdep.h" >> +#include "qemu-common.h" >> +#include "qemu/main-loop.h" >> +#include "audio.h" >> +#include "trace.h" >> + >> +#define AUDIO_CAP "sndio" >> +#include "audio_int.h" >> + >> +/* default latency in ms if no option is set */ >> +#define SNDIO_LATENCY_US 50000 > Maybe you mean "microseconds" in the comment? 50 *seconds* seems a bit > long ;) I have corrected the comment.
© 2016 - 2024 Red Hat, Inc.