ola
ola copied to clipboard
OpenLightingProject
usbdmx: Add support for having multiple inflight transfers
In developing a custom USB->DMX widget for OLA I've noticed that packet latency isn't as consistent as possible when only using a single libusb transfer at a time. Hence here I add support for more than one and also fix a nasty use after free hazard while I'm at it.
Commit messages copied below:
usbdmx: Add support for having multiple inflight transfers
Currently we only use a single active transfer for both sending and reveiving usb packets.
This is suboptimal in terms of latency, USB actually guarantees certain turnaround times for interrupt transfers for example but that only really works if there is always an IRP ready to go in the USB host controller.
According to my measurments interrupt transfers with bInterval=1 can be delayed by some tens of milliseconds instead of happening evey millisecond on the dot if there are periods with no transfers ready to go.
In order to support multiple transfers being inflight I replace the transfer state machinery in AsyncUsbTransceiverBase with a set of inflight transfers and a queue of idle transfers.
The bookkeeping happens in a new TransferCompleteInternal method which wraps the TransferComplete() method provided by subclasses. This split also allowed moving some duplicated logic for LIBUSB_TRANSFER_NO_DEVICE to the superclass.
The number of transfers to allow is passed to AsyncUsb{Sender,Receiver} as a constructor argument since supporting more than one needs some cooperation from the derived classes. Currently everything just defaults to one transfer which shouldn't change the behaviour one bit.
usbdmx: Remove redundant CancelTransfer() calls:
The superclass destructor already calls CancelTransfer, no need to do it in the derived classes also. To prevent this in the future this also make CancelTransfer private.
usbdmx: Fix use after free hazard in destructor
(see diff for commentary)
Changes:
- Actually {fire off,allow} up to the maximum number of transfers in the {receiver,sender}. Since PerformTransfer() is handled in the AsyncUsb{Sender,Receiver} classes we have to do the transfer count handling there.
In theory this should mean widget specific classes shouldn't even be able to notice what the transfer queue depth actually is, so maybe we can just turn this on across all widgets.
Changes:
- Fix the use-after-free fix :) AsyncUsb{Sender,Receiver} have to call CancelTransfer, not the base class, because they close the device handle and we must also not free the device before the transfers complete!
Changes:
- Fix cpplint complaint about two spaces after code, before comment.