There are 4 audio counters, and we've one ioctl() to get each. So
userland has to call the ioctl() one by one, and must take care to
handle cases when a counters are inconsistent (i.e. don't belong to
the same time). Inconsistencies are extreamly rare produce so the
code is hard to test.

This diff adds the AUDIO_GETPOS ioctl to fetch a snapshot of all
counters in a single call, and adapts libsndio and audioctl to use
it. The logic doesn't change, only the way counters are fetched
from the kernel, so no behaviour change.

OK?

Index: sys/sys/audioio.h
===================================================================
RCS file: /cvs/src/sys/sys/audioio.h,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 audioio.h
--- sys/sys/audioio.h   25 Jun 2015 06:43:46 -0000      1.22
+++ sys/sys/audioio.h   28 Jul 2015 07:32:45 -0000
@@ -92,6 +92,13 @@ typedef struct audio_offset {
        u_int   unused[2];
 } audio_offset_t;
 
+struct audio_pos {
+       unsigned int play_pos;  /* total bytes played */
+       unsigned int play_xrun; /* bytes of silence inserted */
+       unsigned int rec_pos;   /* total bytes recorded */
+       unsigned int rec_xrun;  /* bytes dropped */
+};
+
 /*
  * Supported audio encodings
  */
@@ -141,6 +148,7 @@ typedef struct audio_encoding {
 #define AUDIO_GETIOFFS _IOR('A', 32, struct audio_offset)
 #define AUDIO_GETOOFFS _IOR('A', 33, struct audio_offset)
 #define AUDIO_GETPROPS _IOR('A', 34, int)
+#define AUDIO_GETPOS   _IOR('A', 35, struct audio_pos)
 #define  AUDIO_PROP_FULLDUPLEX 0x01
 #define  AUDIO_PROP_MMAP       0x02
 #define  AUDIO_PROP_INDEPENDENT        0x04
Index: sys/dev/audio.c
===================================================================
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.134
diff -u -p -u -p -r1.134 audio.c
--- sys/dev/audio.c     24 Jul 2015 08:56:45 -0000      1.134
+++ sys/dev/audio.c     28 Jul 2015 07:32:46 -0000
@@ -66,9 +66,9 @@ struct audio_buf {
        size_t start;                   /* first byte used in the FIFO */
        size_t used;                    /* bytes used in the FIFO */
        size_t blksz;                   /* DMA block size */
-       unsigned long pos;              /* bytes transferred */
-       unsigned long xrun;             /* bytes lost by xruns */
        struct selinfo sel;             /* to record & wakeup poll(2) */
+       unsigned int pos;               /* bytes transferred */
+       unsigned int xrun;              /* bytes lost by xruns */
        int blocking;                   /* read/write blocking */
 };
 
@@ -106,6 +106,7 @@ struct audio_softc {
        unsigned char silence[4];       /* a sample of silence */
        int pause;                      /* not trying to start DMA */
        int active;                     /* DMA in process */
+       int offs;                       /* offset between play & rec dir */
        void (*conv_enc)(unsigned char *, int); /* encode to native */
        void (*conv_dec)(unsigned char *, int); /* decode to user */
 #if NWSKBD > 0
@@ -348,7 +349,7 @@ audio_pintr(void *addr)
        struct audio_softc *sc = addr;
        unsigned char *ptr;
        size_t count;
-       int error;
+       int error, nblk, todo;
 
        MUTEX_ASSERT_LOCKED(&audio_lock);
        if (!(sc->mode & AUMODE_PLAY) || !sc->active) {
@@ -360,6 +361,23 @@ audio_pintr(void *addr)
                return;
        }
 
+       /*
+        * check if record pointer wrapped, see explanation
+        * in audio_rintr()
+        */
+       if (sc->mode & AUMODE_RECORD) {
+               sc->offs--;
+               nblk = sc->rec.len / sc->rec.blksz;
+               todo = -sc->offs;
+               if (todo >= nblk) {
+                       todo -= todo % nblk;
+                       DPRINTFN(1, "%s: rec ptr wrapped, moving %d blocs\n",
+                           DEVNAME(sc), todo);
+                       while (todo-- > 0)
+                               audio_rintr(sc);
+               }
+       }
+
        sc->play.pos += sc->play.blksz;
        audio_fill_sil(sc, sc->play.data + sc->play.start, sc->play.blksz);
        audio_buf_rdiscard(&sc->play, sc->play.blksz);
@@ -402,7 +420,7 @@ audio_rintr(void *addr)
        struct audio_softc *sc = addr;
        unsigned char *ptr;
        size_t count;
-       int error;
+       int error, nblk, todo;
 
        MUTEX_ASSERT_LOCKED(&audio_lock);
        if (!(sc->mode & AUMODE_RECORD) || !sc->active) {
@@ -414,6 +432,30 @@ audio_rintr(void *addr)
                return;
        }
 
+       /*
+        * Interrupts may be masked by other sub-systems during 320ms
+        * and more. During such a delay the hardware doesn't stop
+        * playing and the play buffer pointers may wrap, this can't be
+        * detected and corrected by low level drivers. This makes the
+        * record stream ahead of the play stream; this is detected as a
+        * hardware anomaly by userland and cause programs to misbehave.
+        *
+        * We fix this by advancing play position by an integer count of
+        * full buffers, so it reaches the record position.
+        */
+       if (sc->mode & AUMODE_PLAY) {
+               sc->offs++;
+               nblk = sc->play.len / sc->play.blksz;
+               todo = sc->offs;
+               if (todo >= nblk) {
+                       todo -= todo % nblk;
+                       DPRINTFN(1, "%s: play ptr wrapped, moving %d blocs\n",
+                           DEVNAME(sc), todo);
+                       while (todo-- > 0)
+                               audio_pintr(sc);
+               }
+       }
+
        sc->rec.pos += sc->rec.blksz;
        audio_buf_wcommit(&sc->rec, sc->rec.blksz);
        if (sc->rec.used == sc->rec.len) {
@@ -464,6 +506,7 @@ audio_start_do(struct audio_softc *sc)
            sc->rec.len, sc->rec.blksz);
 
        error = 0;
+       sc->offs = 0;
        if (sc->mode & AUMODE_PLAY) {
                if (sc->ops->trigger_output) {
                        p.encoding = sc->hw_enc;
@@ -1298,7 +1341,7 @@ audio_drain(struct audio_softc *sc)
 
        xrun = sc->play.xrun;
        while (sc->play.xrun == xrun) {
-               DPRINTF("%s: drain: used = %zu, xrun = %ld\n",
+               DPRINTF("%s: drain: used = %zu, xrun = %d\n",
                    DEVNAME(sc), sc->play.used, sc->play.xrun);
 
                /*
@@ -1486,6 +1529,7 @@ int
 audio_ioctl(struct audio_softc *sc, unsigned long cmd, void *addr)
 {
        struct audio_offset *ao;
+       struct audio_pos *ap;
        int error = 0, fd;
 
        /* block if quiesced */
@@ -1516,6 +1560,15 @@ audio_ioctl(struct audio_softc *sc, unsi
                mtx_enter(&audio_lock);
                ao = (struct audio_offset *)addr;
                ao->samples = sc->rec.pos;
+               mtx_leave(&audio_lock);
+               break;
+       case AUDIO_GETPOS:
+               mtx_enter(&audio_lock);
+               ap = (struct audio_pos *)addr;
+               ap->play_pos = sc->play.pos;
+               ap->play_xrun = sc->play.xrun;
+               ap->rec_pos = sc->rec.pos;
+               ap->rec_xrun = sc->rec.xrun;
                mtx_leave(&audio_lock);
                break;
        case AUDIO_SETINFO:
Index: usr.bin/audioctl/audioctl.c
===================================================================
RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 audioctl.c
--- usr.bin/audioctl/audioctl.c 26 May 2015 18:17:12 -0000      1.28
+++ usr.bin/audioctl/audioctl.c 28 Jul 2015 07:32:46 -0000
@@ -59,9 +59,9 @@ audio_info_t info;
 
 char encbuf[1000];
 
-int properties, fullduplex, perrors, rerrors;
+int properties, fullduplex;
 
-struct audio_offset poffs, roffs;
+struct audio_pos getpos;
 
 struct field {
        const char *name;
@@ -97,8 +97,8 @@ struct field {
        { "play.pause",         &info.play.pause,       UCHAR,  0 },
        { "play.active",        &info.play.active,      UCHAR,  READONLY },
        { "play.block_size",    &info.play.block_size,  UINT,   0 },
-       { "play.bytes",         &poffs.samples,         INT,    READONLY },
-       { "play.errors",        &perrors,               INT,    READONLY },
+       { "play.bytes",         &getpos.play_pos,       UINT,   READONLY },
+       { "play.errors",        &getpos.play_xrun,      UINT,   READONLY },
        { "record.rate",        &info.record.sample_rate,UINT,  0 },
        { "record.sample_rate", &info.record.sample_rate,UINT,  ALIAS },
        { "record.channels",    &info.record.channels,  UINT,   0 },
@@ -109,8 +109,8 @@ struct field {
        { "record.pause",       &info.record.pause,     UCHAR,  0 },
        { "record.active",      &info.record.active,    UCHAR,  READONLY },
        { "record.block_size",  &info.record.block_size,UINT,   0 },
-       { "record.bytes",       &roffs.samples,         INT,    READONLY },
-       { "record.errors",      &rerrors,               INT,    READONLY },
+       { "record.bytes",       &getpos.rec_pos,        UINT,   READONLY },
+       { "record.errors",      &getpos.rec_xrun,       UINT,   READONLY },
        { 0 }
 };
 
@@ -297,16 +297,8 @@ getinfo(int fd)
                err(1, "AUDIO_GETFD");
        if (ioctl(fd, AUDIO_GETPROPS, &properties) < 0)
                err(1, "AUDIO_GETPROPS");
-       if (ioctl(fd, AUDIO_PERROR, &perrors) < 0)
-               err(1, "AUDIO_PERROR");
-       if (ioctl(fd, AUDIO_RERROR, &rerrors) < 0)
-               err(1, "AUDIO_RERROR");
-       if (ioctl(fd, AUDIO_GETOOFFS, &poffs) < 0)
-               err(1, "AUDIO_GETOOFFS");
-       if (ioctl(fd, AUDIO_GETIOFFS, &roffs) < 0)
-               err(1, "AUDIO_GETOIFFS");
-       if (ioctl(fd, AUDIO_GETINFO, &info) < 0)
-               err(1, "AUDIO_GETINFO");
+       if (ioctl(fd, AUDIO_GETPOS, &getpos) < 0)
+               err(1, "AUDIO_GETPOS");
 }
 
 void
Index: lib/libsndio/sio_sun.c
===================================================================
RCS file: /cvs/src/lib/libsndio/sio_sun.c,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 sio_sun.c
--- lib/libsndio/sio_sun.c      24 Jul 2015 08:50:29 -0000      1.15
+++ lib/libsndio/sio_sun.c      28 Jul 2015 07:32:46 -0000
@@ -804,65 +804,46 @@ int
 sio_sun_revents(struct sio_hdl *sh, struct pollfd *pfd)
 {
        struct sio_sun_hdl *hdl = (struct sio_sun_hdl *)sh;
-       struct audio_offset ao;
-       int xrun, dierr = 0, doerr = 0, offset, delta;
+       struct audio_pos ap;
+       int dierr = 0, doerr = 0, offset, delta;
        int revents = pfd->revents;
 
        if (!hdl->sio.started)
                return pfd->revents;
-       if (hdl->sio.mode & SIO_PLAY) {
-               if (ioctl(hdl->fd, AUDIO_GETOOFFS, &ao) < 0) {
-                       DPERROR("sio_sun_revents: GETOOFFS");
-                       hdl->sio.eof = 1;
-                       return POLLHUP;
-               }
-               delta = (ao.samples - hdl->obytes) / hdl->obpf;
-               hdl->obytes = ao.samples;
-               hdl->odelta += delta;
-               if (!(hdl->sio.mode & SIO_REC))
-                       hdl->idelta += delta;
-       }
-       if (hdl->sio.mode & SIO_REC) {
-               if (ioctl(hdl->fd, AUDIO_GETIOFFS, &ao) < 0) {
-                       DPERROR("sio_sun_revents: GETIOFFS");
-                       hdl->sio.eof = 1;
-                       return POLLHUP;
-               }
-               delta = (ao.samples - hdl->ibytes) / hdl->ibpf;
-               hdl->ibytes = ao.samples;
-               hdl->idelta += delta;
-               if (!(hdl->sio.mode & SIO_PLAY))
-                       hdl->odelta += delta;
+       if (ioctl(hdl->fd, AUDIO_GETPOS, &ap) < 0) {
+               DPERROR("sio_sun_revents: GETPOS");
+               hdl->sio.eof = 1;
+               return POLLHUP;
        }
        if (hdl->sio.mode & SIO_PLAY) {
-               if (ioctl(hdl->fd, AUDIO_PERROR, &xrun) < 0) {
-                       DPERROR("sio_sun_revents: PERROR");
-                       hdl->sio.eof = 1;
-                       return POLLHUP;
-               }
-               doerr = xrun - hdl->oerr;
-               hdl->oerr = xrun;
-               if (!(hdl->sio.mode & SIO_REC))
+               delta = (ap.play_pos - hdl->obytes) / hdl->obpf;
+               doerr = (ap.play_xrun - hdl->oerr) / hdl->obpf;
+               hdl->obytes = ap.play_pos;
+               hdl->oerr = ap.play_xrun;
+               hdl->odelta += delta;   
+               if (!(hdl->sio.mode & SIO_REC)) {
+                       hdl->idelta += delta;
                        dierr = doerr;
+               }
                if (doerr > 0)
                        DPRINTFN(2, "play xrun %d\n", doerr);
        }
        if (hdl->sio.mode & SIO_REC) {
-               if (ioctl(hdl->fd, AUDIO_RERROR, &xrun) < 0) {
-                       DPERROR("sio_sun_revents: RERROR");
-                       hdl->sio.eof = 1;
-                       return POLLHUP;
-               }
-               dierr = xrun - hdl->ierr;
-               hdl->ierr = xrun;
-               if (!(hdl->sio.mode & SIO_PLAY))
+               delta = (ap.rec_pos - hdl->ibytes) / hdl->ibpf;
+               dierr = (ap.rec_xrun - hdl->ierr) / hdl->ibpf;
+               hdl->ibytes = ap.rec_pos;
+               hdl->ierr = ap.rec_xrun;
+               hdl->idelta += delta;
+               if (!(hdl->sio.mode & SIO_PLAY)) {
+                       hdl->odelta += delta;
                        doerr = dierr;
+               }
                if (dierr > 0)
                        DPRINTFN(2, "rec xrun %d\n", dierr);
        }
 
        /*
-        * GET{I,O}OFFS report positions including xruns,
+        * GETPOS reports positions including xruns,
         * so we have to substract to get the real position
         */
        hdl->idelta -= dierr;

Reply via email to