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 <zon...@gmail.com>


> @@ -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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to