Bug 207045
Summary: | uvcvideo: properly handle UVC payloads that occur in the middle of bulk URBs | ||
---|---|---|---|
Product: | Drivers | Reporter: | Julian Meyer (jucmeyer) |
Component: | USB | Assignee: | Default virtual assignee for Drivers/USB (drivers_usb) |
Status: | NEW --- | ||
Severity: | normal | CC: | jucmeyer, louise, luciomf, rpgm2k |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | v5.5 | Subsystem: | |
Regression: | No | Bisected commit-id: |
Description
Julian Meyer
2020-03-31 22:11:09 UTC
Narrowing this issue further, it seems that if the max video frame size is less than the max video transfer size, it works, but if it's over the max video transfer size, it does not work: WORKING (low res) ------- bmHint 0x01 bFormatIndex 1 bFrameIndex 3 dwFrameInterval 333333 wKeyFrameRate 0 wPFrameRate 0 wCompQuality 0 wCompWindowSize 0 wDelay 32 dwMaxVideoFrameSize 50688 dwMaxPayloadTransferSize 129024 NOT WORKING (high res) ----------- bmHint 0x01 bFormatIndex 1 bFrameIndex 1 dwFrameInterval 333333 wKeyFrameRate 0 wPFrameRate 0 wCompQuality 0 wCompWindowSize 0 wDelay 32 dwMaxVideoFrameSize 614400 dwMaxPayloadTransferSize 119296 In the non-working case, first, I see this in the log when it gets set up: [41790.164920] uvcvideo: Allocated 5 URB buffers of 32x512 bytes each. I'm not sure why it's 5, but we know that each URB buffer is 16384 bytes large and then I see this message in the log a bunch: [41790.355807] uvcvideo: payload size is: 131072, max is: 119296 My guess here is that it's sending dwMaxVideoFrameSize and assuming that is the dwMaxPayloadTransferSize. So, it tries to send 614400 in a single payload (not sure about the terminology here), but the buffer is only 119296 bytes large. This means that it would not work if the max video frame size is greater than the max payload transfer size (buffer "overflows" and UVC assumes it's the end of the frame), but it would work if the maxVideoFrameSize is smaller than dwMaxPayloadTransferSize. Also, it makes sense because if I run it at a high resolution, the bottom 1/3 of the picture is cut off/corrupted whereas the top part is just fine. In this case, dwMaxVideoFrameSize is probably about 30% larger than dwMaxPayloadTransferSize. I did more research and I found that the problem is that the camera is sending the next UVC payload immediately after finishing the previous one instead of waiting for the next URB. I ran my camera at 640x480@30fps and found the following information: First, the camera reports the max payload size and max frame size as follows: Max Payload Size: 119296 Max Frame Size: 614400 Actual Received Payload Size (sum of all payloads): 614472 This number makes sense because it includes (frame size/payload size =) 6 payload headers of 12 bytes each, so 614472 bytes total. The last URB is truncated because it fits in 37 full 16K URBs + one 8264 byte URB. uvc_video_decode_bulk calls the following functions: 1. uvc_video_decode_start - find header length and decode it 2. uvc_video_decode_meta - decode the rest of the header metadata 3. uvc_video_decode_data - decodes data up to the max payload size and if it overflows, assume the frame is ended 4. uvc_video_decode_end - if this payload was end-of-frame, start the next frame uvc_video_decode_bulk is called from uvc_video_complete which is the completion handler for each URB. This makes it so that each URB must contain one or less payloads because we only process at most one payload per call to the completion handler. In the case of my camera, the driver hits the max payload size and assumes it's finished (the next payload is in the next URB). Then, it tries to parse the header for the next URB assuming it's a new payload even though it's actually in the middle of a payload. This causes FID mismatches and all sorts of other issues. Proposed Fix: When we see a URB that's larger than the maximum payload size, assume it's another payload and start processing the new payload. I submitted a patch here: https://lore.kernel.org/patchwork/patch/1219476/ Hi Julian, I've been testing this for a few days now, mostly with google meets at 720p. Working like a charm. I'll post if something goes wrong. (In reply to Lucio MF from comment #4) > Hi Julian, > I've been testing this for a few days now, mostly with google meets at 720p. > Working like a charm. I'll post if something goes wrong. So I've been testing a little more this patch and although it seems to been working properly (it fixes the issues with broken video image on higher resolution) there are some situations in witch the system freezes completely. I couldn't find a pattern just using it and don't really know how to debug this, I don't have to much experience in kernel development or debugging. Could it be posible that in some situation len = urb->actual_length; will keep getting 0 or other value minor to 0, that will break the while condition? Hmm... are you using this patch? https://lore.kernel.org/patchwork/patch/1227775/ I think I fixed that bug in v2/3. (In reply to Julian Meyer from comment #6) > Hmm... are you using this patch? > > https://lore.kernel.org/patchwork/patch/1227775/ > > I think I fixed that bug in v2/3. Thanks! I was definitely running an older version of your patch. Now freezes are gone :-D I've been using this patch on Ubuntu 20.04 with kernel 5.4 and now 5.8 without issue. Many thanks to you! Any chance this could be merged eventually? Hi Julian, I keep using this patch almost everyday and it mostly works fine. Although I find out that there are still some "stress" situations where it freezes the whole system. One can almost "force" the freeze in three situations: - Using camera filters in Google Meets for longs periods. - Closing a call and starting one right away - Opening two programs that requires the camera Let me know if you're interested in me running some specific test to help solve this situation. |