The UAC2 function driver currently responds to all packets at all times
with wMaxPacketSize packets. That results in way too fast audio
playback as the function driver (which is in fact supposed to define
the audio stream pace) delivers as fast as it can.

Fix this by sizing each packet correctly with the following steps:

 a) Set the packet's size by dividing the nominal data rate by the
    playback endpoint's interval.q

 b) If there is a residual value from the calculation in a), add
    it to a accumulator to keep track of it across packets.

 c) If the accumulator has gathered at least the number of bytes
    that are needed for one sample frame, increase the packet size.

This way, the packet size calculation will get rid of any kind of
imprecision that would otherwise occur with a simple division over
time.

Signed-off-by: Daniel Mack <zon...@gmail.com>
---
 drivers/usb/gadget/function/f_uac2.c | 66 ++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac2.c 
b/drivers/usb/gadget/function/f_uac2.c
index 246a778..06c364a 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -92,6 +92,10 @@ struct snd_uac2_chip {
 
        struct snd_card *card;
        struct snd_pcm *pcm;
+
+       /* timekeeping for the playback endpoint */
+       unsigned int p_interval;
+       unsigned int p_residue;
 };
 
 #define BUFF_SIZE_MAX  (PAGE_SIZE * 16)
@@ -191,8 +195,43 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request 
*req)
 
        spin_lock_irqsave(&prm->lock, flags);
 
-       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+               struct audio_dev *agdev = uac2_to_agdev(uac2);
+               struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
+               unsigned int fsz = opts->p_ssize * num_channels(opts->p_chmask);
+               unsigned int rate = opts->p_srate * fsz;
+
+               /*
+                * For each IN packet, calculate the minimum packet size by
+                * dividing the nominal bytes per second as required by the
+                * current audio format by the endpoint's interval.
+                */
+               req->length = min_t(unsigned int,
+                                   rate / uac2->p_interval, prm->max_psize);
+
+               /*
+                * If there is a residual value from the division, add it to
+                * the residue accumulator.
+                */
+               uac2->p_residue += rate % uac2->p_interval;
+
+               /*
+                * Whenever there are more bytes in the accumulator than we
+                * need to add one more sample frame, increase this packet's
+                * size and decrease the accumulator.
+                */
+               if (uac2->p_residue / uac2->p_interval >= fsz) {
+                       req->length += fsz;
+                       uac2->p_residue -= fsz * uac2->p_interval;
+               }
+
+               /*
+                * req->actual is used as copy length below. We can safely
+                * override it here as it is not looked at when the packet
+                * is resubmitted on an IN endpoint.
+                */
                req->actual = req->length;
+       }
 
        pending = prm->hw_ptr % prm->period_size;
        pending += req->actual;
@@ -346,6 +385,7 @@ static int uac2_pcm_open(struct snd_pcm_substream 
*substream)
        c_srate = opts->c_srate;
        p_chmask = opts->p_chmask;
        c_chmask = opts->c_chmask;
+       uac2->p_residue = 0;
 
        runtime->hw = uac2_pcm_hardware;
 
@@ -1077,7 +1117,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, 
unsigned alt)
        struct usb_request *req;
        struct usb_ep *ep;
        struct uac2_rtd_params *prm;
-       int i;
+       int req_len, i;
 
        /* No i/f has more than 2 alt settings */
        if (alt > 1) {
@@ -1099,11 +1139,31 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, 
unsigned alt)
                prm = &uac2->c_prm;
                config_ep_by_speed(gadget, fn, ep);
                agdev->as_out_alt = alt;
+               req_len = prm->max_psize;
        } else if (intf == agdev->as_in_intf) {
+               struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
+               unsigned int rate = opts->p_srate * opts->p_ssize *
+                                   num_channels(opts->p_chmask);
+               struct usb_endpoint_descriptor *ep_desc;
+               unsigned int factor;
+
                ep = agdev->in_ep;
                prm = &uac2->p_prm;
                config_ep_by_speed(gadget, fn, ep);
                agdev->as_in_alt = alt;
+
+               /* pre-calculate the playback endpoint's interval */
+               if (gadget->speed == USB_SPEED_FULL) {
+                       ep_desc = &fs_epin_desc;
+                       factor = 1000;
+               } else {
+                       ep_desc = &hs_epin_desc;
+                       factor = 125;
+               }
+
+               uac2->p_interval = (1 << (ep_desc->bInterval - 1)) * factor;
+               req_len = rate / uac2->p_interval;
+               uac2->p_residue = 0;
        } else {
                dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
                return -EINVAL;
@@ -1128,7 +1188,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, 
unsigned alt)
 
                        req->zero = 0;
                        req->context = &prm->ureq[i];
-                       req->length = prm->max_psize;
+                       req->length = req_len;
                        req->complete = agdev_iso_complete;
                        req->buf = prm->rbuf + i * req->length;
                }
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to