Re: [REVIEW] v4l2 loopback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 25, 2009 at 1:07 AM, Alexey Klimov <klimov.linux@xxxxxxxxx> wrote:
> Hello, Vasily
>
> On Tue, 2009-03-24 at 21:27 +0200, vasaka@xxxxxxxxx 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 <vasaka@xxxxxxxxx>
>>
>> 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 <vasaka@xxxxxxxxx>
>>
>> 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.000000000 +0200
>> +++ v4l-dvb.my/linux/drivers/media/video/Kconfig      2009-03-24
>> 12:58:38.000000000 +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.000000000 +0200
>> +++ v4l-dvb.my/linux/drivers/media/video/Makefile     2009-03-24
>> 12:54:59.000000000 +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.000000000 +0300
>> +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.c       2009-03-24
>> 18:51:41.000000000 +0200
>> @@ -0,0 +1,726 @@
>> +/*
>> + *      v4l2loopback.c  --  video 4 linux loopback driver
>> + *
>> + *      Copyright (C) 2005-2009
>> + *          Vasily Levin (vasaka@xxxxxxxxx)
>> + *
>> + *      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,
>> +                        void *priv, struct v4l2_capability *cap)
>> +{
>> +     strcpy(cap->driver, "v4l2 loopback");
>> +     strcpy(cap->card, "Dummy video device");
>> +     cap->version = 1;
>> +     cap->capabilities =
>> +         V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT |
>> +         V4L2_CAP_READWRITE
>> +#ifdef YAVLD_STREAMING
>> +         | V4L2_CAP_STREAMING
>> +#endif
>> +         ;
>> +     return 0;
>> +}
>> +
>> +/******************************************************************************/
>> +/* returns device formats, called on VIDIOC_ENUM_FMT ioctl*/
>> +static int vidioc_enum_fmt_cap(struct file *file, void *fh,
>> +                            struct v4l2_fmtdesc *f)
>> +{
>> +     if (dev->ready_for_capture == 0)
>> +             return -EINVAL;
>> +     if (f->index)
>> +             return -EINVAL;
>> +     strcpy(f->description, "current format");
>> +     f->pixelformat = dev->pix_format.pixelformat;
>> +     return 0;
>> +};
>> +
>> +/******************************************************************************/
>> +/* returns current video format format fmt, called on VIDIOC_G_FMT ioctl */
>> +static int vidioc_g_fmt_cap(struct file *file,
>> +                         void *priv, struct v4l2_format *fmt)
>> +{
>> +     if (dev->ready_for_capture == 0)
>> +             return -EINVAL;
>> +     fmt->fmt.pix = dev->pix_format;
>> +     return 0;
>> +}
>> +
>> +/******************************************************************************/
>> +/* checks if it is OK to change to format fmt, called on VIDIOC_TRY_FMT ioctl
>> + * with v4l2_buf_type set to V4L2_BUF_TYPE_VIDEO_CAPTURE */
>> +/* actual check is done by inner_try_fmt_cap */
>> +/* just checking that pixelformat is OK and set other parameters, app should
>> + * obey this decidion */
>> +static int vidioc_try_fmt_cap(struct file *file,
>> +                           void *priv, struct v4l2_format *fmt)
>> +{
>> +     struct v4l2_loopback_opener *opener = file->private_data;
>> +     opener->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> +     if (dev->ready_for_capture == 0)
>> +             return -EINVAL;
>> +     if (fmt->fmt.pix.pixelformat != dev->pix_format.pixelformat)
>> +             return -EINVAL;
>> +     fmt->fmt.pix = dev->pix_format;
>> +     return 0;
>> +}
>> +
>> +/******************************************************************************/
>> +/* checks if it is OK to change to format fmt, called on VIDIOC_TRY_FMT ioctl
>> + * with v4l2_buf_type set to V4L2_BUF_TYPE_VIDEO_OUTPUT */
>> +/* if format is negotiated do not change it */
>> +static int vidioc_try_fmt_video_output(struct file *file,
>> +                                    void *priv, struct v4l2_format *fmt)
>> +{
>> +     struct v4l2_loopback_opener *opener = file->private_data;
>> +     opener->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>> +     /* TODO(vasaka) loopback does not care about formats writer want to set,
>> +      * maybe it is a good idea to restrict format somehow */
>> +     if (dev->ready_for_capture) {
>> +             fmt->fmt.pix = dev->pix_format;
>> +     } else {
>> +             if (fmt->fmt.pix.sizeimage == 0)
>> +                     return -1;
>> +             dev->pix_format = fmt->fmt.pix;
>> +     }
>> +     return 0;
>> +};
>> +
>> +/******************************************************************************/
>> +/* sets new output format, if possible, called on VIDIOC_S_FMT ioctl
>> + * with v4l2_buf_type set to V4L2_BUF_TYPE_VIDEO_CAPTURE */
>> +/* actually format is set  by input and we even do not check it, just return
>> + * current one, but it is possible to set subregions of input TODO(vasaka) */
>> +static int vidioc_s_fmt_cap(struct file *file,
>> +                         void *priv, struct v4l2_format *fmt)
>> +{
>> +     return vidioc_try_fmt_cap(file, priv, fmt);
>> +}
>> +
>> +/******************************************************************************/
>> +/* sets new output format, if possible, called on VIDIOC_S_FMT ioctl
>> + * with v4l2_buf_type set to V4L2_BUF_TYPE_VIDEO_OUTPUT */
>> +/* allocate data here because we do not know if it will be streaming or
>> + * read/write IO */
>> +static int vidioc_s_fmt_video_output(struct file *file,
>> +                                  void *priv, struct v4l2_format *fmt)
>> +{
>> +     vidioc_try_fmt_video_output(file, priv, fmt);
>
> If i'm not wrong this function returns int. Is it better for good
> programming practice to check returned value ?
>
>> +     if (dev->ready_for_capture == 0) {
>> +             dev->buffer_size = PAGE_ALIGN(dev->pix_format.sizeimage);
>> +             fmt->fmt.pix.sizeimage = dev->buffer_size;
>> +             /* vfree on close file operation in case no open handles left */
>> +             dev->image =
>> +                 vmalloc(dev->buffer_size * dev->buffers_number);
>> +             if (dev->image == NULL)
>> +                     return -EINVAL;
>> +#ifdef DEBUG
>> +             printk(KERNEL_PREFIX "vmallocated %ld bytes\n",
>> +                    dev->buffer_size * dev->buffers_number);
>> +#endif
>
> I saw that usually another approach is used in such cases.
>
>
> They defined macros, something like this:
> #define dprintk(fmt, args...) if (debug)        \
>                do {                            \
>                        printk(KERN_INFO "v4l2-loopback: " fmt, ##args); \
>                } while (0)
>
> And debug can be module parameter or dprintk-macros can be surrounded by
> #ifdef DEBUG and #else..
>
> Probably you can use one variant intstead of many ifdefines in code.
>
>
>> +             init_buffers(dev->buffer_size);
>> +             dev->ready_for_capture = 1;
>> +     }
>> +     return 0;
>> +}
>> +
>> +/******************************************************************************/
>> +/*get some data flaw parameters, only capability, fps and readbuffers
>> has effect
>> + *on this driver, called on VIDIOC_G_PARM*/
>> +static int vidioc_g_parm(struct file *file, void *priv,
>> +                      struct v4l2_streamparm *parm)
>> +{
>> +     /* do not care about type of opener, hope this enums would always be
>> +      * compatible */
>> +     parm->parm.capture = dev->capture_param;
>> +     return 0;
>> +}
>> +
>> +/******************************************************************************/
>> +/*get some data flaw parameters, only capability, fps and readbuffers
>> has effect
>> + *on this driver, called on VIDIOC_S_PARM */
>> +static int vidioc_s_parm(struct file *file, void *priv,
>> +                      struct v4l2_streamparm *parm)
>> +{
>> +#ifdef DEBUG
>> +     printk(KERNEL_PREFIX "vidioc_s_parm called frate=%d/%d\n",
>> +            parm->parm.capture.timeperframe.numerator,
>> +            parm->parm.capture.timeperframe.denominator);
>> +#endif
>> +     switch (parm->type) {
>> +     case V4L2_BUF_TYPE_VIDEO_CAPTURE:{
>> +                     parm->parm.capture = dev->capture_param;
>> +                     return 0;
>> +             }
>> +     case V4L2_BUF_TYPE_VIDEO_OUTPUT:{
>> +                     /* TODO(vasaka) do nothing now, but should set fps if
>> +                      * needed */
>> +                     parm->parm.capture = dev->capture_param;
>> +                     return 0;
>> +             }
>> +     default:{
>> +                     return -1;
>> +             }
>> +     }
>> +}
>> +
>> +/* sets a tv standart, actually we do not need to handle this any spetial way
>> + *added to support effecttv */
>> +static int vidioc_s_std(struct file *file, void *private_data,
>> +                     v4l2_std_id *norm)
>> +{
>> +     return 0;
>> +}
>
> May be inline ?
>
>> +
>> +/* returns set of device inputs, in our case there is only one, but later I may
>> + * add more, called on VIDIOC_ENUMINPUT */
>> +static int vidioc_enum_input(struct file *file, void *fh,
>> +                          struct v4l2_input *inp)
>> +{
>> +     if (dev->ready_for_capture == 0)
>> +             return -EINVAL;
>> +     if (inp->index == 0) {
>> +             strcpy(inp->name, "loopback");
>> +             inp->type = V4L2_INPUT_TYPE_CAMERA;
>> +             inp->audioset = 0;
>> +             inp->tuner = 0;
>> +             inp->std = V4L2_STD_PAL_B;
>> +             inp->status = 0;
>> +             return 0;
>> +     }
>> +     return -EINVAL;
>> +}
>> +
>> +/* which input is currently active, called on VIDIOC_G_INPUT */
>> +int vidioc_g_input(struct file *file, void *fh, unsigned int *i)
>> +{
>> +     if (dev->ready_for_capture == 0)
>> +             return -EINVAL;
>> +     *i = 0;
>> +     return 0;
>> +}
>> +
>> +/* set input, can make sense if we have more than one video src,
>> + * called on VIDIOC_S_INPUT */
>> +int vidioc_s_input(struct file *file, void *fh, unsigned int i)
>> +{
>> +     if (dev->ready_for_capture == 0)
>> +             return -EINVAL;
>> +     if (i == 0)
>> +             return 0;
>> +     return -EINVAL;
>> +}
>> +
>> +/***************************************************************
>> +**************** V4L2 ioctl buffer related calls ***************
>> +***************************************************************/
>> +/* negotiate buffer type, called on VIDIOC_REQBUFS */
>> +/* only mmap streaming supported */
>> +static int vidioc_reqbufs(struct file *file, void *fh,
>> +                       struct v4l2_requestbuffers *b)
>> +{
>> +     switch (b->memory) {
>> +     case V4L2_MEMORY_MMAP:{
>> +                     /* do nothing here, buffers are always allocated*/
>> +                     if (b->count == 0)
>> +                             return 0;
>> +                     b->count = dev->buffers_number;
>> +                     return 0;
>> +             }
>> +     default:{
>> +                     return -EINVAL;
>> +             }
>> +     }
>> +}
>> +
>> +/* returns buffer asked for, called on VIDIOC_QUERYBUF */
>> +/* give app as many buffers as it wants, if it less than 100 :-),
>> + * but map them in our inner buffers */
>> +static int vidioc_querybuf(struct file *file, void *fh,
>> +                        struct v4l2_buffer *b)
>> +{
>> +     enum v4l2_buf_type type = b->type;
>> +     int index = b->index;
>> +     if ((b->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
>> +         (b->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) {
>> +             return -EINVAL;
>> +     }
>> +     if (b->index > 100)
>> +             return -EINVAL;
>> +     *b = dev->buffers[b->index % dev->buffers_number];
>> +     b->type = type;
>> +     b->index = index;
>> +     return 0;
>> +}
>> +
>> +/* put buffer to queue, called on VIDIOC_QBUF */
>> +static int vidioc_qbuf(struct file *file, void *private_data,
>> +                    struct v4l2_buffer *buf)
>> +{
>> +     int index = buf->index % dev->buffers_number;
>> +     if (buf->index > 100)
>> +             return -EINVAL;
>> +     switch (buf->type) {
>> +     case V4L2_BUF_TYPE_VIDEO_CAPTURE:{
>> +                     set_queued(&dev->buffers[index]);
>> +                     return 0;
>> +             }
>> +     case V4L2_BUF_TYPE_VIDEO_OUTPUT:{
>> +                     do_gettimeofday(&dev->buffers[index].timestamp);
>> +                     set_done(&dev->buffers[index]);
>> +                     wake_up_all(&dev->read_event);
>> +                     return 0;
>> +             }
>> +     default:{
>> +                     return -EINVAL;
>> +             }
>> +     }
>> +}
>> +
>> +/* put buffer to dequeue, called on VIDIOC_DQBUF */
>> +static int vidioc_dqbuf(struct file *file, void *private_data,
>> +                     struct v4l2_buffer *buf)
>> +{
>> +     int index;
>> +     struct v4l2_loopback_opener *opener = file->private_data;
>> +     switch (buf->type) {
>> +     case V4L2_BUF_TYPE_VIDEO_CAPTURE:{
>> +                     if ((dev->write_position <= opener->position) &&
>> +                             (file->f_flags&O_NONBLOCK))
>> +                             return -EAGAIN;
>> +                     wait_event_interruptible(dev->read_event,
>> +                                              (dev->write_position >
>> +                                              opener->position));
>> +                     if (dev->write_position > opener->position+2)
>> +                             opener->position = dev->write_position - 1;
>> +                     index = opener->position % dev->buffers_number;
>> +                     if (!(dev->buffers[index].flags&V4L2_BUF_FLAG_MAPPED)) {
>> +                             printk(KERN_INFO
>> +                                    "trying to g\return not mapped buf\n");
>
> I don't know how often this module will be used, but don't you want to
> add module name to this message ?
>
>> +                             return -EINVAL;
>> +                     }
>> +                     ++opener->position;
>> +                     unset_all(&dev->buffers[index]);
>> +                     *buf = dev->buffers[index];
>> +                     return 0;
>> +             }
>> +     case V4L2_BUF_TYPE_VIDEO_OUTPUT:{
>> +                     index = dev->write_position % dev->buffers_number;
>> +                     unset_all(&dev->buffers[index]);
>> +                     *buf = dev->buffers[index];
>> +                     ++dev->write_position;
>> +                     return 0;
>> +             }
>> +     default:{
>> +                     return -EINVAL;
>> +             }
>> +     }
>> +}
>> +
>> +static int vidioc_streamon(struct file *file, void *private_data,
>> +                        enum v4l2_buf_type type)
>> +{
>> +     return 0;
>> +}
>> +
>> +static int vidioc_streamoff(struct file *file, void *private_data,
>> +                         enum v4l2_buf_type type)
>> +{
>> +     return 0;
>> +}
>> +
>> +#ifdef CONFIG_VIDEO_V4L1_COMPAT
>> +int vidiocgmbuf(struct file *file, void *fh, struct video_mbuf *p)
>> +{
>> +     p->frames = dev->buffers_number;
>> +     p->offsets[0] = 0;
>> +     p->offsets[1] = 0;
>> +     p->size = dev->buffer_size;
>> +     return 0;
>> +}
>> +#endif
>> +/************************************************
>> +**************** file operations  ***************
>> +************************************************/
>> +static void vm_open(struct vm_area_struct *vma)
>> +{
>> +     /* TODO(vasaka) do open counter here */
>> +}
>> +
>> +static void vm_close(struct vm_area_struct *vma)
>> +{
>> +     /* TODO(vasaka) do open counter here */
>> +}
>> +
>> +static struct vm_operations_struct vm_ops = {
>> +     .open = vm_open,
>> +     .close = vm_close,
>> +};
>> +
>> +static int v4l2_loopback_mmap(struct file *file,
>> +                           struct vm_area_struct *vma)
>> +{
>> +
>> +     struct page *page = NULL;
>> +
>> +     unsigned long addr;
>> +     unsigned long start = (unsigned long) vma->vm_start;
>> +     unsigned long size = (unsigned long) (vma->vm_end - vma->vm_start);
>> +
>> +#ifdef DEBUG
>> +     printk(KERNEL_PREFIX "entering v4l_mmap(), offset: %lu\n",
>> +            vma->vm_pgoff);
>> +#endif
>> +     if (size > dev->buffer_size) {
>> +             printk(KERNEL_PREFIX
>> +                    "userspace tries to mmap to much, fail\n");
>> +             return -EINVAL;
>> +     }
>> +     if ((vma->vm_pgoff << PAGE_SHIFT) >
>> +         dev->buffer_size * (dev->buffers_number - 1)) {
>> +             printk(KERNEL_PREFIX
>> +                    "userspace tries to mmap to far, fail\n");
>
> Probably, module names in this two messages.
>
>> +             return -EINVAL;
>> +     }
>> +     addr = (unsigned long) dev->image + (vma->vm_pgoff << PAGE_SHIFT);
>> +
>> +     while (size > 0) {
>> +             page = (void *) vmalloc_to_page((void *) addr);
>> +
>> +             if (vm_insert_page(vma, start, page) < 0)
>> +                     return -EAGAIN;
>> +
>> +             start += PAGE_SIZE;
>> +             addr += PAGE_SIZE;
>> +             size -= PAGE_SIZE;
>> +     }
>> +
>> +     vma->vm_ops = &vm_ops;
>> +     vma->vm_private_data = 0;
>> +     dev->buffers[(vma->vm_pgoff<<PAGE_SHIFT)/dev->buffer_size].flags |=
>> +             V4L2_BUF_FLAG_MAPPED;
>> +
>> +     vm_open(vma);
>> +
>> +#ifdef DEBUG
>> +     printk(KERNEL_PREFIX "leaving v4l_mmap()\n");
>> +#endif
>> +
>> +     return 0;
>> +}
>> +
>> +static unsigned int v4l2_loopback_poll(struct file *file,
>> +                                    struct poll_table_struct *pts)
>> +{
>> +     struct v4l2_loopback_opener *opener = file->private_data;
>> +     int ret_mask = 0;
>> +     switch (opener->type) {
>> +             case WRITER: {
>> +                     ret_mask = POLLOUT | POLLWRNORM;
>> +             }
>> +             case READER: {
>> +                     poll_wait(file, &dev->read_event, pts);
>> +                     if (dev->write_position > opener->position)
>> +                             ret_mask =  POLLIN | POLLRDNORM;
>> +             }
>> +             default: {
>> +                     ret_mask = -POLLERR;
>> +             }
>> +     }
>> +     return ret_mask;
>> +}
>> +
>> +/* do not want to limit device opens, it can be as many readers as user want,
>> + * writers are limited by means of setting writer field */
>> +static int v4l_loopback_open(struct file *file)
>> +{
>> +     struct v4l2_loopback_opener *opener;
>> +#ifdef DEBUG
>> +     printk(KERNEL_PREFIX "entering v4l_open()\n");
>> +#endif
>> +     if (dev->open_count == V4L2_LOOPBACK_MAX_OPENERS)
>> +             return -EBUSY;
>> +     /* kfree on close */
>> +     opener = kzalloc(sizeof(*opener), GFP_KERNEL);
>> +     if (opener == NULL)
>> +             return -ENOMEM;
>> +     file->private_data = opener;
>> +     ++dev->open_count;
>> +     return 0;
>> +}
>> +
>> +static int v4l_loopback_close(struct file *file)
>> +{
>> +     struct v4l2_loopback_opener *opener = file->private_data;
>> +#ifdef DEBUG
>> +     printk(KERNEL_PREFIX "entering v4l_close()\n");
>> +#endif
>> +     --dev->open_count;
>> +     /* TODO(vasaka) does the closed file means that mmaped buffers are
>> +      * no more valid and one can free data? */
>> +     if (dev->open_count == 0) {
>> +             vfree(dev->image);
>> +             dev->image = NULL;
>> +             dev->ready_for_capture = 0;
>> +     }
>> +     kfree(opener);
>> +     return 0;
>> +}
>> +
>> +static ssize_t v4l_loopback_read(struct file *file, char __user *buf,
>> +                              size_t count, loff_t *ppos)
>> +{
>> +     int read_index;
>> +     struct v4l2_loopback_opener *opener = file->private_data;
>> +     if ((dev->write_position <= opener->position) &&
>> +             (file->f_flags&O_NONBLOCK)) {
>> +             return -EAGAIN;
>> +     }
>> +     wait_event_interruptible(dev->read_event,
>> +                              (dev->write_position > opener->position));
>> +     if (count > dev->buffer_size)
>> +             count = dev->buffer_size;
>> +     if (dev->write_position > opener->position+2)
>> +             opener->position = dev->write_position - 1;
>> +     read_index = opener->position % dev->buffers_number;
>> +     if (copy_to_user((void *) buf, (void *) (dev->image +
>> +                      dev->buffers[read_index].m.offset), count)) {
>> +             printk(KERN_INFO "failed copy_from_user() in write buf\n");
>
> The same here.
>
>> +             return -EFAULT;
>> +     }
>> +     ++opener->position;
>> +#ifdef DEBUG_RW
>> +     printk(KERNEL_PREFIX "leave v4l2_loopback_read()\n");
>> +#endif
>> +     return count;
>> +}
>> +
>> +static ssize_t v4l_loopback_write(struct file *file,
>> +                               const char __user *buf, size_t count,
>> +                               loff_t *ppos)
>> +{
>> +     int write_index = dev->write_position % dev->buffers_number;
>> +#ifdef DEBUG_RW
>> +     printk(KERNEL_PREFIX
>> +            "v4l2_loopback_write() trying to write %d bytes\n", count);
>> +#endif
>> +     if (count > dev->buffer_size)
>> +             count = dev->buffer_size;
>> +     if (copy_from_user(
>> +                (void *) (dev->image + dev->buffers[write_index].m.offset),
>> +                (void *) buf, count)) {
>> +             printk(KERNEL_PREFIX
>> +                "failed copy_from_user() in write buf, could not write %d\n",
>
> Again.
>
>> +                count);
>> +             return -EFAULT;
>> +     }
>> +     do_gettimeofday(&dev->buffers[write_index].timestamp);
>> +     dev->buffers[write_index].sequence = dev->write_position++;
>> +     wake_up_all(&dev->read_event);
>> +#ifdef DEBUG_RW
>> +     printk(KERNEL_PREFIX "leave v4l2_loopback_write()\n");
>> +#endif
>> +     return count;
>> +}
>> +
>> +/************************************************
>> +**************** init functions *****************
>> +************************************************/
>> +/* init inner buffers, they are capture mode and flags are set as
>> + * for capture mod buffers */
>> +static void init_buffers(int buffer_size)
>> +{
>> +     int i;
>> +     for (i = 0; i < dev->buffers_number; ++i) {
>> +             dev->buffers[i].bytesused = buffer_size;
>> +             dev->buffers[i].length = buffer_size;
>> +             dev->buffers[i].field = V4L2_FIELD_NONE;
>> +             dev->buffers[i].flags = 0;
>> +             dev->buffers[i].index = i;
>> +             dev->buffers[i].input = 0;
>> +             dev->buffers[i].m.offset = i * buffer_size;
>> +             dev->buffers[i].memory = V4L2_MEMORY_MMAP;
>> +             dev->buffers[i].sequence = 0;
>> +             dev->buffers[i].timestamp.tv_sec = 0;
>> +             dev->buffers[i].timestamp.tv_usec = 0;
>> +             dev->buffers[i].type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> +     }
>> +     dev->write_position = 0;
>> +}
>> +
>> +/* fills and register video device */
>> +static void init_vdev(struct video_device *vdev)
>> +{
>> +     strcpy(vdev->name, "Dummy video device");
>> +     vdev->tvnorms = V4L2_STD_NTSC | V4L2_STD_SECAM | V4L2_STD_PAL;/* TODO */
>> +     vdev->current_norm = V4L2_STD_PAL_B, /* do not know what is best here */
>> +     vdev->vfl_type = VFL_TYPE_GRABBER;
>> +     vdev->fops = &v4l2_loopback_fops;
>> +     vdev->ioctl_ops = &v4l2_loopback_ioctl_ops;
>> +     vdev->release = &video_device_release;
>> +     vdev->minor = -1;
>> +#ifdef DEBUG
>> +     vdev->debug = V4L2_DEBUG_IOCTL | V4L2_DEBUG_IOCTL_ARG;
>> +#endif
>> +}
>> +
>> +/* init default capture paramete, only fps may be changed in future */
>> +static void init_capture_param(struct v4l2_captureparm *capture_param)
>> +{
>> +     capture_param->capability = 0;
>> +     capture_param->capturemode = 0;
>> +     capture_param->extendedmode = 0;
>> +     capture_param->readbuffers = V4L2_LOOPBACK_BUFFERS_NUMBER;
>> +     capture_param->timeperframe.numerator = 1;
>> +     capture_param->timeperframe.denominator = 30;
>> +}
>> +
>> +/* init loopback main structure */
>> +static int v4l2_loopback_init(struct v4l2_loopback_device *dev)
>> +{
>> +     dev->vdev = video_device_alloc();
>> +     if (dev->vdev == NULL)
>> +             return -1;
>
> Maybe this error is -ENOMEM ?
>
>> +     init_vdev(dev->vdev);
>> +     init_capture_param(&dev->capture_param);
>> +     dev->buffers_number = V4L2_LOOPBACK_BUFFERS_NUMBER;
>> +     dev->open_count = 0;
>> +     dev->ready_for_capture = 0;
>> +     dev->buffer_size = 0;
>> +     dev->image = NULL;
>> +     /* kfree on module release */
>> +     dev->buffers =
>> +         kzalloc(sizeof(*dev->buffers) * dev->buffers_number,
>> +                 GFP_KERNEL);
>> +     if (dev->buffers == NULL)
>> +             return -ENOMEM;
>> +     init_waitqueue_head(&dev->read_event);
>> +     return 0;
>> +};
>> +
>> +/***********************************************
>> +**************** LINUX KERNEL ****************
>> +************************************************/
>> +static const struct v4l2_file_operations v4l2_loopback_fops = {
>> +      .owner = THIS_MODULE,
>> +      .open = v4l_loopback_open,
>> +      .release = v4l_loopback_close,
>> +      .read = v4l_loopback_read,
>> +      .write = v4l_loopback_write,
>> +      .poll = v4l2_loopback_poll,
>> +      .mmap = v4l2_loopback_mmap,
>> +      .ioctl = video_ioctl2,
>> +};
>> +
>> +static const struct v4l2_ioctl_ops v4l2_loopback_ioctl_ops = {
>> +     .vidioc_querycap = &vidioc_querycap,
>> +     .vidioc_enum_fmt_vid_cap = &vidioc_enum_fmt_cap,
>> +     .vidioc_enum_input = &vidioc_enum_input,
>> +     .vidioc_g_input = &vidioc_g_input,
>> +     .vidioc_s_input = &vidioc_s_input,
>> +     .vidioc_g_fmt_vid_cap = &vidioc_g_fmt_cap,
>> +     .vidioc_s_fmt_vid_cap = &vidioc_s_fmt_cap,
>> +     .vidioc_s_fmt_vid_out = &vidioc_s_fmt_video_output,
>> +     .vidioc_try_fmt_vid_cap = &vidioc_try_fmt_cap,
>> +     .vidioc_try_fmt_vid_out = &vidioc_try_fmt_video_output,
>> +     .vidioc_s_std = &vidioc_s_std,
>> +     .vidioc_g_parm = &vidioc_g_parm,
>> +     .vidioc_s_parm = &vidioc_s_parm,
>> +     .vidioc_reqbufs = &vidioc_reqbufs,
>> +     .vidioc_querybuf = &vidioc_querybuf,
>> +     .vidioc_qbuf = &vidioc_qbuf,
>> +     .vidioc_dqbuf = &vidioc_dqbuf,
>> +     .vidioc_streamon = &vidioc_streamon,
>> +     .vidioc_streamoff = &vidioc_streamoff,
>> +#ifdef CONFIG_VIDEO_V4L1_COMPAT
>> +     .vidiocgmbuf = &vidiocgmbuf,
>> +#endif
>> +};
>> +
>> +int __init init_module()
>> +{
>> +#ifdef DEBUG
>> +     printk(KERNEL_PREFIX "entering init_module()\n");
>> +#endif
>> +     /* kfree on module release */
>> +     dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>> +     if (dev == NULL)
>> +             return -ENOMEM;
>> +     if (v4l2_loopback_init(dev) < 0)
>> +             return -EINVAL;
>
> Well, honestly it's better to return error that you get from
> v4l2_loopback_init because possible v4l2_loopback_init mistake is no
> mem. There is no invalid argument error as i see it.
>
>> +     /* register the device -> it creates /dev/video* */
>> +     if (video_register_device(dev->vdev, VFL_TYPE_GRABBER, -1) < 0) {
>> +             video_device_release(dev->vdev);
>> +             printk(KERN_INFO "failed video_register_device()\n");
>> +             return -EINVAL;
>> +     }
>> +     printk(KERNEL_PREFIX "module installed\n");
>
> Module names or not? :)
>
>> +     return 0;
>> +}
>> +
>> +void __exit cleanup_module()
>> +{
>> +#ifdef DEBUG
>> +     printk(KERNEL_PREFIX "entering cleanup_module()\n");
>> +#endif
>> +     /* unregister the device -> it deletes /dev/video* */
>> +     video_unregister_device(dev->vdev);
>> +     kfree(dev->buffers);
>> +     kfree(dev);
>> +     printk(KERNEL_PREFIX "module removed\n");
>> +}
>> +
>> +
>> +MODULE_DESCRIPTION("YAVLD - V4L2 loopback video device");
>> +MODULE_VERSION("0.0.1");
>> +MODULE_AUTHOR("Vasily Levin");
>> +MODULE_LICENSE("GPL");
>> diff -uprN v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.h
>> v4l-dvb.my/linux/drivers/media/video/v4l2loopback.h
>> --- v4l-dvb.orig/linux/drivers/media/video/v4l2loopback.h     1970-01-01
>> 03:00:00.000000000 +0300
>> +++ v4l-dvb.my/linux/drivers/media/video/v4l2loopback.h       2009-03-24
>> 18:21:58.000000000 +0200
>> @@ -0,0 +1,58 @@
>> +/*
>> + *      v4l2loopback.h  --  video 4 linux loopback driver
>> + *
>> + *      Copyright (C) 2005-2009
>> + *          Vasily Levin (vasaka@xxxxxxxxx)
>> + *
>> + *      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.
>> + *
>> + */
>> +
>> +#ifndef _V4L2LOOPBACK_H
>> +#define      _V4L2LOOPBACK_H
>> +
>> +#include <linux/videodev2.h>
>> +#include <media/v4l2-common.h>
>> + /* fixed inner buffers number */
>> +/* TODO(vasaka) make this module parameter */
>> +#define V4L2_LOOPBACK_BUFFERS_NUMBER 4
>> +#define V4L2_LOOPBACK_MAX_OPENERS 10
>> +
>> +/* 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 choise */
>> +     struct v4l2_buffer *buffers;    /* inner driver buffers */
>> +     int write_position; /* number of last written frame + 1 */
>> +     long buffer_size;
>> +     /* sync stuff */
>> +     int 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 sinc */
>> +     struct v4l2_buffer *buffers;
>> +};
>> +#endif                               /* _V4L2LOOPBACK_H */
>> diff -uprN v4l-dvb.orig/v4l/.config v4l-dvb.my/v4l/.config
>> --- v4l-dvb.orig/v4l/.config  2009-03-24 03:45:35.000000000 +0200
>> +++ v4l-dvb.my/v4l/.config    2009-03-24 12:58:09.000000000 +0200
>> @@ -37,6 +37,7 @@ CONFIG_DVB_B2C2_FLEXCOP_PCI=m
>>  CONFIG_RADIO_AZTECH=m
>>  CONFIG_VIDEO_BT848=m
>>  CONFIG_VIDEO_VIVI=m
>> +CONFIG_VIDEO_V4L2_LOOPBACK=m
>>  CONFIG_DVB_USB_CXUSB=m
>>  CONFIG_USB_GSPCA_FINEPIX=m
>>  CONFIG_SOC_CAMERA_MT9V022=m
>> diff -uprN v4l-dvb.orig/v4l/Kconfig v4l-dvb.my/v4l/Kconfig
>> --- v4l-dvb.orig/v4l/Kconfig  2009-03-24 03:45:35.000000000 +0200
>> +++ v4l-dvb.my/v4l/Kconfig    2009-03-24 12:59:15.000000000 +0200
>> @@ -781,6 +781,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.
>> +
>>  config VIDEO_BT848
>>       tristate "BT848 Video For Linux"
>>       depends on VIDEO_DEV && PCI && I2C && VIDEO_V4L2 && INPUT
>> @@ -2137,7 +2144,7 @@ config USB_S2255
>>       depends on VIDEO_V4L2
>>       select VIDEOBUF_VMALLOC
>>       default n
>> -     help
>> +     ---help---
>
> is this really part of patch ?
>
>>         Say Y here if you want support for the Sensoray 2255 USB device.
>>         This driver can be compiled as a module, called s2255drv.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Best regards, Klimov Alexey
>
>
Thanks for comments, this is first kernel I ever wrote :-)

I also want to ask what was wrong with my post that patchwork tool did
not understand patch properly?

Vasily Levin
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux