[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] audio: Add sndio backend

From: Brad Smith
Subject: Re: [PATCH] audio: Add sndio backend
Date: Thu, 18 Nov 2021 14:25:41 -0500
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:95.0) Gecko/20100101 Thunderbird/95.0

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:
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.


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

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


+        return dev->u.sndio.TYPE;

           return dev->u.spice.TYPE;
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
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
they are not explicitly listed in MAINTAINERS. Does not seem to be
mandatory for QEMU though. Just saying.
Best regards,
Christian Schoenebeck

reply via email to

[Prev in Thread] Current Thread [Next in Thread]