
Ricard Wanderlof wrote:
The addition of 0xffff ("almost 1" in Q16.16 format), resulting in the rounding up of the resulting value, was in the wrong place, as it is the resulting packet size that needs to be rounded up to the nearest integer (e.g. if the resulting packet size is calculated as 55.125 bytes, we need a packet of 56 bytes), rather than the rate value (ep->freqmax).
Tested with an Edirol UA-5 both at 44.1 kHz and 96 kHz.
Signed-off-by: Ricard Wanderlof ricardw@axis.com
V2: Added tested device to commit message. No other changes.
Since this clearly is a sensitive part of the code, here follows a lengthy rationale with examples to illustrate the problem.
There is a maximum size for USB packets, but not for git commit messages. :)
[...] 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.
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.
(Please write Q16.16 values in hex.)
However, if we do the number of bytes calculation in a less obscure way it's more apparent what the true corresponding packet size is: we get (96000 * 1.25 / 8000) * 8 = 120
The value inside the parentheses is the number of frames per packet, and must be rounded.
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 ...)
Regards, Clemens