> 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 <[email protected]> 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 <[email protected]>
>>
>> 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 <[email protected]>
>>
>> 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.000000000 +0300
>> +++ v4l-dvb.orig/linux/drivers/media/video/Kconfig 2009-04-25
>> 04:41:20.000000000 +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.000000000 +0300
>> +++ v4l-dvb.orig/linux/drivers/media/video/Makefile 2009-04-25
>> 04:41:20.000000000 +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.000000000 +0300
>> +++ v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.c 1970-01-01
>> 03:00:00.000000000 +0300
>> @@ -1,732 +0,0 @@
>> -/*
>> - * v4l2loopback.c -- video 4 linux loopback driver
>> - *
>> - * Copyright (C) 2005-2009
>> - * Vasily Levin ([email protected])
>> - *
>> - * 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
