rust-embedded-community / usb-device

Experimental device-side USB framework for microcontrollers in Rust.
MIT License
437 stars 75 forks source link

Isochronous endpoint support #33

Closed twitchyliquid64 closed 1 year ago

twitchyliquid64 commented 4 years ago

Heya,

I would like to implement USB Audio v1 (specifically, input terminal + AudioStreaming interface) however the spec calls for isochronous endpoints.

I think this is possible to implement without breaking changes by just adding a new method to allocate an isynchronous endpoint, which additionally takes two more fields for the Usage Type & Synchronization type. We can then store those in the Endpoint struct, and for isochronous endpoints, compute the correct value of bmAttributes that way.

What do you think? Would you accept a PR for this?

mvirkkunen commented 4 years ago

Unfortunately it's not as simple as just adding a new endpoint type. Isochronous endpoints are quite a bit more complicated than the other types, and I'd rather not have a half baked implementation.

The main complication is synchronization, because there's a somewhat complex model with three synchronization modes and associated feedback endpoints which are used for different methods to synchronize clocks between various things in an isochronous system. I recommend reading the USB 2.0 specification chapter 5.12 ("Special Considerations for Isochronous Transfers") to get an idea of what it involves. For a good implementation this would all have to be abstracted in a way that's reasonable to use.

Additionally each driver would need some work to add the new endpoint type. To have reliable isochronous transfers things such as double buffering (or just queueing more than one packet) will probably become necessary to add, because there are no retries in isochronous transfers. I don't think any driver implements that as of now.

twitchyliquid64 commented 4 years ago

My understanding was usb-device would implement the abstraction between devices and implemented interfaces, specifically:

Under that model, shouldn't most of the logic for feedback exist in the class implementation?

While the feedback / synch mechanism is broadly described in the USB spec, and how to communicate some info about isochronous endpoints is described, the actual details necessary to define the semantics are class specific. For example, interpreting the data from the synch ep requires understanding the format and sample rate in use, and adapting the audio stream requires tracking the average sample rate and comparing it to the synch values in a big, messy, feedback loop. I don't think this makes sense in the context of a USB Hal like usb-device, especially given how specific it is.

Concretely, this is what I had in mind:

Also, why does usb-device need double buffering? There are already class callbacks when transfers are received or finished in the hot path of poll(), so the endpoints will get filled almost immediately anyway, and without the double copy caused by a usb-device buffer.

teburd commented 4 years ago

This is needed for usb audio class devices correct?

mvirkkunen commented 4 years ago

@bfrog Yes, the audio standard needs isochronous endpoints to work to its fullest extent. I think there's a fallback bulk endpoint mode but I'm not sure how well that works in practice.

lkolbly commented 3 years ago

I too would like to implement USB audio... Assuming nobody else has a prototype, I can start trying to proof-of-concept creating the required endpoints.

I looked through USB 2.0 section 5.12 a bit, it seems like representing errors in the API may be a bit tricky. It'll take a bit of poking at.

ianrrees commented 3 years ago

@lkolbly - I've got a local branch with some work on this, will try to clean it up and push it up to github in the next couple days in case it's useful. IIRC there isn't actually any change required to usb-device to make isochronous data endpoints with no synchronization.

ianrrees commented 3 years ago

Here's a branch that just adds isochronous endpoint support, but so far it's enough for my application (edit: along with alternate setting support as in #49 ) and aligns with @twitchyliquid64 's second comment : https://github.com/ianrrees/usb-device/tree/isochronous .

I'm also working on embedded-dma support, per @mvirkkunen 's comment the present scheme wastes too many cycles copying data in/out of the endpoint buffers for my application. Strictly speaking, the DMA work is not related to isochronous transfers, but in practice isochronous is used for higher-bandwidth things like audio.

lkolbly commented 3 years ago

@ianrrees Awesome, thanks! Yeah, it was enough to get me up and going with (unsychronized) audio, and in particular I think the only additional thing this crate needs to support isochronous endpoints is support from libusb so we can add it to the test harness. If that sounds reasonable @mvirkkunen I'll go try and work on that for a bit. (also, we may have to switch to rusb, since it seems like the original libusb is unmaintained)

Double-buffering would be nice for drivers to implement, but I think doesn't necessarily require changes to the API. As it is today, drivers could implement e.g. up to 5 queued packets, and write() I think could transparently append onto the queue (the user then gets notified of the actual send via the endpoint_in_complete callback that exists today).

DMA (or zerocopy in general) would be handy, as well. For some applications you could get away with just requesting that the driver generate a buffer, and leveraging the scope that it gives you:

some_endpoint.write_into(BUF_SIZE, |buf| {
   // The driver has allocated the buffer for its "next" transfer, now we fill it directly
   buf[5] = 123;
});

some_endpoint.read_from(|buf| {
   // buf is the next packet as received by the device
   println!("Got value {}!", buf[0]);
});

although that doesn't work super well for DMA, it does at least save a copy. Also, the existing read/write implementations could be written in terms of this, which might save some code in the drivers (since I believe they currently need to handle the copy).

ianrrees commented 3 years ago

@lkolbly - For my project, I'm using rusb for the higher-level USB stuff, and libusb1-sys for the asynchronous interface to the C libusb which supports isochronous transfers but only via the asynchronous interface. I haven't really looked in to Rust async yet so perhaps using it would be easier, but the way I've done the host-side driver with asynchronous libusb is a bit of a pain. @mvirkkunen what do you think about pulling this in without testing support?

The main problem I encountered with DMA is to do with reading, as the user needs to supply a buffer for the next read before/while retrieving the previously-filled one. I think DMA is orthogonal to isochronous support - the DMA stuff would be useful for bulk transfers too, and isochronous doesn't strictly need them.