Hi Étienne,
On Sunday 09 December 2007, Étienne Bersac wrote:
> Hi,
>
> Follows an updated patch based on Ivan work. It's half weighted, mainly
> because it does not extract and load the AppleUSBVideoSupport OS X
> driver. This is done in userspace using isight-firmware-tools availables
> at http://bersace03.free.fr/ift/ and udev. No SHA1 computation, No
> twice probe, No double device entry, No kernel space firmware loading,
> No is_isight(), etc. isight.h has been merged into uvcvideo.h and
> isight.c has been renamed to uvc_isight.c.
First of all, thanks for your work. Comments inlined.
> Many thanks to Laurent for helping cleaning the patch and properly
> implement the quirk workaround.
>
> Kind regards,
> Étienne.
>
> Signed-off-by: Étienne Bersac <[EMAIL PROTECTED]>
> ---
> Add built-in isight support to uvcvieo driver. Add a decode function
> member to struct uvc_video_queue. The decode function has three
> implementation : isoc, bulk and isight. uvc_isight.c implement the
> isight decode function.
>
> A BUILTIN_ISIGHT quirk is added allowing to determine wether to use
> isight decode function or not.
>
> Index: uvc_video.c
> ===================================================================
> --- uvc_video.c (révision 149)
> +++ uvc_video.c (copie de travail)
> @@ -452,8 +452,9 @@
> /*
> * Completion handler for video URBs.
> */
> -static void uvc_video_complete_isoc(struct urb *urb,
> - struct uvc_video_queue *queue, struct uvc_buffer *buf)
> +static void uvc_video_decode_isoc(struct urb *urb,
> + struct uvc_video_queue *queue,
> + struct uvc_buffer *buf)
> {
> u8 *mem;
> int ret, i;
Please don't cleanup formatting unrelated to iSight support.
> @@ -490,8 +491,9 @@
> }
> }
>
> -static void uvc_video_complete_bulk(struct urb *urb,
> - struct uvc_video_queue *queue, struct uvc_buffer *buf)
> +static void uvc_video_decode_bulk(struct urb *urb,
> + struct uvc_video_queue *queue,
> + struct uvc_buffer *buf)
> {
> u8 *mem;
> int len, ret;
Ditto.
> @@ -585,10 +587,7 @@
> buf = list_entry(queue->irqqueue.next, struct uvc_buffer,
> queue);
> spin_unlock_irqrestore(&queue->irqlock, flags);
>
> - if (urb->number_of_packets)
> - uvc_video_complete_isoc(urb, queue, buf);
> - else
> - uvc_video_complete_bulk(urb, queue, buf);
> + queue->decode(urb, queue, buf);
>
> if ((ret = usb_submit_urb(urb, GFP_ATOMIC)) < 0) {
> uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
> @@ -628,7 +627,8 @@
> * Initialize isochronous URBs and allocate transfer buffers. The packet
> size * is given by the endpoint.
> */
> -static int uvc_init_video_isoc(struct uvc_video_device *video, struct
> usb_host_endpoint *ep) +static int uvc_init_video_isoc(struct
> uvc_video_device *video,
> + struct usb_host_endpoint *ep)
> {
> struct urb *urb;
> unsigned int npackets, i, j;
Ditto.
> @@ -693,7 +693,8 @@
> * Initialize bulk URBs and allocate transfer buffers. The packet size is
> * given by the endpoint.
> */
> -static int uvc_init_video_bulk(struct uvc_video_device *video, struct
> usb_host_endpoint *ep) +static int uvc_init_video_bulk(struct
> uvc_video_device *video,
> + struct usb_host_endpoint *ep)
> {
> struct urb *urb;
> unsigned int pipe, i;
Ditto.
> @@ -747,7 +748,7 @@
> {
> struct usb_interface *intf = video->streaming->intf;
> struct usb_host_interface *alts;
> - struct usb_host_endpoint *ep;
> + struct usb_host_endpoint *ep = NULL;
> int intfnum = video->streaming->intfnum;
> unsigned int bandwidth, psize, i;
> int ret;
This fixes a gcc4 warning. I'll commit it as a separate patch.
> @@ -937,6 +938,16 @@
> video->streaming->cur_frame = frame;
> atomic_set(&video->active, 1);
>
> +
> + /* determine the decode function */
> + if (video->dev->quirks & UVC_QUIRK_BUILTIN_ISIGHT)
> + video->queue.decode = uvc_video_decode_isight;
> + else if (video->streaming->intf->num_altsetting > 1)
> + video->queue.decode = uvc_video_decode_isoc;
> + else
> + video->queue.decode = uvc_video_decode_bulk;
> +
> +
Please remove extra blank lines.
> return 0;
> }
>
> Index: uvc_isight.c
> ===================================================================
> --- uvc_isight.c (révision 0)
> +++ uvc_isight.c (révision 0)
> @@ -0,0 +1,154 @@
> +/*
> + * Copyright (C) 2006-2007 Ivan N. Zlatev <[EMAIL PROTECTED]>
> + * Copyright © 2007 Étienne Bersac <[EMAIL PROTECTED]>
Are non ascii characters allowed in Linux sources ?
> + *
> + * Based on extract.c by Ronald S. Bultje <[EMAIL PROTECTED]>
> + * Firmware loading specifics by Johannes Berg <[EMAIL PROTECTED]>
> + * at http://johannes.sipsolutions.net/MacBook/iSight
There is no firmware loading anymore.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301, USA
> + */
> +
> +#include <linux/usb.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +
> +#include "uvcvideo.h"
> +
> +#define isight_printk(msg...) \
> + printk(KERN_DEBUG "uvcvideo: iSight: " msg)
> +
Unused, please remove.
> +/* Built-in iSight webcams are completely broken. They implement most
> + * of UVC 1.0, but the Apple engineers decided to use a completely
> + * different packet format, although the video data is in YUV. Were
> + * they on crack or just lazy ? As the hardware is 8051-based, it
> + * might be interesting to write an open-source firmware.
I think I'm the author of that message. Now that I know more about the
hardware, I understand the 8051-based controller they used doesn't have the
required hardware to generate UVC-compatible isochronous packets. The comment
should be made a bit less aggressive :-)
> + *
> + * Instead of sending a header at the beginning of each isochronous
> + * transfer payload, the webcam sends a single header per image (on
> + * its own in a packet), followed by packets containing data only.
> + *
> + * Offset Size (bytes) Description
> + * ------------------------------------------------------------------
> + * 0x00 1 Header length
> + * 0x01 1 Flags (UVC-compliant)
> + * 0x02 4 Always equal to '11223344'
> + * 0x06 8 Always equal to 'deadbeefdeadface'
> + * 0x0e 16 Unknown
> + *
> + * The header can be prefixed by an optional, unknown-purpose byte.
> + */
> +
> +static int isight_decode (struct uvc_video_queue *queue,
> + struct uvc_buffer *buf,
> + const __u8 *data, unsigned int len)
Please use two tabs to indent function arguments, don't try to align them on
the opening parenthesis.
> +{
> + static const __u8 hdr[] = {
> + 0x11, 0x22, 0x33, 0x44, 0xde, 0xad, 0xbe, 0xef,
> + 0xde, 0xad, 0xfa, 0xce
> + };
> +
> + unsigned int maxlen, nbytes;
> + __u8 *mem;
> + int is_header = 0;
> +
> + if (buf == NULL)
> + return 0;
> +
> + if ((len >= 14 && memcmp (&data[2], hdr, 12) == 0) ||
> + (len >= 15 && memcmp (&data[3], hdr, 12) == 0)) {
> + uvc_trace(UVC_TRACE_FRAME, "iSight header found\n");
> + is_header = 1;
> + }
> +
> + /* Synchronize to the input stream by waiting for a header packet. */
> + if (buf->state != UVC_BUF_STATE_ACTIVE) {
> + if (!is_header) {
> + uvc_trace(UVC_TRACE_FRAME, "Dropping packet (out of "
> + "sync).\n");
> + return 0;
> + }
> +
> + buf->state = UVC_BUF_STATE_ACTIVE;
> + }
> +
> + /* Mark the buffer as done if we're at the beginning of a new frame.
> + *
> + * Empty buffers (bytesused == 0) don't trigger end of frame detection
> + * as it doesn't make sense to return an empty buffer.
> + */
> + if (is_header && buf->buf.bytesused != 0) {
> + buf->state = UVC_BUF_STATE_DONE;
> + return -EAGAIN;
> + }
> +
> + /* Copy the video data to the buffer. Skip header packets, as they
> + * contain no data.
> + */
> + if (!is_header) {
> + maxlen = buf->buf.length - buf->buf.bytesused;
> + mem = queue->mem + buf->buf.m.offset + buf->buf.bytesused;
> + nbytes = min(len, maxlen);
> + memcpy(mem, data, nbytes);
> + buf->buf.bytesused += nbytes;
> +
> + /* Drop the current frame if the buffer size was exceeded. */
The frame isn't dropped, the comment is incorrect.
> + if (len > maxlen || buf->buf.bytesused == buf->buf.length) {
> + uvc_trace(UVC_TRACE_FRAME, "Frame complete
> (overflow).\n");
> + buf->state = UVC_BUF_STATE_DONE;
> + }
> + }
> +
> + return 0;
> +}
> +
> +void uvc_video_decode_isight (struct urb *urb,
> + struct uvc_video_queue *queue,
> + struct uvc_buffer *buf)
Two tabs please.
> +{
> + int ret, i;
> +
> + for (i = 0; i < urb->number_of_packets; ++i) {
> + 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);
> + /* Breaks 640x480
> + continue;
> + */
Please remove commented code.
> + }
> +
> + /* Decode the payload packet.
> + * uvc_video_decode is entered twice when a frame transition
> + * has been detected because the end of frame can only be
> + * reliably detected when the first packet of the new frame
> + * is processed. The first pass detects the transition and
> + * closes the previous frame's buffer, the second pass
> + * processes the data of the first payload of the new frame.
> + */
> + do {
> + ret = isight_decode(queue, buf,
> + urb->transfer_buffer +
> urb->iso_frame_desc[i].offset,
> +
> urb->iso_frame_desc[i].actual_length);
> +
> + if (buf == NULL)
> + break;
> +
> + if (buf->state == UVC_BUF_STATE_DONE ||
> + buf->state == UVC_BUF_STATE_ERROR)
> + buf = uvc_queue_next_buffer(queue, buf);
> + } while (ret == -EAGAIN);
> + }
> +}
> Index: uvc_driver.c
> ===================================================================
> --- uvc_driver.c (révision 149)
> +++ uvc_driver.c (copie de travail)
> @@ -1667,6 +1667,13 @@
> .bInterfaceProtocol = 0,
> .driver_info = UVC_QUIRK_PROBE_MINMAX
> },
> + /* Apple Built-In iSight */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE,
> + .idVendor = 0x05ac,
> + .idProduct = 0x8501,
> + .driver_info = UVC_QUIRK_PROBE_MINMAX
> + | UVC_QUIRK_BUILTIN_ISIGHT
> + },
> /* Microsoft Lifecam NX-6000 */
> { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
>
> | USB_DEVICE_ID_MATCH_INT_INFO,
>
> Index: uvcvideo.h
> ===================================================================
> --- uvcvideo.h (révision 149)
> +++ uvcvideo.h (copie de travail)
> @@ -311,6 +311,7 @@
> #define UVC_QUIRK_STATUS_INTERVAL 0x00000001
> #define UVC_QUIRK_PROBE_MINMAX 0x00000002
> #define UVC_QUIRK_PROBE_EXTRAFIELDS 0x00000004
> +#define UVC_QUIRK_BUILTIN_ISIGHT 0x00000008
>
> /* Format flags */
> #define UVC_FMT_FLAG_COMPRESSED 0x00000001
> @@ -576,6 +577,9 @@
>
> struct list_head mainqueue;
> struct list_head irqqueue;
> +
> + void (*decode) (struct urb *urb, struct uvc_video_queue *queue,
> + struct uvc_buffer *buf);
> };
>
> struct uvc_video_device {
> @@ -764,6 +768,11 @@
> extern struct usb_host_endpoint *uvc_find_endpoint(
> struct usb_host_interface *alts, __u8 epaddr);
>
> +/* quirks functions */
Could you rename that to 'Quirks support' ?
> +void uvc_video_decode_isight (struct urb *urb, struct uvc_video_queue
> *queue, + struct uvc_buffer *buf);
Two tabs please.
> +
> +
> #endif /* __KERNEL__ */
>
> #endif
> Index: Makefile
> ===================================================================
> --- Makefile (révision 149)
> +++ Makefile (copie de travail)
> @@ -5,7 +5,7 @@
> PWD := $(shell pwd)
>
> obj-m := uvcvideo.o
> -uvcvideo-objs := uvc_driver.o uvc_queue.o uvc_v4l2.o uvc_video.o
> uvc_ctrl.o +uvcvideo-objs := uvc_driver.o uvc_queue.o uvc_v4l2.o
> uvc_video.o uvc_ctrl.o uvc_isight.o
>
> all: uvcvideo
Other than for those cosmetic fixes, the patch looks fine to me. Please
resubmit it, and I'll then wait a few days to see if other people have
comments before committing it.
Thanks a lot for your work.
Best regards,
Laurent Pinchart
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel