Re: speaker(4): unhook driver and manpage from build

2022-04-29 Thread Alexandre Ratchov
On Thu, Apr 28, 2022 at 06:34:00AM -0500, Scott Cheloha wrote:
> speaker(4) is a whimsical thing, but I don't think we should have a
> dedicated chiptune interpreter in the kernel.
> 
> This patch unhooks the driver and the manpage from the build.  The
> driver is built for alpha, amd64, and i386.
> 
> A subsequent patch will move all relevant files to the attic and clean
> up manpage cross references.
> 
> Nothing in base or xenocara includes .
> 
> I see a couple SPKRTONE and SPKRTUNE symbols in ports, but I imagine
> those ports don't use the symbols if they are missing.
> 
> ok?

I can't see any use of this driver.

ok ratchov



Re: sndio: add sio_flush() function

2022-04-28 Thread Alexandre Ratchov
On Wed, Apr 27, 2022 at 10:48:42PM +0200, Caspar Schutijser wrote:
> Hi,
> 
> On Thu, Mar 24, 2022 at 07:11:42AM +0100, Alexandre Ratchov wrote:
> > Most audio/video players do a stop/start cycle whenever the play
> > position is changed, track is changed, etc. Currently, stopping drains
> > the play buffer, which by default is very large (to workaround very
> > long kernel non-preemptive code-paths). This makes player controls
> > sluggish.
> > 
> > This diff adds a new sio_flush() function to workaround the jumbo
> > buffer sizes: it stops playback immediately, discarding buffered
> > data. Basically it's the same as sio_stop() but doesn't wait. The plan
> > is to make players use it.
> > 
> > In the network protocol, sio_flush() is implemented by adding a flag
> > to the message corresponding to sio_stop(). Old sndiod servers ignore
> > it and just work with new libraries. New sndiod servers see that the
> > flag is not set by old libraries and properly drain the play buffer.
> > 
> > Tested with mplayer, mpv and audacious, if we go this way other ports
> > will follow.
> 
> I tested this with Audacious and I like the new behavior a lot! When I
> change the play position, stop playback or change the track it indeed
> responds much faster.
> 
> When pausing playback however, the buffer is still being drained which
> suprised me a little bit. However, after some printf-debugging I
> confirmed my suspicion that Audacious does not call SndioPlugin::flush()
> in that case, so this patch (or sndio in general) doesn't have anything
> to do with that.
> 
> I can't really comment on the diff itself, although that makes sense
> to me.
> 

thank you for testing this.

I retested multimedia/mpv and x11/mplayer as well; besides the need
for an extra REVISION bump, my previous diff still applies.

OK?



Re: sndio: add sio_flush() function

2022-03-24 Thread Alexandre Ratchov
On Thu, Mar 24, 2022 at 07:11:42AM +0100, Alexandre Ratchov wrote:
> Most audio/video players do a stop/start cycle whenever the play
> position is changed, track is changed, etc. Currently, stopping drains
> the play buffer, which by default is very large (to workaround very
> long kernel non-preemptive code-paths). This makes player controls
> sluggish.
> 
> This diff adds a new sio_flush() function to workaround the jumbo
> buffer sizes: it stops playback immediately, discarding buffered
> data. Basically it's the same as sio_stop() but doesn't wait. The plan
> is to make players use it.
> 
> In the network protocol, sio_flush() is implemented by adding a flag
> to the message corresponding to sio_stop(). Old sndiod servers ignore
> it and just work with new libraries. New sndiod servers see that the
> flag is not set by old libraries and properly drain the play buffer.
> 
> Tested with mplayer, mpv and audacious, if we go this way other ports
> will follow.
> 

Here's the diff to make mplayer, mpv and audacious use the new
function.

As you see, sio_stop() and sio_flush() are perfectly
interchangable. The only difference is the time the function takes to
complete (and the resulting silence).

Index: x11/mplayer/Makefile
===
RCS file: /cvs/ports/x11/mplayer/Makefile,v
retrieving revision 1.319
diff -u -p -r1.319 Makefile
--- x11/mplayer/Makefile11 Mar 2022 20:16:48 -  1.319
+++ x11/mplayer/Makefile23 Mar 2022 19:49:08 -
@@ -3,7 +3,7 @@ COMMENT=movie player supporting many fo
 V= 20211106
 FFMPEG_V=  4.4.1
 DISTNAME=  mplayer-${V}
-REVISION=  0
+REVISION=  1
 CATEGORIES=x11 multimedia
 MASTER_SITES=  https://comstyle.com/source/
 EXTRACT_SUFX=  .tar.xz
Index: x11/mplayer/patches/patch-libao2_ao_sndio_c
===
RCS file: x11/mplayer/patches/patch-libao2_ao_sndio_c
diff -N x11/mplayer/patches/patch-libao2_ao_sndio_c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ x11/mplayer/patches/patch-libao2_ao_sndio_c 23 Mar 2022 19:49:08 -
@@ -0,0 +1,21 @@
+Index: libao2/ao_sndio.c
+--- libao2/ao_sndio.c.orig
 libao2/ao_sndio.c
+@@ -179,7 +179,7 @@ static void uninit(int immed)
+  */
+ static void reset(void)
+ {
+-if (!sio_stop(hdl))
++if (!sio_flush(hdl))
+ mp_msg(MSGT_AO, MSGL_ERR, "ao2: reset: couldn't stop\n");
+ delay = 0;
+ if (!sio_start(hdl))
+@@ -235,7 +235,7 @@ static void audio_pause(void)
+  * sndio can't pause, so just stop
+  */
+ prepause_delay = delay;
+-if (!sio_stop(hdl))
++if (!sio_flush(hdl))
+ mp_msg(MSGT_AO, MSGL_ERR, "ao2: pause: couldn't stop\n");
+ delay = 0;
+ }
Index: multimedia/mpv/Makefile
===
RCS file: /cvs/ports/multimedia/mpv/Makefile,v
retrieving revision 1.81
diff -u -p -r1.81 Makefile
--- multimedia/mpv/Makefile 11 Mar 2022 19:39:24 -  1.81
+++ multimedia/mpv/Makefile 23 Mar 2022 19:49:08 -
@@ -4,7 +4,7 @@ GH_ACCOUNT =mpv-player
 GH_PROJECT =   mpv
 GH_TAGNAME =   v0.34.1
 
-REVISION = 0
+REVISION = 1
 
 SHARED_LIBS += mpv 0.2 # 1.109
 
Index: multimedia/mpv/patches/patch-audio_out_ao_sndio_c
===
RCS file: /cvs/ports/multimedia/mpv/patches/patch-audio_out_ao_sndio_c,v
retrieving revision 1.6
diff -u -p -r1.6 patch-audio_out_ao_sndio_c
--- multimedia/mpv/patches/patch-audio_out_ao_sndio_c   11 Mar 2022 19:39:24 
-  1.6
+++ multimedia/mpv/patches/patch-audio_out_ao_sndio_c   23 Mar 2022 19:49:09 
-
@@ -245,8 +245,8 @@ Index: audio/out/ao_sndio.c
 +if (p->playing) {
 +p->playing = false;
 +
-+if (!sio_stop(p->hdl)) {
-+MP_ERR(ao, "reset: couldn't sio_stop()\n");
++if (!sio_flush(p->hdl)) {
++MP_ERR(ao, "reset: couldn't sio_flush()\n");
 +}
 +p->delay = 0;
 +if (!sio_start(p->hdl)) {
Index: audio/audacious/plugins/Makefile
===
RCS file: /cvs/ports/audio/audacious/plugins/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- audio/audacious/plugins/Makefile8 Mar 2022 08:53:50 -   1.10
+++ audio/audacious/plugins/Makefile23 Mar 2022 19:49:09 -
@@ -1,7 +1,7 @@
 COMMENT =  input and output plugins for Audacious
 
 DISTNAME = audacious-plugins-${VERSION}
-REVISION = 2
+REVISION = 3
 
 # BSD / GPL
 PERMIT_PACKAGE =   Yes
Index: audio/audacious/plugins/patches/patch-src_sndio_sndio_cc
===
RCS file: audio/audacious/plug

sndio: add sio_flush() function

2022-03-24 Thread Alexandre Ratchov
Most audio/video players do a stop/start cycle whenever the play
position is changed, track is changed, etc. Currently, stopping drains
the play buffer, which by default is very large (to workaround very
long kernel non-preemptive code-paths). This makes player controls
sluggish.

This diff adds a new sio_flush() function to workaround the jumbo
buffer sizes: it stops playback immediately, discarding buffered
data. Basically it's the same as sio_stop() but doesn't wait. The plan
is to make players use it.

In the network protocol, sio_flush() is implemented by adding a flag
to the message corresponding to sio_stop(). Old sndiod servers ignore
it and just work with new libraries. New sndiod servers see that the
flag is not set by old libraries and properly drain the play buffer.

Tested with mplayer, mpv and audacious, if we go this way other ports
will follow.

ok?

Index: include/sndio.h
===
RCS file: /cvs/src/include/sndio.h,v
retrieving revision 1.13
diff -u -p -r1.13 sndio.h
--- include/sndio.h 28 Jun 2020 05:21:38 -  1.13
+++ include/sndio.h 23 Mar 2022 20:23:29 -
@@ -164,6 +164,7 @@ size_t sio_write(struct sio_hdl *, const
 size_t sio_read(struct sio_hdl *, void *, size_t);
 int sio_start(struct sio_hdl *);
 int sio_stop(struct sio_hdl *);
+int sio_flush(struct sio_hdl *);
 int sio_nfds(struct sio_hdl *);
 int sio_pollfd(struct sio_hdl *, struct pollfd *, int);
 int sio_revents(struct sio_hdl *, struct pollfd *);
Index: lib/libsndio/Symbols.map
===
RCS file: /cvs/src/lib/libsndio/Symbols.map,v
retrieving revision 1.2
diff -u -p -r1.2 Symbols.map
--- lib/libsndio/Symbols.map26 Feb 2020 13:53:58 -  1.2
+++ lib/libsndio/Symbols.map23 Mar 2022 20:23:29 -
@@ -11,6 +11,7 @@
sio_read;
sio_start;
sio_stop;
+   sio_flush;
sio_nfds;
sio_pollfd;
sio_revents;
Index: lib/libsndio/amsg.h
===
RCS file: /cvs/src/lib/libsndio/amsg.h,v
retrieving revision 1.14
diff -u -p -r1.14 amsg.h
--- lib/libsndio/amsg.h 1 Nov 2021 14:43:24 -   1.14
+++ lib/libsndio/amsg.h 23 Mar 2022 20:23:29 -
@@ -96,6 +96,9 @@ struct amsg {
 #define AMSG_DATAMAX   0x1000
uint32_t size;
} data;
+   struct amsg_stop {
+   uint8_t drain;
+   } stop;
struct amsg_ts {
int32_t delta;
} ts;
Index: lib/libsndio/shlib_version
===
RCS file: /cvs/src/lib/libsndio/shlib_version,v
retrieving revision 1.12
diff -u -p -r1.12 shlib_version
--- lib/libsndio/shlib_version  26 Feb 2020 13:53:58 -  1.12
+++ lib/libsndio/shlib_version  23 Mar 2022 20:23:29 -
@@ -1,2 +1,2 @@
 major=7
-minor=1
+minor=2
Index: lib/libsndio/sio.c
===
RCS file: /cvs/src/lib/libsndio/sio.c,v
retrieving revision 1.26
diff -u -p -r1.26 sio.c
--- lib/libsndio/sio.c  1 Nov 2021 14:43:24 -   1.26
+++ lib/libsndio/sio.c  23 Mar 2022 20:23:29 -
@@ -129,6 +129,8 @@ sio_start(struct sio_hdl *hdl)
 int
 sio_stop(struct sio_hdl *hdl)
 {
+   if (hdl->ops->stop == NULL)
+   return sio_flush(hdl);
if (hdl->eof) {
DPRINTF("sio_stop: eof\n");
return 0;
@@ -139,6 +141,28 @@ sio_stop(struct sio_hdl *hdl)
return 0;
}
if (!hdl->ops->stop(hdl))
+   return 0;
+#ifdef DEBUG
+   DPRINTFN(2, "libsndio: polls: %llu, samples = %llu\n",
+   hdl->pollcnt, hdl->cpos);
+#endif
+   hdl->started = 0;
+   return 1;
+}
+
+int
+sio_flush(struct sio_hdl *hdl)
+{
+   if (hdl->eof) {
+   DPRINTF("sio_flush: eof\n");
+   return 0;
+   }
+   if (!hdl->started) {
+   DPRINTF("sio_flush: not started\n");
+   hdl->eof = 1;
+   return 0;
+   }
+   if (!hdl->ops->flush(hdl))
return 0;
 #ifdef DEBUG
DPRINTFN(2, "libsndio: polls: %llu, samples = %llu\n",
Index: lib/libsndio/sio_aucat.c
===
RCS file: /cvs/src/lib/libsndio/sio_aucat.c,v
retrieving revision 1.20
diff -u -p -r1.20 sio_aucat.c
--- lib/libsndio/sio_aucat.c9 Jan 2016 08:27:24 -   1.20
+++ lib/libsndio/sio_aucat.c23 Mar 2022 20:23:29 -
@@ -49,6 +49,7 @@ struct sio_aucat_hdl {
 static void sio_aucat_close(struct sio_hdl *);
 static int sio_aucat_start(struct sio_hdl *);
 static int sio_aucat_stop(struct sio_hdl *);
+static int sio_aucat_flush(struct sio_hdl *);
 static int sio_aucat_setpar(struct sio_hdl *, struct sio_par *);
 static int 

Re: constify *_hw_if

2022-03-21 Thread Alexandre Ratchov
On Mon, Mar 21, 2022 at 12:09:54PM +, Miod Vallat wrote:
> The following diff makes {audio,midi,radio,video}_hw_if structs in the
> kernel const, in order to move them to rodata.
> 

ok ratchov



Re: wskbd_set_mixervolume

2022-02-11 Thread Alexandre Ratchov
On Thu, Feb 10, 2022 at 04:53:59PM +0100, Anton Lindqvist wrote:
> On Wed, Feb 09, 2022 at 09:13:58AM +0100, Alexandre Ratchov wrote:
> > On Tue, Feb 08, 2022 at 06:59:39PM +0100, Anton Lindqvist wrote:
> > > On Tue, Feb 08, 2022 at 07:32:38AM +0100, Alexandre Ratchov wrote:
> > > > On Mon, Feb 07, 2022 at 06:55:21PM +0100, Anton Lindqvist wrote:
> > > > > On Mon, Feb 07, 2022 at 11:21:43AM +0100, Alexandre Ratchov wrote:
> > > > > > On Sun, Feb 06, 2022 at 08:40:35AM +0100, Anton Lindqvist wrote:
> > > > > > > 
> > > > > > > Polished diff. I'm omitting a necessary refactoring commit making
> > > > > > > audio_attach_mi() accept a new cookie argument.
> > > > > > > 
> > > > > > 
> > > > > > I'm not sure to understand the need for the wskbd_audio structure.
> > > > > > Couldn't we just store the cookie in audio and wskbd softc 
> > > > > > structures,
> > > > > > then pass it in the wskbd_set_mixervolume_sc() calls ?
> > > > > 
> > > > > Due to the device caching the data must be stored in either the audio 
> > > > > or
> > > > > wskbd softc and I don't want to expose the softc structures so I ended
> > > > > up introducing wskbd_audio. Dropping the caching would probably make 
> > > > > it
> > > > > possible to only pass down the cookie to wskbd_set_mixervolume_sc() 
> > > > > and
> > > > > always do the audio device lookup.
> > > > > 
> > > > > Is anyone in favor of this approach? I achieves the expected behavior 
> > > > > in
> > > > > my opinion.
> > > > 
> > > > IMHO, handling the volume keys this way won't work in the general
> > > > case. But for the short term we've no other options, have we?
> > > > 
> > > > AFAICS, you're fixing a concrete use-case by tweaking what already
> > > > exists, this won't make things more broken than they already are. I'm
> > > > OK with it.
> > > 
> > > Here's the complete diff including adding a cookie argument to
> > > audio_attach_mi().
> > 
> > Is the caching necessary? device_lookup() seems cheap and there are at
> > most two devices in most cases. Keeping the code minimal especially on
> > rare and non-performace-critical code-paths would be nice.
> > 
> > If you choose to drop the caching, this would allow to just add a a
> > new "cookie" argument to wskbd_set_mixervolume(), similarly to what
> > you did for audio_attach_mi()
> 
> Sure, I ended up adding a new function after all since the
> wskbd_set_mixervolume() prototype is not present in any header at this
> point.
> 

ok ratchov



Re: wskbd_set_mixervolume

2022-02-09 Thread Alexandre Ratchov
On Tue, Feb 08, 2022 at 06:59:39PM +0100, Anton Lindqvist wrote:
> On Tue, Feb 08, 2022 at 07:32:38AM +0100, Alexandre Ratchov wrote:
> > On Mon, Feb 07, 2022 at 06:55:21PM +0100, Anton Lindqvist wrote:
> > > On Mon, Feb 07, 2022 at 11:21:43AM +0100, Alexandre Ratchov wrote:
> > > > On Sun, Feb 06, 2022 at 08:40:35AM +0100, Anton Lindqvist wrote:
> > > > > 
> > > > > Polished diff. I'm omitting a necessary refactoring commit making
> > > > > audio_attach_mi() accept a new cookie argument.
> > > > > 
> > > > 
> > > > I'm not sure to understand the need for the wskbd_audio structure.
> > > > Couldn't we just store the cookie in audio and wskbd softc structures,
> > > > then pass it in the wskbd_set_mixervolume_sc() calls ?
> > > 
> > > Due to the device caching the data must be stored in either the audio or
> > > wskbd softc and I don't want to expose the softc structures so I ended
> > > up introducing wskbd_audio. Dropping the caching would probably make it
> > > possible to only pass down the cookie to wskbd_set_mixervolume_sc() and
> > > always do the audio device lookup.
> > > 
> > > Is anyone in favor of this approach? I achieves the expected behavior in
> > > my opinion.
> > 
> > IMHO, handling the volume keys this way won't work in the general
> > case. But for the short term we've no other options, have we?
> > 
> > AFAICS, you're fixing a concrete use-case by tweaking what already
> > exists, this won't make things more broken than they already are. I'm
> > OK with it.
> 
> Here's the complete diff including adding a cookie argument to
> audio_attach_mi().

Is the caching necessary? device_lookup() seems cheap and there are at
most two devices in most cases. Keeping the code minimal especially on
rare and non-performace-critical code-paths would be nice.

If you choose to drop the caching, this would allow to just add a a
new "cookie" argument to wskbd_set_mixervolume(), similarly to what
you did for audio_attach_mi()



Re: wskbd_set_mixervolume

2022-02-07 Thread Alexandre Ratchov
On Mon, Feb 07, 2022 at 06:55:21PM +0100, Anton Lindqvist wrote:
> On Mon, Feb 07, 2022 at 11:21:43AM +0100, Alexandre Ratchov wrote:
> > On Sun, Feb 06, 2022 at 08:40:35AM +0100, Anton Lindqvist wrote:
> > > 
> > > Polished diff. I'm omitting a necessary refactoring commit making
> > > audio_attach_mi() accept a new cookie argument.
> > > 
> > 
> > I'm not sure to understand the need for the wskbd_audio structure.
> > Couldn't we just store the cookie in audio and wskbd softc structures,
> > then pass it in the wskbd_set_mixervolume_sc() calls ?
> 
> Due to the device caching the data must be stored in either the audio or
> wskbd softc and I don't want to expose the softc structures so I ended
> up introducing wskbd_audio. Dropping the caching would probably make it
> possible to only pass down the cookie to wskbd_set_mixervolume_sc() and
> always do the audio device lookup.
> 
> Is anyone in favor of this approach? I achieves the expected behavior in
> my opinion.

IMHO, handling the volume keys this way won't work in the general
case. But for the short term we've no other options, have we?

AFAICS, you're fixing a concrete use-case by tweaking what already
exists, this won't make things more broken than they already are. I'm
OK with it.



Re: wskbd_set_mixervolume

2022-02-07 Thread Alexandre Ratchov
On Mon, Feb 07, 2022 at 06:58:40PM +0100, Anton Lindqvist wrote:
> On Mon, Feb 07, 2022 at 11:43:43AM +0100, Alexandre Ratchov wrote:
> > On Sat, Feb 05, 2022 at 06:30:43PM +0100, Claudio Jeker wrote:
> > > On Sat, Feb 05, 2022 at 12:28:08PM +0100, Mark Kettenis wrote:
> > > > > Date: Sat, 5 Feb 2022 09:29:42 +0100
> > > > > From: Anton Lindqvist 
> > > > > 
> > > > > Hi,
> > > > > I recently got a USB headset with physical volume buttons, handled by
> > > > > ucc(4). However, after enabling the device in sndiod the volume 
> > > > > buttons
> > > > > does not cause the volume to change. Turns out wskbd_set_mixervolume()
> > > > > is only propagating volume changes to first attached audio device. Is
> > > > > their any good not to consider all attached audio devices?
> > > > 
> > > > I think this is tricky.  The mixer values of different audio devices
> > > > may start out differently and may have different granularity and
> > > > probably operate on a different scale.  This may lead to situations
> > > > where as you turn the volume up and down, the relative output volume
> > > > between devices changes considerably.  I also think that your
> > > > implementation will unmute all audio devices as soon as you touch the
> > > > volume control buttons, which is probably not desirable.
> > > > 
> > > > Thinking about other ways to do this, we could:
> > > > 
> > > > - Add a knob that allows the user to control which audio device is
> > > >   controlled by the volume control buttons.  The choice could include
> > > >   "none" and "all" as well as the individual devices.
> > > > 
> > > > - Add infrastructure to bind specific keyboards to specific audio
> > > >   devices, a bit like how we support binding specific wskbd devices to
> > > >   specific wsdisplay devices.
> > > > 
> > > > The first suggestion is probably relatively easy to achieve.  The
> > > > implementation of the latter would defenitely need more thought and
> > > > discussion.
> > > > 
> > > > The "none" choice above would (partly) solve another issue where
> > > > userland applications see the key presses and act upon them even
> > > > though the kernel already did the volume adjustment.
> > > 
> > > There is a 3rd option of passing the information to sndiod and let it do
> > > the volume scaling.
> > >  
> > 
> > Here's a small WIP program to do so. It also adds a key to cycle
> > through audio devices.
> > 
> > https://github.com/ratchov/sndiokeys
> > 
> > By default it uses Ctrl+Alt+{plus,minus,0} because multimedia keys are
> > already used.
> > 
> > Anton, I've no headphones with buttons to test, does this work for
> > yours?
> 
> Interesting! I cannot get to run under X, it fails in XGetKeyboardMapping:
> 
> (gdb) r
> Starting program: /tmp/sndiokeys/sndiokeys 
> X Error of failed request:  BadAccess (attempt to access private resource 
> denied)
>   Major opcode of failed request:  33 (X_GrabKey)
>   Serial number of failed request:  12
>   Current serial number in output stream:  76
> 
> Breakpoint 1, _libc_exit (status=1) at /home/src2/lib/libc/stdlib/exit.c:54
> 54  /home/src2/lib/libc/stdlib/exit.c: No such file or directory.
> (gdb) bt
> #0  _libc_exit (status=1) at /home/src2/lib/libc/stdlib/exit.c:54
> #1  0x0cfba76512b1 in _XDefaultError () from /usr/X11R6/lib/libX11.so.17.1
> #2  0x0cfba76514a9 in _XError () from /usr/X11R6/lib/libX11.so.17.1
> #3  0x0cfba764d47a in handle_response () from 
> /usr/X11R6/lib/libX11.so.17.1
> #4  0x0cfba764dd50 in _XReply () from /usr/X11R6/lib/libX11.so.17.1
> #5  0x0cfba762d8ba in XGetKeyboardMapping () from 
> /usr/X11R6/lib/libX11.so.17.1
> #6  0x0cf94a4778f7 in grab_keys () at sndiokeys.c:318
> #7  0x0cf94a477279 in main (argc=0, argv=0x7f7dd760) at 
> sndiokeys.c:529
> 
> 

One of the keys is already used (aka grabbed) by another X
program. You've to either stop the other program or start sndiokeys
before it.

Meanwhile, I'll try to add error handling and print a meaningful error
message.



Re: wskbd_set_mixervolume

2022-02-07 Thread Alexandre Ratchov
On Sat, Feb 05, 2022 at 06:30:43PM +0100, Claudio Jeker wrote:
> On Sat, Feb 05, 2022 at 12:28:08PM +0100, Mark Kettenis wrote:
> > > Date: Sat, 5 Feb 2022 09:29:42 +0100
> > > From: Anton Lindqvist 
> > > 
> > > Hi,
> > > I recently got a USB headset with physical volume buttons, handled by
> > > ucc(4). However, after enabling the device in sndiod the volume buttons
> > > does not cause the volume to change. Turns out wskbd_set_mixervolume()
> > > is only propagating volume changes to first attached audio device. Is
> > > their any good not to consider all attached audio devices?
> > 
> > I think this is tricky.  The mixer values of different audio devices
> > may start out differently and may have different granularity and
> > probably operate on a different scale.  This may lead to situations
> > where as you turn the volume up and down, the relative output volume
> > between devices changes considerably.  I also think that your
> > implementation will unmute all audio devices as soon as you touch the
> > volume control buttons, which is probably not desirable.
> > 
> > Thinking about other ways to do this, we could:
> > 
> > - Add a knob that allows the user to control which audio device is
> >   controlled by the volume control buttons.  The choice could include
> >   "none" and "all" as well as the individual devices.
> > 
> > - Add infrastructure to bind specific keyboards to specific audio
> >   devices, a bit like how we support binding specific wskbd devices to
> >   specific wsdisplay devices.
> > 
> > The first suggestion is probably relatively easy to achieve.  The
> > implementation of the latter would defenitely need more thought and
> > discussion.
> > 
> > The "none" choice above would (partly) solve another issue where
> > userland applications see the key presses and act upon them even
> > though the kernel already did the volume adjustment.
> 
> There is a 3rd option of passing the information to sndiod and let it do
> the volume scaling.
>  

Here's a small WIP program to do so. It also adds a key to cycle
through audio devices.

https://github.com/ratchov/sndiokeys

By default it uses Ctrl+Alt+{plus,minus,0} because multimedia keys are
already used.

Anton, I've no headphones with buttons to test, does this work for
yours?



Re: wskbd_set_mixervolume

2022-02-07 Thread Alexandre Ratchov
On Sun, Feb 06, 2022 at 08:40:35AM +0100, Anton Lindqvist wrote:
> 
> Polished diff. I'm omitting a necessary refactoring commit making
> audio_attach_mi() accept a new cookie argument.
> 

I'm not sure to understand the need for the wskbd_audio structure.
Couldn't we just store the cookie in audio and wskbd softc structures,
then pass it in the wskbd_set_mixervolume_sc() calls ?

> diff --git sys/dev/audio.c sys/dev/audio.c
> index 64696025a50..f2b40637771 100644
> --- sys/dev/audio.c
> +++ sys/dev/audio.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include  /* struct wskbd_audio */
>  #include "audio.h"
>  #include "wskbd.h"
>  
> @@ -96,6 +97,8 @@ struct wskbd_vol
>  #define WSKBD_MUTE_DISABLE   2
>  #define WSKBD_MUTE_ENABLE3
>  };
> +
> +int wskbd_set_mixervolume_unit(int, long, long);
>  #endif
>  
>  /*
> @@ -2455,10 +2458,6 @@ wskbd_mixer_init(struct audio_softc *sc)
>   };
>   int i;
>  
> - if (sc->dev.dv_unit != 0) {
> - DPRINTF("%s: not configuring wskbd keys\n", DEVNAME(sc));
> - return;
> - }
>   for (i = 0; i < sizeof(spkr_names) / sizeof(spkr_names[0]); i++) {
>   if (wskbd_initvol(sc, >spkr,
>   spkr_names[i].cn, spkr_names[i].dn))
> @@ -2569,13 +2568,55 @@ wskbd_set_mixermute(long mute, long out)
>   return 0;
>  }
>  
> +int
> +wskbd_set_mixervolume_sc(struct wskbd_audio *audio, long dir, long out)
> +{
> + if (audio->dev != NULL) {
> + if ((audio->dev->dv_flags & DVF_ACTIVE) == 0) {
> + /* Audio device gone, fallback to audio0. */
> + device_unref(audio->dev);
> + audio->dev = NULL;
> + audio->unit = 0;
> + }
> + } else if (audio->cookie != NULL) {
> + void *cookie;
> + int i;
> +
> + cookie = audio->cookie;
> + audio->cookie = NULL;
> + for (i = 0; i < audio_cd.cd_ndevs; i++) {
> + struct audio_softc *sc;
> +
> + sc = (struct audio_softc *)device_lookup(_cd, i);
> + if (sc == NULL)
> + continue;
> + if (sc->cookie != cookie) {
> + device_unref(>dev);
> + continue;
> + }
> +
> + audio->dev = (struct device *)sc;
> + audio->unit = i;
> + break;
> + }
> + }
> +
> + return wskbd_set_mixervolume_unit(audio->unit, dir, out);
> +}
> +
>  int
>  wskbd_set_mixervolume(long dir, long out)
> +{
> + return wskbd_set_mixervolume_unit(0, dir, out);
> +}
> +
> +int
> +wskbd_set_mixervolume_unit(int unit, long dir, long out)
>  {
>   struct audio_softc *sc;
>   struct wskbd_vol *vol;
>  
> - sc = (struct audio_softc *)device_lookup(_cd, 0);
> + sc = (struct audio_softc *)device_lookup(_cd, unit);
>   if (sc == NULL)
>   return ENODEV;
>   vol = out ? >spkr : >mic;
> diff --git sys/dev/usb/uaudio.c sys/dev/usb/uaudio.c
> index 0a19d512a5c..d86019311e0 100644
> --- sys/dev/usb/uaudio.c
> +++ sys/dev/usb/uaudio.c
> @@ -3841,7 +3841,7 @@ uaudio_attach(struct device *parent, struct device 
> *self, void *aux)
>   /* print a nice uaudio attach line */
>   uaudio_print(sc);
>  
> - audio_attach_mi(_hw_if, sc, NULL, >dev);
> + audio_attach_mi(_hw_if, sc, arg->cookie, >dev);
>  }
>  
>  int
> diff --git sys/dev/usb/ucc.c sys/dev/usb/ucc.c
> index f23f32990bb..705a31b327b 100644
> --- sys/dev/usb/ucc.c
> +++ sys/dev/usb/ucc.c
> @@ -104,7 +104,7 @@ void  ucc_attach(struct device *, struct device *, 
> void *);
>  int  ucc_detach(struct device *, int);
>  void ucc_intr(struct uhidev *, void *, u_int);
>  
> -void ucc_attach_wskbd(struct ucc_softc *);
> +void ucc_attach_wskbd(struct ucc_softc *, void *);
>  int  ucc_enable(void *, int);
>  void ucc_set_leds(void *, int);
>  int  ucc_ioctl(void *, u_long, caddr_t, int, struct proc *);
> @@ -680,7 +680,7 @@ ucc_attach(struct device *parent, struct device *self, 
> void *aux)
>  
>   /* Cannot load an empty map. */
>   if (sc->sc_maplen > 0)
> - ucc_attach_wskbd(sc);
> + ucc_attach_wskbd(sc, uha->uaa->cookie);
>  }
>  
>  int
> @@ -772,7 +772,7 @@ unknown:
>  }
>  
>  void
> -ucc_attach_wskbd(struct ucc_softc *sc)
> +ucc_attach_wskbd(struct ucc_softc *sc, void *cookie)
>  {
>   static const struct wskbd_accessops accessops = {
>   .enable = ucc_enable,
> @@ -784,6 +784,7 @@ ucc_attach_wskbd(struct ucc_softc *sc)
>   .keymap = >sc_keymap,
>   .accessops  = ,
>   .accesscookie   = sc,
> + .audiocookie= cookie,
>   };
>  
>   sc->sc_keydesc[0].name = KB_US;
> diff --git sys/dev/usb/usb_subr.c sys/dev/usb/usb_subr.c
> index 7d8480f0f01..fc5808bfeb1 

Re: wskbd_set_mixervolume

2022-02-06 Thread Alexandre Ratchov
On Sat, Feb 05, 2022 at 06:30:43PM +0100, Claudio Jeker wrote:
> On Sat, Feb 05, 2022 at 12:28:08PM +0100, Mark Kettenis wrote:
> > > Date: Sat, 5 Feb 2022 09:29:42 +0100
> > > From: Anton Lindqvist 
> > > 
> > > Hi,
> > > I recently got a USB headset with physical volume buttons, handled by
> > > ucc(4). However, after enabling the device in sndiod the volume buttons
> > > does not cause the volume to change. Turns out wskbd_set_mixervolume()
> > > is only propagating volume changes to first attached audio device. Is
> > > their any good not to consider all attached audio devices?
> > 
> > I think this is tricky.  The mixer values of different audio devices
> > may start out differently and may have different granularity and
> > probably operate on a different scale.  This may lead to situations
> > where as you turn the volume up and down, the relative output volume
> > between devices changes considerably.  I also think that your
> > implementation will unmute all audio devices as soon as you touch the
> > volume control buttons, which is probably not desirable.
> > 
> > Thinking about other ways to do this, we could:
> > 
> > - Add a knob that allows the user to control which audio device is
> >   controlled by the volume control buttons.  The choice could include
> >   "none" and "all" as well as the individual devices.
> > 
> > - Add infrastructure to bind specific keyboards to specific audio
> >   devices, a bit like how we support binding specific wskbd devices to
> >   specific wsdisplay devices.
> > 
> > The first suggestion is probably relatively easy to achieve.  The
> > implementation of the latter would defenitely need more thought and
> > discussion.
> > 
> > The "none" choice above would (partly) solve another issue where
> > userland applications see the key presses and act upon them even
> > though the kernel already did the volume adjustment.
> 
> There is a 3rd option of passing the information to sndiod and let it do
> the volume scaling.
>  

Agreed, this will fix most volume keys-related problems.



Re: wskbd_set_mixervolume

2022-02-06 Thread Alexandre Ratchov
On Sat, Feb 05, 2022 at 09:29:42AM +0100, Anton Lindqvist wrote:
> Hi,
> I recently got a USB headset with physical volume buttons, handled by
> ucc(4). However, after enabling the device in sndiod the volume buttons
> does not cause the volume to change. Turns out wskbd_set_mixervolume()
> is only propagating volume changes to first attached audio device. Is
> their any good not to consider all attached audio devices?

This is a hack to make pc keyboard volume keys mimic volume keys of
old laptops, which used to simply control internal speaker/headphones
level. Hence the use of the first device (in most cases the internal).

> The diff below gives me the desired behavior by propagating volume
> changes to all attached audio devices.
> 

Your diff is correct.

However it's based on a broken hack, so it won't work as expected:
Currently volume keys change the volume but also propagate to X as
XF86XK_Audio{Raise,Lower}Volume events which in turn may change the
volume a second time in a different way, possibly of different
device. To increase further the confusion, X keys auto-repeat, while
volume key changes through wskbd_set_mixervolume() don't.

For instance, in one terminal start "sndioctl -m" to monitor volume
changes then play something with ports/mplayer and observe volume
changes:

$ sndioctl -m 
...
output[0].level=0.322   # changed
output[1].level=0.322   # changed
app/mplayer0.level=0.567# changed
output[0].level=0.353   # changed
output[1].level=0.353   # changed
app/mplayer0.level=0.583# changed

IMHO, adding a sysctl or wsconsctl knob, as kettenis@ suggested, would
make your keys "work", and allow to start experimenting with proper
user-space solution.



Re: sndiod: -F does not switch back to preferred device

2021-12-25 Thread Alexandre Ratchov
On Sat, Dec 25, 2021 at 02:51:08PM +, Klemens Nanni wrote:
> 
> either "devices that are" or "device that is", I'd say the latter.
> 

commited with "device that is".

BTW, the example of the -F option description seems more appropriate
for the new hot plugging section.

OK?

diff --git a/sndiod/sndiod.8 b/sndiod/sndiod.8
index b39d021..5226d80 100644
--- a/sndiod/sndiod.8
+++ b/sndiod/sndiod.8
@@ -193,11 +193,6 @@ the one given with the previous
 or
 .Fl F
 option will be used.
-For instance, specifying a USB device following a
-PCI device allows
-.Nm
-to use the USB one preferably when it's connected
-and to fall back to the PCI one when it's disconnected.
 .It Fl f Ar device
 Add this
 .Xr sndio 7
@@ -539,6 +534,15 @@ Instead,
 must be used to change the
 .Va server.device
 control.
+.Pp
+For instance, specifying a USB device with
+.Fl F
+following a PCI device with
+.Fl f
+allows
+.Nm
+to use the USB one preferably when it's connected
+and to fall back to the PCI one when it's disconnected.
 .Sh EXAMPLES
 Start server using default parameters, creating an
 additional sub-device for output to channels 2:3 only (rear speakers



Re: sndiod: -F does not switch back to preferred device

2021-12-25 Thread Alexandre Ratchov
On Sat, Dec 25, 2021 at 01:34:19PM +, Klemens Nanni wrote:
> 
> This reads OK kn, but I suspect others will object to the hotplugd(8)
> reference.
> 

here's a new one, with attach/detach replaced by plug/unplug and no
ref to hotplug(8)

OK?

diff --git a/sndiod/sndiod.8 b/sndiod/sndiod.8
index 1f95097..c31da66 100644
--- a/sndiod/sndiod.8
+++ b/sndiod/sndiod.8
@@ -185,8 +185,10 @@ Only the signedness and the precision are mandatory.
 Examples:
 .Va u8 , s16le , s24le3 , s24le4lsb .
 .It Fl F Ar device
-Specify an alternate device to use.
-If it doesn't work, the one given with the previous
+Same as
+.Fl f
+except that if the device is disconnected,
+the one given with the previous
 .Fl f
 or
 .Fl F
@@ -196,11 +198,6 @@ PCI device allows
 .Nm
 to use the USB one preferably when it's connected
 and to fall back to the PCI one when it's disconnected.
-Alternate devices may be switched with the
-.Va server.device
-control of the
-.Xr sndioctl 1
-utility.
 .It Fl f Ar device
 Add this
 .Xr sndio 7
@@ -528,6 +525,20 @@ behave normally, while streams connected to
 wait for the MMC start signal and start synchronously.
 Regardless of which device a stream is connected to,
 its playback volume knob is exposed.
+.Sh HOT PLUGGING
+If devices specified with
+.Fl F
+are unavailable when needed or unplugged at runtime,
+.Nm
+will attempt to seamlessly fall back to the last device specified.
+.Pp
+.Nm
+will not automatically switch to specified devices that is plugged at runtime.
+Instead,
+.Xr sndioctl 1
+must be used to change the
+.Va server.device
+control.
 .Sh EXAMPLES
 Start server using default parameters, creating an
 additional sub-device for output to channels 2:3 only (rear speakers



switch sndiod & aucat to 24-bit ?

2021-12-21 Thread Alexandre Ratchov
This diff switches internal sample representation frem 16-bit to
24-bit fixed-point. Resampling is already 24-bit only, so there's
little point in keeping the current 16-bit code.

The default device precision doesn't change, i.e. we still request the
same 16-bit mode (and convert everything to 24-bit when needed), so
there's no risk of exposing driver bugs (yet).

If you've the 24-bit capable hardware, adding "-e s24" to sndiod_flags
will reduce the quantization noise, without the need for rebuilding
with -DADATA_BITS=24 anymore.

objections? OK?

Index: sndiod/dsp.h
===
RCS file: /cvs/src/usr.bin/sndiod/dsp.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 dsp.h
--- sndiod/dsp.h25 May 2021 08:06:12 -  1.10
+++ sndiod/dsp.h21 Dec 2021 17:53:12 -
@@ -25,28 +25,14 @@
  * boundary is excluded. We represent them as signed fixed point numbers
  * of ADATA_BITS. We also assume that 2^(ADATA_BITS - 1) fits in a int.
  */
-#ifndef ADATA_BITS
-#define ADATA_BITS 16
-#endif
+#define ADATA_BITS 24
 #define ADATA_LE   (BYTE_ORDER == LITTLE_ENDIAN)
 #define ADATA_UNIT (1 << (ADATA_BITS - 1))
 
-#if ADATA_BITS == 16
-
-#define ADATA_MUL(x,y) (((int)(x) * (int)(y)) >> (ADATA_BITS - 1))
-
-typedef short adata_t;
-
-#elif ADATA_BITS == 24
-
 #define ADATA_MUL(x,y) \
((int)(((long long)(x) * (long long)(y)) >> (ADATA_BITS - 1)))
 
 typedef int adata_t;
-
-#else
-#error "only 16-bit and 24-bit precisions are supported"
-#endif
 
 /*
  * The FIR is sampled and stored in a table of fixed-point numbers
Index: sndiod/sndiod.8
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 sndiod.8
--- sndiod/sndiod.8 18 Dec 2021 21:41:49 -  1.12
+++ sndiod/sndiod.8 21 Dec 2021 17:53:13 -
@@ -564,11 +564,6 @@ $ sndiod -r 48000 -b 480 -z 240
 Resampling is low quality; down-sampling especially should be avoided
 when recording.
 .Pp
-Processing is done using 16-bit arithmetic,
-thus samples with more than 16 bits are rounded.
-16 bits (i.e. 97dB dynamic) are largely enough for most applications though.
-Processing precision can be increased to 24-bit at compilation time though.
-.Pp
 If
 .Fl a Ar off
 is used,
Index: sndiod/sndiod.c
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v
retrieving revision 1.47
diff -u -p -u -p -r1.47 sndiod.c
--- sndiod/sndiod.c 1 Nov 2021 14:43:25 -   1.47
+++ sndiod/sndiod.c 21 Dec 2021 17:53:13 -
@@ -85,6 +85,13 @@
 #define DEFAULT_BUFSZ  7680
 #endif
 
+/*
+ * default device precision
+ */
+#ifndef DEFAULT_BITS
+#define DEFAULT_BITS   16
+#endif
+
 void sigint(int);
 void sighup(int);
 void opt_ch(int *, int *);
@@ -486,7 +493,11 @@ main(int argc, char **argv)
pmax = 1;
rmin = 0;
rmax = 1;
-   aparams_init();
+   par.bits = DEFAULT_BITS;
+   par.bps = APARAMS_BPS(par.bits);
+   par.le = ADATA_LE;
+   par.sig = 1;
+   par.msb = 0;
mode = MODE_PLAY | MODE_REC;
dev_first = dev_next = NULL;
port_first = port_next = NULL;
Index: aucat/aucat.1
===
RCS file: /cvs/src/usr.bin/aucat/aucat.1,v
retrieving revision 1.116
diff -u -p -u -p -r1.116 aucat.1
--- aucat/aucat.1   22 Apr 2020 05:37:00 -  1.116
+++ aucat/aucat.1   21 Dec 2021 17:53:13 -
@@ -88,7 +88,7 @@ Increase log verbosity.
 .It Fl e Ar enc
 Encoding of the audio file.
 The default is
-.Va s16 .
+.Va s24 .
 Encoding names use the following scheme: signedness
 .Po
 .Va s
Index: aucat/aucat.c
===
RCS file: /cvs/src/usr.bin/aucat/aucat.c,v
retrieving revision 1.177
diff -u -p -u -p -r1.177 aucat.c
--- aucat/aucat.c   12 Jan 2021 15:46:53 -  1.177
+++ aucat/aucat.c   21 Dec 2021 17:53:13 -
@@ -1393,7 +1393,11 @@ main(int argc, char **argv)
rate = DEFAULT_RATE;
cmin = 0;
cmax = 1;
-   aparams_init();
+   par.bits = ADATA_BITS;
+   par.bps = APARAMS_BPS(par.bits);
+   par.le = ADATA_LE;
+   par.sig = 1;
+   par.msb = 1;
hdr = AFILE_HDR_AUTO;
n_flag = 0;
port = NULL;
Index: aucat/dsp.h
===
RCS file: /cvs/src/usr.bin/aucat/dsp.h,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 dsp.h
--- aucat/dsp.h 25 May 2021 08:06:12 -  1.8
+++ aucat/dsp.h 21 Dec 2021 17:53:13 -
@@ -25,28 +25,14 @@
  * boundary is excluded. We represent them as signed fixed point numbers
  * of ADATA_BITS. We also assume that 2^(ADATA_BITS - 1) fits in a int.
  */
-#ifndef ADATA_BITS
-#define ADATA_BITS  

Re: sndiod: -F does not switch back to preferred device

2021-12-20 Thread Alexandre Ratchov
On Fri, Dec 17, 2021 at 07:11:41PM +, Klemens Nanni wrote:
> 
> So we've concluded that the hotplpugging framework needs work, fine.
> 
> I'd still like to improve sndiod(8) regarding its own behaviour.
> 
> How about the same diff modulo hotplug referencing/examples?
> This helps me understand `-f' and `-F' usage better.
> 
> Feedback? Objections? OK?
>
> Index: sndiod.8
> ===
> RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
> retrieving revision 1.11
> diff -u -p -r1.11 sndiod.8
> --- sndiod.8  16 Jul 2021 15:05:58 -  1.11
> +++ sndiod.8  17 Dec 2021 19:02:21 -
> @@ -185,7 +185,7 @@ Only the signedness and the precision ar
>  Examples:
>  .Va u8 , s16le , s24le3 , s24le4lsb .
>  .It Fl F Ar device
> -Specify an alternate device to use.
> +Specify a preferred device to use on startup.
>  If it doesn't work, the one given with the last
>  .Fl f
>  or

Recently we dropped the "alternate device name" hack and now -F and -f
are the same, except that -f may fail, while -F tries the next
device. AFAICS, -F will disappear completely soon.

What about just saying it's the same as -f?

(BTW, the paragraph about USB may be moved to the new "HOT PLUGGING"
section, if we go this way)

> @@ -528,6 +528,24 @@ behave normally, while streams connected
>  wait for the MMC start signal and start synchronously.
>  Regardless of which device a stream is connected to,
>  its playback volume knob is exposed.
> +.Sh HOT PLUGGING
> +.Nm
> +If preferred devices specified with
> +.Fl F
> +are unavailable at startup or detach at runtime,
> +.Nm
> +will attempt to seamlessly fall back to the last device specified.
> +.Pp
> +.Nm
> +will not automatically switch to specified devices that attach at runtime.
> +Instead,
> +.Xr sndioctl 1
> +must be used to change the
> +.Va server.device
> +control.
> +.Pp
> +.Xr hotplugd 8
> +can be used to implement seamless switching at runtime.
>  .Sh EXAMPLES
>  Start server using default parameters, creating an
>  additional sub-device for output to channels 2:3 only (rear speakers
> 
> 

"At startup" suggests this is when the daemon starts, imho "When a
device is needed" is more accurate (devices are kept closed when not
used, so when a device is needed again, iteration over -Ff restarts).

With these tweaks, I end-up with the diff below.

As this is a the "hot plugging" section, would it make sense to use
plugged/unplugged instead of attach/detach words?


diff --git a/sndiod/sndiod.8 b/sndiod/sndiod.8
index 910ce12..7dd72e5 100644
--- a/sndiod/sndiod.8
+++ b/sndiod/sndiod.8
@@ -185,8 +185,10 @@ Only the signedness and the precision are mandatory.
 Examples:
 .Va u8 , s16le , s24le3 , s24le4lsb .
 .It Fl F Ar device
-Specify an alternate device to use.
-If it doesn't work, the one given with the previous
+Same as
+.Fl f
+except that if the device is disconnected,
+the one given with the previous
 .Fl f
 or
 .Fl F
@@ -196,11 +198,6 @@ PCI device allows
 .Nm
 to use the USB one preferably when it's connected
 and to fall back to the PCI one when it's disconnected.
-Alternate devices may be switched with the
-.Va server.device
-control of the
-.Xr sndioctl 1
-utility.
 .It Fl f Ar device
 Add this
 .Xr sndio 7
@@ -528,6 +525,23 @@ behave normally, while streams connected to
 wait for the MMC start signal and start synchronously.
 Regardless of which device a stream is connected to,
 its playback volume knob is exposed.
+.Sh HOT PLUGGING
+If devices specified with
+.Fl F
+are unavailable when needed or detach at runtime,
+.Nm
+will attempt to seamlessly fall back to the last device specified.
+.Pp
+.Nm
+will not automatically switch to specified devices that attach at runtime.
+Instead,
+.Xr sndioctl 1
+must be used to change the
+.Va server.device
+control.
+.Pp
+.Xr hotplugd 8
+can be used to implement seamless switching at runtime.
 .Sh EXAMPLES
 Start server using default parameters, creating an
 additional sub-device for output to channels 2:3 only (rear speakers



Re: sndiod: Remove ambiguous use of 'last' in man page

2021-12-18 Thread Alexandre Ratchov
On Sat, Dec 18, 2021 at 10:27:34AM +, Jason McIntyre wrote:
> On Sat, Dec 18, 2021 at 10:51:28AM +0100, Richard Ulmer wrote:
> > Hi,
> > after reading about using USB audio interfaces in
> > https://www.openbsd.org/faq/faq13.html I looked up what -F and -f mean
> > in sndiod(8) but was briefly confused by the word 'last'. I commonly
> > understand 'last' as either 'previous' or 'at the end of the list' and
> > in this case the latter popped into my head first, which is the wrong
> > meaning in this case (if I'm not misunderstanding things).
> > 
> 
> hi.
> 
> i agree that "previous" is probably less ambiguous, so i'm ok with that
> change.
> 
> > Maybe the attached patch could prevent this confusion. I also changed
> > 'options' to singular, because it seems to me like it refers to the
> > (single) previous -f/-F option.
> > 
> 
> this one really depends on how the options are thought of, so either
> version is ok really. but with your proposed word change of "previous",
> singulAr does read better.
> 
> so, i'm fine with this diff and happy to commit. i'll let it sit a
> little in case anyone has an issue (i.e. we've both misunderstood the
> text!)
> 

thanks, both changes makes the man page better explain what the
code does

ok ratchov



Re: sndiod: -F does not switch back to preferred device

2021-12-09 Thread Alexandre Ratchov
On Thu, Dec 09, 2021 at 09:02:07PM -0700, Theo de Raadt wrote:
> 
> I think drivers, or maybe this can be done in higher-level subsystems, need
> to be modified such that events get sent when a device is *actually ready*,
> and the call from config_attach() should be deleted.
> 

For audio and MIDI, I think we're not very far from this.

Currently, as soon as audio_attach() returns, both hardware and
audio(4) internal structures are properly initialized. So all the
audio_{open,read,write,ioctl,close}() interfaces are safe to call.

What happens in higher layers is unclear to me? for instance, what
needs to be done for open(2) to reach audioopen() with no races?



Re: sndiod: -F does not switch back to preferred device

2021-12-09 Thread Alexandre Ratchov
On Thu, Dec 09, 2021 at 02:04:06PM +0100, Solene Rapenne wrote:
> 
> where does sndioctl server.device= come from?
> 
> on my system I don't have the server.device property
> 

There's one server.device knob for each "-s" to controls on which
device are player (or recorded from).

> > sndioctl server.device
> server.device: no such control
> 
> I use this line in rc.conf.local
> sndiod_flags=-f rsnd/0 -m play,rec,mon -s rec -s mon -F rsnd/1
> 
> 

Do you have an AUDIODEVICE environment variable set? If so, could you
verify that it's set to "snd/rec", "snd/mon" or "snd/default"?

These are supposed to work:

sndioctl -f snd/mon server.device
sndioctl -f snd/rec server.device
sndioctl -f snd/default server.device

While these are not:

sndioctl -f snd/0 server.device
sndioctl -f snd/1 server.device

because the 0 or 1 _is_ the device, so there's no device to choose.

(this applies to -current, above were added after release)



Re: sndiod: -F does not switch back to preferred device

2021-12-09 Thread Alexandre Ratchov
On Wed, Dec 08, 2021 at 10:30:08PM +, Klemens Nanni wrote:
> Following https://www.openbsd.org/faq/faq13.html#usbaudio and reading
> sndiod(8)'s
> 
>   -F device
>   Specify an alternate device to use.  If it doesn't work, the one
>   given with the last -f or -F options will be used.  For 
> instance,
>   specifying a USB device following a PCI device allows sndiod to
>   use the USB one preferably when it's connected and to fall back
>   to the PCI one when it's disconnected.  Alternate devices may be
>   switched with the server.device control of the sndioctl(1)
>   utility.
> 
> I configured things as follows in order to play audio via USB and
> fall back to internal sound if USB is not available:
> 
>   $ rcctl get sndiod flags
>   -f rsnd/0 -F rsnd/1
> 
> Plugging in an USB headset and restarting sndiod or forcing the device
> with `sndioctl server.device=1' then plays sound via USB.
> 
> Unplugging the device makes playback fall back to internal sound.
> 
> But plugging USB back in does not prefer USB to internal as I'd expect
> now. I am currently resorting to the following hotplugd(8) script to
> always select the USB sound device whenever I plug it in:
> 
>   #!/bin/ksh
>   set -Cefu -o pipefail
> 
>   readonly DEVCLASS=$1 DEVNAME=$2
>   typeset -i devid
> 
>   case "${DEVCLASS}-${DEVNAME}" in
>   0-audio*)   # switch sndio(4) to USB headset when plugging it in
>   devid=${DEVNAME#audio}
>   sndioctl server.device=${devid}
>   ;;
>   esac
> 

IFAIU, audio devices always match the audio[0-9] pattern, so testing
the device class is not necessary, is it? FWIW, I use a similar
script.

> I'd expect sndiod to *always* use USB whenever possible.
> 
> Is this uni-directional behaviour of sndiod intentional/by-design?

sndiod doesn't use directly hotplug(4), so it can't obtain by itself a
notification when a new device is plugged in order to switch to
it. Using hotplugd(8) is the right way to make audio hotplug work,
both daemons complete each other.

When sndiod needs the audio device (i.e. when the first client
connects), it will try all alternate devices (reverse -F options
order, the USB headset is first in your case). This gives a false
impression of hotplug support (indeed, you plug the USB headset, start
an audio player and it just works!). But that's just a side effect of
device priority/fail-over: if the internal device is already open when
you plug the USB headset, sndiod won't switch to it until it needs to
reopen the device.

Certain programs (including browsers), tend to keep the device open
even when they remain silent. So when actual playback starts sndiod
doesn't need to open a device and doesn't "see" the new USB headset. I
guess that's what happens on your system, but hotplugd(8) handles
this.

When devices are unplugged, we don't need hotplug because the device
stops working (input/output error) and sndiod switch to the next one
of the fail-over list.

> If so, can we clarify the manual?

Sure. While -F option description seems exact, maybe we need an extra
paragraph or FAQ entry to explain how to use it with other tools like
hotplugd and sndioctl. What about this wording?

HOT-PLUG SUPPORT
 If a device is unplugged while in use, sndiod will attempt to switch to
 one of the alternate devices (-F), if any.  This is seamless to programs
 connected to sndiod.

 Later, when the device is connected again, the server.device control of
 the sndioctl(1) utility could be used to switch back to it, without the
 need to restart all audio programs.  This last step could be automated
 using hotplugd(8).  For instance, if sndiod is started with:

   $ sndiod -f rsnd/0 -F rsnd/1 -F rsnd/2 -F rsnd/3

 then, the following hotplugd(8) attach script could be used to
 automatically switch to the last connected device:

   #!/bin/sh

   DEVNAME=$2

   case $DEVNAME in
   audio[0-3])
   sndioctl server.device=${DEVNAME#audio}
   ;;
   esac

Any opinions where to put such information?



better audio defaults: please test

2021-11-04 Thread Alexandre Ratchov
The current sndiod latency (minimum time between when the program
plays something and when sound reaches Joe's ears) is too large and
makes OpenBSD unpleasant to use for telephony, games, and makes
controls of video players slugish.

The defaut latency (of 160ms) was set ~10 years ago to workaround
various problems: KERNEL_LOCK used to block audio processing for very
long, azalia(4) and uaudio(4) were unable to recover after an error,
which aggravated the problem.

The kernel improved a lot the last decade and such large buffers are
not necessary anymore. I think something between 20ms and 40ms is a
better default for the average OpenBSD system:

 * audio-conferencing software and games requires no sndiod_flags
   tweaks anymore

 * on modern machines (like my 7 years old i5-2500K) building a kernel
   doesn't make audio stutterer

 * sndiod_flags tweaks will still be needed for:
- very slow or overloaded machines used for audio
- machines running heavy/bogus SMM code
- real-time synths & effects (20ms is still too small)

Please try to switch you system to 40ms buffers (i.e. 1920 samples at
the default 48kHz rate), for instance either apply diff below or
simply do:

rcctl set sndiod flags -z 480 -b 1920
rcctl restart sndiod

then report any significant increase of stuttering, and what
software/hardware triggers it. If you think 20ms or 30ms (i.e. 960 and
1440 sample buffers) are better, let me know as well.

Note that OpenBSD is not real-time (neither are programs we run) so
audio may stutter no matter how large the buffers are. The goal here
is to get a ballance between disconfort caused by latency and
probability of stuttering for the average OpenBSD system.

If we reach a consensus, here's the diff to make above settings the
default.

OK?

Index: sndiod.c
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v
retrieving revision 1.47
diff -u -p -r1.47 sndiod.c
--- sndiod.c1 Nov 2021 14:43:25 -   1.47
+++ sndiod.c1 Nov 2021 15:28:37 -
@@ -82,7 +82,7 @@
  * buffer size if neither ``-z'' nor ``-b'' is used
  */
 #ifndef DEFAULT_BUFSZ
-#define DEFAULT_BUFSZ  7680
+#define DEFAULT_BUFSZ  1920
 #endif
 
 void sigint(int);



midi: defer selwakeup() calls to softintr to fix locking bug

2021-10-29 Thread Alexandre Ratchov
selwakeup() needs to be protected by KERNEL_LOCK, but we're not
allowed to grab KERNEL_LOCK on interrupt context because midi runs at
IPL_AUDIO with the audio_lock held.

Furthermore, doing so is a locking order bug: syscall code-path grabs
KERNEL_LOCK first while interrupt code-path does the opposite when
calling selwakeup().

Fix this for midi(4) as we did years ago for audio(4): defer
selwakeup() calls to a softintr. This diff is mostly copied from
audio(4) to make both look similar.

ok?

Index: midi.c
===
RCS file: /cvs/src/sys/dev/midi.c,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 midi.c
--- midi.c  29 Oct 2021 13:24:50 -  1.49
+++ midi.c  29 Oct 2021 15:26:30 -
@@ -32,6 +32,8 @@
 #include 
 #include 
 
+#define IPL_SOFTMIDI   IPL_SOFTNET
+#define DEVNAME(sc)((sc)->dev.dv_xname)
 
 intmidiopen(dev_t, int, int, struct proc *);
 intmidiclose(dev_t, int, int, struct proc *);
@@ -84,6 +86,25 @@ const struct filterops midiread_filtops 
 };
 
 void
+midi_buf_wakeup(void *addr)
+{
+   struct midi_buffer *buf = addr;
+
+   if (buf->blocking) {
+   wakeup(>blocking);
+   buf->blocking = 0;
+   }
+   /*
+* As long as selwakeup() grabs the KERNEL_LOCK() make sure it is
+* already held here to avoid lock ordering problems with `audio_lock'
+*/
+   KERNEL_ASSERT_LOCKED();
+   mtx_enter(_lock);
+   selwakeup(>sel);
+   mtx_leave(_lock);
+}
+
+void
 midi_iintr(void *addr, int data)
 {
struct midi_softc  *sc = (struct midi_softc *)addr;
@@ -97,13 +118,14 @@ midi_iintr(void *addr, int data)
return; /* discard data */
 
MIDIBUF_WRITE(mb, data);
-   if (mb->used == 1) {
-   if (sc->rchan) {
-   sc->rchan = 0;
-   wakeup(>rchan);
-   }
-   selwakeup(>rsel);
-   }
+
+   /*
+* As long as selwakeup() needs to be protected by the
+* KERNEL_LOCK() we have to delay the wakeup to another
+* context to keep the interrupt context KERNEL_LOCK()
+* free.
+*/
+   softintr_schedule(sc->inbuf.softintr);
 }
 
 int
@@ -131,9 +153,9 @@ midiread(dev_t dev, struct uio *uio, int
error = EWOULDBLOCK;
goto done_mtx;
}
-   sc->rchan = 1;
-   error = msleep_nsec(>rchan, _lock, PWAIT | PCATCH,
-   "mid_rd", INFSLP);
+   sc->inbuf.blocking = 1;
+   error = msleep_nsec(>inbuf.blocking, _lock,
+   PWAIT | PCATCH, "mid_rd", INFSLP);
if (!(sc->dev.dv_flags & DVF_ACTIVE))
error = EIO;
if (error)
@@ -206,11 +228,14 @@ void
 midi_out_stop(struct midi_softc *sc)
 {
sc->isbusy = 0;
-   if (sc->wchan) {
-   sc->wchan = 0;
-   wakeup(>wchan);
-   }
-   selwakeup(>wsel);
+
+   /*
+* As long as selwakeup() needs to be protected by the
+* KERNEL_LOCK() we have to delay the wakeup to another
+* context to keep the interrupt context KERNEL_LOCK()
+* free.
+*/
+   softintr_schedule(sc->outbuf.softintr);
 }
 
 void
@@ -276,8 +301,8 @@ midiwrite(dev_t dev, struct uio *uio, in
 */
goto done_mtx;
}
-   sc->wchan = 1;
-   error = msleep_nsec(>wchan, _lock,
+   sc->outbuf.blocking = 1;
+   error = msleep_nsec(>outbuf.blocking, _lock,
PWAIT | PCATCH, "mid_wr", INFSLP);
if (!(sc->dev.dv_flags & DVF_ACTIVE))
error = EIO;
@@ -327,9 +352,9 @@ midipoll(dev_t dev, int events, struct p
}
if (revents == 0) {
if (events & (POLLIN | POLLRDNORM))
-   selrecord(p, >rsel);
+   selrecord(p, >inbuf.sel);
if (events & (POLLOUT | POLLWRNORM))
-   selrecord(p, >wsel);
+   selrecord(p, >outbuf.sel);
}
mtx_leave(_lock);
device_unref(>dev);
@@ -349,11 +374,11 @@ midikqfilter(dev_t dev, struct knote *kn
error = 0;
switch (kn->kn_filter) {
case EVFILT_READ:
-   klist = >rsel.si_note;
+   klist = >inbuf.sel.si_note;
kn->kn_fop = _filtops;
break;
case EVFILT_WRITE:
-   klist = >wsel.si_note;
+   klist = >outbuf.sel.si_note;
kn->kn_fop = _filtops;
break;
default:
@@ -376,7 +401,7 @@ filt_midirdetach(struct knote *kn)
struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
 

Re: Please test: full poll/select(2) switch

2021-10-29 Thread Alexandre Ratchov
On Fri, Oct 29, 2021 at 01:12:06PM +0100, Martin Pieuchot wrote:
> On 29/10/21(Fri) 13:12, Alexandre Ratchov wrote:
> > On Sat, Oct 23, 2021 at 10:40:56AM +0100, Martin Pieuchot wrote:
> > > Diff below switches both poll(2) and select(2) to the kqueue-based
> > > implementation.
> > > 
> > > In addition it switches libevent(3) to use poll(2) by default for
> > > testing purposes.
> > > 
> > > I don't have any open bug left with this diff and I'm happily running
> > > GNOME with it.  So I'd be happy if you could try to break it and report
> > > back.
> > > 
> > 
> > Without the below diff (copied from audio(4) driver), kernel panics
> > upon the first MIDI input byte.
> 
> What is the panic?  The mutex is taken recursively, right?
>  

Exactly, this is the "locking against myself", panic.

AFAIU, the interrupt handler grabs the audio_lock and calls
midi_iintr(). It calls selwakeup(), which in turn calls
filt_midiread(), which attempts to grab the audio_lock a second time.

> > ok? suggestion for a better fix?
> 
> Without seeing the panic, I'm guessing this is correct.
> 
> That suggest kevent(2) wasn't safe to use with midi(4).
> 

Yes, this is the very first time midi(4) is used with kevent(2).



Re: Please test: full poll/select(2) switch

2021-10-29 Thread Alexandre Ratchov
On Sat, Oct 23, 2021 at 10:40:56AM +0100, Martin Pieuchot wrote:
> Diff below switches both poll(2) and select(2) to the kqueue-based
> implementation.
> 
> In addition it switches libevent(3) to use poll(2) by default for
> testing purposes.
> 
> I don't have any open bug left with this diff and I'm happily running
> GNOME with it.  So I'd be happy if you could try to break it and report
> back.
> 

Without the below diff (copied from audio(4) driver), kernel panics
upon the first MIDI input byte.

ok? suggestion for a better fix?

Index: midi.c
===
RCS file: /cvs/src/sys/dev/midi.c,v
retrieving revision 1.48
diff -u -p -r1.48 midi.c
--- midi.c  25 Dec 2020 12:59:52 -  1.48
+++ midi.c  29 Oct 2021 11:09:47 -
@@ -386,9 +386,11 @@ filt_midiread(struct knote *kn, long hin
struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
int retval;
 
-   mtx_enter(_lock);
+   if ((hint & NOTE_SUBMIT) == 0)
+   mtx_enter(_lock);
retval = !MIDIBUF_ISEMPTY(>inbuf);
-   mtx_leave(_lock);
+   if ((hint & NOTE_SUBMIT) == 0)
+   mtx_leave(_lock);
 
return (retval);
 }
@@ -409,9 +411,11 @@ filt_midiwrite(struct knote *kn, long hi
struct midi_softc *sc = (struct midi_softc *)kn->kn_hook;
intretval;
 
-   mtx_enter(_lock);
+   if ((hint & NOTE_SUBMIT) == 0)
+   mtx_enter(_lock);
retval = !MIDIBUF_ISFULL(>outbuf);
-   mtx_leave(_lock);
+   if ((hint & NOTE_SUBMIT) == 0)
+   mtx_leave(_lock);
 
return (retval);
 }




sndio: initial bits for multiple devices support, please test

2021-10-25 Thread Alexandre Ratchov
Hi,

Current sndiod logic supports one device only: If two -f are used then
two independent instances of the whole server are created, each with
its own set of clients, options, controls, and so on. This diff shifts
sndiod towards a more flexible (and conceptually simpler) model:

  - clients connect to logical devices (created with -s option)

  - logical devices route data to/from physical devices registered
with -f option

This logic removes obstacles to working on multiple devices support,
including a single client using one device for recording and another
for playback.

This logic makes not only sndiod internals simpler but allows to
simplify sndio(7) naming scheme. Instead of:

"snd/."

we only need:

"snd/"

where  becomes the logical device (or "virtual device" to use
Ingo's words). This doesn't affect setup using default. For those
using an advanced configuration, there is a compatibility layer making
the change almost transparent.

This change also makes -F and -f rougly the same: logical device can
be migrated from one physical device to another, so the need for the
whole "alternate device" hack^Wconcept vanishes.

Documentation change is the bare minimum. Man pages could be
simplified, I'll do so as soon as MIDI has received the same
treatment (next step).

The diff is big, but it mostly moves existing bits from one object to
another, so there's little risk of breaking things, but please test
and report regressions, if any.

Index: usr.bin/sndiod/dev.c
===
RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
retrieving revision 1.102
diff -u -p -u -p -r1.102 dev.c
--- usr.bin/sndiod/dev.c3 May 2021 04:29:50 -   1.102
+++ usr.bin/sndiod/dev.c23 Oct 2021 13:40:34 -
@@ -45,16 +45,13 @@ struct dev *dev_new(char *, struct apara
 unsigned int, unsigned int, unsigned int, unsigned int);
 void dev_adjpar(struct dev *, int, int, int);
 int dev_allocbufs(struct dev *);
-int dev_open(struct dev *);
 void dev_freebufs(struct dev *);
-void dev_close(struct dev *);
 int dev_ref(struct dev *);
 void dev_unref(struct dev *);
 int dev_init(struct dev *);
 void dev_done(struct dev *);
 struct dev *dev_bynum(int);
 void dev_del(struct dev *);
-void dev_setalt(struct dev *, unsigned int);
 unsigned int dev_roundof(struct dev *, unsigned int);
 void dev_wakeup(struct dev *);
 
@@ -930,10 +927,8 @@ dev_new(char *path, struct aparams *par,
return NULL;
}
d = xmalloc(sizeof(struct dev));
-   d->alt_list = NULL;
-   dev_addname(d,path);
+   d->path = path;
d->num = dev_sndnum++;
-   d->alt_num = -1;
 
d->reqpar = *par;
d->reqmode = mode;
@@ -948,6 +943,7 @@ dev_new(char *path, struct aparams *par,
d->slot_list = NULL;
d->master = MIDI_MAXCTL;
d->master_enabled = 0;
+   d->alt_next = d;
snprintf(d->name, CTL_NAMEMAX, "%u", d->num);
for (pd = _list; *pd != NULL; pd = &(*pd)->next)
;
@@ -957,51 +953,6 @@ dev_new(char *path, struct aparams *par,
 }
 
 /*
- * add a alternate name
- */
-int
-dev_addname(struct dev *d, char *name)
-{
-   struct dev_alt *a;
-
-   if (d->alt_list != NULL && d->alt_list->idx == DEV_NMAX - 1) {
-   log_puts(name);
-   log_puts(": too many alternate names\n");
-   return 0;
-   }
-   a = xmalloc(sizeof(struct dev_alt));
-   a->name = name;
-   a->idx = (d->alt_list == NULL) ? 0 : d->alt_list->idx + 1;
-   a->next = d->alt_list;
-   d->alt_list = a;
-   return 1;
-}
-
-/*
- * set prefered alt device name
- */
-void
-dev_setalt(struct dev *d, unsigned int idx)
-{
-   struct dev_alt **pa, *a;
-
-   /* find alt with given index */
-   for (pa = >alt_list; (a = *pa)->idx != idx; pa = >next)
-   ;
-
-   /* detach from list */
-   *pa = a->next;
-
-   /* attach at head */
-   a->next = d->alt_list;
-   d->alt_list = a;
-
-   /* reopen device with the new alt */
-   if (idx != d->alt_num)
-   dev_reopen(d);
-}
-
-/*
  * adjust device parameters and mode
  */
 void
@@ -1099,9 +1050,6 @@ dev_allocbufs(struct dev *d)
 int
 dev_open(struct dev *d)
 {
-   char name[CTL_NAMEMAX];
-   struct dev_alt *a;
-
d->mode = d->reqmode;
d->round = d->reqround;
d->bufsz = d->reqbufsz;
@@ -1123,16 +1071,6 @@ dev_open(struct dev *d)
if (!dev_allocbufs(d))
return 0;
 
-   /* if there are multiple alt devs, add server.device knob */
-   if (d->alt_list->next != NULL) {
-   for (a = d->alt_list; a != NULL; a = a->next) {
-   snprintf(name, sizeof(name), "%d", a->idx);
-   ctl_new(CTL_DEV_ALT, d, >idx,
-   CTL_SEL, d->name, "server", -1, "device",
-   name, -1, 1, a->idx == d->alt_num);
- 

Re: human-readable audio device descriptions

2021-07-07 Thread Alexandre Ratchov
On Wed, Jul 07, 2021 at 03:13:37PM +0100, Edd Barrett wrote:
> Hi,
> 
> This diff is an attempt to make identifying audio devices easier by
> giving each device a human-readable description that can be read from
> userspace (and without scraping dmesg).
> 
> This is implemented on a per-audio-device basis using a new `descr`
> interface to `audio_hw_if`. For now I've only implemented this on:
> 
>  - azalia(4), which uses the codec info, e.g. `Realtek ALC1150`
> 
>  - uaudio(4), which uses the USB vendor and model info, e.g.
>`Logitech HD Pro Webcam C920`.
> 
> For drivers, which currently don't implement this interface, the
> description string defaults to the audio device node name, e.g.
> `uaudio2`.
> 
> The string is exposed to userspace via the AUDIO_GETDEV ioctl(2). I've
> reclaimed the space currently occupied by the (unused) `version` and
> `config` fields in `audio_device_t` and used it for a new `descr` field.
> Thus the size of `audio_device_t` remains the same for now.
> 
> I've also made audioctl(8) display the description string, e.g.:
> ```
> # audioctl   
> name=azalia1
> descr=Realtek ALC1150
> ...
> # audioctl -f /dev/audioctl1 
> name=uaudio0
> descr=SteelSeries SteelSeries Arctis 
> ...
> ```
> 
> I'm hoping that later we can have sndiod read these strings and expose
> them to its clients. For example, I'd like for sndioctl(1) to allow
> choosing a device based on the device description string (instead of the
> sndio device index), and for xfce4-mixer to allow choosing a device via
> its description in the GUI using a drop-down box.
> 
> Diff follows.
> 

[...]

> @@ -4019,6 +4021,37 @@ azalia_set_nblks(void *v, int mode,
>   nblks = HDA_BDL_MAX;
>  
>   return nblks;
> +}
> +
> +void
> +azalia_descr(void *arg, char *descr, size_t len)
> +{
> + azalia_t *az = arg;
> + codec_t *codec;
> + const char *vendor;
> + char buf[MAX_AUDIO_DESCR_LEN];
> + int i;
> +
> + *descr = '\0';
> + for (i = 0; i < az->ncodecs; i++) {

azalia(4) fetches all codecs, but uses (and supports) only one of
them. You should copy the name of the one in use, ie
az->codecs[az->codecno].

> + codec = >codecs[i];
> + /* adapted from azalia_print_codec() */
> + if (codec->name == NULL) {
> + vendor = pci_findvendor(codec->vid >> 16);
> + if (vendor == NULL)
> + snprintf(buf, MAX_AUDIO_DESCR_LEN,
> + "0x%04x/0x%04x", codec->vid >> 16,
> + codec->vid & 0x);
> + else
> + snprintf(buf, MAX_AUDIO_DESCR_LEN, "%s/0x%04x",
> + vendor, codec->vid & 0x);
> + } else
> + snprintf(buf, MAX_AUDIO_DESCR_LEN, "%s", codec->name);
> +
> + if (i > 0)
> + strlcat(descr, ", ", len);
> + strlcat(descr, buf, len);
> + }
>  }
>  
>  int
> Index: sys/sys/audioio.h
> ===
> RCS file: /cvs/src/sys/sys/audioio.h,v
> retrieving revision 1.27
> diff -u -p -r1.27 audioio.h
> --- sys/sys/audioio.h 14 Sep 2016 06:12:20 -  1.27
> +++ sys/sys/audioio.h 7 Jul 2021 10:23:09 -
> @@ -76,10 +76,10 @@ struct audio_status {
>   * audio devices.
>   */
>  #define MAX_AUDIO_DEV_LEN16
> +#define MAX_AUDIO_DESCR_LEN  32
>  typedef struct audio_device {
>   char name[MAX_AUDIO_DEV_LEN];
> - char version[MAX_AUDIO_DEV_LEN];
> - char config[MAX_AUDIO_DEV_LEN];
> + char descr[MAX_AUDIO_DESCR_LEN]; /* device description string */
>  } audio_device_t;
>

The intent of the "name" field was to be human-readable as well. You
could just replace it. There's no benefit in having two fields for the
same thing.



Re: human-readable audio device descriptions

2021-07-07 Thread Alexandre Ratchov
On Wed, Jul 07, 2021 at 08:25:16AM -0600, Theo de Raadt wrote:
> > This diff is an attempt to make identifying audio devices easier by
> > giving each device a human-readable description that can be read from
> > userspace (and without scraping dmesg).
> 
> But why does anyone want that?
> 
> Isn't everyone served best when there is a public portable interface,
> and all the back-ends must try to work the same?

Exactly. The long-term aim is to expose this information to programs
through sndiod with a stable libsndio interface, which in turn would
ease switching between audio devices.

This diff is the first step; IMO, it's not necessary until we get the
whole chain, but it doesn't hurt either; it improves audioctl(8)
output, which is a dignostics-only tool.



Re: add PCI IDs for Thinkpad X1 Extreme Gen 3, enable WLAN

2021-04-19 Thread Alexandre Ratchov
On Thu, Apr 15, 2021 at 03:24:56PM +0200, Ivo Sbalzarini wrote:
> Thanks a lot! 
> 
> I also got the sound working on this machine now. As far as I 
> can tell, both speakers work and Dolby Atmos works, too, thanks 
> to the awesome quirk by Joshua Stein (jcs@).
> 
> I’m not sure I did everything right here, though it works. 
> Below is the diff.
> 
> cheers,
> - ivo
> 
> 
> Index: azalia.c
> ===
> RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> retrieving revision 1.259
> diff -u -p -r1.259 azalia.c
> --- azalia.c  25 Oct 2020 07:22:06 -  1.259
> +++ azalia.c  15 Apr 2021 13:18:07 -
> @@ -490,7 +490,8 @@ azalia_configure_pci(azalia_t *az)
>  
>  const struct pci_matchid azalia_pci_devices[] = {
>   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_200SERIES_U_HDA },
> - { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }
> + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA },
> + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_CAVS }
>  };
>  
>  int
> Index: azalia_codec.c
> ===
> RCS file: /cvs/src/sys/dev/pci/azalia_codec.c,v
> retrieving revision 1.183
> diff -u -p -r1.183 azalia_codec.c
> --- azalia_codec.c16 Jan 2021 07:02:39 -  1.183
> +++ azalia_codec.c15 Apr 2021 13:18:07 -
> @@ -162,9 +162,16 @@ azalia_codec_init_vtbl(codec_t *this)
>   break;
>   case 0x10ec0285:
>   this->name = "Realtek ALC285";
> - if (this->subid == 0x229217aa)   /* Thinkpad X1 Carbon 
> 7 */
> - this->qrks |= AZ_QRK_ROUTE_SPKR2_DAC |
> - AZ_QRK_WID_CLOSE_PCBEEP;
> + if (PCI_VENDOR(this->subid) == PCI_VENDOR_LENOVO) {
> +if (this->subid == 0x22c017aa) { /* Thinkpad X1 Extreme 
> 3 */
> +this->name = "Realtek ALC3286";
> +this->qrks |= AZ_QRK_DOLBY_ATMOS |
> +   AZ_QRK_ROUTE_SPKR2_DAC;
> + } 
> + else if (this->subid == 0x229217aa)  /* Thinkpad X1 Carbon 
> 7 */
> +this->qrks |= AZ_QRK_ROUTE_SPKR2_DAC |
> +AZ_QRK_WID_CLOSE_PCBEEP;
> +  }
>   break;
>   case 0x10ec0287:
>   this->name = "Realtek ALC287";
> 
> 
> 

Thanks for looking at this. Few comments:

- the "PCI_VENDOR(this->subid) == PCI_VENDOR_LENOVO" compares the 16
  lower bits of the subid. It's not necessary because below we compare
  the full 32 bits.

- the "name" field is the codec name; it is identified by the "vid"
  field, so it shouldn't depend on the device subid.

- style: indentation is (8 char) TAB, see style(9).

With above tweaks, I ended up this diff. Could you confirm it still
makes audio work?

Index: azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.259
diff -u -p -u -p -r1.259 azalia.c
--- azalia.c25 Oct 2020 07:22:06 -  1.259
+++ azalia.c19 Apr 2021 15:37:32 -
@@ -490,7 +490,8 @@ azalia_configure_pci(azalia_t *az)
 
 const struct pci_matchid azalia_pci_devices[] = {
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_200SERIES_U_HDA },
-   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }
+   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA },
+   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_CAVS }
 };
 
 int
Index: azalia_codec.c
===
RCS file: /cvs/src/sys/dev/pci/azalia_codec.c,v
retrieving revision 1.183
diff -u -p -u -p -r1.183 azalia_codec.c
--- azalia_codec.c  16 Jan 2021 07:02:39 -  1.183
+++ azalia_codec.c  19 Apr 2021 15:37:32 -
@@ -162,9 +162,15 @@ azalia_codec_init_vtbl(codec_t *this)
break;
case 0x10ec0285:
this->name = "Realtek ALC285";
-   if (this->subid == 0x229217aa)   /* Thinkpad X1 Carbon 
7 */
+   if (this->subid == 0x229217aa) {
+   /* Thinkpad X1 Carbon 7 */
this->qrks |= AZ_QRK_ROUTE_SPKR2_DAC |
AZ_QRK_WID_CLOSE_PCBEEP;
+} else if (this->subid == 0x22c017aa) {
+   /* Thinkpad X1 Extreme 3 */
+   this->qrks |= AZ_QRK_DOLBY_ATMOS |
+   AZ_QRK_ROUTE_SPKR2_DAC;
+   }
break;
case 0x10ec0287:
this->name = "Realtek ALC287";



Re: sndiod: allow mixing of duplex, record-only and play-only audio devices

2021-02-28 Thread Alexandre Ratchov
On Sat, Feb 27, 2021 at 09:59:21PM +, Edd Barrett wrote:
> 
> This is mostly a no-op for sndiod in the default configuration (except
> that play-only devices now accept recording clients). If you use
> alternative devices (-F), then it's possible for a record-only device to
> be found first, which may be confusing if you just want to hear sound.
> We can only assume that if you deviate from defaults, then you know what
> you are doing.

thanks for fixing this, ok ratchov



Re: sndiod: Move controls out of the device structure, please test & review

2021-01-29 Thread Alexandre Ratchov
On Fri, Jan 29, 2021 at 12:42:37PM +0100, Alexandre Ratchov wrote:
> Moving to a global server-wide controls list is necessary to expose
> controls that are not associated to a particular device (ex. a device
> selector).
> 
> The current hack to use the device-side sioctl_desc->addr variable as
> client-side key can't work anymore. So, we use a unique dynamically
> allocated ctl->addr key; this is which much cleaner. A new "scope"
> enum (with two "void *" arguments) is used to determine what the
> control does control. This adds a lot of flexibility and allows to
> easily add new control types that are not associated to devices.
> 
> The diff touches many parts of the code and is quite big. Please test
> and report regressions.
> 

Thanks for the feedback, new diff below. It fixes a server crash when
a client issues an invalid request.

Index: dev.c
===
RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
retrieving revision 1.89
diff -u -p -u -p -r1.89 dev.c
--- dev.c   29 Jan 2021 11:38:23 -  1.89
+++ dev.c   29 Jan 2021 15:15:38 -
@@ -106,6 +106,7 @@ struct slotops zomb_slotops = {
zomb_exit
 };
 
+struct ctl *ctl_list = NULL;
 struct dev *dev_list = NULL;
 unsigned int dev_sndnum = 0;
 
@@ -370,12 +371,14 @@ dev_midi_master(struct dev *d)
master = d->master;
else {
master = 0;
-   for (c = d->ctl_list; c != NULL; c = c->next) {
+   for (c = ctl_list; c != NULL; c = c->next) {
if (c->type != CTL_NUM ||
-   strcmp(c->group, "") != 0 ||
+   strcmp(c->group, d->name) != 0 ||
strcmp(c->node0.name, "output") != 0 ||
strcmp(c->func, "level") != 0)
continue;
+   if (c->u.any.arg0 != d)
+   continue;
v = (c->curval * 127 + c->maxval / 2) / c->maxval;
if (master < v)
master = v;
@@ -465,7 +468,7 @@ dev_midi_omsg(void *arg, unsigned char *
slot_array[chan].opt->dev != d)
return;
slot_setvol(slot_array + chan, msg[2]);
-   dev_onval(d, CTLADDR_SLOT_LEVEL(chan), msg[2]);
+   ctl_onval(CTL_SLOT_LEVEL, slot_array + chan, NULL, msg[2]);
return;
}
x = (struct sysex *)msg;
@@ -479,7 +482,7 @@ dev_midi_omsg(void *arg, unsigned char *
if (len == SYSEX_SIZE(master)) {
dev_master(d, x->u.master.coarse);
if (d->master_enabled) {
-   dev_onval(d, CTLADDR_MASTER,
+   ctl_onval(CTL_DEV_MASTER, d, NULL,
   x->u.master.coarse);
}
}
@@ -1005,14 +1008,16 @@ dev_master(struct dev *d, unsigned int m
if (d->mode & MODE_PLAY)
dev_mix_adjvol(d);
} else {
-   for (c = d->ctl_list; c != NULL; c = c->next) {
+   for (c = ctl_list; c != NULL; c = c->next) {
+   if (c->scope != CTL_HW || c->u.hw.dev != d)
+   continue;
if (c->type != CTL_NUM ||
-   strcmp(c->group, "") != 0 ||
+   strcmp(c->group, d->name) != 0 ||
strcmp(c->node0.name, "output") != 0 ||
strcmp(c->func, "level") != 0)
continue;
v = (master * c->maxval + 64) / 127;
-   dev_setctl(d, c->addr, v);
+   ctl_setval(c, v);
}
}
 }
@@ -1070,7 +1075,7 @@ dev_new(char *path, struct aparams *par,
d->master = MIDI_MAXCTL;
d->mtc.origin = 0;
d->tstate = MMC_STOP;
-   d->ctl_list = NULL;
+   snprintf(d->name, CTL_NAMEMAX, "%u", d->num);
d->next = dev_list;
dev_list = d;
return d;
@@ -1208,10 +1213,8 @@ dev_allocbufs(struct dev *d)
 int
 dev_open(struct dev *d)
 {
-   int i;
char name[CTL_NAMEMAX];
struct dev_alt *a;
-   struct slot *s;
 
d->master_enabled = 0;
d->mode = d->reqmode;
@@ -1235,23 +1238,12 @@ dev_open(struct dev *d)
if (!dev_allocbufs(d))
return 0;
 
-   for (i = 0, s = slot_array; i < DEV_NSLOT; i++, s++) {
-   

sndiod: Move controls out of the device structure, please test & review

2021-01-29 Thread Alexandre Ratchov
Moving to a global server-wide controls list is necessary to expose
controls that are not associated to a particular device (ex. a device
selector).

The current hack to use the device-side sioctl_desc->addr variable as
client-side key can't work anymore. So, we use a unique dynamically
allocated ctl->addr key; this is which much cleaner. A new "scope"
enum (with two "void *" arguments) is used to determine what the
control does control. This adds a lot of flexibility and allows to
easily add new control types that are not associated to devices.

The diff touches many parts of the code and is quite big. Please test
and report regressions.

Besides that no behavior change.

OK?

Index: dev.c
===
RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
retrieving revision 1.89
diff -u -p -u -p -r1.89 dev.c
--- dev.c   29 Jan 2021 11:38:23 -  1.89
+++ dev.c   29 Jan 2021 11:39:08 -
@@ -106,6 +106,7 @@ struct slotops zomb_slotops = {
zomb_exit
 };
 
+struct ctl *ctl_list = NULL;
 struct dev *dev_list = NULL;
 unsigned int dev_sndnum = 0;
 
@@ -370,12 +371,14 @@ dev_midi_master(struct dev *d)
master = d->master;
else {
master = 0;
-   for (c = d->ctl_list; c != NULL; c = c->next) {
+   for (c = ctl_list; c != NULL; c = c->next) {
if (c->type != CTL_NUM ||
-   strcmp(c->group, "") != 0 ||
+   strcmp(c->group, d->name) != 0 ||
strcmp(c->node0.name, "output") != 0 ||
strcmp(c->func, "level") != 0)
continue;
+   if (c->u.any.arg0 != d)
+   continue;
v = (c->curval * 127 + c->maxval / 2) / c->maxval;
if (master < v)
master = v;
@@ -465,7 +468,7 @@ dev_midi_omsg(void *arg, unsigned char *
slot_array[chan].opt->dev != d)
return;
slot_setvol(slot_array + chan, msg[2]);
-   dev_onval(d, CTLADDR_SLOT_LEVEL(chan), msg[2]);
+   ctl_onval(CTL_SLOT_LEVEL, slot_array + chan, NULL, msg[2]);
return;
}
x = (struct sysex *)msg;
@@ -479,7 +482,7 @@ dev_midi_omsg(void *arg, unsigned char *
if (len == SYSEX_SIZE(master)) {
dev_master(d, x->u.master.coarse);
if (d->master_enabled) {
-   dev_onval(d, CTLADDR_MASTER,
+   ctl_onval(CTL_DEV_MASTER, d, NULL,
   x->u.master.coarse);
}
}
@@ -1005,14 +1008,16 @@ dev_master(struct dev *d, unsigned int m
if (d->mode & MODE_PLAY)
dev_mix_adjvol(d);
} else {
-   for (c = d->ctl_list; c != NULL; c = c->next) {
+   for (c = ctl_list; c != NULL; c = c->next) {
+   if (c->scope != CTL_HW || c->u.hw.dev != d)
+   continue;
if (c->type != CTL_NUM ||
-   strcmp(c->group, "") != 0 ||
+   strcmp(c->group, d->name) != 0 ||
strcmp(c->node0.name, "output") != 0 ||
strcmp(c->func, "level") != 0)
continue;
v = (master * c->maxval + 64) / 127;
-   dev_setctl(d, c->addr, v);
+   ctl_setval(c, v);
}
}
 }
@@ -1070,7 +1075,7 @@ dev_new(char *path, struct aparams *par,
d->master = MIDI_MAXCTL;
d->mtc.origin = 0;
d->tstate = MMC_STOP;
-   d->ctl_list = NULL;
+   snprintf(d->name, CTL_NAMEMAX, "%u", d->num);
d->next = dev_list;
dev_list = d;
return d;
@@ -1208,10 +1213,8 @@ dev_allocbufs(struct dev *d)
 int
 dev_open(struct dev *d)
 {
-   int i;
char name[CTL_NAMEMAX];
struct dev_alt *a;
-   struct slot *s;
 
d->master_enabled = 0;
d->mode = d->reqmode;
@@ -1235,23 +1238,12 @@ dev_open(struct dev *d)
if (!dev_allocbufs(d))
return 0;
 
-   for (i = 0, s = slot_array; i < DEV_NSLOT; i++, s++) {
-   if (s->opt == NULL || s->opt->dev != d || s->name[0] == 0)
-   continue;
-   slot_ctlname(s, name, CTL_NAMEMAX);
-   dev_addctl(d, "app", CTL_NUM,
-   CTLADDR_SLOT_LEVEL(i),
-   name, -1, "level",
-   NULL, -1, 127, s->vol);
-   }
-
/* if there are multiple alt devs, add server.device knob */
if (d->alt_list->next != NULL) {
  

Re: search usbd_interfaces in case of non-compliant device

2021-01-26 Thread Alexandre Ratchov
On Tue, Jan 26, 2021 at 08:56:29PM +, Edd Barrett wrote:
> Hi,
> 
> I've recently come across a uaudio device which doesn't work in OpenBSD:
> 
> uaudio2 at uhub0 port 1 configuration 1 interface 3 "E+ Corp. DAC Audio" rev 
> 1.10/0.01 addr 2
> uaudio2: class v1, full-speed, async, channels: 2 play, 0 rec, 3 ctls
> audio3 at uaudio2
> 
> When opening the audio device, the kernel reports `uaudio0: can't get iface
> handle` in the dmesg buffer.
> 
> I posted information about the device and sthen@ and kettenis@ spotted
> the problem: the device exposes non-consecutive interface numbers, and
> is thus not compliant with the USB standard.
> 
> It has interfaces 0, 1 and 3, in that order. The audio stream is on
> interface 3, which the kernel expects to find at index 3 in the ifaces
> array, but actually it's at index 2!
> 
> I've pasted a load of info about the device (lsusb and debug kernel
> output) here:
> https://gist.github.com/vext01/958756d0fd515cc321f99a3ee1e3351a
> 
> The following diff makes this device work. It searches the ifaces array
> in the event that the correct entry is not found at the expect index.
> This should be a "no functional change" for all existing compliant
> devices.
> 
> I've tested this for the past two days and all seems well, but I'd like
> some USB stack hackers to verify it.
> 

This also fixes another audio device. AFAICS this diff doesn't change
the code-path for compliant devices, so I don't see how this could
cause problems

ok ratchov

(could you split the long comment line, please)



Re: sndiod: (mostly) suppress aliasing noise

2020-12-20 Thread Alexandre Ratchov
On Sun, Dec 20, 2020 at 05:58:22PM +0200, Paul Irofti wrote:
> Hi,
> 
> Interesting diff.
> 
> I did not have time to look at it thoroughly, but here are a few
> observations:
> 

Hi,

Thanks for looking at this.

  - why do you keep the symmetric filter coefficients? (this could halve your
> while-loop computations too, right?)
> 

The filter function is indeed symetric:

h(t) = h(-t)

but they are used at points that are not symetric, we need:

...
h(-2 + x)
h(-1 + x)
h(0 + x)
h(1 + x)
h(2 + x)
...

h(-1 + x) is not the same as h(1 + x), so using the symetry to save
CPU cycles is not easy

I also tried to store only half of the static table to save few bytes;
this makes the code larger (and messy) so the benefit is not that
important for such a small filter order.

>  - the diff's mainlobe look kind of strange at the end, why is that? What
> kind of filter did you use? 
> the side-lobes seem also a bit strange as they
> do not decrease monotonically nor are the first sidelobes small and then
> increasing

To measure the response, we're supposed to apply the filter to a
peak. But on the link I sent the plot it the result of the whole
8k->48k conversion, so the peak is not really peak anymore and the
result is harder to understand.

Here's plot of the Fourier transform of the coefficients in the code:

https://vm.caoua.org/src/coef-dft.png

which should look way more familiar (x-axis is multiples of the
Nyquist frequency, y-axis is the Fourier transform amplitude in dB)

The filter is an ideal "sinc" low-pass with the cut-off frequency set
to 0.75 of the Nyquist frequency, multiplied by a Blackman window of
length 8:

  h(t) = 0.75 * sinc(0.75 * pi * t) * win(t / 8)

where win(t) is the Blackman window function with a = 0.16, defined as:

  win(t) = 0.5 * ((1 - a) + cos(2 * pi * x) + a * cos(4 * pi * x))

At 48kHz, the transition band starts at 0.75 * 24kHz = 18kHz.  Besides
preserving audible frequencies, with this choice the ideal filter's
impulse response (the pure sinc function) crosses zero at the edges of
the window, which makes the result smoother (derivative is 0) and in
turn improves the roll-off.

>   - would be nice to see some SNR comparisons if you can to better show the
> effects of your anti-aliasing effort
> 

Here's an array of measurements I did.

https://vm.caoua.org/src/sweeps.html

The signal and the aliasing amplitudes are represented for each
frequency for various conversion ratios. Anything except the
increasing white curve (the signal, 0dB) is aliasing, the color indicates
its strength.



sndiod: (mostly) suppress aliasing noise

2020-12-19 Thread Alexandre Ratchov
Hi,

The current sndiod resampling algorithm is very basic mainly to keep
CPU usage very low, which used to make sense for the zaurus. So,
resampling produces aliasing noise, easily audible in 8kHz to 48kHz
conversions but present in other cases.

The diff below reduces the aliasing noise. It's a trade-off between
audio quality, CPU consumption and code simplicity. It adds a better
resampling filter (8-th order FIR low-pass), which mostly suppresses
aliasing noise in the audible part of the spectrum.

Few plots here: https://vm.caoua.org/src

I measured the CPU usage of resampling 16-bit stereo from 44.1kHz to
48kHz.

On a ~7 years old i5-2500K:
  - current code:   0.07% CPU
  - with diff:  0.45% CPU

On a ~15 years old Thinkpad x40:
  - current:0.25% CPU
  - with diff:  1.90% CPU

This is still acceptable even for slow machines, IMHO. Enjoy and let
me know if this introduces any regression.

Index: dsp.c
===
RCS file: /cvs/src/usr.bin/sndiod/dsp.c,v
retrieving revision 1.15
diff -u -p -r1.15 dsp.c
--- dsp.c   10 Dec 2020 17:30:49 -  1.15
+++ dsp.c   19 Dec 2020 23:38:24 -
@@ -38,6 +38,75 @@ int aparams_ctltovol[128] = {
26008,  27029,  28090,  29193,  30339,  31530,  32768
 };
 
+int resamp_filt[RESAMP_LENGTH / RESAMP_STEP + 1] = {
+ 0,   0,   3,   9,  22,  42,  73, 116,
+   174, 248, 341, 454, 589, 749, 934,1148,
+  1392,1666,1974,2316,2693,3107,3560,4051,
+  4582,5154,5766,6420,7116,7853,8632,9451,
+ 10311,   11210,   12148,   13123,   14133,   15178,   16253,   17359,
+ 18491,   19647,   20824,   22018,   23226,   24443,   25665,   26888,
+ 28106,   29315,   30509,   31681,   32826,   33938,   35009,   36033,
+ 37001,   37908,   38744,   39502,   40174,   40750,   41223,   41582,
+ 41819,   41925,   41890,   41704,   41358,   40842,   40147,   39261,
+ 38176,   36881,   35366,   33623,   31641,   29411,   26923,   24169,
+ 21140,   17827,   14222,   10317,6105,1580,   -3267,   -8440,
+-13944,  -19785,  -25967,  -32492,  -39364,  -46584,  -54153,  -62072,
+-70339,  -78953,  -87911,  -97209, -106843, -116806, -127092, -137692,
+   -148596, -159795, -171276, -183025, -195029, -207271, -219735, -232401,
+   -245249, -258259, -271407, -284670, -298021, -311434, -324880, -338329,
+   -351750, -365111, -378378, -391515, -404485, -417252, -429775, -442015,
+   -453930, -465477, -476613, -487294, -497472, -507102, -516137, -524527,
+   -532225, -539181, -545344, -550664, -555090, -558571, -561055, -562490,
+   -562826, -562010, -559990, -556717, -552139, -546205, -538866, -530074,
+   -519779, -507936, -494496, -479416, -462652, -444160, -423901, -401835,
+   -377923, -352132, -324425, -294772, -263143, -229509, -193847, -156134,
+   -116348,  -74474,  -30494,   15601,   63822,  114174,  11,  221283,
+278037,  336916,  397911,  461009,  526194,  593446,  662741,  734054,
+807354,  882608,  959779, 1038826, 1119706, 1202370, 1286768, 1372846,
+   1460546, 1549808, 1640566, 1732753, 1826299, 1921130, 2017169, 2114336,
+   2212550, 2311723, 2411770, 2512598, 2614116, 2716228, 2818836, 2921841,
+   3025142, 3128636, 3232218, 3335782, 3439219, 3542423, 3645282, 3747687,
+   3849526, 3950687, 4051059, 4150530, 4248987, 4346320, 4442415, 4537163,
+   4630453, 4722177, 4812225, 4900493, 4986873, 5071263, 5153561, 5233668,
+   5311485, 5386917, 5459872, 5530259, 5597992, 5662986, 5725160, 5784436,
+   5840739, 5893999, 5944148, 5991122, 6034862, 6075313, 6112422, 6146142,
+   6176430, 6203247, 6226559, 6246335, 6262551, 6275185, 6284220, 6289647,
+   6291456, 6289647, 6284220, 6275185, 6262551, 6246335, 6226559, 6203247,
+   6176430, 6146142, 6112422, 6075313, 6034862, 5991122, 5944148, 5893999,
+   5840739, 5784436, 5725160, 5662986, 5597992, 5530259, 5459872, 5386917,
+   5311485, 5233668, 5153561, 5071263, 4986873, 4900493, 4812225, 4722177,
+   4630453, 4537163, 4442415, 4346320, 4248987, 4150530, 4051059, 3950687,
+   3849526, 3747687, 3645282, 3542423, 3439219, 3335782, 3232218, 3128636,
+   3025142, 2921841, 2818836, 2716228, 2614116, 2512598, 2411770, 2311723,
+   2212550, 2114336, 2017169, 1921130, 1826299, 1732753, 1640566, 1549808,
+   1460546, 1372846, 1286768, 1202370, 1119706, 1038826,  959779,  882608,
+807354,  734054,  662741,  593446,  526194,  461009,  397911,  336916,
+278037,  221283,  11,  114174,   63822,   15601,  -30494,  -74474,
+   -116348, -156134, -193847, -229509, -263143, -294772, -324425, -352132,
+   -377923, -401835, -423901, -444160, -462652, -479416, -494496, -507936,
+   -519779, 

Re: delays in sensors thread

2020-12-11 Thread Alexandre Ratchov
On Fri, Dec 11, 2020 at 09:07:45AM +0100, Marcus Glocker wrote:
> 
> After doing some deeper analyzes in to asmc_wait() I agree to that.
> Something seems to go fundamental wrong there.  In every asmc_update()
> execution, I can see asmc_wait() timeout 9 times, always on the
> ASMC_ACCEPT check.  That means out of ~48ms to execute asmc_update(),
> we spend 45ms waiting for timeouts!
> 
> When this can be fixed, I guess we should be fine with the remaining
> ~3ms to execute asmc_update().
> 

Even 3ms seems the wrong order of magnitude; that's ~300 CPU
cycles on a 1GHz processor. This is 3 usb frames, so this will still
hurt audio in low-latency corner cases.



Re: delays in sensors thread

2020-12-10 Thread Alexandre Ratchov
On Thu, Dec 10, 2020 at 05:27:16PM +0100, Marcus Glocker wrote:
> Hi All,
> 
> I recently started to play around with uvideo(4) and uaudio(4) on my
> amd64 iMacs.  There I quickly noticed regular freezes when streaming
> USB video or audio.  On some of those machines it was very frequent,
> like every few seconds the video or audio stream did freeze for ~1s,
> then resume, while the rest of the system did continue to operate fine.
> 
> First I found that when running the machine with an SP kernel, the issue
> disappears.

On SP kernels, interrupts are still working while the CPU is spinning
in kernel mode (as long as current IPL permits it). That's why audio
works better.

> Secondly some debugging hours, and quite some e-mail
> exchanges with mpi@ later, I found that the freeze is getting triggered
> by the asmc(4) driver, specifically by the sensor_task_register()
> update function.  My first intention was to change
> sensor_task_register() to call taskq_create() with the TASKQ_MPSAFE
> flag for a test, to remove the kernel lock, which also resolved the
> freezing with an MP kernel. [1]
> 
> In the end I found that the asmc(4) sensor update code is calling a
> busy loop in asmc_wait(), where the delay call is spending ~50ms in
> average.  Doing that during the KERNEL_LOCK() is resulting in
> noticeable USB ISOC transfer delays.  Obviously replacing the delay(9)
> with tsleep_nsec(9) in asmc(4) did fix the issue as well. [2]
> 
> I'm not sure if just applying diff [2] to the driver is the right
> approach finally or if we need to take a more generic path to address
> this problem.  Any feedback, help, comments appreciated.
> 

Would asmc(4) work if we sleep for very long (tsleep_nsec() has no
upper bound)? Spinning during 50ms in kernel mode doesn't look right,
so using tsleep() looks as a step forward as long as sleeping doesn't
break asmc(4).

AFAIU, calling delay(10) + yield() would have a similar effect.



sio_open.3: clarify what sio_start() does

2020-11-27 Thread Alexandre Ratchov
this wording is shorter and more precise and complete.

ok?

Index: sio_open.3
===
RCS file: /cvs/src/lib/libsndio/sio_open.3,v
retrieving revision 1.51
diff -u -p -r1.51 sio_open.3
--- sio_open.3  20 Nov 2020 12:09:45 -  1.51
+++ sio_open.3  27 Nov 2020 18:02:16 -
@@ -387,17 +387,17 @@ bitmasks should always be used.
 .Ss Starting and stopping the device
 The
 .Fn sio_start
-function puts the device in a waiting state:
-the device will wait for playback data to be provided
-(using the
-.Fn sio_write
-function).
-Once enough data is queued to ensure that play buffers
-will not underrun, actual playback is started automatically.
-If record mode only is selected, then recording starts
-immediately.
+function prepares the devices to start.
+Once the play buffer is full, i.e.
+.Fa sio_par.bufsz
+samples are queued with
+.Fn sio_write ,
+playback starts automatically.
+If record mode only is selected, then
+.Fn sio_start
+starts recording immediately.
 In full-duplex mode, playback and recording will start
-synchronously as soon as enough data to play is available.
+synchronously as soon as the play buffer is full.
 .Pp
 The
 .Fn sio_stop



Re: sndioctl.1: group is optional

2020-11-20 Thread Alexandre Ratchov
On Fri, Nov 20, 2020 at 05:49:00PM +0100, Klemens Nanni wrote:
> Not every control has a group as the manual wording says, i.e.
> 
>   $ sndioctl
>   input.level=0.486
>   input.mute=0
>   output.level=1.000
>   output.mute=0
>   app/aucat0.level=1.000
> 
> Feedack? OK?

OK ratchov@

If the control has a group, the group must be specified (this is
different from channel numbers: not specifying channel number means
select "all channels").

> Index: sndioctl.1
> ===
> RCS file: /cvs/src/usr.bin/sndioctl/sndioctl.1,v
> retrieving revision 1.14
> diff -u -p -r1.14 sndioctl.1
> --- sndioctl.127 May 2020 17:34:45 -  1.14
> +++ sndioctl.120 Nov 2020 16:46:45 -
> @@ -72,8 +72,8 @@ The set of available controls depends on
>  Commands use the following two formats to display and change
>  controls respectively:
>  .Pp
> -.Dl group/stream[channel].function
> -.Dl group/stream[channel].function=value
> +.Dl [group/]stream[channel].function
> +.Dl [group/]stream[channel].function=value
>  .Pp
>  On the left-hand side are specified the control group (if any),
>  the affected stream name, and the optional channel number.
> 
> 



Re: AUDIORECDEVICE environment variable in sndio lib

2020-11-18 Thread Alexandre Ratchov
On Tue, Nov 17, 2020 at 05:09:28PM +, Stuart Henderson wrote:
> On 2020/11/17 17:13, Peter J. Philipp wrote:
> > Hi,
> > 
> > I have a mic on snd/1 and speakers on snd/0.  I had tried a lot of different
> > settings with audacity port but couldn't get this to work, so I chose the
> > method of last resort.  Below is a patch to allow an AUDIORECDEVICE 
> > environment
> > variable specifying the wanted microphone.
> 
> This seems a worthwhile addition.
> 
> > Index: sio.c
> > ===
> > RCS file: /cvs/src/lib/libsndio/sio.c,v
> > retrieving revision 1.24
> > diff -u -p -u -r1.24 sio.c
> > --- sio.c   29 Jun 2019 06:05:26 -  1.24
> > +++ sio.c   17 Nov 2020 16:13:04 -
> > @@ -43,6 +43,7 @@ sio_open(const char *str, unsigned int m
> >  {
> > static char devany[] = SIO_DEVANY;
> > struct sio_hdl *hdl;
> > +   char *rec = NULL;
> >  
> >  #ifdef DEBUG
> > _sndio_debug_init();
> > @@ -55,6 +56,12 @@ sio_open(const char *str, unsigned int m
> > str = getenv("AUDIODEVICE");
> > if (str == NULL)
> > str = devany;
> > +   rec = getenv("AUDIORECDEVICE");
> > +   if (rec == NULL)
> > +   rec = devany;
> > +
> > +   if (rec && mode == SIO_REC)
> > +   str = rec;
> 
> If AUDIORECDEVICE is unset, it would be better to fallback to
> AUDIODEVICE rather than directly to devany.
> 

Thanks; semarie suggested a similar diff, so below is an attempt to
take into account all the suggestions:

- add AUDIOPLAYDEVICE, to handle play-only devices as well. We've the
  very same problem for them.

- use AUDIODEVICE if play-only (rec-only) mode is used but the
  corresponding AUDIOPLAYDEVICE (AUDIORECDEVICE) var is not defined

- minimal update for AUDIO{PLAY,REC}DEVICE in the man page

OK?

Index: sio.c
===
RCS file: /cvs/src/lib/libsndio/sio.c,v
retrieving revision 1.24
diff -u -p -r1.24 sio.c
--- sio.c   29 Jun 2019 06:05:26 -  1.24
+++ sio.c   18 Nov 2020 09:49:24 -
@@ -52,7 +52,12 @@ sio_open(const char *str, unsigned int m
if (str == NULL) /* backward compat */
str = devany;
if (strcmp(str, devany) == 0 && !issetugid()) {
-   str = getenv("AUDIODEVICE");
+   if ((mode & SIO_PLAY) == 0)
+   str = getenv("AUDIORECDEVICE");
+   if ((mode & SIO_REC) == 0)
+   str = getenv("AUDIOPLAYDEVICE");
+   if (mode == (SIO_PLAY | SIO_REC) || str == NULL)
+   str = getenv("AUDIODEVICE");
if (str == NULL)
str = devany;
}
Index: sndio.7
===
RCS file: /cvs/src/lib/libsndio/sndio.7,v
retrieving revision 1.24
diff -u -p -r1.24 sndio.7
--- sndio.7 18 Jul 2020 05:01:14 -  1.24
+++ sndio.7 18 Nov 2020 09:49:24 -
@@ -151,9 +151,11 @@ If
 .Cm default
 is used as the audio device, the program will use the
 one specified in the
-.Ev AUDIODEVICE
-environment variable.
-If it is not set, the program first tries to connect to
+.Ev AUDIODEVICE , AUDIOPLAYDEVICE
+and/or
+.Ev AUDIORECDEVICE
+environment variables.
+If they are not set, the program first tries to connect to
 .Li snd/0 .
 If that fails, it then tries to use
 .Li rsnd/0 .
@@ -190,10 +192,20 @@ and contains 128 bits of raw random data
 If a session needs to be shared between multiple users, they
 can connect to the server using the same cookie.
 .Sh ENVIRONMENT
-.Bl -tag -width "AUDIODEVICEXXX" -compact
+.Bl -tag -width "AUDIOPLAYDEVICE" -compact
 .It Ev AUDIODEVICE
 Audio device descriptor to use
 when no descriptor is explicitly specified to a program.
+.It Ev AUDIOPLAYDEVICE
+Audio device descriptor to use for play-only mode
+when no descriptor is explicitly specified to a program.
+Overrides
+.Ev AUDIODEVICE .
+.It Ev AUDIORECDEVICE
+Audio device descriptor to use for record-only mode
+when no descriptor is explicitly specified to a program.
+Overrides
+.Ev AUDIODEVICE .
 .It Ev MIDIDEVICE
 MIDI port descriptor to use
 when no descriptor is explicitly specified to a program.



Re: mixerctl names

2020-10-19 Thread Alexandre Ratchov
On Sat, Oct 17, 2020 at 05:52:58PM +0200, Jan Stary wrote:
> Currently, mixerctl.conf(5) says
> 
>   Most devices have a number of digital to analogue converters
>   (DACs), used for sound playback, and each DAC has a corresponding
>   output mixer. The mixers are labelled “mix” or “sel”.
> 
> That doesn't seem to be the case, at least not universaly
> as the wording seems to imply. For example, this is
> mixerctl output on a Thinkpad T400:
> 
>   inputs.dac-0:1=222,222
>   inputs.dac-2:3=222,222
>   inputs.beep=0
>   record.adc-2:3_source=mic2
>   record.adc-2:3=219,219
>   record.adc-0:1_source=mic
>   record.adc-0:1=219,219
>   outputs.hp_source=dac-0:1
>   outputs.hp_boost=on
>   inputs.mic=189,189
>   outputs.mic_dir=input-vr80
>   outputs.spkr_source=dac-2:3
>   outputs.spkr_eapd=on
>   inputs.mic2=189,189
>   outputs.hp_sense=unplugged
>   outputs.mic_sense=unplugged
>   outputs.master=240,240
>   outputs.master.mute=off
>   outputs.master.slaves=
>   record.volume=240,240
>   record.volume.mute=off
>   record.volume.slaves=
>   record.enable=sysctl
> 
> Apparently, it has two DACS (for the speakers and the headphones).
> The current wording might confuse the user into thinking he has
> no output mixer, but the
> 
>   inputs.dac-0:1=222,222
>   inputs.dac-2:3=222,222
> 
> do control the respective volumes,
> while no "mix" or "sel" exists.
> 
> Similarly for recording via the two ADCs.
> 

It depends on the hardware. Basically, azalia exposes the schematic of
the mixer and, in turn, the driver exposes all "widgets" to the upper
layers, with a generated name. This is very similar to uaudio.

Certain devices have no "mix_xxx" widgets others have. On one of my
machines:

$ doas mixerctl | egrep '(inputs|outputs).mix' | wc -l 
  20

desktop computers have many jacks and tend to have many
mixers. The "sel_xxx" widgets are rare but still exist.



Re: pledge(2) sndioctl(1)

2020-05-24 Thread Alexandre Ratchov
On Fri, May 22, 2020 at 08:10:54AM +0100, Ricardo Mestre wrote:
> Hello,
> 
> I tried to open the raw device but now it seems I was to sleepy to
> figure out that I couldn't access it due to sndiod(8) having the device
> opened earlier and therefore coundn't reach that code path.
> 
> Here's the audio promise added, but maybe it raises the question again
> if these utilities should have direct access to the devices, and also
> includes sndiod(8) not running when this is done?
> 
> ratchov@ will this change eventually? Otherwise the below keeps that
> code path happy.

As devices nodes are disabled by default for regular users, the
"audio" promise doesn't add additional possibilities for regular
users.  On the other hand, running sndioctl as root on the raw device
is useful for testing and experimenting.

ok ratchov



Re: Audio over hdmi

2020-05-01 Thread Alexandre Ratchov
On Fri, May 01, 2020 at 05:16:10PM +0200, Damien Couderc wrote:
> 
> So if I'm not wrong it could be possible to set the -f option with the
> analog device and the -F option with the digital-only one.
> 
> That said, it would work only if you have two audio device (e.g. HDMI from
> video card as audio0 and analog from soundcard as audio1).
> 
> This is not true on Thinkpad laptops for example because they have two
> output codecs on the same device for both analog and digital (and only one
> is kept actually).
> 
> Maybe we could make first analog and first digital codec available on each
> audio device. Then sndiod would take the first analog and the first digital
> from the devices in the given order. Does it sound sane?

-F is just a tool to *ease* switching between devices, but doesn't
solve the "default device problem" completely. It's intended to be
used as follows:

-f rsnd/0 -F rsnd/1 -F rsnd/2 ...

which might become the default. This way if there's only one
sound-card, it's used. If the user connects a new device, the new one
is used until it's disconnected. That works by pure luck because
attach order maches device priority.

To apply this model to hdmi, there must be a mean to detect if it's
connected to speakers and make it fail if it is not, so it can be
skipped. If we can't detect whether it's connected, we must be sure it
won't be used instead of the others. But even if hdmi has properly
detected speakers, the user may want to use the analog one because it
is more capable, for instance for telephony.

AFAICS, this general problem can't be solved by just adding the hdmi
device with the proper minor number, changes will be needed in many
places of the audio sub-system.

In the short term if you make sure the digital codec and the hdmi-only
device properly work, never attache as audio0 and speakers are
detected, the current defaults will work and nobody will
complain. This would be a big step forward.

my 2 cents



Re: Audio over hdmi

2020-05-01 Thread Alexandre Ratchov
On Fri, May 01, 2020 at 03:01:04PM +0200, Mark Kettenis wrote:
> > Date: Fri, 1 May 2020 14:17:56 +0200
> > From: Alexandre Ratchov 
> > 
> > On Fri, May 01, 2020 at 01:11:16PM +0200, Damien Couderc wrote:
> > > 
> > > Speaking of the hdmi-only devices that were disabled in 2009: does the
> > > project still stand on this position in 2020? I made a quick search and it
> > > seems that more than half of the screens are audio capable now. I 
> > > understand
> > > the defaults back in 2009, but now is it still true?
> > 
> > There's nothing wrong with hdmi-only devices. As long as audio works
> > by default with no tweaks, nobody will object to re-enabling
> > them. AFAIK, this was the only reason to disable them.
> 
> Right.  The main issue was that by default we only send output to
> audio0.  On many machines the audio device associated with the HDMI
> port appears before the audio device that is associated with the
> speakers and/or headphone jack on our laptops.  Therefore by default
> audio would go to an unconnected HDMI.  Just enabling digital-only
> devices would not work.
> 
> There are a couple things we could do here.
> 
> 1. Make sndiod(8) responsible for picking a default output device.
> 
> 2. Use locators (like I did for drm(4) and wsdisplay(4)) such that
>audio0 is always the non-HDMI audio device.
> 
> Option #2 has the downside that if your HDMI audio device is the only
> supported audio device in the system, it will still be audio1 and
> therefore not the default device.  On the other hand that has the
> benefit that if you later plug something like uaudio(4) into your
> system it will become the default device.

Since recently, sndiod can switch between devices. So with minimal
tweaks we could make it use audio1 if audio0 is not present.

Detecting if the hdmi codec is commected could ease this further

> Option #1 would give us more flexibility, but I'm not sure if the
> current audio(4) ioctls allow us to implement the proper heuristics.
> 
> > > About the multi-codec devices, how do you see it ? Keeping all the codecs
> > > and adding a knob to switch between analog and digital to select the 
> > > codec > 
> > This seems to make sense.
> 
> It's never entirely clear to me what exactly a "codec" is.  

Basically there's are two chips: a PCI chip (the host) that does only
DMA and talks through a simple serial link to the other chip (the
codec) which has a bunch of DACs, ADCs, optical transmitters, the HDMI
interfaces and so on. The interface between these is standardized.

HDA host chips can handle multiple codecs, not sure if they are trully
independent, specs say they are.



Re: Default device in audioctl and mixerctl

2020-05-01 Thread Alexandre Ratchov
On Fri, May 01, 2020 at 01:34:54PM +0200, Damien Couderc wrote:
> Hi,
> 
> I noticed that audioctl and mixerctl both use /dev/audioctl0 as a default
> since the reimplementation of the new api.
> 
> Shouldn't it use the /dev/audioctl link instead to permit choosing which
> device we want as the default?
> 

There's no audioctl symlink anymore, it was removed few years ago.

The default device is the one set with sndiod_flags, this way the same
device is used to play, record and volume control.

{audio,mixerctl}ctl are lower layer tools which are no longer for
everyday use and don't participate to the default device logic



Re: Audio over hdmi

2020-05-01 Thread Alexandre Ratchov
On Fri, May 01, 2020 at 01:11:16PM +0200, Damien Couderc wrote:
> 
> Speaking of the hdmi-only devices that were disabled in 2009: does the
> project still stand on this position in 2020? I made a quick search and it
> seems that more than half of the screens are audio capable now. I understand
> the defaults back in 2009, but now is it still true?

There's nothing wrong with hdmi-only devices. As long as audio works
by default with no tweaks, nobody will object to re-enabling
them. AFAIK, this was the only reason to disable them.

> About the multi-codec devices, how do you see it ? Keeping all the codecs
> and adding a knob to switch between analog and digital to select the codec ?

This seems to make sense.



Re: Install: Invalid group _sndiop in #163 amd64

2020-04-29 Thread Alexandre Ratchov
On Wed, Apr 29, 2020 at 01:26:51PM +0200, Otto Moerbeek wrote:
> On Wed, Apr 29, 2020 at 01:02:29PM +0200, Otto Moerbeek wrote:
> 
> > On Wed, Apr 29, 2020 at 10:28:37AM +, Oliver Marugg wrote:
> > 
> > > Possible typo group _sniop shoud be _sndiod:
> > 
> > Nope. _sniop is the correct name. The real issue is that that group isn't in
> > the /etc/group file used during install.
> > 
> > How that can happen I do not know yet.
> 
> Just did an install using install67.fg and did not see that error.
> 

There was a snap ~1-2 weeks ago with a broken bsd.rd (with /etc/groups
missing _sndiop).

Now this is fixed.



Re: Audio over hdmi

2020-04-25 Thread Alexandre Ratchov
On Sat, Apr 25, 2020 at 05:16:03PM +0200, Damien Couderc wrote:
> > 
> > I can see in the full dmesg that there are two different FTYPE results
> > provided during azalia_codec_init and only the first one seems to be
> > displayed in the mixerctl output.
> > 
> > I think that maybe mixerctl does not recurse all the audio functions or
> > that some bits aren't recorded. I'll take a look.
> 
> Ok, I did a bit of reading and from what I understood there are 2 detected
> codecs (0 and 2) for the same instance of azalia but the latest is deleted
> in azalia_init_codecs() because there is only one that can be enabled.
> 
> Alexandre could maybe tell us what is possible to do here. I think it's
> maybe possible to keep all detected codecs but to change which one is
> enabled.

Hi, 

Sorry couldn't test your diff, my machines have one codec only.

The azalia driver can handle only one codec at a time. Multiple codecs
can't be presented as different devices because they can't be used at
the same time. Quickly adding support for multiple codecs would need
to expose "mixer" controls of all the codecs and the ability to switch
between them. This may look difficult but in most (all?) real-world
cases there is at most 1 analog and 1 digital codec and the latter has
very few controls, if any.

Independenty, support for the hdmi-only devices, present on certain
graphics cards was removed as they were not useful, but used to attach
as audio0, i.e. they took the place of the default audio device, which
made audio not work by default. But this is not related to your patch.

HTH



mixerctl: use /dev/audioctl0 instead of /dev/mixer by default

2020-04-02 Thread Alexandre Ratchov
The /dev/audioctlN files, when used in O_WRONLY mode offer the same
functionality as /dev/mixerN.

Ports don't use /dev/mixerN anymore, by switching mixerctl(4) to
/dev/audioctlN too, we remove the last /dev/mixerN user.

OK?

Index: mixerctl.1
===
RCS file: /cvs/src/usr.bin/mixerctl/mixerctl.1,v
retrieving revision 1.35
diff -u -p -r1.35 mixerctl.1
--- mixerctl.1  30 Jul 2018 17:24:25 -  1.35
+++ mixerctl.1  2 Apr 2020 20:22:34 -
@@ -68,7 +68,7 @@ This is the default, if no parameters ar
 .It Fl f Ar file
 Specify an alternative audio mixing device.
 The default is
-.Pa /dev/mixer0 .
+.Pa /dev/audioctl0 .
 .It Fl n
 Suppress printing of the variable name.
 .It Fl q
@@ -153,7 +153,7 @@ The audio mixer device to use.
 .El
 .Sh FILES
 .Bl -tag -width "/etc/mixerctl.confXXX" -compact
-.It Pa /dev/mixer0
+.It Pa /dev/audioctl0
 Default mixer audio device.
 .It Pa /etc/mixerctl.conf
 .Nm
Index: mixerctl.c
===
RCS file: /cvs/src/usr.bin/mixerctl/mixerctl.c,v
retrieving revision 1.32
diff -u -p -r1.32 mixerctl.c
--- mixerctl.c  28 Jun 2019 13:35:02 -  1.32
+++ mixerctl.c  2 Apr 2020 20:22:34 -
@@ -249,7 +249,7 @@ main(int argc, char **argv)
int ndev;
 
if ((file = getenv("MIXERDEVICE")) == 0 || *file == '\0')
-   file = "/dev/mixer";
+   file = "/dev/audioctl0";
 
while ((ch = getopt(argc, argv, "af:nqtvw")) != -1) {
switch (ch) {
@@ -284,15 +284,12 @@ main(int argc, char **argv)
if (argc == 0 && tflag == 0)
aflag = 1;
 
-   if (unveil(file, "rw") == -1)
+   if (unveil(file, "w") == -1)
err(1, "unveil");
 
-   if ((fd = open(file, O_RDWR)) == -1) {
+   if ((fd = open(file, O_WRONLY)) == -1) {
if (unveil(file, "r") == -1)
err(1, "unveil");
-
-   if ((fd = open(file, O_RDONLY)) == -1)
-   err(1, "%s", file);
}
 
if (unveil(NULL, NULL) == -1)



Re: libossaudio: start using sndio

2020-04-02 Thread Alexandre Ratchov
On Thu, Apr 02, 2020 at 05:21:45PM +0100, Stuart Henderson wrote:
> On 2020/04/02 17:13, Landry Breuil wrote:
> > On Wed, Apr 01, 2020 at 07:27:12PM +0200, Alexandre Ratchov wrote:
> > > ping!
> > > 
> > > FWIW, the diff below affects the following ports:
> > >   - emulators/gambatte
> > >   - gstreamer-0.10 mixer plugin
> > >   - sysutils/conky
> > >   - sysutils/gkrellm
> > >   - sysutil/tpb
> > >   - telephony/iaxclient
> > >   - anything depending on gstreamer, ex xfce4-mixer
> > > 
> > > If you use one of above ports and want to test, just apply this diff
> > > to src/lib/libossaudio in base, rebuild it and reinstall it. The ABI
> > > is not changing, so no need to rebuild or update ports.
> 
> btw, most of these ports are either not really used (tpb, iaxclient),
> or where the mixer functionality isn't hugely important (conky, gkrellm),
> so Landry's tests here should cover things fairly well.

I've tested conky and gkrellm, it works. They show the new controls,
but this is an improvement, imho



Re: libossaudio: start using sndio

2020-04-02 Thread Alexandre Ratchov
On Thu, Apr 02, 2020 at 05:13:49PM +0200, Landry Breuil wrote:
> On Wed, Apr 01, 2020 at 07:27:12PM +0200, Alexandre Ratchov wrote:
> > ping!
> > 
> > FWIW, the diff below affects the following ports:
> > - emulators/gambatte
> > - gstreamer-0.10 mixer plugin
> > - sysutils/conky
> > - sysutils/gkrellm
> > - sysutil/tpb
> > - telephony/iaxclient
> > - anything depending on gstreamer, ex xfce4-mixer
> > 
> > If you use one of above ports and want to test, just apply this diff
> > to src/lib/libossaudio in base, rebuild it and reinstall it. The ABI
> > is not changing, so no need to rebuild or update ports.
> 
> I've briefly tested it with xfce4-mixer which uses gst 0.10 - on a t430u
> with:
> 
> azalia0 at pci0 dev 27 function 0 "Intel 7 Series HD Audio" rev 0x04: msi
> azalia0: codecs: Realtek ALC269, Intel/0x2806, using Realtek ALC269
> 
> previously, it saw two controls, one for the output acting/wired to
> mixerctl values inputs.dac-0:1 & inputs.dac-2:3 (ie both changes upon
> control changes), and one for the input wired to inputs.mic
> 
> with the diff, xfce4-mixer now sees the 3 expected controls:
> * 'volume', wired to aucatctl 'master' / sndioctl 'output.level' value
> * 'input gain', wired to mixerctl record.volume, record.adc-0:1 &
> * record.adc-2:3 (and to sndioctl hw/input.level)
> * 'output gain', wired to mixerctl outputs.master, inputs.dac-0:1 &
> * inputs.dac-2:3 (and to sndioctl hw/output.level)
> 
> trying to toggle the 'mute' button in xfce4-mixer (for the 'input gain'
> only) leads to 'Error setting mixer recording devices (0x0): Invalid
> argument' msgs on stderr but im not sure it matters.
> 

This is "ok", it tries to select the input source, but as there's only
one choice exposed it complains on stderr.

> toggling the mute button for volume and input gain set the level to 0
> instead of toggling the hw/output.mute button but im not sure it's a big
> deal either.

This is expected as well. The mute controls, instead xfce4-mixer saves
the current level and sets it to zero. When the control is unmuted, it
sets the level to the saved value.

> all in all that seems an improvement to me, at least does what's
> expected for gst 0.10/xfce4-mixer.
> 

Many thanks for looking at this.



libossaudio: start using sndio

2020-04-01 Thread Alexandre Ratchov
ping!

FWIW, the diff below affects the following ports:
- emulators/gambatte
- gstreamer-0.10 mixer plugin
- sysutils/conky
- sysutils/gkrellm
- sysutil/tpb
- telephony/iaxclient
- anything depending on gstreamer, ex xfce4-mixer

If you use one of above ports and want to test, just apply this diff
to src/lib/libossaudio in base, rebuild it and reinstall it. The ABI
is not changing, so no need to rebuild or update ports.

- Forwarded message from Alexandre Ratchov  -

Date: Tue, 17 Mar 2020 12:27:40 +0100
From: Alexandre Ratchov 
To: po...@openbsd.org
Subject: libossaudio: important diff to test
User-Agent: Mutt/1.11.4 (2019-03-13)

Hi,

This diff makes libossaudio use sndio instead of the kernel mixer(4)
interface. I post it to ports@ as it's only used by few ports.

Switching libossaudio to sndio implies that programs using it will
always use the right device (instead of the first one) and will always
see at least the sndiod master level control.

With this diff, libossaudio will expose the following controls:
  - output.level, sndiod master level, always present
  - hw/output.level, hardware output level, present on certain devices only
  - hw/input.level, hardware input level, present on certain devices only

OK?

-- Alexandre

Index: lib/libossaudio/Makefile
===
RCS file: /cvs/src/lib/libossaudio/Makefile,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 Makefile
--- lib/libossaudio/Makefile16 Jul 2014 20:02:17 -  1.5
+++ lib/libossaudio/Makefile22 Feb 2020 16:07:05 -
@@ -4,9 +4,11 @@
 LIB=   ossaudio
 MAN=   ossaudio.3
 
-SRCS=  ossaudio.c
+SRCS=  ossaudio.c aucat.c debug.c sioctl.c sioctl_aucat.c sioctl_sun.c
 
 CPPFLAGS+= -I${.CURDIR}
+
+.PATH: ${.CURDIR}/../libsndio
 
 includes:
@cd ${.CURDIR}; cmp -s soundcard.h ${DESTDIR}/usr/include/soundcard.h 
|| \
Index: lib/libossaudio/ossaudio.c
===
RCS file: /cvs/src/lib/libossaudio/ossaudio.c,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 ossaudio.c
--- lib/libossaudio/ossaudio.c  28 Jun 2019 13:32:42 -  1.20
+++ lib/libossaudio/ossaudio.c  22 Feb 2020 16:07:05 -
@@ -36,26 +36,38 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
-
+#include 
+#include 
+#include 
+#include 
 #include "soundcard.h"
-#undef ioctl
 
-#define GET_DEV(com) ((com) & 0xff)
+#ifdef DEBUG
+#define DPRINTF(...) do { fprintf(stderr, __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(...) do {} while (0)
+#endif
 
-#define TO_OSSVOL(x)   (((x) * 100 + 127) / 255)
-#define FROM_OSSVOL(x) x) > 100 ? 100 : (x)) * 255 + 50) / 100)
+#define GET_DEV(com) ((com) & 0xff)
+#define INTARG (*(int*)argp)
 
-static struct audiodevinfo *getdevinfo(int);
+struct control {
+   struct control *next;
+   int type;   /* one of SOUND_MIXER_xxx */
+   int chan;   /* 0 -> left, 1 -> right, -1 -> mono */
+   int addr;   /* sioctl control id */
+   int value;  /* current value */
+   int max;
+};
 
 static int mixer_ioctl(int, unsigned long, void *);
-static int opaque_to_enum(struct audiodevinfo *di, audio_mixer_name_t *label, 
int opq);
-static int enum_to_ord(struct audiodevinfo *di, int enm);
-static int enum_to_mask(struct audiodevinfo *di, int enm);
 
-#define INTARG (*(int*)argp)
+static int initialized;
+static struct control *controls;
+static struct sioctl_hdl *hdl;
+static char *dev_name = SIO_DEVANY;
+static struct pollfd *pfds;
 
 int
 _oss_ioctl(int fd, unsigned long com, ...)
@@ -71,201 +83,163 @@ _oss_ioctl(int fd, unsigned long com, ..
else if (IOCGROUP(com) == 'M')
return mixer_ioctl(fd, com, argp);
else
-   return ioctl(fd, com, argp);
+   return (ioctl)(fd, com, argp);
 }
 
-/* If the mixer device should have more than MAX_MIXER_DEVS devices
- * some will not be available to Linux */
-#define MAX_MIXER_DEVS 64
-struct audiodevinfo {
-   int done;
-   dev_t dev;
-   ino_t ino;
-   int16_t devmap[SOUND_MIXER_NRDEVICES],
-   rdevmap[MAX_MIXER_DEVS];
-   char names[MAX_MIXER_DEVS][MAX_AUDIO_DEV_LEN];
-   int enum2opaque[MAX_MIXER_DEVS];
-u_long devmask, recmask, stereomask;
-   u_long caps, recsource;
-};
-
-static int
-opaque_to_enum(struct audiodevinfo *di, audio_mixer_name_t *label, int opq)
+/*
+ * new control
+ */
+static void
+mixer_ondesc(void *unused, struct sioctl_desc *d, int val)
 {
-   int i, o;
+   struct control *i, **pi;
+   int type;
 
-   for (i = 0; i < MAX_MIXER_DEVS; i++) {
-   o = di->enum2opaque[i];
-   if (o == opq)
-   break;
-   if (o == -1 && label != NULL &&
-   !strncmp(di->names[i], label-&g

Re: umidi: missing NULL checks

2020-03-26 Thread Alexandre Ratchov
On Mon, Mar 23, 2020 at 10:51:06PM +0100, Tobias Heider wrote:
> In alloc_all_jacks() the variables 'sc_in_jacks' and 'sc_out_checks'
> are set to NULL if 'sc_in_num_jacks' and 'sc_out_num_jacks' are 0.
> Further down both are dereferenced unconditionally. I added explicit NULL
> checks where I think they belong.
> I think 'sc_in_ep' and 'sc_out_ep' can also be NULL, but I am not
> entirely sure about those.
> 
> Objections or ok?
> 

sc_{out,in}_jacks is used only if sc_out_num_jacks > 0, i.e. if
there's at least one iteration. But if sc_out_num_jacks > 0, then
sc_{out,in}_jacks is properly initialized by this chunk:

sc->sc_out_jacks =
sc->sc_out_num_jacks ? sc->sc_jacks : NULL;
sc->sc_in_jacks =
sc->sc_in_num_jacks ? sc->sc_jacks + sc->sc_out_num_jacks : NULL;

AFAICS, the code is correct; but the style looks misleading to me.

> Index: dev/usb/umidi.c
> ===
> RCS file: /mount/openbsd/cvs/src/sys/dev/usb/umidi.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 umidi.c
> --- dev/usb/umidi.c   16 Mar 2020 16:12:43 -  1.53
> +++ dev/usb/umidi.c   23 Mar 2020 21:50:49 -
> @@ -724,50 +724,58 @@ alloc_all_jacks(struct umidi_softc *sc)
>   sc->sc_in_jacks =
>   sc->sc_in_num_jacks ? sc->sc_jacks+sc->sc_out_num_jacks : NULL;
>  
> - jack = >sc_out_jacks[0];
> - for (i=0; isc_out_num_jacks; i++) {
> - jack->opened = 0;
> - jack->binded = 0;
> - jack->arg = NULL;
> - jack->u.out.intr = NULL;
> - jack->intr = 0;
> - jack->cable_number = i;
> - jack++;
> + if (sc->sc_out_jacks) {
> + jack = >sc_out_jacks[0];
> + for (i=0; isc_out_num_jacks; i++) {
> + jack->opened = 0;
> + jack->binded = 0;
> + jack->arg = NULL;
> + jack->u.out.intr = NULL;
> + jack->intr = 0;
> + jack->cable_number = i;
> + jack++;
> + }
>   }
> - jack = >sc_in_jacks[0];
> - for (i=0; isc_in_num_jacks; i++) {
> - jack->opened = 0;
> - jack->binded = 0;
> - jack->arg = NULL;
> - jack->u.in.intr = NULL;
> - jack->cable_number = i;
> - jack++;
> + if (sc->sc_in_jacks) {
> + jack = >sc_in_jacks[0];
> + for (i=0; isc_in_num_jacks; i++) {
> + jack->opened = 0;
> + jack->binded = 0;
> + jack->arg = NULL;
> + jack->u.in.intr = NULL;
> + jack->cable_number = i;
> + jack++;
> + }
>   }
>  
>   /* assign each jacks to each endpoints */
> - jack = >sc_out_jacks[0];
> - ep = >sc_out_ep[0];
> - for (i=0; isc_out_num_endpoints; i++) {
> - rjack = >jacks[0];
> - for (j=0; jnum_jacks; j++) {
> - *rjack = jack;
> - jack->endpoint = ep;
> - jack++;
> - rjack++;
> + if (sc->sc_out_jacks && sc->sc_out_ep) {
> + jack = >sc_out_jacks[0];
> + ep = >sc_out_ep[0];
> + for (i=0; isc_out_num_endpoints; i++) {
> + rjack = >jacks[0];
> + for (j=0; jnum_jacks; j++) {
> + *rjack = jack;
> + jack->endpoint = ep;
> + jack++;
> + rjack++;
> + }
> + ep++;
>   }
> - ep++;
>   }
> - jack = >sc_in_jacks[0];
> - ep = >sc_in_ep[0];
> - for (i=0; isc_in_num_endpoints; i++) {
> - rjack = >jacks[0];
> - for (j=0; jnum_jacks; j++) {
> - *rjack = jack;
> - jack->endpoint = ep;
> - jack++;
> - rjack++;
> + if (sc->sc_in_jacks && sc->sc_in_ep) {
> + jack = >sc_in_jacks[0];
> + ep = >sc_in_ep[0];
> + for (i=0; isc_in_num_endpoints; i++) {
> + rjack = >jacks[0];
> + for (j=0; jnum_jacks; j++) {
> + *rjack = jack;
> + jack->endpoint = ep;
> + jack++;
> + rjack++;
> + }
> + ep++;
>   }
> - ep++;
>   }
>  
>   return USBD_NORMAL_COMPLETION;
> 



Re: Fix brightness control on ASUS 1005PXD

2020-03-16 Thread Alexandre Ratchov
On Mon, Mar 16, 2020 at 10:16:34AM +0100, Patrick Wildt wrote:
> Otherwise, if we want to do that in acpivout_get_brightness(),
> I guess we can update acpivout_select_brightness() and its caller
> to remove the check for -1, since there will be no -1 anymore?
> 

Sure, I missed those. Here's a new diff with the checks
removed. Tested and works as expected on my machne.

Index: acpivout.c
===
RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v
retrieving revision 1.19
diff -u -p -r1.19 acpivout.c
--- acpivout.c  8 Feb 2020 19:08:17 -   1.19
+++ acpivout.c  16 Mar 2020 14:42:17 -
@@ -227,8 +227,10 @@ acpivout_get_brightness(struct acpivout_
aml_freevalue();
DPRINTF(("%s: BQC = %d\n", DEVNAME(sc), level));
 
-   if (level < sc->sc_bcl[0] || level > sc->sc_bcl[sc->sc_bcl_len -1])
-   level = -1;
+   if (level < sc->sc_bcl[0])
+   level = sc->sc_bcl[0];
+   else if (level > sc->sc_bcl[sc->sc_bcl_len - 1])
+   level = sc->sc_bcl[sc->sc_bcl_len - 1];
 
return (level);
 }
@@ -239,9 +241,6 @@ acpivout_select_brightness(struct acpivo
int nindex, level;
 
level = sc->sc_brightness;
-   if (level == -1)
-   return level;
-
nindex = acpivout_find_brightness(sc, nlevel);
if (sc->sc_bcl[nindex] == level) {
if (nlevel > level && (nindex + 1 < sc->sc_bcl_len))
@@ -375,13 +374,11 @@ acpivout_set_param(struct wsdisplay_para
}
if (sc != NULL && sc->sc_bcl_len != 0) {
nindex = acpivout_select_brightness(sc, dp->curval);
-   if (nindex != -1) {
-   sc->sc_brightness = sc->sc_bcl[nindex];
-   acpi_addtask(sc->sc_acpi,
-   acpivout_set_brightness, sc, 0);
-   acpi_wakeup(sc->sc_acpi);
-   return 0;
-   }
+   sc->sc_brightness = sc->sc_bcl[nindex];
+   acpi_addtask(sc->sc_acpi,
+   acpivout_set_brightness, sc, 0);
+   acpi_wakeup(sc->sc_acpi);
+   return 0;
}
return -1;
default:



Re: Fix brightness control on ASUS 1005PXD

2020-03-16 Thread Alexandre Ratchov
On Mon, Mar 16, 2020 at 10:16:34AM +0100, Patrick Wildt wrote:
> On Sat, Mar 14, 2020 at 04:28:26AM +0100, Alexandre Ratchov wrote:
> > On ASUS 1001PXD, _BQC returns an out of range value which makes
> > acpivout_get_brightness() return -1, in turn breaking the
> > display.brightness control (wsconsctl displays a mangled value).
> > 
> > This diff ignores the out of range value and makes the brighness
> > control just work again.
> > 
> > OK?
> 
> With the current code _BQC is only called on attach, where we
> probably want a value higher than the lowest one.  I wonder if
> maybe we should move this check into the attach functions, and
> in case of an error like this, set a reasonable brightness?

Just to be sure that I understand, do you suggest to change the
brightness at attach time?



Fix brightness control on ASUS 1005PXD

2020-03-13 Thread Alexandre Ratchov
On ASUS 1001PXD, _BQC returns an out of range value which makes
acpivout_get_brightness() return -1, in turn breaking the
display.brightness control (wsconsctl displays a mangled value).

This diff ignores the out of range value and makes the brighness
control just work again.

OK?

Index: acpivout.c
===
RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v
retrieving revision 1.19
diff -u -p -r1.19 acpivout.c
--- acpivout.c  8 Feb 2020 19:08:17 -   1.19
+++ acpivout.c  14 Mar 2020 03:19:02 -
@@ -227,8 +227,10 @@ acpivout_get_brightness(struct acpivout_
aml_freevalue();
DPRINTF(("%s: BQC = %d\n", DEVNAME(sc), level));
 
-   if (level < sc->sc_bcl[0] || level > sc->sc_bcl[sc->sc_bcl_len -1])
-   level = -1;
+   if (level < sc->sc_bcl[0])
+   level = sc->sc_bcl[0];
+   else if (level > sc->sc_bcl[sc->sc_bcl_len - 1])
+   level = sc->sc_bcl[sc->sc_bcl_len - 1];
 
return (level);
 }



Re: Audio control API

2020-02-25 Thread Alexandre Ratchov
On Tue, Feb 25, 2020 at 01:02:06PM +0100, Alexandre Ratchov wrote:
> On Thu, Feb 13, 2020 at 04:52:12AM +, Raf Czlonka wrote:
> > 
> > Hi Alexandre,
> > 
> > I have to say that I also find the two ranges mildly confusing,
> > i.e. 0-255 in one place, and 0-127 in another. In terms of units,
> > personally, I'm used to, and quite like, the granularity of 0-255.
> > 
> > Again, not my place so others will certainly be more help here.
> > 
> > One more point regarding the interface, though.
> > 
> > This is the way mixerctl(1) currently behaves:
> > 
> > $ mixerctl outputs.master 
> > outputs.master=255,255
> > $ mixerctl outputs.master=100 
> > outputs.master: 255,255 -> 100,100
> > $ mixerctl outputs.master=300 
> > outputs.master: 100,100 -> 255,255
> > 
> > Should sndioctl(1) behave the same way?
> 
> Many thanks for the feedback.
> 
> After some thinking and experimenting, floats in the [0:1] seem the
> simplest option. It avoids discussions about preferences and allows
> arbitrary precision (if needed, later).
> 
> Furthermore, as all controls are in the [0:1] range, it makes sense to
> request the user to provide numbers between 0 and 1. Providing numbers
> outside this range indicates he is misunderstanding how the program
> works.
> 
> Below are all 3 base diffs combined (libsndio, sndiod, sndioctl,
> libossaudio), to ease testing.

And here's the ports part of the diff including audio/gqmpeg,
sysutils/tray-app and x11/i3status ports.

Index: sysutils/tray-app/Makefile
===
RCS file: /cvs/ports/sysutils/tray-app/Makefile,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 Makefile
--- sysutils/tray-app/Makefile  12 Jul 2019 20:49:53 -  1.8
+++ sysutils/tray-app/Makefile  22 Feb 2020 16:06:19 -
@@ -5,7 +5,7 @@ ONLY_FOR_ARCHS= ${APM_ARCHS}
 COMMENT=   small utilities for X11 system tray: eject, battery, mixer
 
 DISTNAME=  tray-app-0.3.1
-REVISION=  0
+REVISION=  1
 
 CATEGORIES=sysutils x11
 
Index: sysutils/tray-app/patches/patch-sound_Makefile
===
RCS file: /cvs/ports/sysutils/tray-app/patches/patch-sound_Makefile,v
retrieving revision 1.1.1.1
diff -u -p -u -p -r1.1.1.1 patch-sound_Makefile
--- sysutils/tray-app/patches/patch-sound_Makefile  17 Sep 2013 11:21:50 
-  1.1.1.1
+++ sysutils/tray-app/patches/patch-sound_Makefile  22 Feb 2020 16:06:19 
-
@@ -1,6 +1,7 @@
 $OpenBSD: patch-sound_Makefile,v 1.1.1.1 2013/09/17 11:21:50 sthen Exp $
 sound/Makefile.origMon Mar 12 08:46:04 2012
-+++ sound/Makefile Tue Sep 17 11:39:18 2013
+Index: sound/Makefile
+--- sound/Makefile.orig
 sound/Makefile
 @@ -8,13 +8,13 @@ MAN=
  
  gtk_CFLAGS!= pkg-config --cflags gtk+-2.0
@@ -8,7 +9,8 @@ $OpenBSD: patch-sound_Makefile,v 1.1.1.1
 -CFLAGS= -W -Wall -g -O0 -I../lib $(gtk_CFLAGS)
 +CFLAGS+= -W -Wall -I../lib $(gtk_CFLAGS)
  LDFLAGS= -L../lib $(gtk_LDFLAGS)
- LDADD= -ltrayapp
+-LDADD= -ltrayapp
++LDADD= -ltrayapp -lsndio
  
 -BINDIR=/usr/local/libexec/tray-app
 +BINDIR=${TRUEPREFIX}/libexec/tray-app
Index: sysutils/tray-app/patches/patch-sound_sound_c
===
RCS file: sysutils/tray-app/patches/patch-sound_sound_c
diff -N sysutils/tray-app/patches/patch-sound_sound_c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ sysutils/tray-app/patches/patch-sound_sound_c   22 Feb 2020 16:06:19 
-
@@ -0,0 +1,516 @@
+$OpenBSD$
+
+Index: sound/sound.c
+--- sound/sound.c.orig
 sound/sound.c
+@@ -4,12 +4,13 @@
+  */
+ #include 
+ #include 
+-#include 
++#include 
+ 
+ #include 
+ 
+ #include 
+ #include 
++#include 
+ #include 
+ #include 
+ 
+@@ -19,16 +20,20 @@
+ #include "sound-1.xpm"
+ #include "sound-2.xpm"
+ 
++struct control {
++  struct control *next;
++  unsigned int addr;
++  unsigned int value;
++  unsigned int max;
++  int ismute;
++};
++
+ static void   usage(const char *prog);
+-static intget_mixer_index(int fd, mixer_devinfo_t *devinfo);
+-static intget_volume(int fd, mixer_devinfo_t *devinfo, u_char *volume);
+-static intset_volume(int fd, mixer_devinfo_t *devinfo, u_char volume);
+-static intget_mute(int fd, mixer_devinfo_t *devinfo, int *mute);
+-static intset_mute(int fd, mixer_devinfo_t *devinfo, int mute);
++static void   set_state(int ismute, int mute);
++static void   get_state(int *rvolume, int *rmute);
+ 
+ static void   prepare_tooltip(int mute, u_char volume, char *text, size_t sz);
+ 
+-static gboolean   cb_timer(GtkWidget *widget);
+ static void   cb_button_toggled(GtkWidget *widget, gpointer data);
+ static void   cb_scale_value_changed(GtkScale *scale, GtkAdjustment *adj

Re: Audio control API

2020-02-25 Thread Alexandre Ratchov
open.3 mio_open.3 sndio.7
+MAN=   sio_open.3 mio_open.3 sioctl_open.3 sndio.7
 SRCS=  debug.c aucat.c sio_aucat.c sio_sun.c sio.c \
-   mio_rmidi.c mio_aucat.c mio.c
+   mio_rmidi.c mio_aucat.c mio.c \
+   sioctl_aucat.c sioctl_sun.c sioctl.c
 CFLAGS+=-DDEBUG
 COPTS+=-Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith 
-Wundef
 
Index: lib/libsndio/Symbols.map
===
RCS file: /cvs/src/lib/libsndio/Symbols.map,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 Symbols.map
--- lib/libsndio/Symbols.map26 Dec 2017 19:12:22 -  1.1
+++ lib/libsndio/Symbols.map24 Feb 2020 06:03:28 -
@@ -27,10 +27,22 @@
mio_revents;
mio_eof;
 
+   sioctl_open;
+   sioctl_close;
+   sioctl_ondesc;
+   sioctl_onval;
+   sioctl_setval;
+   sioctl_nfds;
+   sioctl_pollfd;
+   sioctl_revents;
+   sioctl_eof;
+
mio_rmidi_getfd;
mio_rmidi_fdopen;
sio_sun_getfd;
sio_sun_fdopen;
+   sioctl_sun_getfd;
+   sioctl_sun_fdopen;
local:
*;
 };
Index: lib/libsndio/amsg.h
===
RCS file: /cvs/src/lib/libsndio/amsg.h,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 amsg.h
--- lib/libsndio/amsg.h 12 Jul 2019 06:30:55 -  1.12
+++ lib/libsndio/amsg.h 24 Feb 2020 06:03:28 -
@@ -43,6 +43,11 @@
 #define AUCAT_PORT 11025
 
 /*
+ * limits
+ */
+#define AMSG_CTL_NAMEMAX   16  /* max name length */
+
+/*
  * WARNING: since the protocol may be simultaneously used by static
  * binaries or by different versions of a shared library, we are not
  * allowed to change the packet binary representation in a backward
@@ -64,6 +69,9 @@ struct amsg {
 #define AMSG_HELLO 10  /* say hello, check versions and so ... */
 #define AMSG_BYE   11  /* ask server to drop connection */
 #define AMSG_AUTH  12  /* send authentication cookie */
+#define AMSG_CTLSUB13  /* ondesc/onctl subscription */
+#define AMSG_CTLSET14  /* set control value */
+#define AMSG_CTLSYNC   15  /* end of controls descriptions */
uint32_t cmd;
uint32_t __pad;
union {
@@ -108,7 +116,38 @@ struct amsg {
 #define AMSG_COOKIELEN 16
uint8_t cookie[AMSG_COOKIELEN];
} auth;
+   struct amsg_ctlsub {
+   uint8_t desc, val;
+   } ctlsub;
+   struct amsg_ctlset {
+   uint16_t addr, val;
+   } ctlset;
} u;
+};
+
+/*
+ * network representation of sioctl_node structure
+ */
+struct amsg_ctl_node {
+   char name[AMSG_CTL_NAMEMAX];
+   int16_t unit;
+   uint8_t __pad[2];
+};
+
+/*
+ * network representation of sioctl_desc structure
+ */
+struct amsg_ctl_desc {
+   struct amsg_ctl_node node0; /* affected channels */
+   struct amsg_ctl_node node1; /* dito for AMSG_CTL_{SEL,VEC,LIST} */
+   char func[AMSG_CTL_NAMEMAX];/* parameter function name */
+   char group[AMSG_CTL_NAMEMAX];   /* group of the control */
+   uint8_t type;   /* see sioctl_desc structure */
+   uint8_t __pad1[1];
+   uint16_t addr;  /* control address */
+   uint16_t maxval;
+   uint16_t curval;
+   uint32_t __pad2[3];
 };
 
 /*
Index: lib/libsndio/shlib_version
===
RCS file: /cvs/src/lib/libsndio/shlib_version,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 shlib_version
--- lib/libsndio/shlib_version  26 Dec 2017 15:23:33 -  1.11
+++ lib/libsndio/shlib_version  24 Feb 2020 06:03:28 -
@@ -1,2 +1,2 @@
 major=7
-minor=0
+minor=1
Index: lib/libsndio/sioctl.c
===
RCS file: lib/libsndio/sioctl.c
diff -N lib/libsndio/sioctl.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ lib/libsndio/sioctl.c   24 Feb 2020 06:03:30 -
@@ -0,0 +1,177 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2014-2020 Alexandre Ratchov 
+ *
+ * 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, A

Re: Audio control API, part 1: libsndio, sndiod bits

2020-02-24 Thread Alexandre Ratchov
On Thu, Feb 13, 2020 at 05:15:34AM +, Raf Czlonka wrote:
> On Sun, Feb 09, 2020 at 12:13:02PM GMT, Alexandre Ratchov wrote:
> > +++ lib/libsndio/sioctl_aucat.c 8 Feb 2020 14:49:37 -
> > [...]
> > + * Copyright (c) 2010-2011 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ lib/libsndio/sioctl_open.3  8 Feb 2020 14:49:37 -
> > [...]
> > +.\" Copyright (c) 2011 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ lib/libsndio/sioctl_priv.h  8 Feb 2020 14:49:38 -
> > [...]
> > + * Copyright (c) 2008 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ lib/libsndio/sioctl_sun.c   8 Feb 2020 14:49:38 -
> > [...]
> > + * Copyright (c) 2010-2011 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ lib/libsndio/sioctl.c   8 Feb 2020 14:49:37 -
> > [...]
> > + * Copyright (c) 2008 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ usr.bin/sndioctl/sndioctl.1 9 Feb 2020 11:05:02 -
> > [...]
> > +.\" Copyright (c) 2007 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ usr.bin/sndioctl/sndioctl.c 9 Feb 2020 11:05:02 -
> > [...]
> > + * Copyright (c) 2007-2011 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ usr.bin/sndiod/dev_sioctl.c 8 Feb 2020 14:49:38 -
> > [...]
> > + * Copyright (c) 2014 Alexandre Ratchov 
> > 
> > [...]
> > 
> > +++ usr.bin/sndiod/dev_sioctl.h 8 Feb 2020 14:49:38 -
> > [...]
> > + * Copyright (c) 2014 Alexandre Ratchov 
> > 
> > [...]
> > 
> 
> Hi Alexandre,
> 
> Shouldn't all of these dates be adjusted?
> 

Sure; added 2020 as copyright year. Thanks.



Re: Audio control API, part 1: libsndio, sndiod bits

2020-02-12 Thread Alexandre Ratchov
On Wed, Feb 12, 2020 at 09:28:38PM +0100, Jan Stary wrote:
> Hi,
> 
> On Feb 09 13:13:02, a...@caoua.org wrote:
> > cd /usr/src
> > patch -p0 <1.diff
> > patch -p0 <2.diff
> > patch -p0 <3.diff
> > cd /usr/src/include && doas make includes
> > cd /usr/src/lib/libsndio && make obj && make && doas make install
> > cd /usr/src/lib/libossaudio && make obj && make && doas make install
> > cd /usr/src/usr.bin/sndiod && make obj && make && doas make install
> > cd /usr/src/usr.bin/sndioctl && make obj && make && doas make install
> > doas rcctl restart sndiod
> > cd /usr/ports
> > patch -p0 <4.diff
> > cd /usr/ports/audio/gqmpeg && make && make install
> > cd /usr/ports/sysutils/tray-app && make && make install
> > 
> > Feedback about the new features
> > is of course welcome as well.
> 
> sndioctl -m works fine with e.g. mplayer,
> i.e. sndioctl notices the "added" app,
> and reports the volume changes.

great :)

> With ffplay and firefox:youtube, volume changes are not reported.
> Is that expected? Do they tweak their "internal" volume knob
> but not the volume they register with sndio? As opposed to mplayer?

Yes exactly. I haven't checked ffplay, but firefox uses an internal
volume control because its internal API has no "getter", this is
explained here:

https://marc.info/?l=openbsd-ports=152641946326955



Re: Audio control API, part 1: libsndio, sndiod bits

2020-02-12 Thread Alexandre Ratchov
On Wed, Feb 12, 2020 at 09:22:20PM +0100, Jan Stary wrote:
> Hi,
> 
> On Feb 09 13:13:02, a...@caoua.org wrote:
> > cd /usr/src
> > patch -p0 <1.diff
> > patch -p0 <2.diff
> > patch -p0 <3.diff
> > cd /usr/src/include && doas make includes
> > cd /usr/src/lib/libsndio && make obj && make && doas make install
> > cd /usr/src/lib/libossaudio && make obj && make && doas make install
> > cd /usr/src/usr.bin/sndiod && make obj && make && doas make install
> > cd /usr/src/usr.bin/sndioctl && make obj && make && doas make install
> > doas rcctl restart sndiod
> > 
> > I'm very interested in any regression.
> 
> After restarting sndiod (with empty flags),
> starting sndioctl -m makes sndiod crash with
> 
>   Feb 12 21:02:31 box /bsd: sndiod[95433]: pledge "tty", syscall 54
> 
> That's ioctl(2). I don't see that in any of the three pledge(2) calls
> revealed by a grep in (the patched) .../sndiod/, but apparently
> the new code does call ioctl().

sndiod does unauthorized ioctls on this kernel, see below

> 
> Strangely, this happens on
> 
> OpenBSD 6.6-current (GENERIC.MP) #0: Tue Feb  4 17:33:19 CET 2020
> h...@box.stare.cz:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> 
> but not on
> 
> OpenBSD 6.6-current (GENERIC.MP) #0: Sun Feb  9 17:30:47 CET 2020
> h...@dell.stare.cz:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> 
> (Did I miss somethong pledge() related in that window?)
> 

Yes, there was a kernel commit to allow programs with the "audio"
promise (like sndiod) to use the mixer ioctl()s.



Re: Audio control API, part 4: switch ports to sndio API

2020-02-12 Thread Alexandre Ratchov
On Sun, Feb 09, 2020 at 01:27:27PM +0100, Alexandre Ratchov wrote:
> There are two ports only using the kernel-based API, this diff updates
> them to use new libsndio-based API:
>   - sysutils/tray-app
>   - audio/gqmpeg
> 
> As a side effect, this fixes gqmpeg crashes, makes these programs use
> the right device, and make them work on many uaudio(4) and envy(4)
> devices with no hardware master level knob. Also when one program
> changes the volume, slider of other programs are properly updated.
> 

I forgot x11/i3status; below is a diff to make it use sndio.

Index: Makefile
===
RCS file: /cvs/ports/x11/i3status/Makefile,v
retrieving revision 1.58
diff -u -p -r1.58 Makefile
--- Makefile27 Sep 2019 20:33:22 -  1.58
+++ Makefile12 Feb 2020 13:55:43 -
@@ -5,7 +5,7 @@ ONLY_FOR_ARCHS= ${APM_ARCHS}
 COMMENT=   generate a statusbar for use with i3/xmobar/dzen2
 
 DISTNAME=  i3status-2.13
-REVISION=  1
+REVISION=  2
 CATEGORIES=x11 sysutils
 
 HOMEPAGE=  https://i3wm.org/i3status/
@@ -28,7 +28,9 @@ BUILD_DEPENDS=textproc/asciidoc>=8.6.8
 LIB_DEPENDS=   devel/libconfuse \
devel/libyajl
 
-CONFIGURE_STYLE =  gnu
+AUTOCONF_VERSION = 2.69
+AUTOMAKE_VERSION = 1.16
+CONFIGURE_STYLE =  autoreconf
 SEPARATE_BUILD =   Yes
 
 FAKE_FLAGS +=  sysconfdir=${PREFIX}/share/examples/i3status/
Index: patches/patch-Makefile_am
===
RCS file: patches/patch-Makefile_am
diff -N patches/patch-Makefile_am
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-Makefile_am   12 Feb 2020 13:55:43 -
@@ -0,0 +1,37 @@
+$OpenBSD$
+
+Index: Makefile.am
+--- Makefile.am.orig
 Makefile.am
+@@ -1,5 +1,3 @@
+-@CODE_COVERAGE_RULES@
+-
+ echo-version:
+   @echo "@I3STATUS_VERSION@"
+ 
+@@ -30,6 +28,7 @@ i3status_CFLAGS = \
+   $(PULSE_CFLAGS) \
+   $(NLGENL_CFLAGS) \
+   $(ALSA_CFLAGS) \
++  $(SNDIO_CFLAGS) \
+   $(PTHREAD_CFLAGS)
+ 
+ i3status_CPPFLAGS = \
+@@ -42,6 +41,7 @@ i3status_LDADD = \
+   $(PULSE_LIBS) \
+   $(NLGENL_LIBS) \
+   $(ALSA_LIBS) \
++  $(SNDIO_LIBS) \
+   $(PTHREAD_LIBS)
+ 
+ i3status_SOURCES = \
+@@ -69,7 +69,8 @@ i3status_SOURCES = \
+   src/print_wireless_info.c \
+   src/print_file_contents.c \
+   src/process_runs.c \
+-  src/pulse.c
++  src/pulse.c \
++  src/sndio.c
+ 
+ dist_sysconf_DATA = \
+   i3status.conf
Index: patches/patch-Makefile_in
===
RCS file: patches/patch-Makefile_in
diff -N patches/patch-Makefile_in
--- patches/patch-Makefile_in   6 Jul 2019 20:20:27 -   1.1
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,15 +0,0 @@
-$OpenBSD: patch-Makefile_in,v 1.1 2019/07/06 20:20:27 jasper Exp $
-
-The CODE_COVERAGE_RULES fragment contains an unmatched "if" clause.
-
-Index: Makefile.in
 Makefile.in.orig
-+++ Makefile.in
-@@ -1851,7 +1851,6 @@ uninstall-man: uninstall-man1
- 
- .PRECIOUS: Makefile
- 
--@CODE_COVERAGE_RULES@
- 
- echo-version:
-   @echo "@I3STATUS_VERSION@"
Index: patches/patch-configure_ac
===
RCS file: patches/patch-configure_ac
diff -N patches/patch-configure_ac
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-configure_ac  12 Feb 2020 13:55:43 -
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+Index: configure.ac
+--- configure.ac.orig
 configure.ac
+@@ -91,6 +91,13 @@ case $host_os in
+   ;;
+ esac
+ 
++# if sndio is available, define USE_SNDIO
++AC_CHECK_HEADER(sndio.h,
++  [AC_CHECK_LIB([sndio], [sio_open], [
++  AC_SUBST(SNDIO_LIBS, "-lsndio")
++  AC_DEFINE([USE_SNDIO], [], [Use sndio])
++  ], [])], [])
++
+ dnl TODO: check for libbsd for GNU/kFreeBSD
+ 
+ # Checks for programs.
Index: patches/patch-include_i3status_h
===
RCS file: patches/patch-include_i3status_h
diff -N patches/patch-include_i3status_h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-include_i3status_h12 Feb 2020 13:55:43 -
@@ -0,0 +1,13 @@
+$OpenBSD$
+
+Index: include/i3status.h
+--- include/i3status.h.orig
 include/i3status.h
+@@ -232,6 +232,7 @@ int volume_pulseaudio(uint32_t sink_idx, const char *s
+ bool description_pulseaudio(uint32_t sink_idx, const char *sink_name, char 
buffer[MAX_SINK_DESCRIPTION_LEN]);
+ bool pulse_initialize(void);
+ void print_file_contents(yajl_gen json_gen, char *buffer, const char *title, 
const char *path, const char *format, const char *format_bad, const int 
max_chars);
++int volume_sndio(void);
+ 
+ /* socket file descriptor for general purpose

Re: Audio control API, part 1: libsndio, sndiod bits

2020-02-11 Thread Alexandre Ratchov
On Tue, Feb 11, 2020 at 07:01:28PM +0100, Florian Obser wrote:
> I've been running the base diffs since you posted them. Firefox,
> chrome and mpv still make noise :)
> 
> I'm puzzled by this:
> 
> $ cat /etc/mixerctl.conf  
> outputs.master=255,255
> record.enable=off
> 
> $ mixerctl outputs.master 
> outputs.master=255,255
> 
> $ sndioctl 
> output.level=127
> 
> I don't understand how they relate and why one goes to 255 and the
> other to 127.
> The error reporting is confusing, too:
> 
> $ sndioctl output.level=128   
> integer overflow
> 
> But no regressions to report :)
> 

Thanks, the code is base on MIDI bits, which uses the 0..127 range;
sndiod, aucat and many codecs also use the 0..127 range. Anyway,
replaced the error message by:

$ sndioctl output.level=128 
128: expected integer in the 0..127 range

[...]

I'm wondering if persents or floating points in the [0:1] range would
be less confusing and solve most "units" problems.



Re: Audio control API, part 2: add new sndioctl(1) utility

2020-02-10 Thread Alexandre Ratchov
On Mon, Feb 10, 2020 at 09:59:09AM +, Raf Czlonka wrote:
> On Sun, Feb 09, 2020 at 12:14:47PM GMT, Alexandre Ratchov wrote:
> > Here's a new sndioctl utility similar to mixerctl(1) but using the new
> > sndio API. Example:
> > 
> > $ sndioctl 
> > output.level=127
> > app/aucat0.level=127
> > app/firefox0.level=127
> > app/firefox1.level=12
> > app/midisyn0.level=127
> > app/mpv0.level=127
> > app/prog5.level=127
> > app/prog6.level=127
> > app/prog7.level=127
> > hw/input.level=62
> > hw/input.mute=0
> > hw/output.level=63
> > hw/output.mute=0
> > 
> 
> Hi Alexandre,
> 
> Just a quick question.
> 
> Is there a good reason to have the above using "slash" ('/') as the
> first separator instead of the, more familiar, "dot" ('.') known
> from sysctl(8)'s MIB (Management Information Base) style names or
> even the "pseudo" MIB know from mixerctl(1)?

Hi,

I don't know if the following qualifies as a "good reason". The first
part (the group) is a prefix of the control identifier. The identifier
itself has a strict "[channel]." format. The prefix
is not always present, examples:

output.level<- sndiod volume knob
hw/output.level <- underlying hardware volume knob

I tried to avoid the group part, but as mixers may be nested it seems
necessary to avoid name clashes.

In the sndioctl syntax, we could replace '/' by '.' but this looks
confusing as the syntax doesn't map directly to the underlying
model. But maybe we should hide such developer-centric details and
just use only dots to make this look as a MIB.

Another option I've considered is to drop the group concept in the API
and simply prefix the stream name to make it unique; in turn we obtain
a flat control list. It's uglier and seems to complicate GUIs
task. For instance the group part could be used to represent controls
of different groups in different sections or to filter-out certain
groups).



Re: Audio control API, part 2: add new sndioctl(1) utility

2020-02-09 Thread Alexandre Ratchov
On Sun, Feb 09, 2020 at 07:20:15PM +0100, Landry Breuil wrote:
> On Sun, Feb 09, 2020 at 01:14:47PM +0100, Alexandre Ratchov wrote:
> > Here's a new sndioctl utility similar to mixerctl(1) but using the new
> > sndio API. Example:
> > 
> > $ sndioctl 
> > output.level=127
> > app/aucat0.level=127
> > app/firefox0.level=127
> > app/firefox1.level=12
> > app/midisyn0.level=127
> > app/mpv0.level=127
> > app/prog5.level=127
> > app/prog6.level=127
> > app/prog7.level=127
> > hw/input.level=62
> > hw/input.mute=0
> > hw/output.level=63
> > hw/output.mute=0
> 
> i suppose that replaces audio/aucatctl port ?

Yes. But aucatctl will continue to work, it's based on the MIDI
interface that will stay for now. The advantage of sndioctl is that it
exposes selected hardware controls, while aucatctl exposes sndiod
internal knobs only.

> audio/cmixer relies on the
> latter but i'll have no issue migrating to it if sndioctl gets added.
> 

The -m option might be useful to cmixer to get updates of the knobs
state without the need to restart sndioctl periodically.



Audio control API, part 4: switch ports to sndio API

2020-02-09 Thread Alexandre Ratchov
There are two ports only using the kernel-based API, this diff updates
them to use new libsndio-based API:
  - sysutils/tray-app
  - audio/gqmpeg

As a side effect, this fixes gqmpeg crashes, makes these programs use
the right device, and make them work on many uaudio(4) and envy(4)
devices with no hardware master level knob. Also when one program
changes the volume, slider of other programs are properly updated.

enjoy,

Index: audio/gqmpeg/Makefile
===
RCS file: /cvs/ports/audio/gqmpeg/Makefile,v
retrieving revision 1.64
diff -u -p -u -p -r1.64 Makefile
--- audio/gqmpeg/Makefile   12 Jul 2019 20:43:33 -  1.64
+++ audio/gqmpeg/Makefile   4 Feb 2020 22:28:34 -
@@ -3,7 +3,7 @@
 COMMENT=   front-end to various audio players
 
 DISTNAME=  gqmpeg-0.91.1
-REVISION=  14
+REVISION=  15
 CATEGORIES=audio
 
 HOMEPAGE=  http://gqmpeg.sourceforge.net/
Index: audio/gqmpeg/patches/patch-src_Makefile_in
===
RCS file: audio/gqmpeg/patches/patch-src_Makefile_in
diff -N audio/gqmpeg/patches/patch-src_Makefile_in
--- /dev/null   1 Jan 1970 00:00:00 -
+++ audio/gqmpeg/patches/patch-src_Makefile_in  4 Feb 2020 22:28:34 -
@@ -0,0 +1,14 @@
+$OpenBSD$
+
+Index: src/Makefile.in
+--- src/Makefile.in.orig
 src/Makefile.in
+@@ -342,7 +342,7 @@ gqmpeg_SOURCES = \
+   $(module_mpg123) $(module_xmp) $(module_ogg123) $(module_radio)
+ 
+ 
+-gqmpeg_LDADD = $(GTK_LIBS) $(LIBPNG)
++gqmpeg_LDADD = $(GTK_LIBS) $(LIBPNG) -lsndio
+ 
+ EXTRA_DIST = \
+   $(extra_SLIK)   \
Index: audio/gqmpeg/patches/patch-src_mixer_c
===
RCS file: /cvs/ports/audio/gqmpeg/patches/patch-src_mixer_c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 patch-src_mixer_c
--- audio/gqmpeg/patches/patch-src_mixer_c  14 Oct 2007 14:12:42 -  
1.2
+++ audio/gqmpeg/patches/patch-src_mixer_c  4 Feb 2020 22:28:34 -
@@ -1,39 +1,287 @@
 $OpenBSD: patch-src_mixer_c,v 1.2 2007/10/14 14:12:42 jasper Exp $
 src/mixer.c.orig   Tue Sep 10 16:16:26 2002
-+++ src/mixer.cSun Oct 14 15:47:27 2007
-@@ -285,7 +285,11 @@ void mixer_init(gint init_device_id)
- 
-   mixer_device = getenv("MIXERDEVICE");
-   if (mixer_device == NULL)
-+#ifdef __OpenBSD__
-+mixer_device = "/dev/mixer";
-+#else
- mixer_device = "/dev/mixer0";
-+#endif
+Index: src/mixer.c
+--- src/mixer.c.orig
 src/mixer.c
+@@ -39,10 +39,16 @@
+ #include 
+ #endif
  
-   if ((fd = open(mixer_device, O_RDWR)) == -1) {
- perror(mixer_device);
-@@ -362,7 +366,11 @@ static void mixer_set_vol(DeviceData *device, gint vol
- 
-   mixer_device = getenv("MIXERDEVICE");
-   if (mixer_device == NULL)
-+#ifdef __OpenBSD__
-+mixer_device = "/dev/mixer";
-+#else
- mixer_device = "/dev/mixer0";
-+#endif
+-#if defined(__NetBSD__) || defined(__OpenBSD__)
++#if defined(__NetBSD__)
+ #include 
+ #endif
  
-   if ((fd = open(mixer_device, O_RDWR)) == -1) {
- perror(mixer_device);
-@@ -406,7 +414,11 @@ static gint mixer_get_vol(DeviceData *device)
- 
-   mixer_device = getenv("MIXERDEVICE");
-   if (mixer_device == NULL)
-+#ifdef __OpenBSD__
-+mixer_device = "/dev/mixer";
-+#else
- mixer_device = "/dev/mixer0";
++#if defined(__OpenBSD__)
++#include 
++#include 
++#include "display.h"
 +#endif
++
+ #if defined(sun) && defined(__svr4__)
+ #include 
+ #endif
+@@ -267,11 +273,11 @@ static gint mixer_get_vol(DeviceData *device)
+ 
+ /*
+  *
+- * NetBSD and OpenBSD
++ * NetBSD
+  *
+  */
+ 
+-#elif defined(__NetBSD__) || defined(__OpenBSD__)
++#elif defined(__NetBSD__)
+ 
+ mixer_devinfo_t *infos;
+ mixer_ctrl_t *values;
+@@ -442,6 +448,241 @@ static gint mixer_get_vol(DeviceData *device)
+ 
+ /*
+  *
++ * OpenBSD
++ *
++ */
++
++#elif defined(__OpenBSD__)
++
++struct control {
++  struct control *next;
++  unsigned int addr;
++  unsigned int value;
++};
++
++static struct control *controls;
++static struct sioctl_hdl *hdl;
++static struct pollfd *pfds;
++static int initialized;
++
++/*
++ * new control registered
++ */
++static void ondesc(void *unused, struct sioctl_desc *d, int val)
++{
++  struct control *i, **pi;
++
++  if (d == NULL)
++  return;
++
++  /*
++   * delete existing control with the same address
++   */
++  for (pi =  (i = *pi) != NULL; pi = >next) {
++  if (d->addr == i->addr) {
++  *pi = i->next;
++  free(i);
++  break;
++  }
++  }
++
++  /*
++   * SIOCTL_NONE 

Audio control API, part 3: switch -lossaudio to sndio

2020-02-09 Thread Alexandre Ratchov
Certain ports use -lossaudio to get access to the volume knobs. This
diff updates -lossaudio to use the new sndio API. By default, it
exposes both sndiod output level and hardware master input/output
knobs (if present). The source selectors are not exposed any longer as
sndiod doesn't expose them.

As a side effect, this solves two problems:

- makes mixer programs (like the xfce4-mixer port) use the right
  device. Example if you use a usb head-set, the program will display
  knobs of the head-set instead of knobs of the build-in device.

- makes mixer programs work on systems that have no master level
  knobs, like many uaudio(4) or envy(4) devices

The library version stays the same as thigs works exactly as before,
so no need to rebuild ports relying no this hack^Wlibrary.

Index: lib/libossaudio/Makefile
===
RCS file: /cvs/src/lib/libossaudio/Makefile,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 Makefile
--- lib/libossaudio/Makefile16 Jul 2014 20:02:17 -  1.5
+++ lib/libossaudio/Makefile8 Feb 2020 14:49:55 -
@@ -4,9 +4,11 @@
 LIB=   ossaudio
 MAN=   ossaudio.3
 
-SRCS=  ossaudio.c
+SRCS=  ossaudio.c aucat.c debug.c sioctl.c sioctl_aucat.c sioctl_sun.c
 
 CPPFLAGS+= -I${.CURDIR}
+
+.PATH: ${.CURDIR}/../libsndio
 
 includes:
@cd ${.CURDIR}; cmp -s soundcard.h ${DESTDIR}/usr/include/soundcard.h 
|| \
Index: lib/libossaudio/ossaudio.c
===
RCS file: /cvs/src/lib/libossaudio/ossaudio.c,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 ossaudio.c
--- lib/libossaudio/ossaudio.c  28 Jun 2019 13:32:42 -  1.20
+++ lib/libossaudio/ossaudio.c  8 Feb 2020 14:49:55 -
@@ -36,26 +36,41 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
-
+#include 
+#include 
+#include 
+#include 
 #include "soundcard.h"
-#undef ioctl
+
+#ifdef DEBUG
+#define DPRINTF(...) do { fprintf(stderr, __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(...) do {} while (0)
+#endif
 
 #define GET_DEV(com) ((com) & 0xff)
 
-#define TO_OSSVOL(x)   (((x) * 100 + 127) / 255)
-#define FROM_OSSVOL(x) x) > 100 ? 100 : (x)) * 255 + 50) / 100)
+#define TO_OSSVOL(x)   (((x) * 100 + SIOCTL_VALMAX / 2) / SIOCTL_VALMAX)
+#define FROM_OSSVOL(x) x) > 100 ? 100 : (x)) * SIOCTL_VALMAX + 50) / 100)
 
-static struct audiodevinfo *getdevinfo(int);
+#define INTARG (*(int*)argp)
+
+struct control {
+   struct control *next;
+   int type;   /* one of SOUND_MIXER_xxx */
+   int chan;   /* 0 -> left, 1 -> right, -1 -> mono */
+   int addr;   /* sioctl control id */
+   int value;  /* current value */
+};
 
 static int mixer_ioctl(int, unsigned long, void *);
-static int opaque_to_enum(struct audiodevinfo *di, audio_mixer_name_t *label, 
int opq);
-static int enum_to_ord(struct audiodevinfo *di, int enm);
-static int enum_to_mask(struct audiodevinfo *di, int enm);
 
-#define INTARG (*(int*)argp)
+static int initialized;
+static struct control *controls;
+static struct sioctl_hdl *hdl;
+static char *dev_name = SIOCTL_DEVANY;
+static struct pollfd *pfds;
 
 int
 _oss_ioctl(int fd, unsigned long com, ...)
@@ -71,201 +86,162 @@ _oss_ioctl(int fd, unsigned long com, ..
else if (IOCGROUP(com) == 'M')
return mixer_ioctl(fd, com, argp);
else
-   return ioctl(fd, com, argp);
+   return (ioctl)(fd, com, argp);
 }
 
-/* If the mixer device should have more than MAX_MIXER_DEVS devices
- * some will not be available to Linux */
-#define MAX_MIXER_DEVS 64
-struct audiodevinfo {
-   int done;
-   dev_t dev;
-   ino_t ino;
-   int16_t devmap[SOUND_MIXER_NRDEVICES],
-   rdevmap[MAX_MIXER_DEVS];
-   char names[MAX_MIXER_DEVS][MAX_AUDIO_DEV_LEN];
-   int enum2opaque[MAX_MIXER_DEVS];
-u_long devmask, recmask, stereomask;
-   u_long caps, recsource;
-};
-
-static int
-opaque_to_enum(struct audiodevinfo *di, audio_mixer_name_t *label, int opq)
+/*
+ * new control
+ */
+static void
+mixer_ondesc(void *unused, struct sioctl_desc *d, int val)
 {
-   int i, o;
+   struct control *i, **pi;
+   int type;
 
-   for (i = 0; i < MAX_MIXER_DEVS; i++) {
-   o = di->enum2opaque[i];
-   if (o == opq)
-   break;
-   if (o == -1 && label != NULL &&
-   !strncmp(di->names[i], label->name, sizeof di->names[i])) {
-   di->enum2opaque[i] = opq;
+   if (d == NULL)
+   return;
+
+   /*
+* delete existing control with the same address
+*/
+   for (pi =  (i = *pi) != NULL; pi = >next) {
+   if (d->addr == i->addr) {
+   *pi = i->next;
+   free(i);
break;
}
}
-   if (i >= MAX_MIXER_DEVS)
-   i 

Audio control API, part 2: add new sndioctl(1) utility

2020-02-09 Thread Alexandre Ratchov
Here's a new sndioctl utility similar to mixerctl(1) but using the new
sndio API. Example:

$ sndioctl 
output.level=127
app/aucat0.level=127
app/firefox0.level=127
app/firefox1.level=12
app/midisyn0.level=127
app/mpv0.level=127
app/prog5.level=127
app/prog6.level=127
app/prog7.level=127
hw/input.level=62
hw/input.mute=0
hw/output.level=63
hw/output.mute=0

Configuration parameters that are not exposed by sndiod will be
handled by audioctl(1), including the /etc/mixerctl.conf file at
system startup.

Originally the program was designed to handle modern many-channel
devices by presenting many-channel knobs on a single line; this
feature isn't used yet as the corresponding kernel bits are missing.

Index: usr.bin/Makefile
===
RCS file: /cvs/src/usr.bin/Makefile,v
retrieving revision 1.161
diff -u -p -u -p -r1.161 Makefile
--- usr.bin/Makefile9 Aug 2019 06:18:25 -   1.161
+++ usr.bin/Makefile9 Feb 2020 11:05:02 -
@@ -22,7 +22,7 @@ SUBDIR= apply arch at aucat audioctl awk
pr printenv printf quota radioctl rcs rdist rdistd \
readlink renice rev rpcgen rpcinfo rs rsync rup rusers rwall \
sdiff script sed sendbug shar showmount signify skey \
-   skeyaudit skeyinfo skeyinit sndiod snmp \
+   skeyaudit skeyinfo skeyinit sndioctl sndiod snmp \
sort spell split ssh stat su systat \
tail talk tcpbench tee telnet tftp tic time \
tmux top touch tput tr true tset tsort tty usbhidaction usbhidctl \
Index: usr.bin/sndioctl/Makefile
===
RCS file: usr.bin/sndioctl/Makefile
diff -N usr.bin/sndioctl/Makefile
--- /dev/null   1 Jan 1970 00:00:00 -
+++ usr.bin/sndioctl/Makefile   9 Feb 2020 11:05:02 -
@@ -0,0 +1,5 @@
+#  $OpenBSD$
+
+PROG=  sndioctl
+LDADD+=-lsndio
+.include 
Index: usr.bin/sndioctl/sndioctl.1
===
RCS file: usr.bin/sndioctl/sndioctl.1
diff -N usr.bin/sndioctl/sndioctl.1
--- /dev/null   1 Jan 1970 00:00:00 -
+++ usr.bin/sndioctl/sndioctl.1 9 Feb 2020 11:05:02 -
@@ -0,0 +1,148 @@
+.\" $OpenBSD$
+.\"
+.\" Copyright (c) 2007 Alexandre Ratchov 
+.\"
+.\" 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.
+.\"
+.Dd $Mdocdate: April 8 2011 $
+.Dt SNDIOCTL 1
+.Os
+.Sh NAME
+.Nm sndioctl
+.Nd control audio parameters
+.Sh SYNOPSIS
+.Nm
+.Bk -words
+.Op Fl iv
+.Op Fl f Ar device
+.Op Ar command ...
+.Ek
+.Nm
+.Bk -words
+.Fl d
+.Ek
+.Sh DESCRIPTION
+The
+.Nm
+utility can display or change parameters of
+.Xr sndio 7
+audio devices.
+The options are as follows:
+.Bl -tag -width Ds
+.It Fl d
+Dump the raw list of available parameters and exit.
+Useful as a debug tool.
+.It Fl f Ar device
+Use this
+.Xr sndio 7
+audio device.
+.It Fl m
+Monitor and display audio parameters changes.
+.It Fl i
+Display characteristics of requested parameters
+instead of their values.
+.It Fl v
+Enable verbose mode, a.k.a. multi-channel mode.
+By default parameters affecting different channels
+of the same stream are disguised as a single mono
+parameter to hide details that are not essential.
+.El
+.Pp
+If no commands are specified all valid parameters are displayed on
+.Em stdout .
+Unless
+.Fl d ,
+.Fl m ,
+or
+.Fl i
+are used, displayed parameters are valid commands.
+The set of available controls depends on the control device.
+.Pp
+Commands use the following two formats to display and set
+parameters respectively:
+.Pp
+.Dl group/stream[channel].function
+.Dl group/stream[channel].function=value
+.Pp
+On the left-hand side are specified the optional parameter group,
+the affected stream name, and the optional channel number.
+Examples of left-hand side terms:
+.Pp
+.Dl output.level
+.Dl hw/spkr[6].mute
+.Pp
+There are 4 parameter types: switches, numbers, selectors, and vectors.
+.Pp
+Numbers are specified in decimal and follow the same semantics
+as MIDI controllers.
+Values are in the 0..127 range and 64 is the neutral state (if applicable).
+Two-state controls (switches) take either 0 or 1 as value,
+typically corresponding to 

Audio control API, part 1: libsndio, sndiod bits

2020-02-09 Thread Alexandre Ratchov
_arg, desc, val);
+}
+
+void
+_sioctl_onval_cb(struct sioctl_hdl *hdl, unsigned int addr, unsigned int val)
+{
+   DPRINTF("_sioctl_onval_cb: %u -> %u\n", addr, val);
+   if (hdl->ctl_cb)
+   hdl->ctl_cb(hdl->ctl_arg, addr, val);
+}
+
+int
+sioctl_setval(struct sioctl_hdl *hdl, unsigned int addr, unsigned int val)
+{
+   if (!(hdl->mode & SIOCTL_WRITE))
+   return 0;
+   return hdl->ops->setctl(hdl, addr, val);
+}
Index: lib/libsndio/sioctl_aucat.c
===
RCS file: lib/libsndio/sioctl_aucat.c
diff -N lib/libsndio/sioctl_aucat.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ lib/libsndio/sioctl_aucat.c 8 Feb 2020 14:49:37 -
@@ -0,0 +1,272 @@
+/*
+ * Copyright (c) 2010-2011 Alexandre Ratchov 
+ *
+ * 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.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "debug.h"
+#include "aucat.h"
+#include "sioctl_priv.h"
+
+struct sioctl_aucat_hdl {
+   struct sioctl_hdl sioctl;
+   struct aucat aucat;
+   struct sioctl_desc desc;
+   struct amsg_ctl_desc buf[16];
+   size_t buf_wpos;
+   int dump_wait;
+};
+
+static void sioctl_aucat_close(struct sioctl_hdl *);
+static int sioctl_aucat_nfds(struct sioctl_hdl *);
+static int sioctl_aucat_pollfd(struct sioctl_hdl *, struct pollfd *, int);
+static int sioctl_aucat_revents(struct sioctl_hdl *, struct pollfd *);
+static int sioctl_aucat_setctl(struct sioctl_hdl *, unsigned int, unsigned 
int);
+static int sioctl_aucat_onval(struct sioctl_hdl *);
+static int sioctl_aucat_ondesc(struct sioctl_hdl *);
+
+/*
+ * operations every device should support
+ */
+struct sioctl_ops sioctl_aucat_ops = {
+   sioctl_aucat_close,
+   sioctl_aucat_nfds,
+   sioctl_aucat_pollfd,
+   sioctl_aucat_revents,
+   sioctl_aucat_setctl,
+   sioctl_aucat_onval,
+   sioctl_aucat_ondesc
+};
+
+static int
+sioctl_aucat_rdata(struct sioctl_aucat_hdl *hdl)
+{
+   struct sioctl_desc desc;
+   struct amsg_ctl_desc *c;
+   size_t rpos;
+   int n;
+
+   while (hdl->aucat.rstate == RSTATE_DATA) {
+
+   /* read entries */
+   while (hdl->buf_wpos < sizeof(hdl->buf) &&
+   hdl->aucat.rstate == RSTATE_DATA) {
+   n = _aucat_rdata(>aucat,
+   (unsigned char *)hdl->buf + hdl->buf_wpos,
+   sizeof(hdl->buf) - hdl->buf_wpos,
+   >sioctl.eof);
+   if (n == 0 || hdl->sioctl.eof)
+   return 0;
+   hdl->buf_wpos += n;
+   }
+
+   /* parse entries */
+   c = hdl->buf;
+   rpos = 0;
+   while (rpos < hdl->buf_wpos) {
+   strlcpy(desc.group, c->group, SIOCTL_NAMEMAX);
+   strlcpy(desc.node0.name, c->node0.name, SIOCTL_NAMEMAX);
+   desc.node0.unit = (int16_t)ntohs(c->node0.unit);
+   strlcpy(desc.node1.name, c->node1.name, SIOCTL_NAMEMAX);
+   desc.node1.unit = (int16_t)ntohs(c->node1.unit);
+   strlcpy(desc.func, c->func, SIOCTL_NAMEMAX);
+   desc.type = c->type;
+   desc.addr = ntohs(c->addr);
+   _sioctl_ondesc_cb(>sioctl,
+   , ntohs(c->curval));
+   rpos += sizeof(struct amsg_ctl_desc);
+   c++;
+   }
+   hdl->buf_wpos = 0;
+   }
+   return 1;
+}
+
+/*
+ * execute the next message, return 0 if blocked
+ */
+static int
+sioctl_aucat_runmsg(struct sioctl_aucat_hdl *hdl)
+{
+   if (!_aucat_rmsg(>aucat, >sioctl.eof))
+   return 0;
+   switch (ntohl(hdl->aucat.rmsg.cmd)) {
+   case AMSG_DATA:
+   hdl->buf_wpos = 0;
+   if (!sioctl_aucat_rdata(hdl))
+   return 0;
+   break;
+   case AMSG_CTLSET:
+   DPRINTF("sioctl_aucat_runmsg:

Re: get rid of almost empty /etc/examples/mixerctl.conf

2020-02-08 Thread Alexandre Ratchov
On Sat, Feb 08, 2020 at 11:04:10PM +0100, Ingo Schwarze wrote:
> Hi Theo,
> 
> you have a point, that was a lot of cheap talk and no patch.
> 
> I don't aim at changing yacc(1) grammars.  I think most parts of
> OpenBSD configuration systems already have sane defaults and most
> configuration syntaxes are already good with respect to simplicity
> and usability.  At least "most", maybe even all.
> 
> > The example files were moved from /etc, so that /etc contained fewer
> > files for unconfigured software.
> 
> That was a very good step, and it was also good to not encumber it
> by trying to do more at the same time.
> 
> > Little to no effort was put into curating them.  That's the gap that
> > occured.
> 
> So, here i'm putting a few pennies where my mouth is, starting from the
> shortest file in /etc/examples/.  The example is so short that cut and
> paste from the manual page will certainly cause no trouble, and the
> file also isn't a special case with respect to permissions,
> it is -rw-r--r-- root:wheel.
> 
> OK?
>   Ingo
> 
> 
> P.S.
> When preparing this, i noticed i already had the change to
> mixerctl.conf.5 in my tree.  It looks like i started it
> in the past, then forgot about it.
> 
> P.P.S.
> Note that i will not propose to get rid of *all* of /etc/examples/,
> but just of trivial examples that provide no benefit by being outside
> the manual page.  There are certainly some that make sense as they
> are.

I agree that the mixerctl.conf is pointless. Furthermore not all
devices have an outputs.master knob (ex. envy(4) and uaudio(4) don't),
so the example is wrong in certain cases.

thanks for looking at this

ok ratchov



Include AUDIO_MIXER_xxx ioctls in the "audio" pledge() promise

2020-02-02 Thread Alexandre Ratchov
The plan is to make sndiod need to control volume knobs. This diff
adds the AUDIO_MIXER_xxx ioctls to the "audio" pledge. Another option
would be to introduce a new "audioctl" promise, but IMHO we don't seed
such fine-grained promises.

OK?

Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.256
diff -u -p -u -p -r1.256 kern_pledge.c
--- kern_pledge.c   8 Dec 2019 23:08:59 -   1.256
+++ kern_pledge.c   2 Feb 2020 09:02:27 -
@@ -1123,6 +1123,9 @@ pledge_ioctl(struct proc *p, long com, s
case AUDIO_SETPAR:
case AUDIO_START:
case AUDIO_STOP:
+   case AUDIO_MIXER_DEVINFO:
+   case AUDIO_MIXER_READ:
+   case AUDIO_MIXER_WRITE:
if (fp->f_type == DTYPE_VNODE &&
vp->v_type == VCHR &&
cdevsw[major(vp->v_rdev)].d_open == audioopen)



Add support for notifications about audio(4) "mixer" controls changes

2020-01-25 Thread Alexandre Ratchov
The diff bellow allows programs to get notifications about changes of
audio controls. Programs open the /dev/audioctlN device node and use
the read(4) syscall to get the index of each changed control.

There may be only one reader at a time. Supporting multiple readers
would be much more complicated and the purpose of this diff is to
allow sndiod(8) to expose the controls to programs. So, there will be
one reader only.

To test the diff, run:

hexdump -ve '1/4 "%d\n"'  0
struct wskbd_vol spkr, mic;
struct task wskbd_task;
@@ -1192,6 +1205,8 @@ audio_attach(struct device *parent, stru
sc->mix_nent = mi->index;
sc->mix_ents = mallocarray(sc->mix_nent,
sizeof(struct mixer_ctrl), M_DEVBUF, M_WAITOK);
+   sc->mix_evbuf = mallocarray(sc->mix_nent,
+   sizeof(struct mixer_ev), M_DEVBUF, M_WAITOK | M_ZERO);
 
ent = sc->mix_ents;
mi->index = 0;
@@ -1329,6 +1344,7 @@ audio_detach(struct device *self, int fl
}
 
/* free resources */
+   free(sc->mix_evbuf, M_DEVBUF, sc->mix_nent * sizeof(struct mixer_ev));
free(sc->mix_ents, M_DEVBUF, sc->mix_nent * sizeof(struct mixer_ctrl));
audio_buf_done(sc, >play);
audio_buf_done(sc, >rec);
@@ -1718,6 +1734,28 @@ audio_ioctl(struct audio_softc *sc, unsi
return error;
 }
 
+void
+audio_event(struct audio_softc *sc, int addr)
+{
+   struct mixer_ev *e;
+
+   mtx_enter(_lock);
+   if (sc->mix_isopen) {
+   e = sc->mix_evbuf + addr;
+   if (!e->pending) {
+   e->pending = 1;
+   e->next = sc->mix_pending;
+   sc->mix_pending = e;
+   }
+   if (sc->mix_blocking) {
+   wakeup(>mix_blocking);
+   sc->mix_blocking = 0;
+   }
+   selwakeup(>mix_sel);
+   }
+   mtx_leave(_lock);
+}
+
 int
 audio_mixer_devinfo(struct audio_softc *sc, struct mixer_devinfo *devinfo)
 {
@@ -1784,6 +1822,7 @@ audio_mixer_set(struct audio_softc *sc, 
return error;
if (sc->ops->commit_settings)
return sc->ops->commit_settings(sc->arg);
+   audio_event(sc, c->dev);
return 0;
}
 
@@ -1834,6 +1873,107 @@ audio_ioctl_mixer(struct audio_softc *sc
 }
 
 int
+audio_mixer_read(struct audio_softc *sc, struct uio *uio, int ioflag)
+{  
+   struct mixer_ev *e;
+   int data;
+   int error;
+
+   DPRINTF("%s: mixer read: resid = %zd\n", DEVNAME(sc), uio->uio_resid);
+
+   /* block if quiesced */
+   while (sc->quiesce)
+   tsleep_nsec(>quiesce, 0, "mix_qrd", INFSLP);
+
+   mtx_enter(_lock);
+
+   /* if there are no events then sleep */
+   while (!sc->mix_pending) {
+   if (ioflag & IO_NDELAY) {
+   mtx_leave(_lock);
+   return EWOULDBLOCK;
+   }
+   DPRINTF("%s: mixer read sleep\n", DEVNAME(sc));
+   sc->mix_blocking = 1;
+   error = msleep_nsec(>mix_blocking,
+   _lock, PWAIT | PCATCH, "mix_rd", INFSLP);
+   if (!(sc->dev.dv_flags & DVF_ACTIVE))
+   error = EIO;
+   if (error) {
+   DPRINTF("%s: mixer read woke up error = %d\n",
+   DEVNAME(sc), error);
+   mtx_leave(_lock);
+   return error;
+   }
+   }
+
+   /* at this stage, there is an event to transfer */
+   while (uio->uio_resid >= sizeof(int) && sc->mix_pending) {
+   e = sc->mix_pending;
+   sc->mix_pending = e->next;
+   e->pending = 0;
+   data = e - sc->mix_evbuf;
+   mtx_leave(_lock);
+   DPRINTF("%s: mixer read: %u\n", DEVNAME(sc), data);
+   error = uiomove(, sizeof(int), uio);
+   if (error)
+   return error;
+   mtx_enter(_lock);
+   }
+
+   mtx_leave(_lock);
+   return 0;
+}
+
+int
+audio_mixer_poll(struct audio_softc *sc, int events, struct proc *p)
+{
+   int revents = 0;
+
+   mtx_enter(_lock);
+   if (sc->mix_isopen && sc->mix_pending)
+   revents |= events & (POLLIN | POLLRDNORM);
+   if (revents == 0) {
+   if (events & (POLLIN | POLLRDNORM))
+   selrecord(p, >mix_sel);
+   }
+   mtx_leave(_lock);
+   return revents;
+}
+
+int
+audio_mixer_open(struct audio_softc *sc, int flags)
+{
+   DPRINTF("%s: flags = 0x%x\n", __func__, flags);
+
+   if (flags & FREAD) {
+   if (sc->mix_isopen)
+   return EBUSY;
+   sc->mix_isopen = 1;
+   }
+   return 0;
+}
+
+int
+audio_mixer_close(struct audio_softc *sc, int flags)
+{
+   int i;
+
+   DPRINTF("%s: flags 

Re: eso(4): msleep(9) -> msleep_nsec(9)

2020-01-16 Thread Alexandre Ratchov
On Thu, Jan 16, 2020 at 10:32:21PM -0600, Scott Cheloha wrote:
> Ticks to milliseconds.
> 
> Pretty sure all we need to do to convert the drain timeouts is
> substitute 1000 for hz in each expression.
> 
> While we're here, I noticed that at halt time for input/output we
> msleep to allow a drain and then immediately relinquish the mutex
> after coming out of the sleep.  We could just opt not to reenter the
> mutex with PNORELOCK, though that might make the code harder to read.
> Maybe you have a preference?

I don't have the hardware to test, but this looks correct.

> Otherwise, ok?
> 

ok ratchov


> Index: pci/esovar.h
> ===
> RCS file: /cvs/src/sys/dev/pci/esovar.h,v
> retrieving revision 1.6
> diff -u -p -r1.6 esovar.h
> --- pci/esovar.h  21 Sep 2010 20:11:44 -  1.6
> +++ pci/esovar.h  17 Jan 2020 04:29:28 -
> @@ -129,7 +129,7 @@ struct eso_softc {
>   void(*sc_rintr)(void *);
>   void *  sc_rarg;
>  
> - /* Auto-initialize DMA transfer block drain timeouts, in ticks */
> + /* Auto-initialize DMA transfer block drain timeouts, in milliseconds */
>   int sc_pdrain;
>   int sc_rdrain;
>  
> Index: pci/eso.c
> ===
> RCS file: /cvs/src/sys/dev/pci/eso.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 eso.c
> --- pci/eso.c 14 Dec 2019 12:49:50 -  1.45
> +++ pci/eso.c 17 Jan 2020 04:29:28 -
> @@ -760,8 +760,8 @@ eso_halt_output(void *hdl)
>   ESO_IO_A2DMAM_DMAENB);
>  
>   sc->sc_pintr = NULL;
> - error = msleep(>sc_pintr, _lock, PWAIT, "esoho", 
> sc->sc_pdrain);
> - mtx_leave(_lock);
> + error = msleep_nsec(>sc_pintr, _lock, PWAIT | PNORELOCK,
> + "esoho", MSEC_TO_NSEC(sc->sc_pdrain));
>   
>   /* Shut down DMA completely. */
>   eso_write_mixreg(sc, ESO_MIXREG_A2C1, 0);
> @@ -787,8 +787,8 @@ eso_halt_input(void *hdl)
>   DMA37MD_WRITE | DMA37MD_DEMAND);
>  
>   sc->sc_rintr = NULL;
> - error = msleep(>sc_rintr, _lock, PWAIT, "esohi", 
> sc->sc_rdrain);
> - mtx_leave(_lock);
> + error = msleep_nsec(>sc_rintr, _lock, PWAIT | PNORELOCK,
> + "esohi", MSEC_TO_NSEC(sc->sc_rdrain));
>  
>   /* Shut down DMA completely. */
>   eso_write_ctlreg(sc, ESO_CTLREG_A1C2,
> @@ -1605,8 +1605,8 @@ eso_trigger_output(void *hdl, void *star
>   sc->sc_pintr = intr;
>   sc->sc_parg = arg;
>  
> - /* Compute drain timeout. */
> - sc->sc_pdrain = hz * (blksize * 3 / 2) / 
> + /* Compute drain timeout in milliseconds. */
> + sc->sc_pdrain = 1000 * (blksize * 3 / 2) / 
>   (param->sample_rate * param->channels * param->bps);
>  
>   /* DMA transfer count (in `words'!) reload using 2's complement. */
> @@ -1688,8 +1688,8 @@ eso_trigger_input(void *hdl, void *start
>   sc->sc_rintr = intr;
>   sc->sc_rarg = arg;
>  
> - /* Compute drain timeout. */
> - sc->sc_rdrain = hz * (blksize * 3 / 2) / 
> + /* Compute drain timeout in milliseconds. */
> + sc->sc_rdrain = 1000 * (blksize * 3 / 2) / 
>   (param->sample_rate * param->channels * param->bps);
>  
>   /* Set up ADC DMA converter parameters. */



Re: pci(4): autri(4), eap(4): tsleep(9) -> tsleep_nsec(9)

2020-01-11 Thread Alexandre Ratchov
On Sat, Jan 11, 2020 at 02:33:07AM -0600, Scott Cheloha wrote:
> Both of these cards have a 100ms sleep when closed.
> 
> ok?
> 

sure, ok ratchov


> Index: pci/eap.c
> ===
> RCS file: /cvs/src/sys/dev/pci/eap.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 eap.c
> --- pci/eap.c 14 Dec 2019 12:48:32 -  1.57
> +++ pci/eap.c 11 Jan 2020 08:29:27 -
> @@ -1536,7 +1536,9 @@ eap_midi_close(void *addr)
>  {
>   struct eap_softc *sc = addr;
>  
> - tsleep(sc, PWAIT, "eapclm", hz/10); /* give uart a chance to drain */
> + /* give uart a chance to drain */
> + tsleep_nsec(sc, PWAIT, "eapclm", MSEC_TO_NSEC(100));
> +
>   EWRITE1(sc, EAP_UART_CONTROL, 0);
>   EWRITE4(sc, EAP_ICSC, EREAD4(sc, EAP_ICSC) & ~EAP_UART_EN);
>  
> Index: pci/autri.c
> ===
> RCS file: /cvs/src/sys/dev/pci/autri.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 autri.c
> --- pci/autri.c   4 Dec 2019 20:21:35 -   1.43
> +++ pci/autri.c   11 Jan 2020 08:29:27 -
> @@ -1352,7 +1352,8 @@ autri_midi_close(void *addr)
>  
>   DPRINTF(("autri_midi_close()\n"));
>  
> - tsleep(sc, PWAIT, "autri", hz/10); /* give uart a chance to drain */
> + /* give uart a chance to drain */
> + tsleep_nsec(sc, PWAIT, "autri", MSEC_TO_NSEC(100));
>  
>   sc->sc_iintr = NULL;
>   sc->sc_ointr = NULL;
> 



Re: bktr(4) (again): tsleep(9) -> tsleep_nsec(9)

2020-01-10 Thread Alexandre Ratchov
On Fri, Jan 10, 2020 at 08:23:14PM -0600, Scott Cheloha wrote:
> Whoops, missed one last time.
> 
> ok?

ok

> 
> Index: pci/bktr/bktr_tuner.c
> ===
> RCS file: /cvs/src/sys/dev/pci/bktr/bktr_tuner.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 bktr_tuner.c
> --- pci/bktr/bktr_tuner.c 14 Mar 2015 03:38:49 -  1.8
> +++ pci/bktr/bktr_tuner.c 11 Jan 2020 02:20:51 -
> @@ -900,7 +900,7 @@ do_afc( bktr_ptr_t bktr, int addr, int f
>   origFrequency = frequency;
>  
>   /* wait for first setting to take effect */
> - tsleep( BKTR_SLEEP, PZERO, "tuning", hz/8 );
> + tsleep_nsec( BKTR_SLEEP, PZERO, "tuning", MSEC_TO_NSEC(1000 / 8) );
>  
>   if ( (status = i2cRead( bktr, addr + 1 )) < 0 )
>   return( -1 );



Re: midi(4): *sleep(9) -> *sleep_nsec(9)

2019-12-19 Thread Alexandre Ratchov
On Wed, Dec 18, 2019 at 10:41:56AM -0600, Scott Cheloha wrote:
> On Wed, Dec 18, 2019 at 09:54:43AM +0100, Alexandre Ratchov wrote:
> > On Tue, Dec 17, 2019 at 07:09:02PM -0600, Scott Cheloha wrote:
> > > The only conversion I'm having trouble with is the tsleep().
> > > The comment says "20ms", but then we use some arithmetic
> > > to derive a count of ticks.
> > > 
> > > Given
> > > 
> > >   hz * MIDI_MAXWRITE / MIDI_RATE
> > > 
> > > You have hz ticks/second, and 32 bytes, and 3125 bytes/second, so you
> > > have
> > > 
> > > hz ticks   32 bytes   3125 bytes
> > >  *  / --
> > > second1   second
> > > 
> > >   = 32 * hz ticks
> > > -
> > > 3125
> > > 
> > >   = 1 ticks
> > > 
> > > if hz = 100, with integer division.
> > > 
> > > I'm not sure how to use the constants to produce a count of
> > > milliseconds.  Maybe I'm just having a slow day.
> > 
> > The problem is that close() may reset the transmitter before the few
> > bytes of its internal buffer is sent on the wire; there's no "wait for
> > completion" feature in such simple hardware, so we just wait few
> > milliseconds.
> > 
> > The transmitter buffer size is around 16 bytes, the byte rate is 3125
> > bytes/second.  So if we wait at least 16B / 3125B/s = 5.12ms, we're
> > safe. Waiting 10ms-20ms is enough and is unnoticeable.
> 
> Just to be sure we're all on the same page, the comment says 20ms is
> 64 bytes' worth, and MIDI_MAXWRITE is 32 bytes, but you're saying the
> buffer is around 16 bytes.  Is any of this inconsistent?

Yeah, the comment doesn't match the code.

16B is what "typical" UARTs buffer. MIDI_MAXWRITE and 64 are both
upper bounds of it.

Waiting 20ms is safer. MIDI_MAXWRITE could be removed completely once
your diff is in.



Re: midi(4): *sleep(9) -> *sleep_nsec(9)

2019-12-18 Thread Alexandre Ratchov
On Tue, Dec 17, 2019 at 07:09:02PM -0600, Scott Cheloha wrote:
> The only conversion I'm having trouble with is the tsleep().
> The comment says "20ms", but then we use some arithmetic
> to derive a count of ticks.
> 
> Given
> 
>   hz * MIDI_MAXWRITE / MIDI_RATE
> 
> You have hz ticks/second, and 32 bytes, and 3125 bytes/second, so you
> have
> 
> hz ticks   32 bytes   3125 bytes
>  *  / --
> second1   second
> 
>   = 32 * hz ticks
> -
> 3125
> 
>   = 1 ticks
> 
> if hz = 100, with integer division.
> 
> I'm not sure how to use the constants to produce a count of
> milliseconds.  Maybe I'm just having a slow day.

The problem is that close() may reset the transmitter before the few
bytes of its internal buffer is sent on the wire; there's no "wait for
completion" feature in such simple hardware, so we just wait few
milliseconds.

The transmitter buffer size is around 16 bytes, the byte rate is 3125
bytes/second.  So if we wait at least 16B / 3125B/s = 5.12ms, we're
safe. Waiting 10ms-20ms is enough and is unnoticeable.

ok ratchov



Re: bktr(4): tsleep(9) -> tsleep_nsec(9)

2019-12-16 Thread Alexandre Ratchov
On Sun, Dec 15, 2019 at 10:28:32PM -0600, Scott Cheloha wrote:
> I don't have any of these cards, but these are straightforward conversions.
> 
> ok?
> 

ok



Re: envy(4): *sleep(9) -> *sleep_nsec(9)

2019-11-23 Thread Alexandre Ratchov
On Fri, Nov 22, 2019 at 06:41:25PM -0600, Scott Cheloha wrote:
> ok?
> 

sure, thanks



Re: tsleep_nsec(9) for wsdisplay(4)

2019-10-08 Thread Alexandre Ratchov
On Tue, Oct 08, 2019 at 05:47:04PM +0200, Frederic Cambus wrote:
> Hi tech@,
> 
> Here is a diff to convert all tsleep(9) calls in wsdisplay(4)
> to tsleep_nsec(9).
> 
> Comments? OK?
> 

looks right, ok ratchov@

> Index: sys/dev/wscons/wsdisplay.c
> ===
> RCS file: /cvs/src/sys/dev/wscons/wsdisplay.c,v
> retrieving revision 1.133
> diff -u -p -r1.133 wsdisplay.c
> --- sys/dev/wscons/wsdisplay.c1 Jul 2019 19:38:40 -   1.133
> +++ sys/dev/wscons/wsdisplay.c8 Oct 2019 14:40:56 -
> @@ -1830,7 +1830,8 @@ wsdisplay_switch(struct device *dev, int
>   s = spltty();
>  
>   while (sc->sc_resumescreen != WSDISPLAY_NULLSCREEN && res == 0)
> - res = tsleep(>sc_resumescreen, PCATCH, "wsrestore", 0);
> + res = tsleep_nsec(>sc_resumescreen, PCATCH, "wsrestore",
> + INFSLP);
>   if (res) {
>   splx(s);
>   return (res);
> @@ -1980,7 +1981,7 @@ wsscreen_switchwait(struct wsdisplay_sof
>   if (no == WSDISPLAY_NULLSCREEN) {
>   s = spltty();
>   while (sc->sc_focus && res == 0) {
> - res = tsleep(sc, PCATCH, "wswait", 0);
> + res = tsleep_nsec(sc, PCATCH, "wswait", INFSLP);
>   }
>   splx(s);
>   return (res);
> @@ -1995,7 +1996,7 @@ wsscreen_switchwait(struct wsdisplay_sof
>   s = spltty();
>   if (scr != sc->sc_focus) {
>   scr->scr_flags |= SCR_WAITACTIVE;
> - res = tsleep(scr, PCATCH, "wswait2", 0);
> + res = tsleep_nsec(scr, PCATCH, "wswait2", INFSLP);
>   if (scr != sc->sc_scr[no])
>   res = ENXIO; /* disappeared in the meantime */
>   else
> 



cdio.1: remove reference to "aucat socket"

2019-10-02 Thread Alexandre Ratchov
The "aucat socket" was removed ~7 years ago. I've dropped
the .Xr to audio(4) as the output could be anything on the network.

OK?

Index: cdio.1
===
RCS file: /cvs/src/usr.bin/cdio/cdio.1,v
retrieving revision 1.62
diff -u -p -u -p -r1.62 cdio.1
--- cdio.1  19 Mar 2017 18:40:11 -  1.62
+++ cdio.1  2 Oct 2019 06:05:45 -
@@ -99,11 +99,7 @@ Unlike
 .Ic play ,
 the CD player need not be connected to an audio device;
 instead it rips tracks from disk and outputs audio data to
-the default
-.Xr audio 4
-device or
-.Xr aucat 1
-socket.
+the default audio device.
 Both individual tracks and track ranges may be specified.
 If range is specified in descending order tracks will be played in descending 
order.
 If the first value in the range is omitted, tracks from first track on disk to 
the specified one will be played.



Re: new USB audio class v2.0 driver

2019-09-25 Thread Alexandre Ratchov
On Wed, Sep 25, 2019 at 08:39:10PM +0100, cho...@jtan.com wrote:
> Alexandre Ratchov writes:
> > On Wed, Sep 25, 2019 at 05:15:22PM +0100, cho...@jtan.com wrote:
> > > I have a similar problem to Alexander Hof with a presonus audio usb
> > > device, where attaching it reports 'only one AC iface allowed' and the
> > > device remains (apparently totally) inaccessible.
> > >
> > > dmes and lsusb included below.
> > >
> >
> > According to dmesg, this is 6.5, which doesn't contain the fix for
> > this problem. Could you try the device on a -current system?
> 
> I did look through the post 6.5 changes in CVS but couldn't see anything
> relevent, possibly for reasons that will become clear.
> 
> > Programmable clocks are OK, the comment states that we don't support
> > multiple clock sources simultaneously. When audio starts, all parts of
> > the device must be clocked by the same clock source.
> >
> > Pro audio interfaces use necessarily a single clock because otherwise
> > they wouldn't be unstable in DAWs and/or for real-time effects.
> >
> > FWIW, I've never seen -- or even heard of -- devices using multiple
> > clock sources simultaneously.
> 
> Good, because that was a complete red herring. The _other_ instance of
> '%s: only one AC iface allowed' is the one associated with the fault, in
> uaudio_process_conf(). Perhaps the error message at the end of
> uaudio_process_ac() could read '%s: only one distinct clock source
> allowed'?
> 
> There is good news and bad news about -current. The good news is that
> the upgrade was seamless and the device is registered without reporting
> an error. The bad news is that I'd eventually managed to notice the
> empty (?) control interface and skip it, but it still didn't/doesn't
> work:
> 
> uaudio0 at uhub3 port 2 configuration 1 interface 1 "PreSonus AudioBox USB 
> 96" rev 2.00/1.12 addr 4
> uaudio0: class v2, high-speed, async, channels: 2 play, 2 rec, 0 ctls
> audio1 at uaudio0
> umidi0 at uhub3 port 2 configuration 1 interface 4 "PreSonus AudioBox USB 96" 
> rev 2.00/1.12 addr 4
> umidi0: (genuine USB-MIDI)
> umidi0: out=1, in=1
> midi0 at umidi0: 
> ugen1 at uhub3 port 2 configuration 1 "PreSonus AudioBox USB 96" rev 
> 2.00/1.12 addr 4
> 
> ludmilla$ audioctl -f /dev/audioctl1
> name=uaudio0
> mode=
> pause=0
> active=0
> nblks=2
> blksz=960
> rate=48000
> encoding=s16le
> play.channels=2
> play.bytes=0
> play.errors=0
> record.channels=2
> record.bytes=0
> record.errors=0
> 
> ludmilla$ mixerctl -f /dev/mixer1
> record.enable=sysctl
> 
> ludmilla$ sysctl |grep -e mix -e audio
> kern.audio.record=0
> 
> And from mplayer with AUDIODEVICE=snd/1:
> ...
> ao2: can't open sndio
> ...

You need to set

sndiod_flags=-frsnd/0 -frsnd/1

in /etc/rc.conf.local and restart sndiod. This is for sndiod to
configure this device as well.

Another option is to use the new -F option, so that sndiod will use
the USB device if it's connected and the internal if it
isn't. Example:

sndiod_flags=-frsnd/0 -Frsnd/1

in this case, no need to export AUDIODEVICE=snd/1, assuming the device
works ;-)



Re: new USB audio class v2.0 driver

2019-09-25 Thread Alexandre Ratchov
On Wed, Sep 25, 2019 at 08:13:09PM +0100, cho...@jtan.com wrote:
> Alexandre Ratchov writes:
> > On Wed, Sep 25, 2019 at 05:15:22PM +0100, cho...@jtan.com wrote:
> > > I have a similar problem to Alexander Hof with a presonus audio usb
> > > device, where attaching it reports 'only one AC iface allowed' and the
> > > device remains (apparently totally) inaccessible.
> > > 
> > > dmes and lsusb included below.
> > > 
> >
> > According to dmesg, this is 6.5, which doesn't contain the fix for
> > this problem. Could you try the device on a -current system?
> 
> I did look through the post 6.5 changes in CVS but couldn't see anything
> relevent, possibly for reasons that will become clear.
> 

This is the commit:


revision 1.142
date: 2019/05/09 06:58:13;  author: ratchov;  state: Exp;  lines: +5 -8;  
commitid: 3l3sLX3Zz5BTanPk;
Skip empty control interfaces when parsing descriptors.

Even if having multiple control interface descriptors is not allowed
by the UAC spec, there's no reason to stop as long as a proper control
interface was processed.


But there are fixes for other problems, so I'd suggest using 6.6

> > Programmable clocks are OK, the comment states that we don't support
> > multiple clock sources simultaneously. When audio starts, all parts of
> > the device must be clocked by the same clock source.
> >
> > Pro audio interfaces use necessarily a single clock because otherwise
> > they wouldn't be unstable in DAWs and/or for real-time effects.
> >
> > FWIW, I've never seen -- or even heard of -- devices using multiple
> > clock sources simultaneously.
> 
> Good, because that was a complete red herring. The _other_ instance of
> '%s: only one AC iface allowed' is the one associated with the fault, in
> uaudio_process_conf().

It was removed and the code fixed. The problem was that certain
devices with MIDI ports (like yours) may expose two Audio Control (aka
AC) interfaces one for audio and one (empty) for MIDI; this is not
allowed by the standard. The new code accepts multiple AC interfaces,
it picks the right one and ignores the empty ones.

> Perhaps the error message at the end of
> uaudio_process_ac() could read '%s: only one distinct clock source
> allowed'?

Actually we support multiple clock sources, but distinct sources may
not clock distinct parts of the device.

The "domain" word is the one used in the UAC specification; maybe too
technical, but it's exact. Note that there are few other
developper-centric messages, mainly to help improving the driver in
case certain "rare" devices pop-up one day.



Re: new USB audio class v2.0 driver

2019-09-25 Thread Alexandre Ratchov
On Wed, Sep 25, 2019 at 05:15:22PM +0100, cho...@jtan.com wrote:
> I have a similar problem to Alexander Hof with a presonus audio usb
> device, where attaching it reports 'only one AC iface allowed' and the
> device remains (apparently totally) inaccessible.
> 
> dmes and lsusb included below.
> 

According to dmesg, this is 6.5, which doesn't contain the fix for
this problem. Could you try the device on a -current system?

> The device and the box it plugs into are both available for prodding and
> poking if it might help as neither is in use (but note I am not on tech@).
> 
> It would appear that this assumption doesn't fully hold on some,
> professional?, usb audio hardware:
> 
> /*
>  * Find common clock unit. We assume all terminals
>  * belong to the same clock domain (ie are connected
>  * to the same source)
>  */
> 
> A cheap USB sound card I have to hand does not include the programmable
> clock in its lsusb output, or indeed anything marked CLOCK_SOURCE.
> 

Programmable clocks are OK, the comment states that we don't support
multiple clock sources simultaneously. When audio starts, all parts of
the device must be clocked by the same clock source.

Pro audio interfaces use necessarily a single clock because otherwise
they wouldn't be unstable in DAWs and/or for real-time effects.

FWIW, I've never seen -- or even heard of -- devices using multiple
clock sources simultaneously.



Re: switching between audio devices with no playback interruption

2019-09-20 Thread Alexandre Ratchov
On Tue, Sep 17, 2019 at 09:57:28AM +0200, Alexandre Ratchov wrote:
> 
> > Regarding the race I was talking about, doesn't dev_reopen() close the
> > current device before trying to open a new one, possibly finding an
> > incompatible device or failing to open it?  What happens in this case?
> 
> After a SIGHUP, if the new device is incompatible, the old one is
> reopened and sound continues. Now I understand your remark: oops!
> close() and open() are called in the wrong order, so another process
> could steal the device. I'll fix that.
> 

Here's a new diff with the fix. Now, the new device is opened before
closing the old one. I've fixed two other bugs in the pure audio
logic.

Index: dev.c
===
RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
retrieving revision 1.61
diff -u -p -u -p -r1.61 dev.c
--- dev.c   19 Sep 2019 05:10:19 -  1.61
+++ dev.c   20 Sep 2019 07:17:54 -
@@ -969,7 +969,8 @@ dev_new(char *path, struct aparams *par,
return NULL;
}
d = xmalloc(sizeof(struct dev));
-   d->path = xstrdup(path);
+   d->path_list = NULL;
+   namelist_add(>path_list, path);
d->num = dev_sndnum++;
d->opt_list = NULL;
 
@@ -1174,6 +1175,74 @@ dev_close(struct dev *d)
dev_freebufs(d);
 }
 
+/*
+ * Close the device, but attempt to migrate everything to a new sndio
+ * device.
+ */
+int
+dev_reopen(struct dev *d)
+{
+   struct slot *s;
+   long long pos;
+   unsigned int pstate;
+   int delta;
+
+   /* not opened */
+   if (d->pstate == DEV_CFG)
+   return 1;
+
+   /* save state */
+   delta = d->delta;
+   pstate = d->pstate;
+
+   if (!dev_sio_reopen(d))
+   return 0;
+
+   /* reopen returns a stopped device */
+   d->pstate = DEV_INIT;
+
+   /* reallocate new buffers, with new parameters */
+   dev_freebufs(d);
+   dev_allocbufs(d);
+
+   /*
+* adjust time positions, make anything go back delta ticks, so
+* that the new device can start at zero
+*/
+   for (s = d->slot_list; s != NULL; s = s->next) {
+   pos = (long long)s->delta * d->round + s->delta_rem;
+   pos -= (long long)delta * s->round;
+   s->delta_rem = pos % (int)d->round;
+   s->delta = pos / (int)d->round;
+   if (log_level >= 3) {
+   slot_log(s);
+   log_puts(": adjusted: delta -> ");
+   log_puti(s->delta);
+   log_puts(", delta_rem -> ");
+   log_puti(s->delta_rem);
+   log_puts("\n");
+   }
+
+   /* reinitilize the format conversion chain */
+   slot_initconv(s);
+   }
+   if (d->tstate == MMC_RUN) {
+   d->mtc.delta -= delta * MTC_SEC;
+   if (log_level >= 2) {
+   dev_log(d);
+   log_puts(": adjusted mtc: delta ->");
+   log_puti(d->mtc.delta);
+   log_puts("\n");
+   }
+   }
+
+   /* start the device if needed */
+   if (pstate == DEV_RUN)
+   dev_wakeup(d);
+
+   return 1;
+}
+
 int
 dev_ref(struct dev *d)
 {
@@ -1280,7 +1349,7 @@ dev_del(struct dev *d)
}
midi_del(d->midi);
*p = d->next;
-   xfree(d->path);
+   namelist_clear(>path_list);
xfree(d);
 }
 
Index: dev.h
===
RCS file: /cvs/src/usr.bin/sndiod/dev.h,v
retrieving revision 1.21
diff -u -p -u -p -r1.21 dev.h
--- dev.h   12 Jul 2019 06:30:55 -  1.21
+++ dev.h   20 Sep 2019 07:17:54 -
@@ -159,7 +159,7 @@ struct dev {
 #define DEV_INIT   1   /* stopped */
 #define DEV_RUN2   /* playin & recording */
unsigned int pstate;/* one of above */
-   char *path; /* sio path */
+   struct name *path_list;
 
/*
 * actual parameters and runtime state (i.e. once opened)
@@ -201,6 +201,7 @@ extern struct dev *dev_list;
 
 void dev_log(struct dev *);
 void dev_close(struct dev *);
+int dev_reopen(struct dev *);
 struct dev *dev_new(char *, struct aparams *, unsigned int, unsigned int,
 unsigned int, unsigned int, unsigned int, unsigned int);
 struct dev *dev_bynum(int);
Index: fdpass.c
===
RCS file: /cvs/src/usr.bin/sndiod/fdpass.c,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 fdpass.c
--- fdpass.c28 Jun 2019 13:35:03 -  1.6
+++ fdpass.c20 Sep 2019 07:

Re: switching between audio devices with no playback interruption

2019-09-17 Thread Alexandre Ratchov
On Mon, Sep 16, 2019 at 03:44:32PM -0300, Martin Pieuchot wrote:
> On 16/09/19(Mon) 19:31, Alexandre Ratchov wrote:
> > On Sun, Sep 15, 2019 at 03:01:36PM -0300, Martin Pieuchot wrote:
> > > On 29/08/19(Thu) 10:52, Alexandre Ratchov wrote:
> > > > The diff below allows to specifiy "an alternate device" to use in case
> > > > the audio device is disconnected. If so, programs continue playing
> > > > using the other device.
> > > > 
> > > > For instance, if dmesg contains:
> > > > 
> > > > audio0 at azalia0
> > > > audio1 at uaudio0
> > > > 
> > > > and /etc/rc.conf.local contains:
> > > > 
> > > > sndiod_flags=-f rsnd/0 -F rsnd/1
> > > > 
> > > > then sndiod will try to use the usb device by default. If it's
> > > > disconnected, programs continue using the internal one. Note that
> > > > there's no way to detect that the usb device is reconnected;
> > > 
> > > Instead of doing a open/reopen dance in userland, which is inherently
> > > racy, did you consider introducing a driver abstracting the various
> > > audio(4) drivers attached?  I'm thinking of something like wsmux(4) for
> > > audio.  This should allow us to transparently attach/detach usb audio
> > > devices.
> > 
> > I don't see any race conditions: once a device disappears, it is
> > closed and another one is opened in place. This only relies on calls
> > to open() and close() which are not racy. Unlike with hotplug(4),
> > there's no concurency between multiple code-paths; it's all
> > sequencial.
> 
> I understand the logic for closing an existing device, but what about
> plugging a new one?  I understand that this is not the scope of the
> diff, I wondering if that would fit in the model you're suggesting.
> 
> It is just a matter of sending SIGHUP, so I could fake that when
> uaudio(4) attaches for example?

Yes. That's the best we can do with "unix file open/close semantics"
only. Going further, if desired, would require some kind of kernel
support.

There's a full range of "clever" and complicated ways to handle
hot-plugging automatically (and to configure it!). I don't know what's
the best one, some experimenting is needed first.

> Regarding the race I was talking about, doesn't dev_reopen() close the
> current device before trying to open a new one, possibly finding an
> incompatible device or failing to open it?  What happens in this case?

After a SIGHUP, if the new device is incompatible, the old one is
reopened and sound continues. Now I understand your remark: oops!
close() and open() are called in the wrong order, so another process
could steal the device. I'll fix that.

> > I've considered the wsmux-style device for audio. Multiple audio
> > devices can't be combined, so the audio mux device would be a switch
> > between the devices, which is roughly what the diff does.
> > 
> > Switching between devices, when possible, is not trivial, because the
> > state of one device must be restored on another device. It may require
> > channel mapping and/or format conversions, in case the devices don't
> > have the same capabilities. We don't want such complicated code to run
> > with full kernel privileges and to have access to all the physical
> > memory. FWIW, this was one of the reasons for moving the audio
> > sub-system completely to user-space, leaving in the kernel only the
> > code to access the hardware.
> 
> I understand.
> 
> Another question, if a machine has multiple audio devices, including an
> USB one, can't we assume that we should try this one first?  Or could a
> choice mechanism like kern.timecounter with default value could be
> implemented to make this new feature work out of the box?

Almost. The reverse attaching order seems to cover most practical use
cases. If the user coonects a USB device, it's probably because he
wants to use it, so it would make sense for it to be tried first.

But a choice mechanism seems necessary in certain cases. For instance
"pro" audio interfaces need to be handled separately; certain USB
devices (ex. webcams) come with an unused audio interface that the
user may want to skip.

> 
> Finally, is there any downside of having a smaller DEFAULT_ROUND?

No. FWIW, the large default block and buffer sizes used to be
necessary 10+ years ago because MP kernels could delay interrupts for
more than a audio block which used to confuse azalia(4) and envy(4)
permanently. Now drivers are resilient to interrupt loss and any block
size could be used. Recent MP kernels behave better.



Re: switching between audio devices with no playback interruption

2019-09-16 Thread Alexandre Ratchov
On Sun, Sep 15, 2019 at 03:01:36PM -0300, Martin Pieuchot wrote:
> On 29/08/19(Thu) 10:52, Alexandre Ratchov wrote:
> > The diff below allows to specifiy "an alternate device" to use in case
> > the audio device is disconnected. If so, programs continue playing
> > using the other device.
> > 
> > For instance, if dmesg contains:
> > 
> > audio0 at azalia0
> > audio1 at uaudio0
> > 
> > and /etc/rc.conf.local contains:
> > 
> > sndiod_flags=-f rsnd/0 -F rsnd/1
> > 
> > then sndiod will try to use the usb device by default. If it's
> > disconnected, programs continue using the internal one. Note that
> > there's no way to detect that the usb device is reconnected;
> 
> Instead of doing a open/reopen dance in userland, which is inherently
> racy, did you consider introducing a driver abstracting the various
> audio(4) drivers attached?  I'm thinking of something like wsmux(4) for
> audio.  This should allow us to transparently attach/detach usb audio
> devices.

I don't see any race conditions: once a device disappears, it is
closed and another one is opened in place. This only relies on calls
to open() and close() which are not racy. Unlike with hotplug(4),
there's no concurency between multiple code-paths; it's all
sequencial.

I've considered the wsmux-style device for audio. Multiple audio
devices can't be combined, so the audio mux device would be a switch
between the devices, which is roughly what the diff does.

Switching between devices, when possible, is not trivial, because the
state of one device must be restored on another device. It may require
channel mapping and/or format conversions, in case the devices don't
have the same capabilities. We don't want such complicated code to run
with full kernel privileges and to have access to all the physical
memory. FWIW, this was one of the reasons for moving the audio
sub-system completely to user-space, leaving in the kernel only the
code to access the hardware.

Furthermore, putting such code in the kernel would add an extra audio
software layer, while we've all the necessary bits in sndiod.

IMO, if in the future something ends up difficult to implement in
user-space I'm not against pulling parts of the code in the kernel,
but for now this doesn't seem necessary.


> > internal one will keep playing until all programs stop; then sndiod
> > will retry the usb one again. To force sndiod to retry the usb one,
> > send SIGHUP.
> 
> I'm not a fan of having such hotplug logic in userland because it is by
> default racy and opens a can of worm for being able to configure how / 
> when a driver becomes the default one.
> 

The configuration logic is simple and well-defined: there's an ordered
list of devices. Whenever a device disappears, it's closed and the
first working device of the list is opend. Configuring this is a
matter of providing the (oredered) device list; this would be
necessary in kernel implementation as well.

> > As we're at it, the same logic is applied to MIDI as well; it makes
> > possible to swap instuments without restarting programs.
> > 
> > For all this to work, both audio devices must support the same rate
> > (48kHz by default) and the same block size (now 10ms by default). This
> > requires the block size calculation to be fixed in the audio(9) layer,
> > so this diff is needed as well:
> 
> With the idea above you could attach all 'converted' audio drivers to
> the mux ;)

AFAIK, all drivers are now "fixed" (hacks remain, but they still
work). Now, whether switching between the devices is possible depends
only on the hardware and the device operating mode (which is not known
at attach time, btw).



Re: audio: fix block size calculation

2019-09-03 Thread Alexandre Ratchov
On Tue, Sep 03, 2019 at 02:03:52PM -0300, Martin Pieuchot wrote:
> On 18/08/19(Sun) 08:02, Alexandre Ratchov wrote:
> > Currently the block size calculations are broken by design: the driver
> > provides a round_blocksize() function which must retrun a valid block
> > size in *bytes*. Unfortunately, since the driver doesn't know if it's
> > called for the play or for the record block size, it's mathematically
> > impossible to calculate the block size in all cases if play and record
> > number of channels are different. As a consequence, there are
> > half-working and weired hacks to find a usable block sizes.
> > 
> > The diff below addresses this by adding two new driver functions,
> > which are very simple to use:
> > 
> > set_blksz() - calculate and set the block size in *frames*, it's
> > necessarily common to play and recording directions no matter
> > the number of channels,
> > 
> > set_nblks() - calculate the number of blocks per buffer for the given
> > direction.
> > 
> > the diff below shows how to properly calculate the block size in
> > azalia and uaudio. The plan is to convert all drivers from
> > round_blocksize() to the new functions and to delete
> > round_blocksize().
> > 
> > Why is this important? besides for removing ugly (and risky) hacks, we
> > want all our drivers to support common block sizes in the 5ms-50ms
> > range. This would allow to implement switching between audio devices:
> > for instance, start playback on a USB device, unplug the cable and
> > continue on azalia.
> 
> While the cleanup in itself makes sense to me.  I'm unsure if continuing
> to play on a secondary audio device is what we want.  Nowadays phones
> seems to stop music players if an audio device is disconnected.
> 
> Let's assume I'm in a hackathon hearing music via a USB device, if I
> unplug it, accidentally or not, I'd find more logical that the player
> stop instead of forcing the whole room to listen my music.

I totally agree, making this the default would be a mistake as it's
against the "least surprise" principle. It seems practical in few
specific cases though: for instance I use it to temporarilly connect a
usb headset in order to answer a phone call in firefox without the
need to restart sndiod and firefox and then call back the other
person.

> > OK?
> 
> Diff is ok with me, if you think it makes sense to do this change anyway
> :)
> 

Sure, the change is very useful.



switching between audio devices with no playback interruption

2019-08-29 Thread Alexandre Ratchov
The diff below allows to specifiy "an alternate device" to use in case
the audio device is disconnected. If so, programs continue playing
using the other device.

For instance, if dmesg contains:

audio0 at azalia0
audio1 at uaudio0

and /etc/rc.conf.local contains:

sndiod_flags=-f rsnd/0 -F rsnd/1

then sndiod will try to use the usb device by default. If it's
disconnected, programs continue using the internal one. Note that
there's no way to detect that the usb device is reconnected; the
internal one will keep playing until all programs stop; then sndiod
will retry the usb one again. To force sndiod to retry the usb one,
send SIGHUP.

As we're at it, the same logic is applied to MIDI as well; it makes
possible to swap instuments without restarting programs.

For all this to work, both audio devices must support the same rate
(48kHz by default) and the same block size (now 10ms by default). This
requires the block size calculation to be fixed in the audio(9) layer,
so this diff is needed as well:

https://marc.info/?l=openbsd-tech=156610818325864

Please test even if you don't use this feature as this diff changes
the default audio block size (not all USB host controllers support
large transfers).

OK?

Index: dev.c
===
RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
retrieving revision 1.58
diff -u -p -u -p -r1.58 dev.c
--- dev.c   29 Aug 2019 07:38:15 -  1.58
+++ dev.c   29 Aug 2019 07:52:08 -
@@ -968,7 +968,8 @@ dev_new(char *path, struct aparams *par,
return NULL;
}
d = xmalloc(sizeof(struct dev));
-   d->path = xstrdup(path);
+   d->path_list = NULL;
+   namelist_add(>path_list, path);
d->num = dev_sndnum++;
d->opt_list = NULL;
 
@@ -1173,6 +1174,94 @@ dev_close(struct dev *d)
dev_close_do(d);
 }
 
+/*
+ * Close the device, but attempt to migrate everything to a new sndio
+ * device.
+ */
+void
+dev_reopen(struct dev *d)
+{
+   struct slot *s;
+   long long pos;
+   unsigned int mode, round, bufsz, rate, pstate;
+   int delta;
+
+   /* not opened */
+   if (d->pstate == DEV_CFG)
+   return;
+
+   if (log_level >= 1) {
+   dev_log(d);
+   log_puts(": reopening device\n");
+   }
+
+   /* save state */
+   mode = d->mode;
+   round = d->round;
+   bufsz = d->bufsz;
+   rate = d->rate;
+   delta = d->delta;
+   pstate = d->pstate;
+
+   /* close device */
+   dev_close_do(d);
+
+   /* open device */
+   if (!dev_open_do(d)) {
+   if (log_level >= 1) {
+   dev_log(d);
+   log_puts(": found no working alternate device\n");
+   }
+   dev_exitall(d);
+   return;
+   }
+
+   /* check if new parameters are compatible with old ones */
+   if (d->mode != mode ||
+   d->round != round ||
+   d->bufsz != bufsz ||
+   d->rate != rate) {
+   if (log_level >= 1) {
+   dev_log(d);
+   log_puts(": alternate device not compatible\n");
+   }
+   dev_close(d);
+   return;
+   }
+
+   /*
+* adjust time positions, make anything go back delta ticks, so
+* that the new device can start at zero
+*/
+   for (s = d->slot_list; s != NULL; s = s->next) {
+   pos = (long long)(d->round - delta) * s->round + s->delta_rem;
+   s->delta_rem = pos % d->round;
+   s->delta += pos / (int)d->round;
+   s->delta -= s->round;
+   if (log_level >= 2) {
+   slot_log(s);
+   log_puts(": adjusted: delta -> ");
+   log_puti(s->delta);
+   log_puts(", delta_rem -> ");
+   log_puti(s->delta_rem);
+   log_puts("\n");
+   }
+   }
+   if (d->tstate == MMC_RUN) {
+   d->mtc.delta -= delta * MTC_SEC;
+   if (log_level >= 2) {
+   dev_log(d);
+   log_puts(": adjusted mtc: delta ->");
+   log_puti(d->mtc.delta);
+   log_puts("\n");
+   }
+   }
+
+   /* start the device if needed */
+   if (pstate == DEV_RUN)
+   dev_wakeup(d);
+}
+
 int
 dev_ref(struct dev *d)
 {
@@ -1279,7 +1368,7 @@ dev_del(struct dev *d)
}
midi_del(d->midi);
*p = d->next;
-   xfree(d->path);
+   namelist_clear(>path_list);
xfree(d);
 }
 
Index: dev.h
===
RCS file: /cvs/src/usr.bin/sndiod/dev.h,v
retrieving revision 1.21
diff -u -p -u -p -r1.21 dev.h
--- dev.h   12 Jul 2019 06:30:55 -  1.21
+++ dev.h   29 

audio: fix block size calculation

2019-08-18 Thread Alexandre Ratchov
Currently the block size calculations are broken by design: the driver
provides a round_blocksize() function which must retrun a valid block
size in *bytes*. Unfortunately, since the driver doesn't know if it's
called for the play or for the record block size, it's mathematically
impossible to calculate the block size in all cases if play and record
number of channels are different. As a consequence, there are
half-working and weired hacks to find a usable block sizes.

The diff below addresses this by adding two new driver functions,
which are very simple to use:

set_blksz() - calculate and set the block size in *frames*, it's
necessarily common to play and recording directions no matter
the number of channels,

set_nblks() - calculate the number of blocks per buffer for the given
direction.

the diff below shows how to properly calculate the block size in
azalia and uaudio. The plan is to convert all drivers from
round_blocksize() to the new functions and to delete
round_blocksize().

Why is this important? besides for removing ugly (and risky) hacks, we
want all our drivers to support common block sizes in the 5ms-50ms
range. This would allow to implement switching between audio devices:
for instance, start playback on a USB device, unplug the cable and
continue on azalia.

OK?

Index: share/man/man9/audio.9
===
RCS file: /cvs/src/share/man/man9/audio.9,v
retrieving revision 1.27
diff -u -p -r1.27 audio.9
--- share/man/man9/audio.9  12 Mar 2019 08:18:34 -  1.27
+++ share/man/man9/audio.9  18 Aug 2019 05:28:35 -
@@ -82,6 +82,10 @@ struct audio_hw_if {
void (*)(void *), void *, struct audio_params *);
void(*copy_output)(void *hdl, size_t bytes);
void(*underrun)(void *hdl);
+   int (*set_blksz)(void *, int,
+   struct audio_params *, struct audio_params *, int);
+   int (*set_nblks)(void *, int, int,
+   struct audio_params *, int);
 };
 
 struct audio_params {
@@ -417,6 +421,56 @@ Drivers using bounce buffers for transfe
 ring buffer and the device must implement this method to skip
 one block from the audio ring buffer and transfer the
 corresponding amount of silence to the device.
+.It Fn "int (*set_blksz)" "void *hdl" "int mode" \
+"struct audio_params *play" "struct audio_params *rec" "int blksz"
+This function is called to set the audio block size.
+.Fa mode
+is a combination of the
+.Dv AUMODE_RECORD
+and
+.Dv AUMODE_PLAY
+flags indicating the current mode set with the
+.Fn open
+function.
+The
+.Fa play
+and
+.Fa rec
+structures contain the current encoding set with the
+.Fn set_params
+function.
+.Fa blksz
+is the desired block size in frames.
+It may be adjusted to match hardware constraints.
+This function returns the adjusted block size.
+.It Fn "int (*set_nblks)" "void *hdl" "int dir" "int blksz" \
+"struct audio_params *params" "int nblks"
+This function is called to set the number of audio blocks in
+the ring buffer.
+.Fa dir
+is either the
+.Dv AUMODE_RECORD
+or the
+.Dv AUMODE_PLAY
+flag, indicating which ring buffer size is set.
+The
+.Fa params
+structure contains the encoding parameters set by the
+.Fn set_params
+method.
+.Fa blksz
+is the current block size in frames set with the
+.Fa set_params
+function.
+The
+.Fa params
+structure is the current encoding parameters, set with the
+.Fn set_params
+function.
+.Fa nblks
+is the desired number of blocks in the ring buffer.
+It may be lowered to at least two, to match hardware constraints.
+This function returns the adjusted number of blocks.
 .El
 .Pp
 If the audio hardware is capable of input from more
Index: sys/dev/audio.c
===
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.180
diff -u -p -r1.180 audio.c
--- sys/dev/audio.c 17 Aug 2019 05:04:56 -  1.180
+++ sys/dev/audio.c 18 Aug 2019 05:28:36 -
@@ -193,6 +193,31 @@ audio_gcd(unsigned int a, unsigned int b
return a;
 }
 
+/*
+ * Calculate the least block size (in frames) such that both the
+ * corresponding play and/or record block sizes (in bytes) are multiple
+ * of the given number of bytes.
+ */
+int
+audio_blksz_bytes(int mode,
+   struct audio_params *p, struct audio_params *r, int bytes)
+{
+   unsigned int np, nr;
+
+   if (mode & AUMODE_PLAY) {
+   np = bytes / audio_gcd(p->bps * p->channels, bytes);
+   if (!(mode & AUMODE_RECORD))
+   nr = np;
+   }
+   if (mode & AUMODE_RECORD) {
+   nr = bytes / audio_gcd(r->bps * r->channels, bytes);
+   if (!(mode & AUMODE_PLAY))
+   np = nr;
+   }
+
+   return nr * np / audio_gcd(nr, np);
+}
+
 int
 audio_buf_init(struct audio_softc *sc, struct audio_buf *buf, int dir)
 {
@@ -625,11 +650,19 @@ audio_canstart(struct 

Re: Stop using PUSER in tsleep(9)

2019-07-09 Thread Alexandre Ratchov
On Mon, Jul 08, 2019 at 06:43:34PM -0300, Martin Pieuchot wrote:
> PUSER has been used since the import of ___thrsleep(2) / librthread.
> This value has been then copied to futex(2).  No other tsleep(9) call
> use this value.  I'd like to stop using it to be able to differentiate
> sleeping priorities that will be always < PUSER, to running priorities.
> The only running priorities that could be < PUSER are the one of niced
> programs like sndiod(9).
> 
> Ok?

Seems correct. Use of PWAIT shouldn't hurt interactive programs which
use PSOCK (or better).

Why do you want to differenciate sleeping priorities from running
priorities?



Re: expose host capabilities relative to usb isochronous xfers

2019-06-13 Thread Alexandre Ratchov
On Thu, Jun 13, 2019 at 06:03:03AM +0300, Artturi Alm wrote:
> On Wed, Jun 12, 2019 at 03:55:59PM +0200, Alexandre Ratchov wrote:
> > Currenty USB device driver code has no way to obtain how many frames
> > can be scheduled on the HC. If it attempts to schedule too many
> > frames, usbd_transfer() fails or silently misbehaves.
> > 
> > For audio this is a big problem because the max frames count
> > determines the block size which must be reported to upper layers
> > before any transfer starts. This makes impossible to implement uaudio
> > properly. Currently there's a temporary hack to workaround this, which
> > limits the block size. Now that uaudio is in and works, it's time to
> > start removing such hacks.
> > 
> > Similarly, driver code needs to know the minimum number of frames per
> > transfer to get an interrupt (ie the completion callback). Indeed, if
> > the transfer frame count is not rounded to it, we silently miss the
> > interrupt, the completion call-back is not called and playback
> > stutters.
> > 
> > The diff below adds a new usbd_bus_info() function which reports the
> > maximum frames that can be scheduled and the minimum frames per
> > transfer to get an interrupt.
> > 
> > [snip]
> > Index: usbdi.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> > retrieving revision 1.100
> > diff -u -p -u -p -r1.100 usbdi.c
> > --- usbdi.c 18 Nov 2018 16:33:26 -  1.100
> > +++ usbdi.c 12 Jun 2019 13:33:41 -
> > @@ -275,6 +275,12 @@ usbd_close_pipe(struct usbd_pipe *pipe)
> > return (USBD_NORMAL_COMPLETION);
> >  }
> >  
> > +void
> > +usbd_bus_info(struct usbd_device *dev, struct usbd_bus_info *info)
> > +{
> > +   dev->bus->methods->info(dev, info);
> > +}
> > +
> >  usbd_status
> >  usbd_transfer(struct usbd_xfer *xfer)
> >  {
> > [snip]
> 
> i think above is not good enough with only ehci, ohci, uhci and xhci
> being taken care of in [snip]'ed parts of the diff unfortunately:]
> just thinking about dwc2.
> 

Thanks. I grep'ed for usbd_bus_methods, and dwc2 seems to be the only
one I missed. Below is a new diff with dwc2 bits and few fixes. I've
no arm/octeon machines to test, anyone has one for a quick test?

Interestingly, dwc2 supports at most DWC2_MAXISOCPACKETS frames, which
is defined to 16 only. Without this diff, the uaudio driver would
attempt to schedule ~48 frames which would trigger a KASSERT() in
dwc2_device_start() immediately.

BTW, 16 is too small for usb2.0 devices to work at all. I'd recommend
512, which is two blocks of 20ms each.

Index: ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.204
diff -u -p -r1.204 ehci.c
--- ehci.c  31 Mar 2019 06:16:38 -  1.204
+++ ehci.c  13 Jun 2019 07:25:31 -
@@ -113,6 +113,7 @@ struct ehci_pipe {
 u_int8_t   ehci_reverse_bits(u_int8_t, int);
 
 usbd_statusehci_open(struct usbd_pipe *);
+void   ehci_info(struct usbd_device *, struct usbd_bus_info *);
 intehci_setaddr(struct usbd_device *, int);
 void   ehci_poll(struct usbd_bus *);
 void   ehci_softintr(void *);
@@ -225,6 +226,7 @@ struct usbd_bus_methods ehci_bus_methods
.do_poll = ehci_poll,
.allocx = ehci_allocx,
.freex = ehci_freex,
+   .info = ehci_info,
 };
 
 struct usbd_pipe_methods ehci_root_ctrl_methods = {
@@ -491,6 +493,21 @@ ehci_init(struct ehci_softc *sc)
sc->sc_flsize * sizeof(struct ehci_soft_itd *));
usb_freemem(>sc_bus, >sc_fldma);
return (err);
+}
+
+void
+ehci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+   /*
+* XXX: even if most hosts have 1024 frame lists, only 256
+* frame transfers work reliably. As soon as 256 is exceeded,
+* we start getting zeroed frames if multiple device are
+* running simultaneously. Set this to sc->sc_fl_size, once
+* ehci is fixed. Interrups occur every 1m, despite the
+* EHCI_CMD_ITC_2 setting.
+*/
+   info->nframes_max = 256;
+   info->intr_thres = (dev->speed >= USB_SPEED_HIGH) ? 8 : 1;
 }
 
 int
Index: ohci.c
===
RCS file: /cvs/src/sys/dev/usb/ohci.c,v
retrieving revision 1.156
diff -u -p -r1.156 ohci.c
--- ohci.c  11 Mar 2019 17:50:08 -  1.156
+++ ohci.c  13 Jun 2019 07:25:32 -
@@ -87,6 +87,7 @@ usbd_status   ohci_alloc_std_chain(struct 
struct ohci_soft_td **);
 
 usbd_statusohci_open(struct usbd_pipe *);
+void   ohci_info(struct usbd_devi

expose host capabilities relative to usb isochronous xfers

2019-06-12 Thread Alexandre Ratchov
Currenty USB device driver code has no way to obtain how many frames
can be scheduled on the HC. If it attempts to schedule too many
frames, usbd_transfer() fails or silently misbehaves.

For audio this is a big problem because the max frames count
determines the block size which must be reported to upper layers
before any transfer starts. This makes impossible to implement uaudio
properly. Currently there's a temporary hack to workaround this, which
limits the block size. Now that uaudio is in and works, it's time to
start removing such hacks.

Similarly, driver code needs to know the minimum number of frames per
transfer to get an interrupt (ie the completion callback). Indeed, if
the transfer frame count is not rounded to it, we silently miss the
interrupt, the completion call-back is not called and playback
stutters.

The diff below adds a new usbd_bus_info() function which reports the
maximum frames that can be scheduled and the minimum frames per
transfer to get an interrupt.

Index: ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.204
diff -u -p -u -p -r1.204 ehci.c
--- ehci.c  31 Mar 2019 06:16:38 -  1.204
+++ ehci.c  12 Jun 2019 13:33:38 -
@@ -113,6 +113,7 @@ struct ehci_pipe {
 u_int8_t   ehci_reverse_bits(u_int8_t, int);
 
 usbd_statusehci_open(struct usbd_pipe *);
+void   ehci_info(struct usbd_device *, struct usbd_bus_info *);
 intehci_setaddr(struct usbd_device *, int);
 void   ehci_poll(struct usbd_bus *);
 void   ehci_softintr(void *);
@@ -225,6 +226,7 @@ struct usbd_bus_methods ehci_bus_methods
.do_poll = ehci_poll,
.allocx = ehci_allocx,
.freex = ehci_freex,
+   .info = ehci_info,
 };
 
 struct usbd_pipe_methods ehci_root_ctrl_methods = {
@@ -493,6 +498,21 @@ ehci_init(struct ehci_softc *sc)
return (err);
 }
 
+void
+ehci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+   /*
+* XXX: even if most hosts have 1024 frame lists, only 256
+* frame transfers work reliably. As soon as 256 is exceeded,
+* we start getting zeroed frames if multiple device are
+* running simultaneously. Set this to sc->sc_fl_size, once
+* ehci is fixed. Interrups occur every 1m, despite the
+* EHCI_CMD_ITC_2 setting.
+*/
+   info->nframes_max = 256;
+   info->intr_thres = (dev->speed >= USB_SPEED_HIGH) ? 8 : 1;
+}
+
 int
 ehci_intr(void *v)
 {
Index: ohci.c
===
RCS file: /cvs/src/sys/dev/usb/ohci.c,v
retrieving revision 1.156
diff -u -p -u -p -r1.156 ohci.c
--- ohci.c  11 Mar 2019 17:50:08 -  1.156
+++ ohci.c  12 Jun 2019 13:33:40 -
@@ -87,6 +87,7 @@ usbd_status   ohci_alloc_std_chain(struct 
struct ohci_soft_td **);
 
 usbd_statusohci_open(struct usbd_pipe *);
+void   ohci_info(struct usbd_device *, struct usbd_bus_info *);
 intohci_setaddr(struct usbd_device *, int);
 void   ohci_poll(struct usbd_bus *);
 void   ohci_softintr(void *);
@@ -236,6 +237,7 @@ struct usbd_bus_methods ohci_bus_methods
.do_poll = ohci_poll,
.allocx = ohci_allocx,
.freex = ohci_freex,
+   .info = ohci_info,
 };
 
 struct usbd_pipe_methods ohci_root_ctrl_methods = {
@@ -931,6 +933,13 @@ ohci_init(struct ohci_softc *sc)
  bad1:
usb_freemem(>sc_bus, >sc_hccadma);
return (err);
+}
+
+void
+ohci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+   info->nframes_max = OHCI_SITD_CHUNK;
+   info->intr_thres = 1;
 }
 
 struct usbd_xfer *
Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.144
diff -u -p -u -p -r1.144 uaudio.c
--- uaudio.c9 May 2019 07:09:04 -   1.144
+++ uaudio.c12 Jun 2019 13:33:41 -
@@ -382,6 +382,7 @@ struct uaudio_softc {
unsigned int ufps;  /* USB frames per second */
unsigned int sync_pktsz;/* size of sync packet */
unsigned int host_nframes;  /* max frames we can schedule */
+   unsigned int host_intr_thres;   /* min frames to get an interrupt */
 
int diff_nsamp; /* samples play is ahead of rec */
int diff_nframes;   /* frames play is ahead of rec */
@@ -2820,12 +2821,15 @@ uaudio_stream_open(struct uaudio_softc *
s->data_pipe = NULL;
s->sync_pipe = NULL;
 
-   s->nframes_mask = 0;
-   i = a->fps;
-   while (i > 1000) {
-   s->nframes_mask = (s->nframes_mask << 1) | 1;
-   i >>= 1;
-   }
+   /*
+* Find the smallest transfer size (power of two), larger than
+* the poll interval and the interrupt threshold.
+*/
+   i = 0;
+   while (a->fps < 

Re: scheduler small changes

2019-05-17 Thread Alexandre Ratchov
On Wed, May 15, 2019 at 09:05:32AM -0500, Amit Kulkarni wrote:
> 
> This diff survived multiple tens of kernel builds, a bsd.sp build,
> bsd.rd build, a bsd.mp without these changes, userland/xenocara, a
> make regress a few days ago all on 3 desktops on amd64. Ran under
> all possible scenarios listed in previous sentence. No major
> invasive changes attempted, all tools should work as is, if its
> broken, please let me know. This is faster than current, but not
> sure by how much, placebo effect in play.

CPU-bound processes (builds) run at the same priorority so both the
current and the proposed algorithms should split them in equal time
slices resulting in the same behavior.

> 
> Not sure if the reduction from 32 queues to a single TAILQ would be
> accepted, but tried it anyway, it is definitely faster. This code
> tries to reduce the complexity in deciding which queue to place the
> proc on. 

The intent of using multiple queues is to allow to wakeup processes
and to make them runnable with the desired priority. To achieve this
with a single queue, this would require to insert the process to wake
up in the "middle" of the TAILQ. This is not a problem as there are
only few runnable processes on a typical system.

Example, on a single CPU machine, suppose a CPU-bound process is
running (ex. gzip compressing a file), we want any I/O-bound processes
(ex. audio player) that wakes up to be inserted on the run queue
before the CPU-bound process.



Re: inteldrm: Use "the flush page mechanism" only on chips having it.

2019-05-09 Thread Alexandre Ratchov
On Thu, May 09, 2019 at 10:26:52PM +1000, Jonathan Gray wrote:
> On Thu, May 09, 2019 at 07:45:37AM +0200, Alexandre Ratchov wrote:
> > In the intel_gtt_chipset_setup() routine, called at initialization,
> > the driver maps pci space for certain chip models only, but later in
> > intel_gtt_chipset_flush() it calls bus_space_write() on it in cases
> > it's not initialized.
> > 
> > This results in crashes during boot with "Pineview" generation chips,
> > which are "gen == 3", but, according to intel_gtt_chipset_setup() have
> > no flush page mechanism.
> > 
> > Fix this by adding a flag indicating whether the feature is available
> > or not. Then use the flag to check if it's OK to write to the pci
> > space. This is the what the driver did before the 14 april update.
> > 
> > OK?
> > 
> > FWIW, first it looked surprising to do nothing in
> > intel_gtt_chipset_flush(), so I also tried to call the "gen < 3"
> > code. It works as well.
> 
> intel_gtt_chipset_setup() is wrong the 915/945 ifp setup should be used
> on pineview as well.  g33 is the only gen3 that is different.
> 
> This was apparently missed in
> 
> 
> revision 1.78
> date: 2010/05/12 16:20:00;  author: oga;  state: Exp;  lines: +2 -0;
> Add Pineview M to intagp and inteldrm.
> 
> Tested (and initial tweaked diff) from Erik Mugele; thanks!
> 
> 
> The original ifp setup bits were added in sys/dev/pci/drm/i915_drv.c
> rev 1.45 and the ifp setup we use today come from that.
> 

Makes much more sense this way, thanks. Tested on laptop and it works
as expected.

ok ratchov@



inteldrm: Use "the flush page mechanism" only on chips having it.

2019-05-08 Thread Alexandre Ratchov
In the intel_gtt_chipset_setup() routine, called at initialization,
the driver maps pci space for certain chip models only, but later in
intel_gtt_chipset_flush() it calls bus_space_write() on it in cases
it's not initialized.

This results in crashes during boot with "Pineview" generation chips,
which are "gen == 3", but, according to intel_gtt_chipset_setup() have
no flush page mechanism.

Fix this by adding a flag indicating whether the feature is available
or not. Then use the flag to check if it's OK to write to the pci
space. This is the what the driver did before the 14 april update.

OK?

FWIW, first it looked surprising to do nothing in
intel_gtt_chipset_flush(), so I also tried to call the "gen < 3"
code. It works as well.

Index: intel_gtt.c
===
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_gtt.c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 intel_gtt.c
--- intel_gtt.c 14 Apr 2019 10:14:52 -  1.2
+++ intel_gtt.c 9 May 2019 05:37:09 -
@@ -120,6 +120,8 @@ intel_gtt_chipset_setup(struct drm_devic
struct inteldrm_softc *dev_priv = dev->dev_private;
struct pci_attach_args bpa;
 
+   dev_priv->has_ifp = 0;
+
if (INTEL_GEN(dev_priv) >= 6)
return;
 
@@ -133,8 +135,10 @@ intel_gtt_chipset_setup(struct drm_devic
if (IS_I915G(dev_priv) || IS_I915GM(dev_priv) || IS_I945G(dev_priv) ||
IS_I945GM(dev_priv)) {
i915_alloc_ifp(dev_priv, );
+   dev_priv->has_ifp = 1;
} else if (INTEL_GEN(dev_priv) >= 4 || IS_G33(dev_priv)) {
i965_alloc_ifp(dev_priv, );
+   dev_priv->has_ifp = 1;
} else {
int nsegs;
/*
@@ -192,7 +196,7 @@ intel_gtt_chipset_flush(void)
 * The write will return when it is done.
 */
if (INTEL_GEN(dev_priv) >= 3) {
-   if (dev_priv->ifp.i9xx.bsh != 0)
+   if (dev_priv->has_ifp && dev_priv->ifp.i9xx.bsh != 0)
bus_space_write_4(dev_priv->ifp.i9xx.bst,
dev_priv->ifp.i9xx.bsh, 0, 1);
} else {



Re: new USB audio class v2.0 driver

2019-04-26 Thread Alexandre Ratchov
On Thu, Apr 25, 2019 at 02:49:35AM -0700, alexh wrote:
> Hi,
> 
> 
> Alexandre Ratchov-2 wrote
> > If you have an audio device that is class compliant (aka vendor claims 
> > it's "driverless" on MacOS) *and* one of the above host/hub/device 
> > combinations then I'd be very interested in test reports. Especially 
> > I'd like to know about possible regressions. 
> 
> I tested with a Focusrite Scarlett 2i4 which should be a class compliant
> v2.0 device, but I get "only one AC iface allowed".
> 
> Here is my dmesg:
> 

Could you install the sysutils/usbutils package and send me the output
of "lsusb -vv" please?

Thanks.



uhci(4): fix delayed completions for isochronous transfers

2019-02-06 Thread Alexandre Ratchov
When an isochronous transfer of n frames is scheduled, the last frame
i.e. frame number (n - 1) is set to generate an interrupt.

To figure out which transfer generated the interrupt, the interrupt
handler iterates over all outstanding transfers and checks if the last
TD of the transfer is active. If it's not active, then the transter is
done and the completion call-back is invoked.

Except that for isochronous transfers the interrupt handler checks the
n-th TD insted of the last one. In turn, the transfer appears as
active and the completion call-back is not called (yet). It will be
called on the interrupt of the next transfer (if any).

OK?

Index: uhci.c
===
RCS file: /cvs/src/sys/dev/usb/uhci.c,v
retrieving revision 1.144
diff -u -p -u -p -r1.144 uhci.c
--- uhci.c  16 Nov 2018 11:57:29 -  1.144
+++ uhci.c  7 Feb 2019 05:21:29 -
@@ -2188,7 +2188,7 @@ uhci_device_isoc_start(struct usbd_xfer 
 #endif
 
/* Find the last TD */
-   i = ux->curframe + xfer->nframes;
+   i = ux->curframe + (xfer->nframes - 1);
if (i >= UHCI_VFRAMELIST_COUNT)
i -= UHCI_VFRAMELIST_COUNT;
end = upipe->u.iso.stds[i];



Re: new USB audio class v2.0 driver

2019-01-23 Thread Alexandre Ratchov
On Wed, Jan 23, 2019 at 01:23:05PM -0200, Martin Pieuchot wrote:
> On 31/12/18(Mon) 16:58, Alexandre Ratchov wrote:
> > Hi,
> > 
> > Here's a new driver for both USB audio class (UAC) v1.0 and v2.0
> > devices, it would replace the current one. It focuses on reliability
> > and proper synchronization, including in low-latency configurations.
> 
> Some comments about the code.  I believe this should go in now so we
> can tackle the remaining issue in tree, so ok mpi@

I got a lot of very helpful test reports. I've almost finished fixing
the problems people reported, so I'll repost a new diff soon and
commit that one instead if you're still OK.

> 
> - Please put the UE_* macro in dev/usb/usb.h (not sys/usb.h) ;o)
> 

;-)

> - You could get rid of UAUDIO_NRATES and use nitems() instead
> 
> - What does UAUDIO_USE_FRAC mean?
> 

If the sample rate is not multiple of the USB frame rate, the number
of samples per frame can't be constant: For instance, if frame rate is
1000 frames-per-second and sample rate is 44100 samples-per-second,
the number of samples per frame would be 44.1. Devices handle this
with a variable number for frames: in this example, most of the frames
carry 44 samples, but every 10 samples we have one 45-sample frame, to
reach the 44.1 samples per frame average. In this case the driver has
to manipulate "fractional" frames.

The diff I posted doesn't handle such rates. It's a pure arithmetic
problem, I didn't want to distract people with it.

> - Some values read from descriptors are passed to malloc(9) w/o being
>   checked.  Take the example of `count' in uaudio_req_ranges().  What's
>   the maximum size for `count' and `req_size'?
>   What's the maximum value for `nframes_max'?
> 

- count is 16-bit, so worst case req_size is roughly 65535 multiplied
  by the word size (at most 4-byte) plus two bytes of header. That's
  less than 256kB. Most devices have a signle range, but who knows...

  I've added a comment to say so.

- I my current version of the diff nframes_max limited to 240 on usb2
  and 30 on usb1, which is 30ms of audio. This is because ehci and
  uhci don't seem to handle large isoc transfers. As 30ms enough for
  audio, I didn't try to understand the reason of the limitation yet.

> - You don't need to call usbd_abort_pipe(9) before calling
>   usbd_close_pipe(9), it does it for you.
> 
> - What about using %s __func__ in DPRINTF() instead of function names
>   to help search through your code? ;o)

thans, fixed these as well.



Re: new USB audio class v2.0 driver

2019-01-01 Thread Alexandre Ratchov
On Tue, Jan 01, 2019 at 11:02:39PM -, Christian Weisgerber wrote:
> On 2018-12-31, Alexandre Ratchov  wrote:
> 
> > Our current USB sub-system has limitations that currently allow only
> > the following combinations:
> >  - USB v2 devices on ehci(4) only
> >  - USB v1 devices on uhci(4), ohci(4) or ehci(4) root hub only
> >
> > If you have an audio device that is class compliant (aka vendor claims
> > it's "driverless" on MacOS) *and* one of the above host/hub/device
> > combinations then I'd be very interested in test reports.
> 
> Just to clarify: The new driver supports ONLY these combinations.
> 

well, the old one supports only these as well, except that it prints
no error message.

> The majority of people who have xhci don't need to try:
> uaudio0: xhci(4) not supported yet, try another port

Thanks for insisting on this. 

Until the xhci driver is finished, those who absolutly want to
test/use the new uaudio driver could try to disable xhci on the boot
prompt. On most machines, ehci will attach instead of xhci. This would
allow using usb 2.0 devices out of the box. To test usb 1.1 devices,
you also need to have a usb connectors on the ehci root hub, certain
machines have them, others don't. On those which have them, there's a
compagnion uhci device, example:

usb0 at ehci0: USB revision 2.0
usb1 at uhci0: USB revision 1.0
usb2 at uhci1: USB revision 1.0
usb3 at uhci2: USB revision 1.0
usb4 at uhci3: USB revision 1.0
uhub0 at usb1 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 
addr 1
uhub1 at usb2 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 
addr 1
uhub2 at usb3 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 
addr 1
uhub3 at usb4 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 
addr 1
uhub4 at usb0 configuration 1 interface 0 "Intel EHCI root hub" rev 2.00/1.00 
addr 1
uaudio0 at uhub1 port 1 configuration 1 interface 1 "ABC C-Media USB Headphone 
Set" rev 1.10/1.00 addr 2
uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 8 ctls



  1   2   3   >