Hi Laurent,
Am 08.09.2011 10:32, schrieb Laurent Pinchart:
Hi Alexey,
On Wednesday 07 September 2011 10:46:41 Alexey Fisher wrote:
On 07.09.2011 10:05, Laurent Pinchart wrote:
On Wednesday 07 September 2011 08:14:45 Alexey Fisher wrote:
Am 06.09.2011 17:24, schrieb Laurent Pinchart:
On Monday 05 September 2011 17:48:42 Alexey Fisher wrote:
Am 31.08.2011 00:32, schrieb Laurent Pinchart:
On Thursday 25 August 2011 09:44:10 Alexey Fisher wrote:
Hi Laurent,
are there any reason why uvc_video_decode_start do not do precise
header size checks? Are there many cameras with broken header size
too?
How precise do you mean ? The driver currently doesn't use much of
the header, so it just makes sure that the header size is smaller
than or equal to the packet size, and that it's at least 2 bytes
long.
I send you patch on what i work now to catch streams with fragmented
packets.. what do you think about it? Will you apply some thing like
this?
I'm not sure about that. Webcams that would require something like
that are so broken that I'm tempted to consider them as not
UVC-compliant. They should be returned to vendors with a loud
complaint.
Your patch might help, but the sad story is that it can't completely
fix the streams. There's always a chance that fragmented packets that
contains no header will start with data that looks like a header. You
won't be able to find a buller-proof solution.
You are right,
the idea is not to show definitely broken frames. If there is some
thing what we can't filter, is ok. we did our best.
I understand. I'm not sure if this should be included in the mainline
uvcvideo driver though. It makes the code more complex to support a
couple of completely broken devices, and doesn't guarantee that those
devices will work correctly.
ok, i give up.
I'm sorry about that. I definitely appreciate the work you've done on
this, but as I said I don't think we should cripple the driver with
complex code just to try to support a bit better a webcam that is
completely broken anyway.
I just thinking about build in uvc compliance tester insight of the
module. Some thing what users can use right in shop or at home before
14 day return guarantee.
You enables compliance test and it print results in in dmesg. One of
test should be header check, error/drop rate, and so on.
That's an interesting idea. It should probably come with a userspace
stress test software as well.
you mean luvcview or some thing other?
I'm thinking about a new command line software that would get/set
controls at high speed for instance, verify that all controls reported
by the device are actually supported, start/stop streaming multiple
times, ... Something that would stress the device and try to crash it
(which is unfortunately often quite easy :-)).
That's nice :)
and on kernel should provide file with statistics. Some thing like:
/sys/kernel/debug/uvcvideo/stats
It should have
- packet count
- frame count
- payloads with error bit
- empthy payloads
- overruns, and underruns (for row)
- jpeg completation (SOI - EOI)
I don't think the driver should parse MJPEG data to extract markers. This can
be done by the userspace application.
Any thing else?
The number of failed and total control requests would also be interesting.
Statistics about the status endpoint would also be interesting, to verify that
the device correctly sends control change events for instance (although those
are not properly supported in the driver yet).
this is initial patch for debugfs interface. I didn't implemented all
requests, i think the size of this patch is any way too big now.
Here is how it's working. After driver is loaded it create
debugfs/usb/uvcvideo directory. If some device is attached it create
device dir like this: 001.007_046d.0991 (first part is usb bus, second
is usb id). In this directory it create file called stats. Here is the
content of this file:
usb id: 046d:0991, usb bus: 001:007
packet size: 944(6)
state: idle <-- show the state of device. capture or idle
start time: 1316585466 <- unix epoch time in seconds.
capture time: 106 <- capture time in seconds.
format: YUV 4:2:2 (YUYV)
resolution: 320x240 @ 30
decode_start: 846880 <- this show how many payloads we started to decoding.
bad_header: 0 <-- haw many payloads was dropped, because of bad header
uvc_empty: 604160 <-- correct uvc payloads without video data
uvc_stream_err: 0 <-- count of payloads with err bit set
sequence: 955 <-- count of fid switches
out_of_sync: 0 <-- out of sync calls
diff --git a/drivers/media/video/uvc/Makefile b/drivers/media/video/uvc/Makefile
index 2071ca8..c26d12f 100644
--- a/drivers/media/video/uvc/Makefile
+++ b/drivers/media/video/uvc/Makefile
@@ -1,5 +1,5 @@
uvcvideo-objs := uvc_driver.o uvc_queue.o uvc_v4l2.o uvc_video.o uvc_ctrl.o \
- uvc_status.o uvc_isight.o
+ uvc_status.o uvc_isight.o uvc_debugfs.o
ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
uvcvideo-objs += uvc_entity.o
endif
diff --git a/drivers/media/video/uvc/uvc_debugfs.c b/drivers/media/video/uvc/uvc_debugfs.c
new file mode 100644
index 0000000..34f354b
--- /dev/null
+++ b/drivers/media/video/uvc/uvc_debugfs.c
@@ -0,0 +1,207 @@
+
+#include <linux/slab.h>
+#include <linux/debugfs.h>
+#include <linux/usb.h>
+
+#include "uvcvideo.h"
+
+#define MAX_DIRNAME 18
+
+#define STAT_BUF_SIZE 1024
+
+struct snap {
+ int slen;
+ char str[STAT_BUF_SIZE];
+};
+
+/* we do not need really precise time, seconds are ok */
+void uvc_debugfs_gettimeofday(__kernel_time_t *time)
+{
+ struct timeval tv;
+
+ do_gettimeofday(&tv);
+ *time = tv.tv_sec;
+}
+
+static int uvc_debugfs_stat_open(struct inode *inode, struct file *file)
+{
+ struct uvc_device *dev = inode->i_private;
+ struct usb_device *udev = dev->udev;
+ struct uvc_debugfs_stat *uvc_stat = dev->debugfs_stat;
+ struct snap *sp;
+
+ sp = kmalloc(sizeof(struct snap), GFP_KERNEL);
+
+ if (sp == NULL)
+ return -ENOMEM;
+
+ sp->slen = 0;
+
+ if (uvc_stat->current_state)
+ uvc_debugfs_gettimeofday(&uvc_stat->last_time);
+
+ /* device descriptor */
+ sp->slen += snprintf(sp->str + sp->slen, STAT_BUF_SIZE - sp->slen,
+ "usb id: %04x:%04x, usb bus: %03i:%03i\n",
+ le16_to_cpu(udev->descriptor.idVendor),
+ le16_to_cpu(udev->descriptor.idProduct),
+ udev->bus->busnum, udev->devnum);
+
+ /* capture state */
+ sp->slen += snprintf(sp->str + sp->slen, STAT_BUF_SIZE - sp->slen,
+ "packet size: %d(%d)\n"
+ "state: %s\n",
+ uvc_stat->best_psize, uvc_stat->altsetting,
+ uvc_stat->current_state ? "capture" : "idle");
+
+ sp->slen += snprintf(sp->str + sp->slen, STAT_BUF_SIZE - sp->slen,
+ "start time: %ld\n"
+ "capture time: %ld\n",
+ uvc_stat->start_time,
+ uvc_stat->last_time - uvc_stat->start_time);
+
+ /* last/current format decsriptor */
+ sp->slen += snprintf(sp->str + sp->slen, STAT_BUF_SIZE - sp->slen,
+ "format: %s\n"
+ "resolution: %ux%u @ %u\n",
+ uvc_stat->format_name,
+ uvc_stat->width, uvc_stat->height,
+ uvc_stat->frame_interval ?
+ 10000000/uvc_stat->frame_interval : 0);
+
+ sp->slen += snprintf(sp->str + sp->slen, STAT_BUF_SIZE - sp->slen,
+ "decode_start: %u\n"
+ "bad_header: %u\n"
+ "uvc_empty: %u\n"
+ "uvc_stream_err: %u\n"
+ "sequence: %u\n"
+ "out_of_sync: %u\n",
+ uvc_stat->decode_start, uvc_stat->bad_header,
+ uvc_stat->uvc_empty,
+ uvc_stat->uvc_stream_err, uvc_stat->sequence,
+ uvc_stat->out_of_sync);
+
+ file->private_data = sp;
+ return 0;
+}
+
+/* nothing to change */
+static ssize_t uvc_debugfs_stat_read(struct file *file, char __user *buf,
+ size_t nbytes, loff_t *ppos)
+{
+ struct snap *sp = file->private_data;
+
+ return simple_read_from_buffer(buf, nbytes, ppos, sp->str, sp->slen);
+}
+
+/* nothing to change */
+static int uvc_debugfs_stat_release(struct inode *inode, struct file *file)
+{
+ struct snap *sp = file->private_data;
+ file->private_data = NULL;
+ kfree(sp);
+ return 0;
+}
+
+/* connect all funcs for debugfs */
+const struct file_operations uvc_debugfs_stat_fops = {
+ .owner = THIS_MODULE,
+ .open = uvc_debugfs_stat_open,
+ .llseek = no_llseek,
+ .read = uvc_debugfs_stat_read,
+ .release = uvc_debugfs_stat_release,
+};
+
+void uvc_debugfs_remove_dir(struct dentry **dir)
+{
+ debugfs_remove_recursive(*dir);
+ *dir = NULL;
+}
+
+int uvc_debugfs_create_root_dir(struct dentry **root_dir)
+{
+ /* filter */
+ if (*root_dir != NULL) {
+ uvc_printk(KERN_INFO, "Trying to assign debugfs/uvcvideo "
+ "directory on existing pointer: %p", *root_dir);
+ return -ENOTEMPTY;
+ }
+
+ /* create dir */
+ *root_dir = debugfs_create_dir("uvcvideo", usb_debug_root);
+
+ if (IS_ERR(*root_dir) || *root_dir == NULL) {
+ uvc_printk(KERN_INFO, "Unable to create uvcvideo directory\n");
+ *root_dir = NULL;
+ return -ENODATA;
+ }
+ return 0;
+}
+
+int uvc_debugfs_create_dev_dir(struct dentry **root_dir,
+ struct uvc_device *dev)
+{
+ struct usb_device *udev = dev->udev;
+ char dir_name[MAX_DIRNAME];
+
+ /* filter */
+ if (dev->debugfs_dev_dir != NULL) {
+ uvc_printk(KERN_INFO, "Trying to assign device directory "
+ "on existing pointer: %p", dev->debugfs_dev_dir);
+ return -ENOTEMPTY;
+ }
+
+ /* Some times create_dev_dir called before create_root_dir */
+ if (*root_dir == NULL)
+ if (uvc_debugfs_create_root_dir(root_dir))
+ return -ENODATA;
+
+
+ snprintf(dir_name, MAX_DIRNAME, "%03i.%03i_%04x.%04x",
+ udev->bus->busnum,
+ udev->devnum,
+ le16_to_cpu(udev->descriptor.idVendor),
+ le16_to_cpu(udev->descriptor.idProduct));
+
+
+ dev->debugfs_dev_dir = debugfs_create_dir(dir_name, *root_dir);
+
+ if (IS_ERR(dev->debugfs_dev_dir) || dev->debugfs_dev_dir == NULL) {
+ uvc_printk(KERN_INFO, "Unable to create %s directory. Err:%p\n",
+ dir_name, dev->debugfs_dev_dir);
+ dev->debugfs_dev_dir = NULL;
+ /* root dir will exist, may be we will have more,
+ * luck next time */
+ return -ENODATA;
+ }
+
+ dev->debugfs_stat = kzalloc(sizeof(struct uvc_debugfs_stat),
+ GFP_KERNEL);
+ if (dev->debugfs_stat == NULL) {
+ uvc_printk(KERN_INFO, "Unable to allocate memory for stats\n");
+ uvc_debugfs_remove_dir(&dev->debugfs_dev_dir);
+ return -ENOMEM;
+ }
+
+ dev->stat_file = debugfs_create_file("stats", 0600,
+ dev->debugfs_dev_dir,
+ dev, &uvc_debugfs_stat_fops);
+
+ if (IS_ERR(dev->stat_file) || dev->stat_file == NULL) {
+ /* debugfs not available, we can use uvcvideo without it */
+ uvc_printk(KERN_INFO, "Unable to create stat file. Err:%p\n",
+ dev->stat_file);
+ kfree(dev->debugfs_stat);
+ dev->debugfs_stat = NULL;
+ uvc_debugfs_remove_dir(&dev->debugfs_dev_dir);
+ dev->stat_file = NULL;
+ return -ENODATA;
+ }
+
+ return 0;
+}
+
+void uvc_debugfs_reset_stats(struct uvc_debugfs_stat *debugfs_stat)
+{
+ memset(debugfs_stat, 0, sizeof(struct uvc_debugfs_stat));
+}
diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c
index d29f9c2..188980d 100644
--- a/drivers/media/video/uvc/uvc_driver.c
+++ b/drivers/media/video/uvc/uvc_driver.c
@@ -49,6 +49,8 @@ static unsigned int uvc_quirks_param = -1;
unsigned int uvc_trace_param;
unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT;
+static struct dentry *uvc_debugfs_root_dir;
+
/* ------------------------------------------------------------------------
* Video formats
*/
@@ -1891,6 +1893,7 @@ static int uvc_probe(struct usb_interface *intf,
uvc_trace(UVC_TRACE_PROBE, "UVC device initialized.\n");
usb_enable_autosuspend(udev);
+ uvc_debugfs_create_dev_dir(&uvc_debugfs_root_dir, dev);
return 0;
error:
@@ -1913,6 +1916,16 @@ static void uvc_disconnect(struct usb_interface *intf)
dev->state |= UVC_DEV_DISCONNECTED;
+ /* If root dir is cleared, dev dir cleared as well.
+ * But In this case, debugfs_dev_dir is a pointer to the forrest.
+ * Remove just in case, some body wont to use it.
+ */
+ if (uvc_debugfs_root_dir) {
+ kfree(dev->debugfs_stat);
+ uvc_debugfs_remove_dir(&dev->debugfs_dev_dir);
+ } else
+ dev->debugfs_dev_dir = NULL;
+
uvc_unregister_video(dev);
}
@@ -2386,13 +2399,16 @@ static int __init uvc_init(void)
int result;
result = usb_register(&uvc_driver.driver);
- if (result == 0)
+ if (result == 0) {
printk(KERN_INFO DRIVER_DESC " (" DRIVER_VERSION ")\n");
+ uvc_debugfs_create_root_dir(&uvc_debugfs_root_dir);
+ }
return result;
}
static void __exit uvc_cleanup(void)
{
+ uvc_debugfs_remove_dir(&uvc_debugfs_root_dir);
usb_deregister(&uvc_driver.driver);
}
diff --git a/drivers/media/video/uvc/uvc_video.c b/drivers/media/video/uvc/uvc_video.c
index 8244167..38278f5 100644
--- a/drivers/media/video/uvc/uvc_video.c
+++ b/drivers/media/video/uvc/uvc_video.c
@@ -410,29 +410,40 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
struct uvc_buffer *buf, const __u8 *data, int len)
{
__u8 fid;
+ struct uvc_debugfs_stat *debugfs_stat = stream->dev->debugfs_stat;
+
+ debugfs_stat->decode_start++;
/* Sanity checks:
* - packet must be at least 2 bytes long
* - bHeaderLength value must be at least 2 bytes (see above)
* - bHeaderLength value can't be larger than the packet size.
*/
- if (len < 2 || data[0] < 2 || data[0] > len)
+ if (len < 2 || data[0] < 2 || data[0] > len) {
+ debugfs_stat->bad_header++;
return -EINVAL;
+ }
/* Skip payloads marked with the error bit ("error frames"). */
if (data[1] & UVC_STREAM_ERR) {
+ debugfs_stat->uvc_stream_err++;
uvc_trace(UVC_TRACE_FRAME, "Dropping payload (error bit "
"set).\n");
return -ENODATA;
}
+ if (data[0] == len)
+ debugfs_stat->uvc_empty++;
+
fid = data[1] & UVC_STREAM_FID;
/* Increase the sequence number regardless of any buffer states, so
* that discontinuous sequence numbers always indicate lost frames.
*/
- if (stream->last_fid != fid)
+ if (stream->last_fid != fid) {
+ debugfs_stat->sequence++;
stream->sequence++;
+ }
/* Store the payload FID bit and return immediately when the buffer is
* NULL.
@@ -454,6 +465,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
struct timespec ts;
if (fid == stream->last_fid) {
+ debugfs_stat->out_of_sync++;
uvc_trace(UVC_TRACE_FRAME, "Dropping payload (out of "
"sync).\n");
if ((stream->dev->quirks & UVC_QUIRK_STREAM_NO_FID) &&
@@ -858,6 +870,7 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
*/
static void uvc_uninit_video(struct uvc_streaming *stream, int free_buffers)
{
+ struct uvc_debugfs_stat *debugfs_stat = stream->dev->debugfs_stat;
struct urb *urb;
unsigned int i;
@@ -871,6 +884,9 @@ static void uvc_uninit_video(struct uvc_streaming *stream, int free_buffers)
stream->urb[i] = NULL;
}
+ debugfs_stat->current_state = 0;
+ uvc_debugfs_gettimeofday(&debugfs_stat->last_time);
+
if (free_buffers)
uvc_free_urb_buffers(stream);
}
@@ -984,6 +1000,7 @@ static int uvc_init_video_bulk(struct uvc_streaming *stream,
static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags)
{
struct usb_interface *intf = stream->intf;
+ struct uvc_debugfs_stat *debugfs_stat = stream->dev->debugfs_stat;
struct usb_host_endpoint *ep;
unsigned int i;
int ret;
@@ -994,6 +1011,8 @@ static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags)
stream->bulk.skip_payload = 0;
stream->bulk.payload_size = 0;
+ uvc_debugfs_reset_stats(debugfs_stat);
+
if (intf->num_altsetting > 1) {
struct usb_host_endpoint *best_ep = NULL;
unsigned int best_psize = 3 * 1024;
@@ -1042,6 +1061,9 @@ static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags)
uvc_trace(UVC_TRACE_VIDEO, "Selecting alternate setting %u "
"(%u B/frame bandwidth).\n", altsetting, best_psize);
+ debugfs_stat->best_psize = best_psize;
+ debugfs_stat->altsetting = altsetting;
+
ret = usb_set_interface(stream->dev->udev, intfnum, altsetting);
if (ret < 0)
return ret;
@@ -1071,6 +1093,13 @@ static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags)
}
}
+ debugfs_stat->current_state = 1;
+ strcpy(debugfs_stat->format_name, stream->cur_format->name);
+ debugfs_stat->width = stream->cur_frame->wWidth;
+ debugfs_stat->height = stream->cur_frame->wHeight;
+ debugfs_stat->frame_interval = stream->ctrl.dwFrameInterval;
+ uvc_debugfs_gettimeofday(&debugfs_stat->start_time);
+
return 0;
}
diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h
index df32a43..71cb173 100644
--- a/drivers/media/video/uvc/uvcvideo.h
+++ b/drivers/media/video/uvc/uvcvideo.h
@@ -496,6 +496,28 @@ struct uvc_streaming {
__u8 last_fid;
};
+struct uvc_debugfs_stat {
+ unsigned int payload_count;
+ unsigned int current_state;
+ unsigned int best_psize;
+ unsigned int altsetting;
+
+ char format_name[32];
+ unsigned int width;
+ unsigned int height;
+ __u32 frame_interval;
+
+ __u32 decode_start;
+ __u32 bad_header;
+ __u32 uvc_stream_err;
+ __u32 uvc_empty;
+ __u32 sequence;
+ __u32 out_of_sync;
+
+ __kernel_time_t start_time;
+ __kernel_time_t last_time;
+};
+
enum uvc_device_state {
UVC_DEV_DISCONNECTED = 1,
};
@@ -533,6 +555,11 @@ struct uvc_device {
__u8 *status;
struct input_dev *input;
char input_phys[64];
+
+ /* debugfs device dir */
+ struct dentry *debugfs_dev_dir;
+ struct dentry *stat_file;
+ struct uvc_debugfs_stat *debugfs_stat;
};
enum uvc_handle_state {
@@ -698,6 +725,14 @@ extern struct usb_host_endpoint *uvc_find_endpoint(
void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream,
struct uvc_buffer *buf);
+/* debugfs functions */
+int uvc_debugfs_create_root_dir(struct dentry **root_dir);
+void uvc_debugfs_remove_dir(struct dentry **dir);
+int uvc_debugfs_create_dev_dir(struct dentry **root_dir,
+ struct uvc_device *dev);
+void uvc_debugfs_reset_stats(struct uvc_debugfs_stat *debugfs_stat);
+void uvc_debugfs_gettimeofday(__kernel_time_t *time);
+
#endif /* __KERNEL__ */
#endif
_______________________________________________
Linux-uvc-devel mailing list
Linux-uvc-devel@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
