Re: [REVIEW] v4l2 loopback
On Thu, 7 May 2009 02:54:00 +0300 Vasily vas...@gmail.com wrote: This patch introduces v4l2 loopback module Hi Vasily, next time it would be useful to summarize what you changed from the previous version, and put a revision number in the Subject, like [PATCH v2] [PATCH v3], etc. Also, the patch has some style problems reported by checkpatch.pl, please fix those. Even if the driver wouldn't go mainline you do want to follow the style guidelines. And some more typos I spotted, see inlined comments. I am not a native English speaker either, so I learned to use spell checkers even in source code comments :) Anyhow, finally I tested the driver, and I have only a question as a user: couldn't it be possible to make a default input enabled by default? This way, if a writer knows the format to use it doesn't have to setup the device format on its own, and can treat the device as a normal file. Thanks, Antonio From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available Typo: Initialy - Initially to Skype, but in fact it have many more uses. Priority: normal Signed-off-by: Vasily Levin vas...@gmail.com diff -uprN v4l-dvb.orig/linux/drivers/media/video/Kconfig v4l-dvb.my/linux/drivers/media/video/Kconfig --- v4l-dvb.orig/linux/drivers/media/video/Kconfig2009-04-25 04:41:20.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/Kconfig 2009-05-07 01:49:38.0 +0300 @@ -479,6 +479,17 @@ config VIDEO_VIVI Say Y here if you want to test video apps or debug V4L devices. In doubt, say N. +config VIDEO_V4L2_LOOPBACK + tristate v4l2 loopback driver + depends on VIDEO_V4L2 VIDEO_DEV + help + Say Y if you want to use v4l2 loopback driver. + Looback driver allows it's user to present any userspace typo: it's - its + video as a v4l2 device that can be handy for tesring purpose, typo: tesring - testing + or for fixing bugs like upside down image, or for adding + nice effects to videochats + This driver can be compiled as a module, called v4l2loopback. + source drivers/media/video/bt8xx/Kconfig config VIDEO_PMS diff -uprN v4l-dvb.orig/linux/drivers/media/video/Makefile v4l-dvb.my/linux/drivers/media/video/Makefile --- v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-05-07 01:31:32.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/Makefile 2009-05-07 01:50:11.0 +0300 @@ -132,6 +132,7 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/ obj-$(CONFIG_VIDEO_CX18) += cx18/ obj-$(CONFIG_VIDEO_VIVI) += vivi.o +obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o obj-$(CONFIG_VIDEO_CX23885) += cx23885/ obj-$(CONFIG_VIDEO_OMAP2)+= omap2cam.o diff -uprN v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c --- v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01 03:00:00.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c 2009-05-07 02:30:08.0 +0300 @@ -0,0 +1,775 @@ +/* + * v4l2loopback.c -- video 4 linux loopback driver + * + * Copyright (C) 2005-2009 + * Vasily Levin (vas...@gmail.com) + * + * 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. + * + */ +#include linux/version.h +#include linux/vmalloc.h +#include linux/mm.h +#include linux/time.h +#include linux/module.h +#include media/v4l2-ioctl.h +#include linux/videodev2.h +#include media/v4l2-common.h + Usually the includes with the same prefix come all near one another, you could move #include linux/videodev2.h one line up. +#define YAVLD_STREAMING + +MODULE_DESCRIPTION(V4L2 loopback video device); +MODULE_VERSION(0.1.1); +MODULE_AUTHOR(Vasily Levin); +MODULE_LICENSE(GPL); + +/* module structures */ +/* TODO(vasaka) use typenames which are common to kernel, but first find out if + * it is needed */ +/* struct keeping state and settings of loopback device */ +struct v4l2_loopback_device { + struct video_device *vdev; + /* pixel and stream format */ + struct v4l2_pix_format pix_format; + struct v4l2_captureparm capture_param; + /* buffers stuff */ + u8 *image; /* pointer to actual buffers data */ + int buffers_number; /* should not be big, 4 is a good choice */ + struct v4l2_buffer *buffers;/* inner driver buffers */ + int write_position; /* number of last written frame + 1 */ + long buffer_size; + /* sync stuff */ + atomic_t open_count; + int ready_for_capture;/* set to true when at least one writer opened + *
Re: [REVIEW] v4l2 loopback
On Thu, May 7, 2009 at 9:28 PM, Antonio Ospite osp...@studenti.unina.it wrote: On Thu, 7 May 2009 02:54:00 +0300 Vasily vas...@gmail.com wrote: This patch introduces v4l2 loopback module Hi Vasily, next time it would be useful to summarize what you changed from the previous version, and put a revision number in the Subject, like [PATCH v2] [PATCH v3], etc. ok Also, the patch has some style problems reported by checkpatch.pl, please fix those. Even if the driver wouldn't go mainline you do want to follow the style guidelines. I have checked it with make commit in v4l-dvb tree, and it did not complain, I'll look into that. And some more typos I spotted, see inlined comments. I am not a native English speaker either, so I learned to use spell checkers even in source code comments :) Anyhow, finally I tested the driver, and I have only a question as a user: couldn't it be possible to make a default input enabled by default? This way, if a writer knows the format to use it doesn't have to setup the device format on its own, and can treat the device as a normal file. I think it is a good improvement, but what format should be default? and even if I'll do so user will have to make syscalls to change format. Also please exclude scopic addres from recipients when you answering this message, I added this addres by mistake Thanks, Vasily Thanks, Antonio From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available Typo: Initialy - Initially to Skype, but in fact it have many more uses. Priority: normal Signed-off-by: Vasily Levin vas...@gmail.com diff -uprN v4l-dvb.orig/linux/drivers/media/video/Kconfig v4l-dvb.my/linux/drivers/media/video/Kconfig --- v4l-dvb.orig/linux/drivers/media/video/Kconfig 2009-04-25 04:41:20.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/Kconfig 2009-05-07 01:49:38.0 +0300 @@ -479,6 +479,17 @@ config VIDEO_VIVI Say Y here if you want to test video apps or debug V4L devices. In doubt, say N. +config VIDEO_V4L2_LOOPBACK + tristate v4l2 loopback driver + depends on VIDEO_V4L2 VIDEO_DEV + help + Say Y if you want to use v4l2 loopback driver. + Looback driver allows it's user to present any userspace typo: it's - its + video as a v4l2 device that can be handy for tesring purpose, typo: tesring - testing + or for fixing bugs like upside down image, or for adding + nice effects to videochats + This driver can be compiled as a module, called v4l2loopback. + source drivers/media/video/bt8xx/Kconfig config VIDEO_PMS diff -uprN v4l-dvb.orig/linux/drivers/media/video/Makefile v4l-dvb.my/linux/drivers/media/video/Makefile --- v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-05-07 01:31:32.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/Makefile 2009-05-07 01:50:11.0 +0300 @@ -132,6 +132,7 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/ obj-$(CONFIG_VIDEO_CX18) += cx18/ obj-$(CONFIG_VIDEO_VIVI) += vivi.o +obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o obj-$(CONFIG_VIDEO_CX23885) += cx23885/ obj-$(CONFIG_VIDEO_OMAP2) += omap2cam.o diff -uprN v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c --- v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01 03:00:00.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c 2009-05-07 02:30:08.0 +0300 @@ -0,0 +1,775 @@ +/* + * v4l2loopback.c -- video 4 linux loopback driver + * + * Copyright (C) 2005-2009 + * Vasily Levin (vas...@gmail.com) + * + * 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. + * + */ +#include linux/version.h +#include linux/vmalloc.h +#include linux/mm.h +#include linux/time.h +#include linux/module.h +#include media/v4l2-ioctl.h +#include linux/videodev2.h +#include media/v4l2-common.h + Usually the includes with the same prefix come all near one another, you could move #include linux/videodev2.h one line up. +#define YAVLD_STREAMING + +MODULE_DESCRIPTION(V4L2 loopback video device); +MODULE_VERSION(0.1.1); +MODULE_AUTHOR(Vasily Levin); +MODULE_LICENSE(GPL); + +/* module structures */ +/* TODO(vasaka) use typenames which are common to kernel, but first find out if + * it is needed */ +/* struct keeping state and settings of loopback device */ +struct v4l2_loopback_device { + struct video_device *vdev; + /* pixel and stream format */ + struct v4l2_pix_format pix_format; + struct v4l2_captureparm capture_param; + /*
Re: [REVIEW] v4l2 loopback
This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Priority: normal Signed-off-by: Vasily Levin vas...@gmail.com diff -uprN v4l-dvb.orig/linux/drivers/media/video/Kconfig v4l-dvb.my/linux/drivers/media/video/Kconfig --- v4l-dvb.orig/linux/drivers/media/video/Kconfig 2009-04-25 04:41:20.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/Kconfig2009-05-07 01:49:38.0 +0300 @@ -479,6 +479,17 @@ config VIDEO_VIVI Say Y here if you want to test video apps or debug V4L devices. In doubt, say N. +config VIDEO_V4L2_LOOPBACK + tristate v4l2 loopback driver + depends on VIDEO_V4L2 VIDEO_DEV + help + Say Y if you want to use v4l2 loopback driver. + Looback driver allows it's user to present any userspace + video as a v4l2 device that can be handy for tesring purpose, + or for fixing bugs like upside down image, or for adding + nice effects to videochats + This driver can be compiled as a module, called v4l2loopback. + source drivers/media/video/bt8xx/Kconfig config VIDEO_PMS diff -uprN v4l-dvb.orig/linux/drivers/media/video/Makefile v4l-dvb.my/linux/drivers/media/video/Makefile --- v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-05-07 01:31:32.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/Makefile 2009-05-07 01:50:11.0 +0300 @@ -132,6 +132,7 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/ obj-$(CONFIG_VIDEO_CX18) += cx18/ obj-$(CONFIG_VIDEO_VIVI) += vivi.o +obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o obj-$(CONFIG_VIDEO_CX23885) += cx23885/ obj-$(CONFIG_VIDEO_OMAP2) += omap2cam.o diff -uprN v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c --- v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01 03:00:00.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c 2009-05-07 02:30:08.0 +0300 @@ -0,0 +1,775 @@ +/* + * v4l2loopback.c -- video 4 linux loopback driver + * + * Copyright (C) 2005-2009 + * Vasily Levin (vas...@gmail.com) + * + * 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. + * + */ +#include linux/version.h +#include linux/vmalloc.h +#include linux/mm.h +#include linux/time.h +#include linux/module.h +#include media/v4l2-ioctl.h +#include linux/videodev2.h +#include media/v4l2-common.h + +#define YAVLD_STREAMING + +MODULE_DESCRIPTION(V4L2 loopback video device); +MODULE_VERSION(0.1.1); +MODULE_AUTHOR(Vasily Levin); +MODULE_LICENSE(GPL); + +/* module structures */ +/* TODO(vasaka) use typenames which are common to kernel, but first find out if + * it is needed */ +/* struct keeping state and settings of loopback device */ +struct v4l2_loopback_device { + struct video_device *vdev; + /* pixel and stream format */ + struct v4l2_pix_format pix_format; + struct v4l2_captureparm capture_param; + /* buffers stuff */ + u8 *image; /* pointer to actual buffers data */ + int buffers_number; /* should not be big, 4 is a good choice */ + struct v4l2_buffer *buffers;/* inner driver buffers */ + int write_position; /* number of last written frame + 1 */ + long buffer_size; + /* sync stuff */ + atomic_t open_count; + int ready_for_capture;/* set to true when at least one writer opened + * device and negotiated format */ + wait_queue_head_t read_event; +}; + +/* types of opener shows what opener wants to do with loopback */ +enum opener_type { + UNNEGOTIATED = 0, + READER = 1, + WRITER = 2, +}; + +/* struct keeping state and type of opener */ +struct v4l2_loopback_opener { + enum opener_type type; + int buffers_number; + int position; /* number of last processed frame + 1 or + * write_position - 1 if reader went out of sync */ + struct v4l2_buffer *buffers; +}; + +/* module parameters */ +static int debug = 0; +module_param(debug, int, 0); +MODULE_PARM_DESC(debug,if debug output is enabled, values are 0, 1 or 2); + +static int max_buffers_number = 4; +module_param(max_buffers_number, int, 0); +MODULE_PARM_DESC(max_buffers_number,how many buffers should be allocated); + +static int max_openers = 10; +module_param(max_openers, int, 0); +MODULE_PARM_DESC(max_openers,how many users can open loopback device); + +/* module constants */ +#define MAX_MMAP_BUFFERS 100 /* max buffers that can be mmaped, actuale they +
Re: [REVIEW] v4l2 loopback
Hi Vasily, Your patch seems to be reversed, not a big deal for review purposes, of course. I think you know that if you are working on a hg clone you can simply issue hg diff to get the right patch, or you could even use 'quilt' to ease your work. Just very few comments about syntax and style, since I am not a v4l dev :) On Mon, 27 Apr 2009 04:22:58 +0300 Vasily vas...@gmail.com wrote: Hello Hans, Here is version with most issues fixed except usage of struct v4l2_device Can you please tell me more what should I use it for? I do not use any subdevice feature. It does not remove usage of video_device struct as I see from vivi driver it just used to be registered and unregistered and for messages, may be I missed something? So can you tell please what I should use it for in loopback driver? Just add it to v4l2_loopback_device structure and registe it? --- This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Priority: normal Signed-off-by: Vasily Levin vas...@gmail.com diff -uprN v4l-dvb.my.p/linux/drivers/media/video/Kconfig v4l-dvb.orig/linux/drivers/media/video/Kconfig --- v4l-dvb.my.p/linux/drivers/media/video/Kconfig2009-04-26 21:30:37.0 +0300 +++ v4l-dvb.orig/linux/drivers/media/video/Kconfig2009-04-25 04:41:20.0 +0300 @@ -479,13 +479,6 @@ config VIDEO_VIVI Say Y here if you want to test video apps or debug V4L devices. In doubt, say N. -config VIDEO_V4L2_LOOPBACK - tristate v4l2 loopback driver - depends on VIDEO_V4L2 VIDEO_DEV - help - Say Y if you want to use v4l2 loopback driver. - This driver can be compiled as a module, called v4l2loopback. - The description here could be improved, don't you think so? source drivers/media/video/bt8xx/Kconfig config VIDEO_PMS diff -uprN v4l-dvb.my.p/linux/drivers/media/video/Makefile v4l-dvb.orig/linux/drivers/media/video/Makefile --- v4l-dvb.my.p/linux/drivers/media/video/Makefile 2009-04-26 21:30:37.0 +0300 +++ v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-04-25 04:41:20.0 +0300 @@ -132,7 +132,6 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/ obj-$(CONFIG_VIDEO_CX18) += cx18/ obj-$(CONFIG_VIDEO_VIVI) += vivi.o -obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o obj-$(CONFIG_VIDEO_CX23885) += cx23885/ obj-$(CONFIG_VIDEO_MX1) += mx1_camera.o diff -uprN v4l-dvb.my.p/linux/drivers/media/video/v4l2loopback.c v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c --- v4l-dvb.my.p/linux/drivers/media/video/v4l2loopback.c 2009-04-27 03:07:08.0 +0300 +++ v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01 03:00:00.0 +0300 @@ -1,732 +0,0 @@ -/* - * v4l2loopback.c -- video 4 linux loopback driver - * - * Copyright (C) 2005-2009 - * Vasily Levin (vas...@gmail.com) - * - * 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. - * - */ Nitpicking here: just one space before the text? -#include linux/version.h -#include linux/vmalloc.h -#include linux/mm.h -#include linux/time.h -#include linux/module.h -#include media/v4l2-ioctl.h -#include v4l2loopback.h - -#define YAVLD_STREAMING - -MODULE_DESCRIPTION(V4L2 loopback video device); -MODULE_VERSION(0.1.1); -MODULE_AUTHOR(Vasily Levin); -MODULE_LICENSE(GPL); - GPL v2? I am not sure if this is of any importance. -/* module parameters */ -static int debug = 0; -module_param(debug, int, 0); -MODULE_PARM_DESC(debug,if debug output is enabled, values are 0, 1 or 2); - To do debug prints, these days, most kernel modules defines DEBUG at the top of the file (just when needed) and then use pr_debug() or better dev_dbg() into code. -static int max_buffers_number = 4; -module_param(max_buffers_number, int, 0); -MODULE_PARM_DESC(max_buffers_number,how many buffers should be allocated); - -static int max_openers = 10; -module_param(max_openers, int, 0); -MODULE_PARM_DESC(max_openers,how many users can open loopback device); - -#define dprintk(fmt, args...)\ - if (debug) {\ - printk(KERN_INFO v4l2-loopback: fmt, ##args);\ - } - - -#define dprintkrw(fmt, args...)\ - if (debug 1) {\ - printk(KERN_INFO v4l2-loopback: fmt, ##args);\ - } - If you choose to use dev_dbg() these two macros could be dropped, you will loose debug levels, but it is not a big loss IMHO. -/* global module data */ -struct v4l2_loopback_device *dev; -/* forward declarations */
Re: [REVIEW] v4l2 loopback
Hi Vasily, Your patch seems to be reversed, not a big deal for review purposes, of course. I think you know that if you are working on a hg clone you can simply issue hg diff to get the right patch, or you could even use 'quilt' to ease your work. Just very few comments about syntax and style, since I am not a v4l dev :) On Mon, 27 Apr 2009 04:22:58 +0300 Vasily vas...@gmail.com wrote: Hello Hans, Here is version with most issues fixed except usage of struct v4l2_device Can you please tell me more what should I use it for? I do not use any subdevice feature. It does not remove usage of video_device struct as I see from vivi driver it just used to be registered and unregistered and for messages, may be I missed something? So can you tell please what I should use it for in loopback driver? Just add it to v4l2_loopback_device structure and registe it? --- This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Priority: normal Signed-off-by: Vasily Levin vas...@gmail.com diff -uprN v4l-dvb.my.p/linux/drivers/media/video/Kconfig v4l-dvb.orig/linux/drivers/media/video/Kconfig --- v4l-dvb.my.p/linux/drivers/media/video/Kconfig 2009-04-26 21:30:37.0 +0300 +++ v4l-dvb.orig/linux/drivers/media/video/Kconfig 2009-04-25 04:41:20.0 +0300 @@ -479,13 +479,6 @@ config VIDEO_VIVI Say Y here if you want to test video apps or debug V4L devices. In doubt, say N. -config VIDEO_V4L2_LOOPBACK -tristate v4l2 loopback driver -depends on VIDEO_V4L2 VIDEO_DEV -help - Say Y if you want to use v4l2 loopback driver. - This driver can be compiled as a module, called v4l2loopback. - The description here could be improved, don't you think so? source drivers/media/video/bt8xx/Kconfig config VIDEO_PMS diff -uprN v4l-dvb.my.p/linux/drivers/media/video/Makefile v4l-dvb.orig/linux/drivers/media/video/Makefile --- v4l-dvb.my.p/linux/drivers/media/video/Makefile 2009-04-26 21:30:37.0 +0300 +++ v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-04-25 04:41:20.0 +0300 @@ -132,7 +132,6 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/ obj-$(CONFIG_VIDEO_CX18) += cx18/ obj-$(CONFIG_VIDEO_VIVI) += vivi.o -obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o obj-$(CONFIG_VIDEO_CX23885) += cx23885/ obj-$(CONFIG_VIDEO_MX1) += mx1_camera.o diff -uprN v4l-dvb.my.p/linux/drivers/media/video/v4l2loopback.c v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c --- v4l-dvb.my.p/linux/drivers/media/video/v4l2loopback.c2009-04-27 03:07:08.0 +0300 +++ v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c1970-01-01 03:00:00.0 +0300 @@ -1,732 +0,0 @@ -/* - * v4l2loopback.c -- video 4 linux loopback driver - * - * Copyright (C) 2005-2009 - * Vasily Levin (vas...@gmail.com) - * - * 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. - * - */ Nitpicking here: just one space before the text? -#include linux/version.h -#include linux/vmalloc.h -#include linux/mm.h -#include linux/time.h -#include linux/module.h -#include media/v4l2-ioctl.h -#include v4l2loopback.h - -#define YAVLD_STREAMING - -MODULE_DESCRIPTION(V4L2 loopback video device); -MODULE_VERSION(0.1.1); -MODULE_AUTHOR(Vasily Levin); -MODULE_LICENSE(GPL); - GPL v2? I am not sure if this is of any importance. -/* module parameters */ -static int debug = 0; -module_param(debug, int, 0); -MODULE_PARM_DESC(debug,if debug output is enabled, values are 0, 1 or 2); - To do debug prints, these days, most kernel modules defines DEBUG at the top of the file (just when needed) and then use pr_debug() or better dev_dbg() into code. Actually, I prefer a debug module parameter. That way you can enable debugging without recompiling. Given the complexity of video drivers that is often very desirable. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW] v4l2 loopback
On Tue, 14 Apr 2009 16:04:50 +0200 Antonio Ospite osp...@studenti.unina.it wrote: On Tue, 14 Apr 2009 15:53:00 +0300 vas...@gmail.com wrote: On Tue, Apr 14, 2009 at 3:12 PM, Mauro Carvalho Chehab mche...@infradead.org wrote: The issue I see is that the V4L drivers are meant to support real devices. This driver that is a loopback for some userspace driver. I don't discuss its value for testing purposes or other random usage, but I can't see why this should be at upstream kernel. So, I'm considering to add it at v4l-dvb tree, but as an out-of-tree driver only. For this to happen, probably, we'll need a few adjustments at v4l build. Cheers, Mauro Mauro, ok, let it be out-of -tree driver, this is also good as I do not have to adapt the driver to each new kernel, but I want to argue alittle about Inclusion of the driver into upstream kernel. Main reason for inclusion to the kernel is ease of use, as I understand installing the out-of-tree driver for some kernel needs downloading of the whole v4l-dvb tree(am I right?). Loopback gives one opportunities to do many fun things with video streams and when it needs just one step to begin using it chances that someone will do something useful with the driver are higher. I, as a target user of vloopback, agree that having it in mainline would be really handy. Think that with a stable vloopback solution, with device detection and parameter setting, we can really make PTP digicams as webcams[1] useful, right now this is tricky and very uncomfortable on kernel update. This is, in fact, a good reason why we shouldn't add it upstream: instead of adding proper V4L interface to PTP and other similar stuff, people could just do some userspace hack with a in-kernel loopback (or even worse: work against Open Source community, by writing binary-only drivers), and use the loopback to make it work with existing applications (ok, there are other forms to provide such things, but we shouldn't make it even easier). I can see the value of a video loopback for development and tests, but those people could easily download some tree with the video loopback driver and use it. Awareness that there is such thing as loopback is also. If the driver is in upstream tree - more people will see it and more chances that more people will participate in loopback getiing better. I'm afraid not. The contributions we generally receive on other drivers from developers that don't participate on v4l-dvb community are generally just API fixups and new board additions. In fact, the people that can help with this driver will be already developing using v4l-dvb tree, so, I doubt you'll have more contributions by having it on kernel. vivi is an upstream driver :-) Even vivi can be seen as a particular case of a vloopback device, can't it? Vivi is just a driver skeleton. It could eventually be removed from upstream, without any real damage. Yet, it is the easiest way for a video app developer to test their driver. Also, vivi is very useful to test newer core improvements, before actually damag^Wchanging the internal API's at the real drivers. I used it with this objective during video_ioctl2() callback changes, during videobuf split into a core and a helper module, and on other similar situations. On the other hand, It is dubious that a distro would provide a kernel with this module enabled. So, even being at the kernel tree, for you to use it, you'll need to download the kernel and compile it by hand, or use v4l-dvb (it is the same case of the DVB dummy frontend, for example). Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW] v4l2 loopback
On Tue, 14 Apr 2009 04:08:41 +0300 vas...@gmail.com wrote: On Mon, Apr 13, 2009 at 2:17 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Thursday 26 March 2009 19:49:10 Vasily wrote: Hello, please review the new version of v4l2 loopback driver. I fixed up comments to the previous submission, waiting for the new ones :-), reposting for patchwork tool --- This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Hi Vasily, It's still on my todo list, but I have had very little time. If you still haven't seen a review in two weeks time, then please send me a reminder. Sorry about this, normally I review things like this much sooner but it has been very hectic lately. Regards, Hans Hi Hans, Thanks for updating, driver is still waiting for review, I am glad to here that it is on a todo list :-) Vasily, It is on my todo list. I've postponed it since it is valuable to have some discussions about this driver. The issue I see is that the V4L drivers are meant to support real devices. This driver that is a loopback for some userspace driver. I don't discuss its value for testing purposes or other random usage, but I can't see why this should be at upstream kernel. So, I'm considering to add it at v4l-dvb tree, but as an out-of-tree driver only. For this to happen, probably, we'll need a few adjustments at v4l build. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW] v4l2 loopback
On Tue, Apr 14, 2009 at 3:12 PM, Mauro Carvalho Chehab mche...@infradead.org wrote: On Tue, 14 Apr 2009 04:08:41 +0300 vas...@gmail.com wrote: On Mon, Apr 13, 2009 at 2:17 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Thursday 26 March 2009 19:49:10 Vasily wrote: Hello, please review the new version of v4l2 loopback driver. I fixed up comments to the previous submission, waiting for the new ones :-), reposting for patchwork tool --- This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Hi Vasily, It's still on my todo list, but I have had very little time. If you still haven't seen a review in two weeks time, then please send me a reminder. Sorry about this, normally I review things like this much sooner but it has been very hectic lately. Regards, Hans Hi Hans, Thanks for updating, driver is still waiting for review, I am glad to here that it is on a todo list :-) Vasily, It is on my todo list. I've postponed it since it is valuable to have some discussions about this driver. The issue I see is that the V4L drivers are meant to support real devices. This driver that is a loopback for some userspace driver. I don't discuss its value for testing purposes or other random usage, but I can't see why this should be at upstream kernel. So, I'm considering to add it at v4l-dvb tree, but as an out-of-tree driver only. For this to happen, probably, we'll need a few adjustments at v4l build. Cheers, Mauro Mauro, ok, let it be out-of -tree driver, this is also good as I do not have to adapt the driver to each new kernel, but I want to argue alittle about Inclusion of the driver into upstream kernel. Main reason for inclusion to the kernel is ease of use, as I understand installing the out-of-tree driver for some kernel needs downloading of the whole v4l-dvb tree(am I right?). Loopback gives one opportunities to do many fun things with video streams and when it needs just one step to begin using it chances that someone will do something useful with the driver are higher. Awareness that there is such thing as loopback is also. If the driver is in upstream tree - more people will see it and more chances that more people will participate in loopback getiing better. vivi is an upstream driver :-) Vasily -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW] v4l2 loopback
On Tue, 14 Apr 2009 15:53:00 +0300 vas...@gmail.com wrote: On Tue, Apr 14, 2009 at 3:12 PM, Mauro Carvalho Chehab mche...@infradead.org wrote: The issue I see is that the V4L drivers are meant to support real devices. This driver that is a loopback for some userspace driver. I don't discuss its value for testing purposes or other random usage, but I can't see why this should be at upstream kernel. So, I'm considering to add it at v4l-dvb tree, but as an out-of-tree driver only. For this to happen, probably, we'll need a few adjustments at v4l build. Cheers, Mauro Mauro, ok, let it be out-of -tree driver, this is also good as I do not have to adapt the driver to each new kernel, but I want to argue alittle about Inclusion of the driver into upstream kernel. Main reason for inclusion to the kernel is ease of use, as I understand installing the out-of-tree driver for some kernel needs downloading of the whole v4l-dvb tree(am I right?). Loopback gives one opportunities to do many fun things with video streams and when it needs just one step to begin using it chances that someone will do something useful with the driver are higher. I, as a target user of vloopback, agree that having it in mainline would be really handy. Think that with a stable vloopback solution, with device detection and parameter setting, we can really make PTP digicams as webcams[1] useful, right now this is tricky and very uncomfortable on kernel update. Awareness that there is such thing as loopback is also. If the driver is in upstream tree - more people will see it and more chances that more people will participate in loopback getiing better. vivi is an upstream driver :-) Even vivi can be seen as a particular case of a vloopback device, can't it? Regards, Antonio [1] http://shell.studenti.unina.it/~ospite/section/it/dev/canoncam.html#English -- A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? Web site: http://www.studenti.unina.it/~ospite Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc pgpEeET9DTixU.pgp Description: PGP signature
Re: [REVIEW] v4l2 loopback
On Thursday 26 March 2009 19:49:10 Vasily wrote: Hello, please review the new version of v4l2 loopback driver. I fixed up comments to the previous submission, waiting for the new ones :-), reposting for patchwork tool --- This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Hi Vasily, It's still on my todo list, but I have had very little time. If you still haven't seen a review in two weeks time, then please send me a reminder. Sorry about this, normally I review things like this much sooner but it has been very hectic lately. Regards, Hans Priority: normal Signed-off-by: Vasily Levin vas...@gmail.com diff -uprN v4l-dvb.orig/linux/drivers/media/video/Kconfig v4l-dvb.my/linux/drivers/media/video/Kconfig --- v4l-dvb.orig/linux/drivers/media/video/Kconfig2009-03-21 07:08:06.0 +0200 +++ v4l-dvb.my/linux/drivers/media/video/Kconfig 2009-03-25 23:36:13.0 +0200 @@ -465,6 +465,13 @@ config VIDEO_VIVI Say Y here if you want to test video apps or debug V4L devices. In doubt, say N. +config VIDEO_V4L2_LOOPBACK + tristate v4l2 loopback driver + depends on VIDEO_V4L2 VIDEO_DEV + help + Say Y if you want to use v4l2 loopback driver. + This driver can be compiled as a module, called v4l2loopback. + source drivers/media/video/bt8xx/Kconfig config VIDEO_SAA6588 diff -uprN v4l-dvb.orig/linux/drivers/media/video/Makefile v4l-dvb.my/linux/drivers/media/video/Makefile --- v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-03-21 07:08:06.0 +0200 +++ v4l-dvb.my/linux/drivers/media/video/Makefile 2009-03-24 12:54:59.0 +0200 @@ -132,6 +132,7 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/ obj-$(CONFIG_VIDEO_CX18) += cx18/ obj-$(CONFIG_VIDEO_VIVI) += vivi.o +obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o obj-$(CONFIG_VIDEO_CX23885) += cx23885/ obj-$(CONFIG_VIDEO_MX3) += mx3_camera.o diff -uprN v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c --- v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01 03:00:00.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c 2009-03-25 23:01:35.0 +0200 @@ -0,0 +1,725 @@ +/* + * v4l2loopback.c -- video 4 linux loopback driver + * + * Copyright (C) 2005-2009 + * Vasily Levin (vas...@gmail.com) + * + * 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. + * + */ +#include linux/version.h +#include linux/vmalloc.h +#include linux/mm.h +#include linux/time.h +#include linux/module.h +#include media/v4l2-ioctl.h +#include v4l2loopback.h + +/* #define DEBUG +#define DEBUG_RW */ +#define YAVLD_STREAMING + +#ifdef DEBUG +#define dprintk(fmt, args...)\ + do {\ + printk(KERN_INFO v4l2-loopback: fmt, ##args);\ + } while (0) +#else +#define dprintk(fmt, args...) +#endif + +#ifdef DEBUG_RW +#define dprintkrw(fmt, args...)\ + do {\ + printk(KERN_INFO v4l2-loopback: fmt, ##args);\ + } while (0) +#else +#define dprintkrw(fmt, args...) +#endif + +/* global module data */ +struct v4l2_loopback_device *dev; +/* forward declarations */ +static void init_buffers(int buffer_size); +static const struct v4l2_file_operations v4l2_loopback_fops; +static const struct v4l2_ioctl_ops v4l2_loopback_ioctl_ops; +/ + my queue helpers *** +/ +/* next functions sets buffer flags and adjusts counters accordingly */ +static void set_done(struct v4l2_buffer *buffer) +{ + buffer-flags |= V4L2_BUF_FLAG_DONE; + buffer-flags = ~V4L2_BUF_FLAG_QUEUED; +} + +static void set_queued(struct v4l2_buffer *buffer) +{ + buffer-flags |= V4L2_BUF_FLAG_QUEUED; + buffer-flags = ~V4L2_BUF_FLAG_DONE; +} + +static void unset_all(struct v4l2_buffer *buffer) +{ + buffer-flags = ~V4L2_BUF_FLAG_QUEUED; + buffer-flags = ~V4L2_BUF_FLAG_DONE; +} +/ + V4L2 ioctl caps and params calls *** +/ +/*** ***/ +/* returns device capabilities, called on VIDIOC_QUERYCAP ioctl*/ +static int
Re: [REVIEW] v4l2 loopback
On Mon, Apr 13, 2009 at 2:17 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Thursday 26 March 2009 19:49:10 Vasily wrote: Hello, please review the new version of v4l2 loopback driver. I fixed up comments to the previous submission, waiting for the new ones :-), reposting for patchwork tool --- This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Hi Vasily, It's still on my todo list, but I have had very little time. If you still haven't seen a review in two weeks time, then please send me a reminder. Sorry about this, normally I review things like this much sooner but it has been very hectic lately. Regards, Hans Hi Hans, Thanks for updating, driver is still waiting for review, I am glad to here that it is on a todo list :-) Vasily -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW] v4l2 loopback
On Wed, Mar 25, 2009 at 1:07 AM, Alexey Klimov klimov.li...@gmail.com wrote: Hello, Vasily On Tue, 2009-03-24 at 21:27 +0200, vas...@gmail.com wrote: Hello, please review new version of v4l2 loopback driver. driver works fine but there are things which I am not shure in. Is it ok not to count mmaped buffers and just free memory when no open file descriptors left? Here is patch against current v4l-dvb tree - This patch introduces v4l2 loopback module From: Vasily Levin vas...@gmail.com This is v4l2 loopback driver which can be used to make available any userspace video as v4l2 device. Initialy it was written to make videoeffects available to Skype, but in fact it have many more uses. Priority: normal Signed-off-by: Vasily Levin vas...@gmail.com diff -uprN v4l-dvb.orig/linux/drivers/media/video/Kconfig v4l-dvb.my/linux/drivers/media/video/Kconfig --- v4l-dvb.orig/linux/drivers/media/video/Kconfig 2009-03-21 07:08:06.0 +0200 +++ v4l-dvb.my/linux/drivers/media/video/Kconfig 2009-03-24 12:58:38.0 +0200 @@ -465,6 +465,13 @@ config VIDEO_VIVI Say Y here if you want to test video apps or debug V4L devices. In doubt, say N. +config VIDEO_V4L2_LOOPBACK + tristate v4l2 loopback driver + depends on VIDEO_V4L2 VIDEO_DEV + help + Say Y if you want to use v4l2 loopback driver. + This driver can be compiled as a module, called v4l2loopback. + source drivers/media/video/bt8xx/Kconfig config VIDEO_SAA6588 @@ -899,7 +906,7 @@ config USB_S2255 depends on VIDEO_V4L2 select VIDEOBUF_VMALLOC default n - help + ---help--- As i understand this change in not part of the patch.. Say Y here if you want support for the Sensoray 2255 USB device. This driver can be compiled as a module, called s2255drv. diff -uprN v4l-dvb.orig/linux/drivers/media/video/Makefile v4l-dvb.my/linux/drivers/media/video/Makefile --- v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-03-21 07:08:06.0 +0200 +++ v4l-dvb.my/linux/drivers/media/video/Makefile 2009-03-24 12:54:59.0 +0200 @@ -132,6 +132,7 @@ obj-$(CONFIG_VIDEO_IVTV) += ivtv/ obj-$(CONFIG_VIDEO_CX18) += cx18/ obj-$(CONFIG_VIDEO_VIVI) += vivi.o +obj-$(CONFIG_VIDEO_V4L2_LOOPBACK) += v4l2loopback.o obj-$(CONFIG_VIDEO_CX23885) += cx23885/ obj-$(CONFIG_VIDEO_MX3) += mx3_camera.o diff -uprN v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c --- v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01 03:00:00.0 +0300 +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c 2009-03-24 18:51:41.0 +0200 @@ -0,0 +1,726 @@ +/* + * v4l2loopback.c -- video 4 linux loopback driver + * + * Copyright (C) 2005-2009 + * Vasily Levin (vas...@gmail.com) + * + * 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. + * + */ +#include linux/version.h +#include linux/vmalloc.h +#include linux/mm.h +#include linux/time.h +#include linux/module.h +#include media/v4l2-ioctl.h +#include v4l2loopback.h + +#define DEBUG +#define DEBUG_RW +#define YAVLD_STREAMING +#define KERNEL_PREFIX YAVLD device: /* Prefix of each kernel message */ +/* global module data */ +struct v4l2_loopback_device *dev; +/* forward declarations */ +static void init_buffers(int buffer_size); +static const struct v4l2_file_operations v4l2_loopback_fops; +static const struct v4l2_ioctl_ops v4l2_loopback_ioctl_ops; +/ + my queue helpers *** +/ +/* next functions sets buffer flags and adjusts counters accordingly */ +void set_done(struct v4l2_buffer *buffer) +{ + buffer-flags |= V4L2_BUF_FLAG_DONE; + buffer-flags = ~V4L2_BUF_FLAG_QUEUED; +} + +void set_queued(struct v4l2_buffer *buffer) +{ + buffer-flags |= V4L2_BUF_FLAG_QUEUED; + buffer-flags = ~V4L2_BUF_FLAG_DONE; +} + +void unset_all(struct v4l2_buffer *buffer) +{ + buffer-flags = ~V4L2_BUF_FLAG_QUEUED; + buffer-flags = ~V4L2_BUF_FLAG_DONE; +} Can this functions be static and inlined ? +/ + V4L2 ioctl caps and params calls *** +/ +/**/ +/* returns device capabilities, called on VIDIOC_QUERYCAP ioctl*/ +static int vidioc_querycap(struct file *file, +