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 -0000      1.27
+++ share/man/man9/audio.9      18 Aug 2019 05:28:35 -0000
@@ -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 -0000      1.180
+++ sys/dev/audio.c     18 Aug 2019 05:28:36 -0000
@@ -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 audio_softc *sc)
 }
 
 int
-audio_setpar_blksz(struct audio_softc *sc)
+audio_setpar_blksz(struct audio_softc *sc,
+    struct audio_params *p, struct audio_params *r)
 {
        unsigned int nr, np, max, min, mult;
        unsigned int blk_mult, blk_max;
 
+       if (sc->ops->set_blksz) {
+               sc->round = sc->ops->set_blksz(sc->arg, sc->mode,
+                   p, r, sc->round);
+               DPRINTF("%s: block size set to: %u\n", DEVNAME(sc), sc->round);
+               return 0;
+       }
+
        /*
         * get least multiplier of the number of frames per block
         */
@@ -706,7 +739,8 @@ audio_setpar_blksz(struct audio_softc *s
 }
 
 int
-audio_setpar_nblks(struct audio_softc *sc)
+audio_setpar_nblks(struct audio_softc *sc,
+    struct audio_params *p, struct audio_params *r)
 {
        unsigned int max;
 
@@ -719,6 +753,12 @@ audio_setpar_nblks(struct audio_softc *s
                        sc->play.nblks = max;
                else if (sc->play.nblks < 2)
                        sc->play.nblks = 2;
+               if (sc->ops->set_nblks) {
+                       sc->play.nblks = sc->ops->set_nblks(sc->arg, sc->mode,
+                           p, sc->round, sc->play.nblks);
+                       DPRINTF("%s: play nblks -> %u\n", DEVNAME(sc),
+                           sc->play.nblks);
+               }
        }
        if (sc->mode & AUMODE_RECORD) {
                /*
@@ -727,6 +767,11 @@ audio_setpar_nblks(struct audio_softc *s
                 * size of maximum reliability during xruns
                 */
                max = sc->rec.datalen / (sc->round * sc->rchan * sc->bps);
+               if (sc->ops->set_nblks) {
+                       max = sc->ops->set_nblks(sc->arg, sc->mode,
+                           r, sc->round, max);
+                       DPRINTF("%s: rec nblks -> %u\n", DEVNAME(sc), max);
+               }
                sc->rec.nblks = max;
        }
        return 0;
@@ -874,11 +919,11 @@ audio_setpar(struct audio_softc *sc)
        }
        audio_calc_sil(sc);
 
-       error = audio_setpar_blksz(sc);
+       error = audio_setpar_blksz(sc, &p, &r);
        if (error)
                return error;
 
-       error = audio_setpar_nblks(sc);
+       error = audio_setpar_nblks(sc, &p, &r);
        if (error)
                return error;
 
Index: sys/dev/audio_if.h
===================================================================
RCS file: /cvs/src/sys/dev/audio_if.h,v
retrieving revision 1.35
diff -u -p -r1.35 audio_if.h
--- sys/dev/audio_if.h  12 Mar 2019 08:16:29 -0000      1.35
+++ sys/dev/audio_if.h  18 Aug 2019 05:28:36 -0000
@@ -133,6 +133,10 @@ struct audio_hw_if {
                    void (*)(void *), void *, struct audio_params *);
        void    (*copy_output)(void *, size_t);
        void    (*underrun)(void *);
+       unsigned int (*set_blksz)(void *, int,
+           struct audio_params *, struct audio_params *, unsigned int);
+       unsigned int (*set_nblks)(void *, int,
+           struct audio_params *, unsigned int, unsigned int);
 };
 
 struct audio_attach_args {
@@ -149,6 +153,8 @@ struct audio_attach_args {
 /* Attach the MI driver(s) to the MD driver. */
 struct device *audio_attach_mi(struct audio_hw_if *, void *, struct device *);
 int           audioprint(void *, const char *);
+int           audio_blksz_bytes(int,
+                  struct audio_params *, struct audio_params *, int);
 
 extern struct mutex audio_lock;
 
Index: sys/dev/pci/azalia.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.250
diff -u -p -r1.250 azalia.c
--- sys/dev/pci/azalia.c        13 Aug 2019 15:28:12 -0000      1.250
+++ sys/dev/pci/azalia.c        18 Aug 2019 05:28:36 -0000
@@ -252,7 +252,10 @@ int        azalia_open(void *, int);
 void   azalia_close(void *);
 int    azalia_set_params(void *, int, int, audio_params_t *,
        audio_params_t *);
-int    azalia_round_blocksize(void *, int);
+unsigned int azalia_set_blksz(void *, int,
+       struct audio_params *, struct audio_params *, unsigned int);
+unsigned int azalia_set_nblks(void *, int,
+       struct audio_params *, unsigned int, unsigned int);
 int    azalia_halt_output(void *);
 int    azalia_halt_input(void *);
 int    azalia_set_port(void *, mixer_ctrl_t *);
@@ -290,7 +293,7 @@ struct audio_hw_if azalia_hw_if = {
        azalia_open,
        azalia_close,
        azalia_set_params,
-       azalia_round_blocksize,
+       NULL,                   /* round_blocksize */
        NULL,                   /* commit_settings */
        NULL,                   /* init_output */
        NULL,                   /* init_input */
@@ -308,7 +311,11 @@ struct audio_hw_if azalia_hw_if = {
        azalia_round_buffersize,
        azalia_get_props,
        azalia_trigger_output,
-       azalia_trigger_input
+       azalia_trigger_input,
+       NULL,                   /* copy_output */
+       NULL,                   /* underrun */
+       azalia_set_blksz,
+       azalia_set_nblks
 };
 
 static const char *pin_devices[16] = {
@@ -3962,25 +3969,30 @@ azalia_set_params(void *v, int smode, in
        return (0);
 }
 
-int
-azalia_round_blocksize(void *v, int blk)
+unsigned int
+azalia_set_blksz(void *v, int mode,
+       struct audio_params *p, struct audio_params *r, unsigned int blksz)
 {
-       azalia_t *az;
-       size_t size;
+       int mult;
+
+       /* must be multiple of 128 bytes */
+       mult = audio_blksz_bytes(mode, p, r, 128);
+
+       blksz += mult - 1;
+       blksz -= blksz % mult;
+
+       return blksz;
+}
 
-       blk &= ~0x7f;           /* must be multiple of 128 */
-       if (blk <= 0)
-               blk = 128;
+unsigned int
+azalia_set_nblks(void *v, int mode,
+       struct audio_params *params, unsigned int blksz, unsigned int nblks)
+{
        /* number of blocks must be <= HDA_BDL_MAX */
-       az = v;
-       size = az->pstream.buffer.size;
-       if (size > HDA_BDL_MAX * blk) {
-               blk = size / HDA_BDL_MAX;
-               if (blk & 0x7f)
-                       blk = (blk + 0x7f) & ~0x7f;
-       }
-       DPRINTFN(1,("%s: resultant block size = %d\n", __func__, blk));
-       return blk;
+       if (nblks > HDA_BDL_MAX)
+               nblks = HDA_BDL_MAX;
+
+       return nblks;
 }
 
 int
Index: sys/dev/usb/uaudio.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.144
diff -u -p -r1.144 uaudio.c
--- sys/dev/usb/uaudio.c        9 May 2019 07:09:04 -0000       1.144
+++ sys/dev/usb/uaudio.c        18 Aug 2019 05:28:37 -0000
@@ -403,7 +403,8 @@ int uaudio_open(void *, int);
 void uaudio_close(void *);
 int uaudio_set_params(void *, int, int, struct audio_params *,
     struct audio_params *);
-int uaudio_round_blocksize(void *, int);
+unsigned int uaudio_set_blksz(void *, int,
+    struct audio_params *, struct audio_params *, unsigned int);
 int uaudio_trigger_output(void *, void *, void *, int,
     void (*)(void *), void *, struct audio_params *);
 int uaudio_trigger_input(void *, void *, void *, int,
@@ -457,7 +458,7 @@ struct audio_hw_if uaudio_hw_if = {
        uaudio_open,            /* open */
        uaudio_close,           /* close */
        uaudio_set_params,      /* set_params */
-       uaudio_round_blocksize, /* round_blocksize */
+       NULL,                   /* round_blocksize */
        NULL,                   /* commit_settings */
        NULL,                   /* init_output */
        NULL,                   /* init_input */
@@ -477,7 +478,8 @@ struct audio_hw_if uaudio_hw_if = {
        uaudio_trigger_output,  /* trigger_output */
        uaudio_trigger_input,   /* trigger_input */
        uaudio_copy_output,     /* copy_output */
-       uaudio_underrun         /* underrun */
+       uaudio_underrun,        /* underrun */
+       uaudio_set_blksz        /* set_blksz */
 };
 
 /*
@@ -3962,57 +3964,43 @@ uaudio_set_params(void *self, int setmod
        return 0;
 }
 
-int
-uaudio_round_blocksize(void *self, int blksz)
+unsigned int
+uaudio_set_blksz(void *self, int mode,
+    struct audio_params *p, struct audio_params *r, unsigned int blksz)
 {
        struct uaudio_softc *sc = self;
-       struct uaudio_alt *a;
-       unsigned int rbpf, pbpf;
-       unsigned int blksz_max;
+       unsigned int fps, fps_min;
+       unsigned int blksz_max, blksz_min;
 
        /*
-        * XXX: We don't know if we're called for the play or record
-        * direction, so we can't calculate maximum blksz. This would
-        * require a change in the audio(9) interface. Meanwhile, we
-        * use the direction with the greatest sample size; it gives
-        * the correct result: indeed, if we return:
-        *
-        *      blksz_max = max(pbpf, rbpf) * nsamp_max
-        *
-        * in turn the audio(4) layer will use:
-        *
-        *      min(blksz_max / pbpf, blksz_max / rbpf)
-        *
-        * which is exactly nsamp_max.
+        * minimum block size is two transfers, see uaudio_stream_open()
         */
-
-       if (sc->mode & AUMODE_PLAY) {
-               a = sc->params->palt;
-               pbpf = a->bps * a->nch;
-       } else
-               pbpf = 1;
-
-       if (sc->mode & AUMODE_RECORD) {
-               a = sc->params->ralt;
-               rbpf = a->bps * a->nch;
-       } else
-               rbpf = 1;
+       fps_min = sc->ufps;
+       if (mode & AUMODE_PLAY) {
+               fps = sc->params->palt->fps;
+               if (fps_min > fps)
+                       fps_min = fps;
+       }
+       if (mode & AUMODE_RECORD) {
+               fps = sc->params->ralt->fps;
+               if (fps_min > fps)
+                       fps_min = fps;
+       }
+       blksz_min = (sc->rate * 2 + fps_min - 1) / fps_min;
 
        /*
-        * Limit the block size to (slightly more than):
-        *
-        *      sc->host_nframes / UAUDIO_NXFERS_MIN
-        *
-        * (micro-)frames of audio. Transfers are slightly larger than
-        * the audio block size (few bytes to make the "safe" block
-        * size plus one extra millisecond). We reserve an extra 15%
-        * for that.
-        */
-       blksz_max = (pbpf > rbpf ? pbpf : rbpf) *
-           sc->rate * (sc->host_nframes / UAUDIO_NXFERS_MIN) / sc->ufps *
-           85 / 100;
+        * max block size is only limited by the number of frames the
+        * host can schedule
+        */
+       blksz_max = sc->rate * (sc->host_nframes / UAUDIO_NXFERS_MIN) /
+           sc->ufps * 85 / 100;
+
+       if (blksz > blksz_max)
+               blksz = blksz_max;
+       else if (blksz < blksz_min)
+               blksz = blksz_min;
 
-       return blksz < blksz_max ? blksz : blksz_max;
+       return blksz;
 }
 
 int

Reply via email to