Re: [PATCH v2] uvc: gadget: uvc: Defer uvcg_complete_buffer() until .complete()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi All


I forgot to add a Fixes tag to this at the time, but I think that this patch:


Fixes: cdda479f15cd ("USB gadget: video class function driver")


The problem it's fixing has as far as I can tell been around since then, though Michael did patch the uvc_video_encode_isoc_sg() function much more recently in 9b969f93bcef ("usb: gadget: uvc: giveback vb2 buffer on req complete"), and this patch relies on the change in Michael's.



On 19/10/2022 13:45, Daniel Scally wrote:
Calling uvcg_complete_buffer() from uvc_video_encode_isoc() sometimes
causes the final isoc packet for a video frame to be delayed long
enough to cause the USB controller to drop it. The first isoc packet
of the next video frame is then received by the host, which interprets
the toggled FID bit correctly such that the stream continues without
interruption, but the first frame will be missing the last isoc
packet's worth of bytes.

To fix the issue delay the call to uvcg_complete_buffer() until the
usb_request's .complete() callback, as already happens when the data
is encoded via uvc_video_encode_isoc_sg(). For consistency's sake the
same change is applied to uvc_video_encode_bulk().

Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
---

Changes in v2:

	- Applied the same change to uvc_video_encode_bulk() for consistency

@Dan - In the end I thought this is probably worth separating from your "usb:
gadget: uvc: fix sg handling in error case" patch, since it fixes a separate
issue by itself. I _think_ they're separable but I wasn't experiencing the
problem you were so I can't test that - let me know if I'm wrong.

@Michael - I dropped your R-b since I made the change to uvc_video_encode_bulk()
too, didn't want to jump the gun :)

  drivers/usb/gadget/function/uvc_video.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index c00ce0e91f5d..42bd4dd1d4a9 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -87,6 +87,7 @@ static void
  uvc_video_encode_bulk(struct usb_request *req, struct uvc_video *video,
  		struct uvc_buffer *buf)
  {
+	struct uvc_request *ureq = req->context;
  	void *mem = req->buf;
  	int len = video->req_size;
  	int ret;
@@ -113,7 +114,7 @@ uvc_video_encode_bulk(struct usb_request *req, struct uvc_video *video,
  		video->queue.buf_used = 0;
  		buf->state = UVC_BUF_STATE_DONE;
  		list_del(&buf->queue);
-		uvcg_complete_buffer(&video->queue, buf);
+		ureq->last_buf = buf;
  		video->fid ^= UVC_STREAM_FID;
video->payload_size = 0;
@@ -194,6 +195,7 @@ static void
  uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
  		struct uvc_buffer *buf)
  {
+	struct uvc_request *ureq = req->context;
  	void *mem = req->buf;
  	int len = video->req_size;
  	int ret;
@@ -213,7 +215,7 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
  		video->queue.buf_used = 0;
  		buf->state = UVC_BUF_STATE_DONE;
  		list_del(&buf->queue);
-		uvcg_complete_buffer(&video->queue, buf);
+		ureq->last_buf = buf;
  		video->fid ^= UVC_STREAM_FID;
  	}
  }



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux