On Fri, Dec 09, 2022 at 12:43:31PM -0600, Scott Cheloha wrote:
> On Fri, Dec 09, 2022 at 12:10:59PM +0100, Alexandre Ratchov wrote:
> > This diff adds an option to display variables periodically. Basically
> > it replaces this usage:
> >
> > while sleep 1; do audioctl play.errors; done
> >
> > by
> >
> > audioctl -w 1 play.errors
> >
> > The purpose of above audioctl commands is to debug underruns, so we
> > don't want to fork a new process and reopen the device. This would
> > trigger longer kernel code-paths and may cause additional underruns
> > than the ones being investigated.
> >
> > OK?
>
> I like the idea, but I would like to tweak some things.
>
> - Add [-w wait] to the first synoptic form in the manpage. It's legal
> to do e.g.
>
> # audioctl -w 1
>
done
> - Call the variable "wait" in audioctl.c to match the manpage.
>
done (used wait_sec, as there's a global wait() symbol).
> - Update usagestr to mention [-w wait].
>
done
> - When polling variables periodically, it's better to use setitimer(2)
> and sigsuspend(2) instead of sleep(3). setitimer(2) keeps the period
> from drifting.
>
> - Let the user SIGINT (^C) out of the program without returning an
> error to the shell.
>
> I'm unsure about this one, but it seems logical to give the user a
> way to gracefully terminate the program. You say in the manpage that
> the program will continue printing until it is interrupted.
>
I just tried these. Synchronizing the display to a clock might make
sense if it was the sound card's clock, but here the result was boiler
with no benefit. The intent of -w is to just show the variables from
time to time, so keeping the code trivial is more important,
IMHO. I've added a comment to say so.
About ^C, I've changed the man page text to "audioctl will display
variables forever." which implies that ^C is out of the scope.
Index: audioctl.8
===================================================================
RCS file: /cvs/src/usr.bin/audioctl/audioctl.8,v
retrieving revision 1.4
diff -u -p -r1.4 audioctl.8
--- audioctl.8 23 Apr 2020 00:16:59 -0000 1.4
+++ audioctl.8 10 Dec 2022 05:50:05 -0000
@@ -35,9 +35,11 @@
.Sh SYNOPSIS
.Nm audioctl
.Op Fl f Ar file
+.Op Fl w Ar wait
.Nm audioctl
.Op Fl n
.Op Fl f Ar file
+.Op Fl w Ar wait
.Ar name ...
.Nm audioctl
.Op Fl nq
@@ -59,6 +61,12 @@ The default is
Suppress printing of the variable name.
.It Fl q
Suppress all output when setting a variable.
+.It Fl w Ar wait
+Pause
+.Ar wait
+seconds between each display.
+.Nm
+will display variables forever.
.It Ar name Ns = Ns Ar value
Attempt to set the specified variable
.Ar name
@@ -130,10 +138,10 @@ audio control devices
audio devices
.El
.Sh EXAMPLES
-Display the number of bytes of silence inserted during play buffer
-underruns since device started:
+Once per-second, display the number of bytes of silence inserted due to buffer
+underruns (since the device started playback):
.Bd -literal -offset indent
-# audioctl play.errors
+# audioctl -w 1 play.errors
.Ed
.Pp
Use signed 24-bit samples and 44100Hz sample rate:
Index: audioctl.c
===================================================================
RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
retrieving revision 1.43
diff -u -p -r1.43 audioctl.c
--- audioctl.c 12 Jul 2021 15:09:19 -0000 1.43
+++ audioctl.c 10 Dec 2022 05:50:05 -0000
@@ -43,6 +43,7 @@ struct field {
#define STR 2
#define ENC 3
int type;
+ int show;
int set;
} fields[] = {
{"name", &rname.name, NULL, STR},
@@ -63,11 +64,11 @@ struct field {
};
const char usagestr[] =
- "usage: audioctl [-f file]\n"
- " audioctl [-n] [-f file] name ...\n"
+ "usage: audioctl [-f file] [-w wait_sec]\n"
+ " audioctl [-n] [-f file] [-w wait_sec] name ...\n"
" audioctl [-nq] [-f file] name=value ...\n";
-int fd, show_names = 1, quiet = 0;
+int fd, show_names = 1, quiet = 0, wait_sec = 0;
/*
* parse encoding string (examples: s8, u8, s16, s16le, s24be ...)
@@ -198,20 +199,9 @@ audio_main(int argc, char **argv)
char *lhs, *rhs;
int set = 0;
- if (ioctl(fd, AUDIO_GETSTATUS, &rstatus) == -1)
- err(1, "AUDIO_GETSTATUS");
- if (ioctl(fd, AUDIO_GETDEV, &rname) == -1)
- err(1, "AUDIO_GETDEV");
- if (ioctl(fd, AUDIO_GETPAR, &rpar) == -1)
- err(1, "AUDIO_GETPAR");
- if (ioctl(fd, AUDIO_GETPOS, &rpos) == -1)
- err(1, "AUDIO_GETPOS");
if (argc == 0) {
- for (f = fields; f->name != NULL; f++) {
- printf("%s=", f->name);
- print_field(f, f->raddr);
- printf("\n");
- }
+ for (f = fields; f->name != NULL; f++)
+ f->show = 1;
}
AUDIO_INITPAR(&wpar);
for (; argc > 0; argc--, argv++) {
@@ -231,15 +221,41 @@ audio_main(int argc, char **argv)
parse_field(f, f->waddr, rhs);
f->set = 1;
set = 1;
- } else {
+ } else
+ f->show = 1;
+ }
+
+ if (set && wait_sec)
+ errx(1, "Can't set variables wait_secically");
+
+ while (1) {
+ if (ioctl(fd, AUDIO_GETSTATUS, &rstatus) == -1)
+ err(1, "AUDIO_GETSTATUS");
+ if (ioctl(fd, AUDIO_GETDEV, &rname) == -1)
+ err(1, "AUDIO_GETDEV");
+ if (ioctl(fd, AUDIO_GETPAR, &rpar) == -1)
+ err(1, "AUDIO_GETPAR");
+ if (ioctl(fd, AUDIO_GETPOS, &rpos) == -1)
+ err(1, "AUDIO_GETPOS");
+ for (f = fields; f->name != NULL; f++) {
+ if (!f->show)
+ continue;
if (show_names)
printf("%s=", f->name);
print_field(f, f->raddr);
printf("\n");
}
+
+ if (wait_sec == 0)
+ break;
+
+ /* ioctls are fast, we neglect drift from real-time clock */
+ sleep(wait_sec);
}
+
if (!set)
return;
+
if (ioctl(fd, AUDIO_SETPAR, &wpar) == -1)
err(1, "AUDIO_SETPAR");
if (ioctl(fd, AUDIO_GETPAR, &wpar) == -1)
@@ -261,9 +277,10 @@ int
main(int argc, char **argv)
{
char *path = "/dev/audioctl0";
+ const char *errstr;
int c;
- while ((c = getopt(argc, argv, "anf:q")) != -1) {
+ while ((c = getopt(argc, argv, "anf:qw:")) != -1) {
switch (c) {
case 'a': /* ignored, compat */
break;
@@ -275,6 +292,11 @@ main(int argc, char **argv)
break;
case 'q':
quiet = 1;
+ break;
+ case 'w':
+ wait_sec = strtonum(optarg, 1, INT_MAX, &errstr);
+ if (errstr != NULL)
+ errx(1, "wait is %s: %s", errstr, optarg);
break;
default:
fputs(usagestr, stderr);
