Re: [PATCH] audio: Add sndio backend

[Brad Smith]

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.