--- On Thu, 6/17/10, Laurent Pinchart <laurent.pinch...@ideasonboard.com> wrote:
> From: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Subject: Re: [Linux-uvc-devel] [RFC] Discard overrun (corrupted) RX images
> To: linux-uvc-devel@lists.berlios.de
> Cc: "P F" <public_fil...@yahoo.com>
> Date: Thursday, June 17, 2010, 10:07 AM
> Hi,
>
> On Wednesday 09 June 2010 02:59:30 P F wrote:
> >
> > First of all, thank you for providing such great Linux
> support for webcams.
>
> You're welcome.
>
> > I have encountered an issue in this area that I hope
> you experts can help
> > with.
> >
> > I am using UVC/V4L on an embedded system, and I am
> experiencing a problem
> > with corrupted camera images. The system
> consists of a custom ASIC with
> > an ARM926EJ-S and OHCI USB host controller connected
> to a UVC camera which
> > supports only MJPEG.
> >
> > The problem I have is that buffers dequeued in user
> space occasionally
> > contain corrupted JPEGs. This corruption ranges
> from slight, with a row
> > of macroblocks shifted, to severe, with the JPEG being
> totally
> > undecompressible.
> >
> > If I enable UVC_TRACE_FRAME, I see several instances
> of:
> > "uvcvideo: USB isochronous frame lost (-70)."
> > in dmesg. I have noticed a correlation between
> the presence of this
> > message and visual corruption in captured JPEGs.
> >
> > I see that this error code corresponds to -ECOMM,
> which in turn corresponds
> > to TD_BUFFEROVERRUN in the isochronous packet
> status. I assume that this
> > is caused by CPU starvation, which should just
> manifest as dropped frames.
>
> That's correct. One or more packets will then be missing
> from the frame,
> resulting in a corrupt image.
>
> > However, I think these overrun frames are not being
> dropped, and instead
> > are being passed to user space in an indeterminate
> state, leading to the
> > visual corruption that I've witnessed.
>
> Right. The UVC driver drops frames that are missing data
> due to a packet
> loss for uncompressed formats only, based on the frame size
> (although this
> behaviour is overrideable by a module parameter for users
> interested in
> getting bad frames).
>
> The driver should also drop corrupted corrupt frames if a
> packet loss has been detected.
>
> > The attached patch (against 2.6.31) is my naïve
> attempt at fixing the
> > problem. It does appear to resolve the
> corruption, but I am not entirely
> > sure it's the correct thing to do. Does it look
> reasonable? I would
> > appreciate any feedback.
>
> Could you please try the following patch ?
>
> diff --git a/drivers/media/video/uvc/uvc_queue.c
> b/drivers/media/video/uvc/uvc_queue.c
> index 133c78d..580b76e 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,7 @@ 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->state =
> UVC_BUF_STATE_QUEUED;
>
> buf->buf.bytesused = 0;
> return buf;
> @@ -497,6 +500,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 d1f8840..398e65a 100644
> --- a/drivers/media/video/uvc/uvcvideo.h
> +++ b/drivers/media/video/uvc/uvcvideo.h
> @@ -385,11 +385,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;
> @@ -568,7 +569,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.6.4.4
>
> --
> Regards,
>
> Laurent Pinchart
>
Hi Laurent. Thank you for your attention on this issue.
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.
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.
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)
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.
I will be on holiday next week, so if you have another patch I won't be
able to test it until June 28.
Regards,
Filter
Index: luvcview.c
===================================================================
--- luvcview.c (revision 19)
+++ luvcview.c (working copy)
@@ -41,7 +41,7 @@
#include <time.h>
#include <sys/time.h>
#include <signal.h>
-#include <X11/Xlib.h>
+//#include <X11/Xlib.h>
#include <SDL/SDL_syswm.h>
#include "v4l2uvc.h"
#include "gui.h"
@@ -507,10 +507,10 @@
if (videoIn->toggleAvi)
printf("\rframe rate: %g ", frmrate);
- SDL_LockYUVOverlay(overlay);
- memcpy(p, videoIn->framebuffer,
- videoIn->width * (videoIn->height) * 2);
- SDL_UnlockYUVOverlay(overlay);
+// SDL_LockYUVOverlay(overlay);
+// memcpy(p, videoIn->framebuffer,
+// videoIn->width * (videoIn->height) * 2);
+// SDL_UnlockYUVOverlay(overlay);
SDL_DisplayYUVOverlay(overlay, &drect);
if (videoIn->getPict) {
Index: v4l2uvc.c
===================================================================
--- v4l2uvc.c (revision 19)
+++ v4l2uvc.c (working copy)
@@ -663,8 +663,8 @@
vd->framecount++;
}
}
- if (jpeg_decode(&vd->framebuffer, vd->tmpbuffer, &vd->width,
- &vd->height) < 0) {
+ if (0/*jpeg_decode(&vd->framebuffer, vd->tmpbuffer, &vd->width,
+ &vd->height) < 0*/) {
printf("jpeg decode errors\n");
goto err;
}
Index: v4l2uvc.h
===================================================================
--- v4l2uvc.h (revision 19)
+++ v4l2uvc.h (working copy)
@@ -36,7 +36,7 @@
#include "dynctrl-logitech.h"
-#define NB_BUFFER 4
+#define NB_BUFFER 1
#define DHT_SIZE 432
<<attachment: 000007.5714286.jpg>>
<<attachment: 000007.6428572.jpg>>
<<attachment: 000007.7142858.jpg>>
_______________________________________________ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
