[alsa-devel] [PATCH v2] ALSA: usb-audio: Fix max packet size calculation for USB audio

[Ricard Wanderlof]

On Sat, 10 Oct 2015, Clemens Ladisch wrote:

There is a maximum size for USB packets, but not for git commit messages. :)

Smiley aside, do you think more of the rationale should be in the commit 
message or is it sufficient the way it is?

> > The problem here is that the addition of 0xffff is in the wrong place. We don't
> > want the rounding up to take place on the frequency, but on the resulting
> > packet size.
> 
> Packets must not contain partial frames, so the rounding must take place
> on the number of frames per packet.

That of course makes perfect sense.

> > Let's take an example. We have a sample rate of 96 kHz, so our
> > ep->freqn is 786432 (see usb_high_speed_rate()). Add 25% (line 612) and we get
> > 983040.  The calculated maxsize is then ((983040 + 65535) * 8) >> 16 = 127 .
> 
> That code was intended to round correctly, but it's obviously wrong
> because the result is not an integer multiple of the frame size.

If that is the case we need to do something like

maxsize = (((freqn + 0xffff) & 0xffff0000) * frame_size) >> 16

where the & 0xffff0000 rounds the Q16.16 number down to the nearest 
integer, so that when multiplying by frame_size we get an integral 
multiple thereof.

In the (trivial) case above (with 983040 = 0xf0000) we get

maxsize = (((0xf0000 + 0xffff) & 0xffff0000) *8) >> 16 = 120

(which is correct).

> (Please write Q16.16 values in hex.)

Ok, will do.

> > This is also corroborated by the wMaxPacketSize check on line 616. Assume
> > that wMaxPacketSize = 108
> 
> It would not make sense to have a value that is not a multiple of the
> frame size.  (I'm not saying that it doesn't occur in practice ...)

In this particular case I was withholding information. :) The actual 
device in question needs a quirk whereby the audio data in each packet is 
prefixed by an u32 containing the number of audio data bytes in the 
packet. So 108 is the actual number, as the corresponding number of audio 
bytes, 104, is a multiple of 8. But of course I should not have used that 
particular maxsize as an example in this patch.

Ok, I'll rework this and resubmit.

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30