I agree with ratchov that in this instance, precise timing isn't important.
10 Dec 2022 05:53:48 Alexandre Ratchov <[email protected]>: > 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);
