Hi Robert,
On Tuesday 05 January 2010 13:49:34 Robert Lukassen wrote:
> Hi,
>
> In Ubuntu 9.10, support for Video Out devices (bulk-only) has been added.
That's right. Just out of curiosity, could you tell me which video output
device you're working with ? I'm only aware of a single one so far.
Support for isochronous video output devices could also be implemented, feel
free to contact me if needed.
> We noticed that when such a device is disconnected from a non-root hub,
> the queued urbs are returned with usb->status == -EPROTO. The video
> completion callback cancels the queue, and does not re-submit the urb.
>
> Because the hub driver needs some time to process the disconnect properly,
> there is a brief interval in time where queued urbs for the disconnected
> device do not result in -ESHUTDOWN, as expected, but with other error
> codes (such as EPROTO).
>
> We have found that (in case of status == -EPROTO) resubmitting the urb and
> not canceling the queue results in the correct behaviour. Eventually one
> of the urbs indeed comes back with status == -ESHUTDOWN and the device is
> properly administrated as disconnected. Subsequent QBUF attempts then also
> result in correct user-space experience.
>
> We propose the following simple patch:
>
> In uvc_video.c:
>
> static void uvc_video_complete(struct urb *urb)
> {
> struct uvc_video_device *video = urb->context;
> struct uvc_video_queue *queue = &video->queue;
> struct uvc_buffer *buf = NULL;
> unsigned long flags;
> int ret;
>
> switch (urb->status) {
> case 0:
> break;
>
> default:
> uvc_printk(KERN_WARNING, "Non-zero status (%d) in video "
> "completion handler.\n", urb->status);
> + break;
>
> case -ENOENT: /* usb_kill_urb() called. */
> if (video->frozen)
> return;
>
> case -ECONNRESET: /* usb_unlink_urb() called. */
> case -ESHUTDOWN: /* The endpoint is being disabled. */
> uvc_queue_cancel(queue, urb->status == -ESHUTDOWN);
> return;
> }
>
> spin_lock_irqsave(&queue->irqlock, flags);
> if (!list_empty(&queue->irqqueue))
> buf = list_first_entry(&queue->irqqueue, struct uvc_buffer,
> queue);
> spin_unlock_irqrestore(&queue->irqlock, flags);
>
> video->decode(urb, video, buf);
>
> if ((ret = usb_submit_urb(urb, GFP_ATOMIC)) < 0) {
> uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
> ret);
> }
> }
That would work in your case, but devices that constantly return -EPROTO will
never see their queue canceled and will flood the kernel log.
A possible solution would be to count the number of URBs returning with the -
EPROTO status and cancel the queue only when a hardcoded limit is reached.
Could you count how many URBs you get back with a -EPROTO state before the
driver receives -ESHUTDOWN ?
Another solution would be to cancel all buffer queues when the uvc_disconnect
function is called. The QBUF ioctl could still be called during to "EPROTO -
ESHUTDOWN" window, but buffers would be woken up again when uvc_disconnect is
called.
The best solution might be a mix between those two: ignore -EPROTO/-EILSEQ/-
ETIME errors up to a maximum number (the threshold could be the total number
or errors, or the number of consecutive errors), and cancel the queues anyway
when the device is disconnected. What's your opinion ?
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-uvc-devel mailing list
Linux-uvc-devel@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
