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



remove unused header from sys/dev/usb/umidi

2018-12-31 Thread Alexandre Ratchov
Found by jcs@.

ok?

Index: umidi.c
===
RCS file: /cvs/src/sys/dev/usb/umidi.c,v
retrieving revision 1.51
diff -u -p -r1.51 umidi.c
--- umidi.c 16 Nov 2018 11:55:56 -  1.51
+++ umidi.c 31 Dec 2018 20:08:57 -
@@ -44,7 +44,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 



Re: athn(4) hostap: mem leak

2018-12-01 Thread Alexandre Ratchov
On Sat, Dec 01, 2018 at 10:14:38AM +0100, Benjamin Baier wrote:
> On Fri, 30 Nov 2018 16:55:42 +0100
> Alexandre Ratchov  wrote:
> 
> > On Fri, Nov 30, 2018 at 01:49:56PM +0100, Benjamin Baier wrote:
> > > Hi
> > > 
> > > There is a leak of *arg in 
> > > dev/usb/if_athn_usb.c:athn_usb_newauth() line 1263
> > > since Rev. 1.49
> > > Because athn_usb_do_async() memcpy's the argument anyway.
> > > 
> > > Found with llvm/scan-build.
> > > 
> > > Instead of adding free(arg) I opted to make this function
> > > more like the other ones which call athn_usb_do_async.
> > >   
> > 
> > Hi,
> > 
> > AFAICS, athn_usb_do_async() will schedule a call to
> > athn_usb_newauth_cb(), which will use arg after the functin has
> > returned. The arg memory location must stay valid after return from
> > athn_usb_newauth(). So we can neither use free() nor a local variable.
> 
> athn_usb_do_async() takes care of that by memcpy-ing arg to cmd->data
> before calling usb_add_task().
> 
> other calls to athn_usb_do_async() do use local variables.
> if_athn_usb.c:1032:athn_usb_do_async(usc, athn_usb_newstate_cb, , 
> sizeof(cmd));
> if_athn_usb.c:1317:athn_usb_do_async(usc, athn_usb_ampdu_tx_start_cb, , 
> sizeof(cmd));
> if_athn_usb.c:1641:athn_usb_do_async(usc, athn_usb_set_key_cb, , 
> sizeof(cmd));
> if_athn_usb.c:1673:athn_usb_do_async(usc, athn_usb_delete_key_cb, , 
> sizeof(cmd));
> 

You're right, I missed the memcpy() call, sorry.

Your diff is correct.



Re: free(9) sizes for sysv_msg

2018-11-30 Thread Alexandre Ratchov
On Fri, Nov 30, 2018 at 04:02:07PM -0200, Martin Pieuchot wrote:
> On 29/11/18(Thu) 22:42, Alexandre Ratchov wrote:
> > On Thu, Nov 29, 2018 at 04:51:19PM -0200, Martin Pieuchot wrote:
> > > Trivial one, ok?
> > > 
> > > Index: kern/sysv_msg.c
> > > ===
> > > RCS file: /cvs/src/sys/kern/sysv_msg.c,v
> > > retrieving revision 1.33
> > > diff -u -p -r1.33 sysv_msg.c
> > > --- kern/sysv_msg.c   15 Sep 2016 02:00:16 -  1.33
> > > +++ kern/sysv_msg.c   29 Nov 2018 18:47:05 -
> > > @@ -699,7 +699,7 @@ sysctl_sysvmsg(int *name, u_int namelen,
> > >   msginfo.msgmni * sizeof(struct msqid_ds);
> > >  
> > 
> > infolen is calculated twice; the first infolen calculation is used as
> > argument to malloc(). Your diff makes the second one the size argument
> > to free(), which doesn't seem correct.
> 
> Thanks for pointing that out.  Revised diff adding & using a different
> variable.
> 

ok ratchov



Re: athn(4) hostap: mem leak

2018-11-30 Thread Alexandre Ratchov
On Fri, Nov 30, 2018 at 01:49:56PM +0100, Benjamin Baier wrote:
> Hi
> 
> There is a leak of *arg in 
> dev/usb/if_athn_usb.c:athn_usb_newauth() line 1263
> since Rev. 1.49
> Because athn_usb_do_async() memcpy's the argument anyway.
> 
> Found with llvm/scan-build.
> 
> Instead of adding free(arg) I opted to make this function
> more like the other ones which call athn_usb_do_async.
> 

Hi,

AFAICS, athn_usb_do_async() will schedule a call to
athn_usb_newauth_cb(), which will use arg after the functin has
returned. The arg memory location must stay valid after return from
athn_usb_newauth(). So we can neither use free() nor a local variable.

The athn_usb_newauth_cb() callback calls free(), so there's no memory
leak unless the callback is cancelled. I don't know it can be
cancelled, I see no code doing so.

> Only compile tested... looking for tests.
> 
> Greetings Ben
> 
> Index: if_athn_usb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 if_athn_usb.c
> --- if_athn_usb.c 6 Sep 2018 11:50:54 -   1.51
> +++ if_athn_usb.c 29 Nov 2018 18:33:40 -
> @@ -1231,7 +1231,7 @@ athn_usb_newauth(struct ieee80211com *ic
>   struct ifnet *ifp = >ic_if;
>   struct athn_node *an = (struct athn_node *)ni;
>   int nsta;
> - struct athn_usb_newauth_cb_arg *arg;
> + struct athn_usb_newauth_cb_arg arg;
>  
>   if (ic->ic_opmode != IEEE80211_M_HOSTAP)
>   return 0;
> @@ -1254,12 +1254,9 @@ athn_usb_newauth(struct ieee80211com *ic
>* In a process context, try to add this node to the
>* firmware table and confirm the AUTH request.
>*/
> - arg = malloc(sizeof(*arg), M_DEVBUF, M_NOWAIT);
> - if (arg == NULL)
> - return ENOMEM;
> - arg->ni = ieee80211_ref_node(ni);
> - arg->seq = seq;
> - athn_usb_do_async(usc, athn_usb_newauth_cb, arg, sizeof(*arg));
> + arg.ni = ieee80211_ref_node(ni);
> + arg.seq = seq;
> + athn_usb_do_async(usc, athn_usb_newauth_cb, , sizeof(arg));
>   return EBUSY;
>  #else
>   return 0;
> 

-- 



Re: free(9) sizes for sysv_msg

2018-11-29 Thread Alexandre Ratchov
On Thu, Nov 29, 2018 at 04:51:19PM -0200, Martin Pieuchot wrote:
> Trivial one, ok?
> 
> Index: kern/sysv_msg.c
> ===
> RCS file: /cvs/src/sys/kern/sysv_msg.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 sysv_msg.c
> --- kern/sysv_msg.c   15 Sep 2016 02:00:16 -  1.33
> +++ kern/sysv_msg.c   29 Nov 2018 18:47:05 -
> @@ -699,7 +699,7 @@ sysctl_sysvmsg(int *name, u_int namelen,
>   msginfo.msgmni * sizeof(struct msqid_ds);
>  

infolen is calculated twice; the first infolen calculation is used as
argument to malloc(). Your diff makes the second one the size argument
to free(), which doesn't seem correct.

>   if (*sizep < infolen) {
> - free(info, M_TEMP, 0);
> + free(info, M_TEMP, infolen);
>   return (ENOMEM);
>   }
>  
> @@ -716,7 +716,7 @@ sysctl_sysvmsg(int *name, u_int namelen,
>  
>   error = copyout(info, where, infolen);
>  
> - free(info, M_TEMP, 0);
> + free(info, M_TEMP, infolen);
>  
>   return (error);
>  
> 



Re: remove midiplay

2018-11-21 Thread Alexandre Ratchov
On Wed, Nov 21, 2018 at 09:01:38PM +1100, Jonathan Gray wrote:
> On Wed, Nov 21, 2018 at 09:35:55AM +0100, Alexandre Ratchov wrote:
> > On Wed, Nov 21, 2018 at 10:00:26AM +1100, Jonathan Gray wrote:
> > > On Tue, Nov 20, 2018 at 10:02:33PM +0100, Alexandre Ratchov wrote:
> > > > Midiplay requires a midi synth to produce sounds. We dropped support
> > > > for OPL2-style chips, as we've two softsynths in ports to render
> > > > midi. Both already handle .mid files, so midiplay is not very useful
> > > > anymore.
> > > > 
> > > > Objections to remove it?
> > > 
> > > What is the alternative for hardware synths?  
> > 
> > There would be no alternative in base. But there are ports that can
> > play .mid files (and much more), the smfplay utility coming with
> > audio/midish is very similar to midiplay:
> > 
> > > I used to use this to send sysex messages to reset my mt32 and play
> > > midi files back when I still had a machine with a midi interface
> > > (via a pci soundcard).
> > 
> > The plan is to add a new midicat utility very soon, which can be used,
> > amongst others, to send sysex files (assuming .syx file format, aka
> > raw midi data).
> > 
> > However, it's very easy to extend it to handle .mid files, so if you
> > think .mid file support in base should stay, I'll do so.
> > 
> > What do you think?
> 
> If there is something comparable in ports or base that is fine.
> 
> It has been a long time since I had everything hooked up and need
> to get around to getting a usb midi adapter one day.

Any interface (aka "usb midi cable") advertised as class-compliant or
driverless should work on OpenBSD.

> I think I may have just cat'd the reset to the raw midi device.
> Probably looked something like the below which is indeed raw midi.
> 
> $ hexdump -C RESET.SYX
>   f0 41 10 16 12 7f 01 f7   |.A..|
> 0008

indeed, this can be sent as-is to the raw device.



Re: remove midiplay

2018-11-21 Thread Alexandre Ratchov
On Wed, Nov 21, 2018 at 10:00:26AM +1100, Jonathan Gray wrote:
> On Tue, Nov 20, 2018 at 10:02:33PM +0100, Alexandre Ratchov wrote:
> > Midiplay requires a midi synth to produce sounds. We dropped support
> > for OPL2-style chips, as we've two softsynths in ports to render
> > midi. Both already handle .mid files, so midiplay is not very useful
> > anymore.
> > 
> > Objections to remove it?
> 
> What is the alternative for hardware synths?  

There would be no alternative in base. But there are ports that can
play .mid files (and much more), the smfplay utility coming with
audio/midish is very similar to midiplay:

> I used to use this to send sysex messages to reset my mt32 and play
> midi files back when I still had a machine with a midi interface
> (via a pci soundcard).

The plan is to add a new midicat utility very soon, which can be used,
amongst others, to send sysex files (assuming .syx file format, aka
raw midi data).

However, it's very easy to extend it to handle .mid files, so if you
think .mid file support in base should stay, I'll do so.

What do you think?



remove midiplay

2018-11-20 Thread Alexandre Ratchov
Midiplay requires a midi synth to produce sounds. We dropped support
for OPL2-style chips, as we've two softsynths in ports to render
midi. Both already handle .mid files, so midiplay is not very useful
anymore.

Objections to remove it?

OK?

Index: Makefile
===
RCS file: /cvs/src/usr.bin/Makefile,v
retrieving revision 1.157
diff -u -p -u -p -r1.157 Makefile
--- Makefile19 Jun 2018 14:01:16 -  1.157
+++ Makefile20 Nov 2018 20:17:06 -
@@ -15,7 +15,7 @@ SUBDIR= apply arch at aucat audioctl awk
libtool lndir \
locale locate lock logger login logname look lorder \
m4 mail make mandoc mesg mg \
-   midiplay mixerctl mkdep mklocale mktemp nc netstat \
+   mixerctl mkdep mklocale mktemp nc netstat \
newsyslog \
nfsstat nice nm nl nohup openssl pagesize passwd paste patch pctr \
pkg-config pkill \
Index: midiplay/Makefile
===
RCS file: midiplay/Makefile
diff -N midiplay/Makefile
--- midiplay/Makefile   13 Feb 2010 13:45:29 -  1.2
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,8 +0,0 @@
-#  $OpenBSD: Makefile,v 1.2 2010/02/13 13:45:29 ratchov Exp $
-#  $NetBSD: Makefile,v 1.1 1998/08/12 21:39:11 augustss Exp $
-#  @(#)Makefile8.1 (Berkeley) 6/6/93
-
-PROG=  midiplay
-LDADD+=-lsndio
-
-.include 
Index: midiplay/midiplay.1
===
RCS file: midiplay/midiplay.1
diff -N midiplay/midiplay.1
--- midiplay/midiplay.1 18 Oct 2011 07:07:25 -  1.16
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,80 +0,0 @@
-.\" $OpenBSD: midiplay.1,v 1.16 2011/10/18 07:07:25 jmc Exp $
-.\" $NetBSD: midiplay.1,v 1.3 1998/08/13 18:26:36 augustss Exp $
-.\"
-.\" Copyright (c) 1998 The NetBSD Foundation, Inc.
-.\" All rights reserved.
-.\"
-.\" Author: Lennart Augustsson
-.\"
-.\" Redistribution and use in source and binary forms, with or without
-.\" modification, are permitted provided that the following conditions
-.\" are met:
-.\" 1. Redistributions of source code must retain the above copyright
-.\"notice, this list of conditions and the following disclaimer.
-.\" 2. Redistributions in binary form must reproduce the above copyright
-.\"notice, this list of conditions and the following disclaimer in the
-.\"documentation and/or other materials provided with the distribution.
-.\"
-.\" THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
-.\" ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
-.\" TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
-.\" PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
-.\" BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
-.\" CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
-.\" SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
-.\" INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
-.\" CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
-.\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
-.\" POSSIBILITY OF SUCH DAMAGE.
-.\"
-.Dd $Mdocdate: October 18 2011 $
-.Dt MIDIPLAY 1
-.Os
-.Sh NAME
-.Nm midiplay
-.Nd play MIDI files
-.Sh SYNOPSIS
-.Nm midiplay
-.Op Fl gmqvx
-.Op Fl f Ar device
-.Op Fl t Ar tempo
-.Op Ar
-.Sh DESCRIPTION
-The
-.Nm
-command plays MIDI files.
-If no file name is given it will play from standard input;
-otherwise it will play the named files.
-.Pp
-The options are as follows:
-.Bl -tag -width Ds
-.It Fl f Ar device
-Specifies the name of the
-.Xr sndio 7
-MIDI device.
-.It Fl g
-Send a
-.Dq General MIDI On
-system exclusive message to the device.
-.It Fl m
-Show MIDI file meta events (copyright, lyrics, etc.).
-.It Fl q
-Do not play the MIDI file, just parse it.
-.It Fl t Ar tempo
-Specifies the tempo.
-Default is 100.
-.It Fl v
-Be verbose.
-If the flag is repeated, the verbosity increases.
-.It Fl x
-Play a small sample sound.
-.El
-.Sh SEE ALSO
-.Xr aucat 1 ,
-.Xr midi 4 ,
-.Xr sndio 7
-.Sh HISTORY
-The
-.Nm
-command first appeared in
-.Nx 1.4 .
Index: midiplay/midiplay.c
===
RCS file: midiplay/midiplay.c
diff -N midiplay/midiplay.c
--- midiplay/midiplay.c 20 Jul 2018 21:47:07 -  1.21
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,491 +0,0 @@
-/* $OpenBSD: midiplay.c,v 1.21 2018/07/20 21:47:07 mestre Exp $*/
-/* $NetBSD: midiplay.c,v 1.8 1998/11/25 22:17:07 augustss Exp $*/
-
-/*
- * Copyright (c) 1998 The NetBSD Foundation, Inc.
- * All rights reserved.
- *
- * This code is derived from software contributed to The NetBSD Foundation
- * by Lennart Augustsson (augus...@netbsd.org).
- *
- * Redistribution and use in source and binary forms, with or without
- * 

Re: free(9) sizes for USB subdevs

2018-11-16 Thread Alexandre Ratchov
On Fri, Nov 16, 2018 at 06:17:08PM -0200, Martin Pieuchot wrote:
> We need to keep track of the number of slots, so add a new field for
> that.
> 
> ok?
> 

ok ratchov



Re: unveil for audioctl

2018-09-17 Thread Alexandre Ratchov
On Mon, Sep 17, 2018 at 01:27:03PM +0100, Ricardo Mestre wrote:
> And of course I missed disabling unveil(2) just right the first call...
> 
> Index: audioctl.c
> ===
> RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
> retrieving revision 1.35
> diff -u -p -u -r1.35 audioctl.c
> --- audioctl.c31 May 2017 04:18:58 -  1.35
> +++ audioctl.c17 Sep 2018 12:26:30 -
> @@ -217,6 +217,11 @@ main(int argc, char **argv)
>   argc -= optind;
>   argv += optind;
>  
> + if (unveil(path, "rw") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> +
>   fd = open(path, O_RDWR);
>   if (fd < 0)
>   err(1, "%s", path);

looks fine, thanks.

ok ratchov



Re: umidi(4) jack count

2018-09-06 Thread Alexandre Ratchov
On Fri, Sep 07, 2018 at 12:40:49AM +0800, Michael Mikonos wrote:
> Hello,
> 
> The umidi(4) driver has three different endpoint allocation
> functions, which are called by alloc_all_endpoints(). The
> initial jack count can be moved here because it is zero
> no matter which allocation function is used.
> Also when we eventually free the jacks the count can
> be reset. Does this look OK?
> 

ok ratchov@



Re: Add $daemon_nice to rc.subr

2018-09-03 Thread Alexandre Ratchov
On Tue, Sep 04, 2018 at 04:58:53AM +0200, Thomas de Grivel wrote:
> 
> And I still feel the default nice priority of 10 is rather a good
> idea.

why?



Re: move more code under UAUDIO_DEBUG

2018-08-29 Thread Alexandre Ratchov
On Mon, Aug 20, 2018 at 05:32:13PM +0800, Michael W. Bombardieri wrote:
> Hello,
> 
> In uaudio_add_mixer() the number of input channels (ichs) is
> only used in a DPRINTF statement so the code to set its value
> can be excluded unless we're building a debug kernel. In my
> understanding uaudio_get_cluster_nchan() has no side effects.
> 

ok ratchov@



Re: audio round_buffersize

2018-08-29 Thread Alexandre Ratchov
On Tue, Aug 14, 2018 at 02:57:05PM +0800, Michael W. Bombardieri wrote:
> Hello,
> 
> Low level audio drivers using the upper layer audio driver's value
> for buffer size don't need to declare a round_buffersize method.
> The 3rd parameter of the method is the value provided by the
> upper layer driver which is used if round_buffersize isn't defined:
> https://github.com/openbsd/src/blob/master/sys/dev/audio.c#L203
> 

Your diff is OK.

FWIW, round_buffersize() is only used to give the maximum buffer size
the driver supports.



Re: uaudioctl() shadow variable 'error'

2018-08-08 Thread Alexandre Ratchov
On Tue, Jul 31, 2018 at 05:53:14PM +0800, Michael W. Bombardieri wrote:
> Hello,
> 
> The function usbioctl() has an error variable and two more which
> are declared in nested blocks, i.e. switch cases for USB_REQUEST
> and USB_DEVICE_GET_FDESC. The following patch to remove the
> nested declarations builds cleanly, and if I'm following things
> correctly the function works the same without them. Sorry if I
> got it wrong.
> 

It's correct, I confirm the bahavior remains unchanged.

ok ratchov@



Re: unveil in sndiod

2018-07-30 Thread Alexandre Ratchov
On Mon, Jul 30, 2018 at 10:02:51AM -0600, Theo de Raadt wrote:
> Alexandre Ratchov  wrote:
> 
> > On Mon, Jul 30, 2018 at 07:56:00AM -0600, Theo de Raadt wrote:
> > > there are a lot of files in /dev ...
> > > 
> > > can you make this tighter?
> > > 
> > 
> > Yes. I'm experimenting around this right now. I'm looking at the
> > following possibilities:
> > 
> > (1) during initialization, parse device names to determine paths, then
> > call unveil() for each file. This can work because sndiod knows in
> > advance all devices it will use.
> 
> Good enough.
> 

here's the diff.

Index: sndiod.c
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v
retrieving revision 1.33
diff -u -p -r1.33 sndiod.c
--- sndiod.c26 Jun 2018 07:12:35 -  1.33
+++ sndiod.c30 Jul 2018 16:57:31 -
@@ -340,9 +340,26 @@ mkopt(char *path, struct dev *d,
return o;
 }
 
+static void
+dounveil(char *name, char *prefix, char *path_prefix)
+{
+   size_t prefix_len;
+   char path[PATH_MAX];
+
+   prefix_len = strlen(prefix);
+
+   if (strncmp(name, prefix, prefix_len) != 0)
+   errx(1, "%s: unsupported device or port format", name);
+   snprintf(path, sizeof(path), "%s%s", path_prefix, name + prefix_len);
+   if (unveil(path, "rw") < 0)
+   err(1, "unveil");
+}
+
 static int
 start_helper(int background)
 {
+   struct dev *d;
+   struct port *p;
struct passwd *pw;
int s[2];
pid_t pid;
@@ -378,6 +395,10 @@ start_helper(int background)
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
err(1, "cannot drop privileges");
}
+   for (d = dev_list; d != NULL; d = d->next)
+   dounveil(d->path, "rsnd/", "/dev/audio");
+   for (p = port_list; p != NULL; p = p->next)
+   dounveil(p->path, "rmidi/", "/dev/rmidi");
if (pledge("stdio sendfd rpath wpath", NULL) < 0)
err(1, "pledge");
while (file_poll())



Re: unveil in sndiod

2018-07-30 Thread Alexandre Ratchov
On Mon, Jul 30, 2018 at 07:56:00AM -0600, Theo de Raadt wrote:
> there are a lot of files in /dev ...
> 
> can you make this tighter?
> 

Yes. I'm experimenting around this right now. I'm looking at the
following possibilities:

(1) during initialization, parse device names to determine paths, then
call unveil() for each file. This can work because sndiod knows in
advance all devices it will use.

(2) Add a new /dev/snd/ directory and move /dev/audio* and /dev/rmidi*
there. Then call unveil for the /dev/snd/ directory.

This allows other audio/midi programs to use unveil (some allow
the user to select the device).

you see other options? thoughts?



unveil in sndiod

2018-07-30 Thread Alexandre Ratchov
A trivial diff for the sndiod "device helper" process.  All this
process does is to open the device and pass it to the main process. So
it can be restricted to /dev.

The other sndiod process has neither of rpath, wpath, cpath, or exec,
so it doesn't need unveil, right?

Index: sndiod.c
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v
retrieving revision 1.33
diff -u -p -r1.33 sndiod.c
--- sndiod.c26 Jun 2018 07:12:35 -  1.33
+++ sndiod.c30 Jul 2018 09:18:32 -
@@ -378,6 +378,8 @@ start_helper(int background)
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
err(1, "cannot drop privileges");
}
+   if (unveil("/dev", "rw") < 0)
+   err(1, "unveil");
if (pledge("stdio sendfd rpath wpath", NULL) < 0)
err(1, "pledge");
while (file_poll())




MAKEDEV: don't make /dev/audio* symlinks

2018-07-27 Thread Alexandre Ratchov
The /dev/audio and /dev/audioctl symlinks aren't used anymore and can
be removed (the /dev/mixer symlink is still used).

OK?

Index: MAKEDEV.common
===
RCS file: /cvs/src/etc/MAKEDEV.common,v
retrieving revision 1.98
diff -u -p -r1.98 MAKEDEV.common
--- MAKEDEV.common  14 Nov 2017 20:21:29 -  1.98
+++ MAKEDEV.common  27 Jul 2018 13:01:50 -
@@ -418,9 +418,7 @@ __devitem(au, audio*, Audio devices,audi
 _mkdev(au, audio*, {-M audio$U c major_au_c $U
M mixer$U   c major_au_c Add($U, 16)
M audioctl$Uc major_au_c Add($U, 192)
-   MKlist[${#MKlist[*]}]=";[ -e audio ] || ln -s audio$U audio"
-   MKlist[${#MKlist[*]}]=";[ -e mixer ] || ln -s mixer$U mixer"
-   MKlist[${#MKlist[*]}]=";[ -e audioctl ] || ln -s audioctl$U 
audioctl"-})dnl
+   MKlist[${#MKlist[*]}]=";[ -e mixer ] || ln -s mixer$U mixer"-})dnl
 __devitem(vi, video*, Video V4L2 devices,video)dnl
 _mkdev(vi, video*, {-M video$U  c major_vi_c $U 600
MKlist[${#MKlist[*]}]=";[ -e video ] || ln -s video$U video"-})dnl



sndio: rename unix socket name

2018-07-27 Thread Alexandre Ratchov
5+ years since aucat was renamed to sndiod; IMHO, we should have
renamed the socket to remove reference to aucat. This avoids wondering
what are these "aucat" files in /tmp.

The socket is only used for IPC between libsndio and sndiod; so you
only need to rebuild and reinstall libsndio and sndiod.

Thoughts? OK?

Index: amsg.h
===
RCS file: /cvs/src/lib/libsndio/amsg.h,v
retrieving revision 1.10
diff -u -p -r1.10 amsg.h
--- amsg.h  9 Jan 2016 08:27:24 -   1.10
+++ amsg.h  27 Jul 2018 06:42:24 -
@@ -24,11 +24,11 @@
  *
  * DIR [ '-' UID ] '/' FILE UNIT
  *
- * example: "/tmp/aucat-1000/aucat0"
+ * example: "/tmp/sndio-1000/sock0"
  *
  */
-#define SOCKPATH_DIR   "/tmp/aucat"
-#define SOCKPATH_FILE  "aucat"
+#define SOCKPATH_DIR   "/tmp/sndio"
+#define SOCKPATH_FILE  "sock"
 #define SOCKPATH_MAX   (1 +\
sizeof(SOCKPATH_DIR) - 1 +  \
sizeof(char) +  \



move sndio session cookie to its own directory

2018-07-26 Thread Alexandre Ratchov
Currently the session cookie is created in:

$HOME/.aucat_cookie

besides being ugly, this makes libsndio difficult to use with
unveil(2). This change is to make libsndio use:

$HOME/.sndio/cookie

instead.

OK?

Index: aucat.c
===
RCS file: /cvs/src/lib/libsndio/aucat.c,v
retrieving revision 1.71
diff -u -p -u -p -r1.71 aucat.c
--- aucat.c 9 Jan 2016 08:27:24 -   1.71
+++ aucat.c 26 Jul 2018 21:37:08 -
@@ -205,7 +205,8 @@ _aucat_wdata(struct aucat *hdl, const vo
 static int
 aucat_mkcookie(unsigned char *cookie)
 {
-#define COOKIE_SUFFIX  "/.aucat_cookie"
+#define COOKIE_DIR "/.sndio"
+#define COOKIE_SUFFIX  "/.sndio/cookie"
 #define TEMPL_SUFFIX   "."
struct stat sb;
char *home, *path = NULL, *tmp = NULL;
@@ -264,11 +265,20 @@ bad_gen:
/*
 * try to save the cookie
 */
+
if (home == NULL)
goto done;
tmp = malloc(path_len + sizeof(TEMPL_SUFFIX));
if (tmp == NULL)
goto done;
+
+   /* create ~/.sndio directory */
+   memcpy(tmp, home, home_len);
+   memcpy(tmp + home_len, COOKIE_DIR, sizeof(COOKIE_DIR));
+   if (mkdir(tmp, 0755) < 0 && errno != EEXIST)
+   goto done;
+
+   /* create cookie file in it */
memcpy(tmp, path, path_len);
memcpy(tmp + path_len, TEMPL_SUFFIX, sizeof(TEMPL_SUFFIX));
fd = mkstemp(tmp);
Index: sio_open.3
===
RCS file: /cvs/src/lib/libsndio/sio_open.3,v
retrieving revision 1.46
diff -u -p -u -p -r1.46 sio_open.3
--- sio_open.3  3 Jan 2017 20:29:28 -   1.46
+++ sio_open.3  26 Jul 2018 21:37:09 -
@@ -690,7 +690,7 @@ promises.
 and
 .Va cpath
 are needed to read, write or create the authentication cookie
-.Pa ~/.aucat_cookie .
+.Pa ~/.sndio/cookie .
 .It
 .Va rpath ,
 .Va wpath ,
Index: sndio.7
===
RCS file: /cvs/src/lib/libsndio/sndio.7,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 sndio.7
--- sndio.7 7 Dec 2017 15:43:03 -   1.19
+++ sndio.7 26 Jul 2018 21:37:09 -
@@ -191,7 +191,7 @@ Users are identified by their
 which is automatically generated by audio or MIDI applications
 upon the first connection to the server.
 The cookie is stored in
-.Pa "$HOME/.aucat_cookie"
+.Pa "$HOME/.sndio/cookie"
 and contains 128 bits of raw random data.
 .Pp
 If a session needs to be shared between multiple users, they



Re: tracking down sources of spin cpu%

2018-07-25 Thread Alexandre Ratchov
On Wed, Jul 25, 2018 at 05:10:05PM +0100, Stuart Henderson wrote:
> On 2018/07/25 15:26, Alexandre Ratchov wrote:
> > On Wed, Jul 25, 2018 at 11:56:04AM +0100, Stuart Henderson wrote:
> > > On 2018/07/25 12:00, Antoine Jacoutot wrote:
> > > > > I see the exact same issue as you do.
> > > > > I'll try with s/modesetting/intel and see if it improves things.
> > > > 
> > > > OK I can already confirm this "fixes" the issue for me.
> > > 
> > > It's been up for long enough now that I agree.
> > > 
> > > For anyone else running into this, here is the xorg.conf stanza to
> > > switch (no other entries are needed in the file).
> > > 
> > > Section "Device"
> > >Identifier  "Intel Graphics"
> > >Driver  "intel"
> > > EndSection
> > > 
> > 
> > Do you remember if the problem occurs with the modesetting driver if
> > you disable acceleration, ie you add:
> > 
> > Option "AccelMethod" "none"
> > 
> > to the "Device" section?
> > 
> 
> I hadn't tried that before. I just gave it a go, didn't see any freezes,
> but browser rendering was so slow as to be useless - Firefox took about
> 10 minutes to get to a state where I could interact with it (didn't help
> that the saved open tab was a complex map I suppose) and ~30 seconds to
> react to anything. chromium wasn't quite as bad, but I couldn't live
> with it.

This is strange, firefox starts almost immediately on both of my
machines (~1.5s on the faster one, i5-2500K) , with:

Section "Device"
Identifier  "Card0"
Driver  "modesetting"
Option  "SWcursor" "on"
Option  "AccelMethod" "none"
EndSection

AFAIU, if AccelMethod is set to none, the device behaves like a bare
framebuffer (given "modern" memory speeds, this covers all desktop
needs, ex. mplayer uses ~6% CPU for a full-screen 720p movie). So,
what you observe may be unrelated to graphics. Which processes consume
CPU and appear to spin?



Re: tracking down sources of spin cpu%

2018-07-25 Thread Alexandre Ratchov
On Wed, Jul 25, 2018 at 11:56:04AM +0100, Stuart Henderson wrote:
> On 2018/07/25 12:00, Antoine Jacoutot wrote:
> > > I see the exact same issue as you do.
> > > I'll try with s/modesetting/intel and see if it improves things.
> > 
> > OK I can already confirm this "fixes" the issue for me.
> 
> It's been up for long enough now that I agree.
> 
> For anyone else running into this, here is the xorg.conf stanza to
> switch (no other entries are needed in the file).
> 
> Section "Device"
>Identifier  "Intel Graphics"
>Driver  "intel"
> EndSection
> 

Do you remember if the problem occurs with the modesetting driver if
you disable acceleration, ie you add:

Option "AccelMethod" "none"

to the "Device" section?



Re: add pledge(2) to midiplay(1)

2018-07-20 Thread Alexandre Ratchov
On Fri, Jul 20, 2018 at 09:12:29AM +0100, Ricardo Mestre wrote:
> Hi,
> 
> This adds pledge to midiplay, it only needs stdio rpath if reading
> files, otherwise (playing the sample or from stdin) only stdio is
> needed.
> 
> OK?

The diff seems correct. I can't test it right now, if you managed to
play .mid files, then ok ratchov



Re: uaudio: fix detection of a bunch of logitech webcams

2018-07-07 Thread Alexandre Ratchov
On Fri, Jul 06, 2018 at 02:56:37PM +0200, Landry Breuil wrote:
> > 
> > To test:
> > 
> > sysctl kern.audio.record=1
> > aucat -f rsnd/1 -o /tmp/foo.wav (if the uaudio device attaches as audio1)
> > 
> > If this actually records sound (mplayer /tmp/foo.wav to check), you've
> > won. if foo.wav is 68 bytes long, then recording doesnt work, interrupts
> > are triggered but no transfers happen..
> 
> I should add to this testing part that you should 'revert' to usb2 for
> proper uaudio(4) testing, usb3/xhci doesnt really work in this usecase
> and will trigger 'error creating pipe: err=INVAL endpt=0x86' errors in
> dmesg.
> 
> If you can, ensure that you use usb2 by disabling usb3 in the bios or
> xhci in the kernel config.

if you're using usb2, you've to connect the audio device to the root hub,
so if you get the "error creating pipe ... " message, try another port.



Re: uaudio: fix detection of a bunch of logitech webcams

2018-07-07 Thread Alexandre Ratchov
On Fri, Jul 06, 2018 at 01:52:57PM +0200, Landry Breuil wrote:
> 
> New diff adding the C250 which also exhibits the same issue. I have an
> okay from mpi@ on the previous diff, but i'd like actual reports that it
> makes the audio device actually working (mine doesnt now, i'm digging
> into it, but i'm pretty sure at some point i had it working...)
> 

ok ratchov@

> To test:
> 
> sysctl kern.audio.record=1
> aucat -f rsnd/1 -o /tmp/foo.wav (if the uaudio device attaches as audio1)
> 
> If this actually records sound (mplayer /tmp/foo.wav to check), you've
> won. if foo.wav is 68 bytes long, then recording doesnt work, interrupts
> are triggered but no transfers happen..
> 

afaics, this is a unrelated problem



Re: sndiod.8: document how to start with multiple devices

2018-06-24 Thread Alexandre Ratchov
On Sun, Jun 24, 2018 at 05:38:11PM +0200, Landry Breuil wrote:
> On Sun, Jun 24, 2018 at 05:23:46PM +0200, Landry Breuil wrote:
> > Hi,
> > 
> > here's an attempt at documenting the usecase i always forget when
> > playing with multiple devices/mics. Feedback/reworking/tweaks welcome.
> 
> New version after super quick feedback from jmc@, unsure where to put
> the comma around 'for example' though.
> 
> Landry
> 
> Index: sndiod.8
> ===
> RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
> retrieving revision 1.2
> diff -u -r1.2 sndiod.8
> --- sndiod.818 Jan 2016 11:38:07 -  1.2
> +++ sndiod.824 Jun 2018 15:37:17 -
> @@ -484,6 +484,17 @@
>  Regardless of which device a stream is connected to,
>  its playback volume knob is exposed.
>  .Sh EXAMPLES
> +Start server at boot using two devices, for example
> +.Xr audio 4
> +device on
> +.Pa snd/0
> +and an external
> +.Xr uaudio 4
> +microphone on
> +.Pa snd/1 :
> +.Pp
> +.Dl # rcctl set sndiod flags '-frsnd/0 -frsnd/1'
> +.Pp
>  Start server using default parameters, creating an
>  additional sub-device for output to channels 2:3 only (rear speakers
>  on most cards), exposing the
> 

I've two remarks: 

- I'd prefer we make this example look like the other examples,
  without refence to rcctl. I think people can figure out by
  themselves how to transpose this to rcctl.

- The exemple is valid for any devices, not only uaudio
  one. Furthermore refs to audio(4) may raise the question "should I
  use device paths like /dev/audio as specified in audio(4)"? The rest
  of the manual uses "sndio audio device" or just "audio device".



Re: ksh "clear-screen" editing command

2018-06-05 Thread Alexandre Ratchov
On Wed, Jun 06, 2018 at 12:20:50AM +0200, Alexander Hall wrote:
> This adds a "clear-screen" editing command to the emacs editing mode.
> This is the same name as bash and zsh uses, and then I stopped looking.
> 
> The default binding of 'redraw' remains for ^L, for now anyway, so
> you'll need to run
>   $ bind ^L=clear-screen"
> when testing.
> 
> $CLEARSTR can be set to an arbitrary character sequence for clearing the 
> screen, should ^[[H^[[J not fit the bill. For example,
>   $ CLEARSTR=$(clear) 

I don't see the point of the environment variable, for a string
standard since 1974 or 1979, I don't remember. Or is this to make the
command do something else?



Re: [patch] add missing pledge to aucat(1).

2018-05-13 Thread Alexandre Ratchov
On Mon, May 07, 2018 at 08:34:32PM +0200, Jesper Wallin wrote:
> On Mon, May 07, 2018 at 03:30:19PM +0200, Jesper Wallin wrote:
> > I've still not been able to test this using MIDI devices, but everything
> > else seems to work as far as I can tell.
> 
> As Theo (tb@) kindly pointed out to me off-list, aucat(1) supports the
> use of multiple -i and/or -o flags, which my previous patch broke.
> 
> Instead, I've now split slot_new() into two functions, where slot_new()
> only sets the path to the file with all the parameters, and slot_parse()
> which loops through all files and parse the headers.
> 
> Same testing as before as well as specifying -i/-o multiple times.
> 
> 

That's what I meant. The "hdr", "rate" and "pars" variables must also
be saved similarly to "path", but this makes the slot structure a
duplicate of the afile structure does, which is ugly and error-prone.

So afile_open() may need also to be split into a "init" and "open"
parts.

Sorry, I thought all this would be much easier :(



Re: [patch] add missing pledge to aucat(1).

2018-05-06 Thread Alexandre Ratchov
On Fri, May 04, 2018 at 05:31:22PM +0200, Theo Buehler wrote:
> On Fri, May 04, 2018 at 09:03:38AM +0200, Alexandre Ratchov wrote:
> > On Thu, May 03, 2018 at 09:48:13PM +0200, Jesper Wallin wrote:
> > > Hi all,
> > > 
> > > I just noticed that aucat(1) is missing pledge.  However, I'm aware that
> > > aucat(1) is talking to sndiod(8), which is being pledged properly.  But
> > > seeing that programs like yes(1) is properly pledged, I don't see any
> > > reason not to pledge aucat(1) as well, unless I'm missing something
> > > obvious.
> > > 
> > 
> > Thanks. The promise list to use audio and/or midi is in the
> > sio_open(2) man page, so the following seem to be needed: stdio,
> > audio, rpath, wpath, cpath, unix, inet, dns.
> > 
> > aucat could be pledged() since the very beginning; imho this makes
> > sense as the "risky" part is slot_new(), when file headers are parsed.
> > 
> 
> While the last submitted patch looks correct to me, I wonder (rather
> naively) if it would be possible to refactor in such a way that
> slot_new() is called only after or from within dev_open(), so we can
> drop the promises at least to "stdio rpath wpath cpath audio" at the
> point where slot_new() is called for the -i or -o options.

slot_new() is used to determine device parameters, which are used to
call dev_open(), so it must be called first.

The only way to do this is to save the list of files and then open the
device, then parse file headers, then configure the device. This is
not trivial.



  1   2   3   >