On Wed, Aug 27, 2014 at 3:23 AM, Daniel Mack <zonque@xxxxxxxxx> wrote:
> 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 <zonque@xxxxxxxxx>
> ---
> 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 fsz = opts->p_ssize *
num_channels(opts->p_chmask);
+ unsigned int rate = opts->p_srate * fsz;
> + 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;
>
+ if (opts->p_srate % uac2->p_interval)
+ req_len += fsz;
.....
> + 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;
>
.... otherwise req[0]->buf might overlap req[1]->buf's first frame
for when we need to send an extra frame.
-Jassi
> }
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
