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
_______________________________________________
Linux-uvc-devel mailing list
Linux-uvc-devel@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
