Hi, On Friday 18 June 2010 21:26:01 P F wrote: > > Hi Laurent. Thank you for your attention on this issue.
You're welcome. CC'ing Mark O'Neill who seem to suffer from the same problem. Mark, is your hardware setup identical ? > I upgraded my uvcvideo driver from 2.6.31 to 2.6.34, and the corrupted > frames were still present. I then applied your patch to 2.6.34, but the > corrupted frames are still present. :-( There was a small mistake in the patch that would result in all frames being discarded when a transfer error was detected. It shouldn't prevent corruption from being detected though. > I originally noticed this problem with a custom application, but in the > process of debugging, I have been able to reproduce it with a modified > version of luvcview as well. My patch for luvcview is attached. The > patch disables JPEG decoding and drawing to the display. It also reduces > the number of v4l2 buffers to 1. These changes are not required to trigger > the frame corruption, but they seem to exacerbate the problem. I realize > that 1 v4l2 buffer might be unreasonable, but I can produce this problem > with 2 or 3 buffers as well. I've tried the patch, but haven't been able to reproduce the problem with a Logitech Portable Webcam C200. > Counterintuitively, having more CPU available (by disabling rendering) > seems to worsen the problem. Also, adding printks for debugging almost > always masks the problem. For these reasons, I think maybe there might be > some cache coherency issue, but my attempts at sprinkling > flush_dcache_page() in the OHCI driver have not helped. (For reference: > http://thread.gmane.org/gmane.linux.usb.general/27072/focus=944990) There could indeed be cache issues, but that thread is about a completely different problem. Your USB host controller uses DMA, and I'd be quite surprised if cache was handled incorrectly in the OHCI driver. > I extracted some JPEGs from an AVI generated by luvcview to give you an > idea of what I'm seeing. I shook the camera while filming some color bars > (shaking also seems to trigger the problem). The files are named by > timestamp, and the middle one shows the characteristic corruption. > > I have seen this behavior consistently on two different models of UVC > camera. I am going to try a non-UVC v4l2 camera to see if this is > actually a platform issue as opposed to a uvcvideo one. What models have you tried ? > I will be on holiday next week, so if you have another patch I won't be > able to test it until June 28. I've attached a new patch to this e-mail that fixes the problem mentioned above. Could you please add a printk statement in uvc_queue_next_buffer when buf->error is set, to check that every occurrence of the "USB isochronous frame lost" message results in the frame being dropped in uvc_queue_next_buffer ? It's also quite possible that, due to a buffer underrun, part of the image gets dropped by the camera. Could you try a YUYV UVC device, and see if you get any frame dropped by the buffer length test despite no USB isochronous frame loss being detected ? >From 3fa13b68dbe14ebe891a6529e9b514ff9922ff16 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart <laurent.pinch...@ideasonboard.com> Date: Thu, 17 Jun 2010 11:52:37 +0200 Subject: [PATCH] uvcvideo: Drop corrupted compressed frames Corrupted video frames are dropped by default by the driver for uncompressed formats. Data corruption is not less problematic for compressed formats, so frame drop should be enabled by default for those formats as well. Mark buffers as faulty when an isochronous packet loss is detected for any format, or when the buffer length doesn't match the image size for uncompressed formats. Drop erroneous buffers regardless of whether the format is compressed or uncompressed. Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> --- drivers/media/video/uvc/uvc_queue.c | 13 +++++++++---- drivers/media/video/uvc/uvc_video.c | 19 +++++++++++-------- drivers/media/video/uvc/uvcvideo.h | 5 +++-- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/drivers/media/video/uvc/uvc_queue.c b/drivers/media/video/uvc/uvc_queue.c index 133c78d..e9928a4 100644 --- a/drivers/media/video/uvc/uvc_queue.c +++ b/drivers/media/video/uvc/uvc_queue.c @@ -78,12 +78,14 @@ * */ -void uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type) +void uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, + int drop_corrupted) { mutex_init(&queue->mutex); spin_lock_init(&queue->irqlock); INIT_LIST_HEAD(&queue->mainqueue); INIT_LIST_HEAD(&queue->irqqueue); + queue->flags = drop_corrupted ? UVC_QUEUE_DROP_CORRUPTED : 0; queue->type = type; } @@ -435,8 +437,10 @@ int uvc_queue_enable(struct uvc_video_queue *queue, int enable) uvc_queue_cancel(queue, 0); INIT_LIST_HEAD(&queue->mainqueue); - for (i = 0; i < queue->count; ++i) + for (i = 0; i < queue->count; ++i) { + queue->buffer[i].error = 0; queue->buffer[i].state = UVC_BUF_STATE_IDLE; + } queue->flags &= ~UVC_QUEUE_STREAMING; } @@ -488,8 +492,8 @@ struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer *nextbuf; unsigned long flags; - if ((queue->flags & UVC_QUEUE_DROP_INCOMPLETE) && - buf->buf.length != buf->buf.bytesused) { + if ((queue->flags & UVC_QUEUE_DROP_CORRUPTED) && buf->error) { + buf->error = 0; buf->state = UVC_BUF_STATE_QUEUED; buf->buf.bytesused = 0; return buf; @@ -497,6 +501,7 @@ struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, spin_lock_irqsave(&queue->irqlock, flags); list_del(&buf->queue); + buf->error = 0; buf->state = UVC_BUF_STATE_DONE; if (!list_empty(&queue->irqqueue)) nextbuf = list_first_entry(&queue->irqqueue, struct uvc_buffer, diff --git a/drivers/media/video/uvc/uvc_video.c b/drivers/media/video/uvc/uvc_video.c index 53f3ef4..e27cf0d 100644 --- a/drivers/media/video/uvc/uvc_video.c +++ b/drivers/media/video/uvc/uvc_video.c @@ -555,6 +555,9 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream, if (urb->iso_frame_desc[i].status < 0) { uvc_trace(UVC_TRACE_FRAME, "USB isochronous frame " "lost (%d).\n", urb->iso_frame_desc[i].status); + /* Mark the buffer as faulty. */ + if (buf != NULL) + buf->error = 1; continue; } @@ -579,8 +582,14 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream, uvc_video_decode_end(stream, buf, mem, urb->iso_frame_desc[i].actual_length); - if (buf->state == UVC_BUF_STATE_READY) + if (buf->state == UVC_BUF_STATE_READY) { + if (buf->buf.length != buf->buf.bytesused && + !(stream->cur_format->flags & + UVC_FMT_FLAG_COMPRESSED)) + buf->error = 1; + buf = uvc_queue_next_buffer(&stream->queue, buf); + } } } @@ -1104,7 +1113,7 @@ int uvc_video_init(struct uvc_streaming *stream) atomic_set(&stream->active, 0); /* Initialize the video buffers queue. */ - uvc_queue_init(&stream->queue, stream->type); + uvc_queue_init(&stream->queue, stream->type, !uvc_no_drop_param); /* Alternate setting 0 should be the default, yet the XBox Live Vision * Cam (and possibly other devices) crash or otherwise misbehave if @@ -1197,12 +1206,6 @@ int uvc_video_enable(struct uvc_streaming *stream, int enable) return 0; } - if ((stream->cur_format->flags & UVC_FMT_FLAG_COMPRESSED) || - uvc_no_drop_param) - stream->queue.flags &= ~UVC_QUEUE_DROP_INCOMPLETE; - else - stream->queue.flags |= UVC_QUEUE_DROP_INCOMPLETE; - ret = uvc_queue_enable(&stream->queue, 1); if (ret < 0) return ret; diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h index 47b20e7..85e2f89 100644 --- a/drivers/media/video/uvc/uvcvideo.h +++ b/drivers/media/video/uvc/uvcvideo.h @@ -398,11 +398,12 @@ struct uvc_buffer { struct list_head queue; wait_queue_head_t wait; enum uvc_buffer_state state; + unsigned int error; }; #define UVC_QUEUE_STREAMING (1 << 0) #define UVC_QUEUE_DISCONNECTED (1 << 1) -#define UVC_QUEUE_DROP_INCOMPLETE (1 << 2) +#define UVC_QUEUE_DROP_CORRUPTED (1 << 2) struct uvc_video_queue { enum v4l2_buf_type type; @@ -581,7 +582,7 @@ extern struct uvc_driver uvc_driver; /* Video buffers queue management. */ extern void uvc_queue_init(struct uvc_video_queue *queue, - enum v4l2_buf_type type); + enum v4l2_buf_type type, int drop_corrupted); extern int uvc_alloc_buffers(struct uvc_video_queue *queue, unsigned int nbuffers, unsigned int buflength); extern int uvc_free_buffers(struct uvc_video_queue *queue); -- 1.7.1 -- Regards, Laurent Pinchart _______________________________________________ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
