Re: [PATCH] audio: Add sndio backend

[WANG Xuerui <i.qemu@xen0n.name>]

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 ;)