Re: [PATCH] audio: Add sndio backend

[Brad Smith <brad@comstyle.com>]

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.