On Wed, Feb 07, 2024 at 01:04:48PM +0000, Kieran Bingham wrote:
Hi Michael, Quoting Michael Grzeschik (2024-02-06 21:30:17)On Tue, Feb 06, 2024 at 03:23:17AM +0000, Thinh Nguyen wrote: >On Tue, Feb 06, 2024, Michael Grzeschik wrote: >> Hi Thinh >> >> I found some strange situation while streaming via uvc-gadget to some >> usb host. It happens when some requests are missed due to higher load on >> the gadget machine. In some cases some requests will reach the host >> twice. In my special case, I added the following changes [1] for the >> host and gadget side. > >Does this only happen to some specific hosts? >Are all the data of the duplicate requests matching or just some bits of >the transfer? Were you able to confirm from some usb analyzer/sniffer >that the data out the wire is actually duplicate? Turns out, this duplicates are just misinterpretations.I'm glad there's no deeper issue to worry about here.The linux uvcvideo driver will parse the uvc header payload twice. (If the FID was incremented inbetween). This led to those double misleading outputs. Although this means that there is a bug in uvc_video_stats_decode for incrementing the error count.Do you plan/are you able to submit a patch to fix this? Hopefully that would prevent anyone else following the same rabbit-hole.
Sure. For now I came up with this changes. This leaves the errorcounts
in a sane range. However with this compiled kernel I saw some issues
with the host machine to freeze up. I will just ensure that this is not
caused by this particular patch.
Regards,
Michael
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 40eafe43d1888..b582698be7f00 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1094,6 +1094,29 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
uvc_video_stats_update(stream);
}
+ /*
+ * Mark the buffer as done if we're at the beginning of a new frame.
+ * End of frame detection is better implemented by checking the EOF
+ * bit (FID bit toggling is delayed by one frame compared to the EOF
+ * bit), but some devices don't set the bit at end of frame (and the
+ * last payload can be lost anyway). We thus must check if the FID has
+ * been toggled.
+ *
+ * stream->last_fid is initialized to -1, so the first isochronous
+ * frame will never trigger an end of frame detection.
+ *
+ * Empty buffers (bytesused == 0) don't trigger end of frame detection
+ * as it doesn't make sense to return an empty buffer. This also
+ * avoids detecting end of frame conditions at FID toggling if the
+ * previous payload had the EOF bit set.
+ */
+ if (fid != stream->last_fid && buf->bytesused != 0) {
+ uvc_dbg(stream->dev, FRAME,
+ "Frame complete (FID bit toggled)\n");
+ buf->state = UVC_BUF_STATE_READY;
+ return -EAGAIN;
+ }
+
uvc_video_clock_decode(stream, buf, data, len);
uvc_video_stats_decode(stream, data, len);
@@ -1140,29 +1163,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
buf->state = UVC_BUF_STATE_ACTIVE;
}
- /*
- * Mark the buffer as done if we're at the beginning of a new frame.
- * End of frame detection is better implemented by checking the EOF
- * bit (FID bit toggling is delayed by one frame compared to the EOF
- * bit), but some devices don't set the bit at end of frame (and the
- * last payload can be lost anyway). We thus must check if the FID has
- * been toggled.
- *
- * stream->last_fid is initialized to -1, so the first isochronous
- * frame will never trigger an end of frame detection.
- *
- * Empty buffers (bytesused == 0) don't trigger end of frame detection
- * as it doesn't make sense to return an empty buffer. This also
- * avoids detecting end of frame conditions at FID toggling if the
- * previous payload had the EOF bit set.
- */
- if (fid != stream->last_fid && buf->bytesused != 0) {
- uvc_dbg(stream->dev, FRAME,
- "Frame complete (FID bit toggled)\n");
- buf->state = UVC_BUF_STATE_READY;
- return -EAGAIN;
- }
-
stream->last_fid = fid;
return data[0];
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
