On Thu, 10 May 2012, Clemens Ladisch wrote:

> Alan Stern wrote:
> > Can you explain briefly how the snd-usb-audio driver decides how many
> > URBs to submit and how many isochronous packets to use in each URB?
> 
> The default is to have 8 ms per URB (which can be adjusted with the
> nrpacks module parameter) and to have enough URBs for a total queue
> length of 24 ms.
> 
> These values are adjusted down if the application has configured the
> period size (which is the application-visible interrupt interval) to be
> smaller.

Looking through the code in data_ep_set_params(), I'm a little puzzled.  
Here's the confusing part, with questions intermingled:

        /* decide how many packets to be used */
        if (is_playback && !snd_usb_endpoint_implicit_feedback_sink(ep)) {

Why is this case (audio-out with an explicit feedback endpoint) treated 
differently?  Just because the packet size can shrink?

                unsigned int minsize, maxpacks;
                /* determine how small a packet can be */
                minsize = (ep->freqn >> (16 - ep->datainterval))
                          * (frame_bits >> 3);
                /* with sync from device, assume it can be 12% lower */
                if (sync_ep)
                        minsize -= minsize >> 3;
                minsize = max(minsize, 1u);

Instead of 1u, shouldn't this be (frame_bits >> 3)?  I assume a packet 
can't contain a fraction of a sample.

                total_packs = (period_bytes + minsize - 1) / minsize;

Here total_packs ends up being the number of packets that will fit into 
period_bytes (rounded up).

                /* we need at least two URBs for queueing */
                if (total_packs < 2) {
                        total_packs = 2;
                } else {
                        /* and we don't want too long a queue either */
                        maxpacks = max(MAX_QUEUE * packs_per_ms, urb_packs * 2);
                        total_packs = min(total_packs, maxpacks);
                }
        } else {
                while (urb_packs > 1 && urb_packs * maxsize >= period_bytes)
                        urb_packs >>= 1;

Is there some reason for doing a right shift each iteration?  Why not
just compute temp = max(1, (period_bytes - 1) / maxsize) and then set
urb_packs to min(urb_packs, temp), with no loop?

                total_packs = MAX_URBS * urb_packs;

Here total_packs in the number of packets used by all URBs, where each
URB must fit into period_bytes.

        }

Why is total_packs so different in the two branches?  You mentioned
above that period_bytes reflects the application-visible interrupt
interval, which would correspond to the size of a single URB.  
Therefore, in the first branch of the "if" statement, it looks like the
value you compute really should be assigned to urb_packs, not
total_packs.

Maybe I have misunderstood what period_bytes is supposed to be.  Still, 
shouldn't it mean the same thing in both branches of the "if" 
statement?

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