On Fri, May 05, 2023, Avichal Rakesh wrote: > > > On 4/20/23 10:20, Laurent Pinchart wrote: > > > > As far as I understand, we have two ways forward here to avoid running > > out of requests to send: sending data as quickly as possible (maximizing > > the number of bytes sent in each packet) and filling up with 0-length > > requests in-between, and spreading the data across packets. I'll call > > the first one burst mode for lack of a better term. > > > > Hey all, > > Apologies for the late reply. My not-so-long work took longer than expected. > I tried the 2 (technically 3, but I'll go over it in a bit) approaches we had > discussed above and the "burst" approach works pretty well. I'll attach that > to this email, and send another email out with the other patch. > > The first thing I tried was to split one video frame over 266 frames, without > changing the number of requests allocated. And it works! However, as Laurent > mentioned, it does add a fair amount of bookkeeping to split a video frame into > the required number of requests. I also hardcoded the number 266 from our > discussion, but I am not sure how to figure out that number dynamically. 266 > also didn't work if the host started sending frames at more than 30fps :/, so > our dynamic calculation would need to take camera's real output fps into > account, which as far as I can tell is not known to the UVC driver. > > With those issues I tried what Laurent called the "burst" approach > (attached below), i.e. send the video frames in as few packets as possible, > and then queue up 0 length packets to keep the ISOC queue happy. This approach > works perfectly as far as I can tell. Locally I tried with a Linux, Window, > and MacOS host with no frame drops or ISOC failures on any of them! That's great! > > In the current patch, UVC gadget driver keeps the ISOC cadence by effectively > maintaining a back-pressure on the USB controller (at least to the best of its > capabilities). Any usb_request available to the UVC gadget gets immediately > queued back to the USB controller. If a video frame is available, the frame is > encoded, if not, the length is set to 0. The idea being that the host's polling This is what the dwc3 controller expects -- keeping up with the data to the negotiated periodic interval. Thanks for the tests. BR, Thinh > and the controller's 'complete' callback will result in a somewhat consistent > cadence for the uvc driver after the initial burst of packets. > > However this does mean that at worst, the new video frames are up to 63 > usb_requests behind, but assuming a 125us per usb_request, that amounts to ~8ms > latency at the worst, which seems acceptable? > > Another concern I had was about how the back-pressure might affect other USB > controllers. DWC3 doesn't seem to be sweating and in local testing I saw no > EXDEVs or frame drops other than when the stream was being transitioned from > one configuration to another, but I don't know how this interaction might go for > other USB controllers. Would you have any insights into non-DWC3 controllers, > and if they might be negatively affected by having up to 64 requests queued at > once? > > Here's the patch, it doesn't currently handle bulk transfers, but I can upload a > formal patch with it if this approach seems acceptable! > > --- > drivers/usb/gadget/function/uvc_video.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) >
