On Wed, 27 Aug 2014, Daniel Mack 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 <[email protected]>
> @@ -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;
> + }
One thing I didn't mention about the algorithm I posted earlier: The
individual packet calculations don't contain any multiplications or
divisions.
The code here is in the hottest part of the driver, so you might want
to optimize it a little. For instance, several of the operations could
be precomputed and stored for later use.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
