On Wed, Jan 23, 2019 at 01:23:05PM -0200, Martin Pieuchot wrote: > On 31/12/18(Mon) 16:58, Alexandre Ratchov wrote: > > Hi, > > > > Here's a new driver for both USB audio class (UAC) v1.0 and v2.0 > > devices, it would replace the current one. It focuses on reliability > > and proper synchronization, including in low-latency configurations. > > Some comments about the code. I believe this should go in now so we > can tackle the remaining issue in tree, so ok mpi@
I got a lot of very helpful test reports. I've almost finished fixing the problems people reported, so I'll repost a new diff soon and commit that one instead if you're still OK. > > - Please put the UE_* macro in dev/usb/usb.h (not sys/usb.h) ;o) > ;-) > - You could get rid of UAUDIO_NRATES and use nitems() instead > > - What does UAUDIO_USE_FRAC mean? > If the sample rate is not multiple of the USB frame rate, the number of samples per frame can't be constant: For instance, if frame rate is 1000 frames-per-second and sample rate is 44100 samples-per-second, the number of samples per frame would be 44.1. Devices handle this with a variable number for frames: in this example, most of the frames carry 44 samples, but every 10 samples we have one 45-sample frame, to reach the 44.1 samples per frame average. In this case the driver has to manipulate "fractional" frames. The diff I posted doesn't handle such rates. It's a pure arithmetic problem, I didn't want to distract people with it. > - Some values read from descriptors are passed to malloc(9) w/o being > checked. Take the example of `count' in uaudio_req_ranges(). What's > the maximum size for `count' and `req_size'? > What's the maximum value for `nframes_max'? > - count is 16-bit, so worst case req_size is roughly 65535 multiplied by the word size (at most 4-byte) plus two bytes of header. That's less than 256kB. Most devices have a signle range, but who knows... I've added a comment to say so. - I my current version of the diff nframes_max limited to 240 on usb2 and 30 on usb1, which is 30ms of audio. This is because ehci and uhci don't seem to handle large isoc transfers. As 30ms enough for audio, I didn't try to understand the reason of the limitation yet. > - You don't need to call usbd_abort_pipe(9) before calling > usbd_close_pipe(9), it does it for you. > > - What about using %s __func__ in DPRINTF() instead of function names > to help search through your code? ;o) thans, fixed these as well.
