Re: [RFC PATCH v2 7/8] media: video: introduce object detection driver module

2012-01-17 Thread Sylwester Nawrocki
On 01/04/2012 09:13 AM, Ming Lei wrote:
 +
 +int odif_open(struct file *file)
 +{
 + struct odif_dev *dev = video_drvdata(file);
 +
 + kref_get(dev-ref);
 + return v4l2_fh_open(file);
 +}

 Individual drivers may need to perform some initialization when
 device node is opened. So I consider taking this possibility away
 from them not a good idea.
 
 OK, it can be handled easily by introducing new callbacks(such as.
 .open_detect and .close_detect for close) in struct odif_ops.

Certainly, we can introduce plenty of new callbacks, another levels of 
indirection that would just obfuscate things. It should rather be done 
like v4l2-mem2mem does, where file operation handlers are implemented 
by each driver and from their open()/close() handlers relevant init
and release functions are called (v4l2_m2m_init/v4l2_m2m_release).

I think we can always change the kernel interfaces once there is more 
devices like OMAP4 FDIF. It may look easy for you when you refer to 
just one hardware implementation with your theoretically generic module. 
Or have you referred to other devices than OMAP4 FDIF when designing it ?

 +static int vidioc_g_od_result(struct file *file, void *priv,
 + struct v4l2_od_result *f)
 +{
 + struct odif_dev *dev = video_drvdata(file);
 + unsigned long flags;
 + struct v4l2_odif_result *tmp;
 + struct v4l2_odif_result *fr = NULL;
 + unsigned int cnt = 0;
 + int ret = -EINVAL;
 +
 + spin_lock_irqsave(dev-lock, flags);
 + list_for_each_entry(tmp,dev-odif_dq.complete, list)
 + if (tmp-frm_seq == f-frm_seq) {
 + fr = tmp;
 + list_del(tmp-list);
 + break;
 + }
 + spin_unlock_irqrestore(dev-lock, flags);
 +
 + if (fr) {
 + ret = 0;
 + cnt = min(f-obj_cnt, fr-obj_cnt);
 + if (cnt)
 + memcpy(f-od, fr-objs,
 + sizeof(struct v4l2_od_object) * cnt);
 + f-obj_cnt = cnt;
 +
 + free_result(fr);

 Some drivers may be designed so they do not discard the result data
 automatically once it is read by user application. AFAICS this module
 doesn't allow such behaviour.

 For instance, it might be required to discard the oldest results data
 when the ring buffer gets filled in.
 
 Looks like no any benefit about keeping the old result data, could you
 give some use cases which require the old results need to be kept for
 some time?

Consider scenario where one thread writes image data and multiple threads
read the FD results. Having all data discarded on first read only one 
thread could get the data. This is not what we want in a standard API.

Also in V4L2 it is always assumed that multiple applications can read
state of a device, and the application priority API [1] aims at
managing the devices state modifications among multiple processes.
 ...
 +static int buffer_init(struct vb2_buffer *vb)
 +{
 + struct odif_dev *dev = vb2_get_drv_priv(vb-vb2_queue);
 +
 + /*
 +  * This callback is called once per buffer, after its allocation.
 +  *
 +  * Vivi does not allow changing format during streaming, but it is
 +  * possible to do so when streaming is paused (i.e. in streamoff 
 state).
 +  * Buffers however are not freed when going into streamoff and so
 +  * buffer size verification has to be done in buffer_prepare, on each
 +  * qbuf.
 +  * It would be best to move verification code here to buf_init and
 +  * s_fmt though.
 +  */
 + dprintk(dev, 1, %s vaddr=%p\n, __func__,
 + vb2_plane_vaddr(vb, 0));
 +
 + return 0;
 +}

 As I already mentioned this empty callback can be removed. Anyway IMO the
 implementation of the buffer queue operations should be left to individual
 drivers. Having them in generic module doesn't sound flexible enough.
 
 IMO, the buffer queue operations can be shared for use case of
 detecting objects from user space images, so it may be kept it in
 generic driver.

Not necessarily, sometimes device specific logic needs to be implemented
in these callbacks. And the ioctl handlers (vidioc_*) should be just 
pass-through for the vb2 callbacks.


 +void odif_add_detection(struct odif_dev *dev,
 + struct v4l2_odif_result *v4l2_fr)
 +{
 + unsigned long flags;
 + struct v4l2_odif_result *old = NULL;
 + struct v4l2_odif_result *tmp;
 +
 + spin_lock_irqsave(dev-lock, flags);
 + list_for_each_entry(tmp,dev-odif_dq.complete, list)
 + if (tmp-frm_seq == v4l2_fr-frm_seq) {
 + old = tmp;
 + list_del(tmp-list);
 + break;
 + }
 + list_add_tail(v4l2_fr-list,dev-odif_dq.complete);
 + spin_unlock_irqrestore(dev-lock, flags);
 +
 + if (old)
 + free_result(old);
 + else
 + atomic_inc(dev-odif_dq.complete_cnt);
 +}

Re: [RFC PATCH v2 7/8] media: video: introduce object detection driver module

2012-01-04 Thread Ming Lei
Hi Sylwester,

Thanks for your review.

On Fri, Dec 30, 2011 at 1:16 AM, Sylwester Nawrocki snj...@gmail.com wrote:
 Hi Ming,

 On 12/14/2011 03:00 PM, Ming Lei wrote:
 This patch introduces object detection generic driver.

 The driver is responsible for all v4l2 stuff, buffer management
 and other general things, and doesn't touch object detection hardware
 directly. Several interfaces are exported to low level drivers
 (such as the coming omap4 FD driver) which will communicate with
 object detection hw module.

 So the driver will make driving object detection hw modules more
 easy.
 TODO:
       - implement object detection setting interfaces with v4l2
       controls or ext controls

 Signed-off-by: Ming Lei ming@canonical.com
 ---
 v2:
       - extend face detection driver to object detection driver
       - introduce subdevice and media entity
       - provide support to detect object from media HW
 ---
  drivers/media/video/Kconfig       |    2 +
  drivers/media/video/Makefile      |    1 +
  drivers/media/video/odif/Kconfig  |    7 +
  drivers/media/video/odif/Makefile |    1 +
  drivers/media/video/odif/odif.c   |  890 
 +
  drivers/media/video/odif/odif.h   |  157 +++
  6 files changed, 1058 insertions(+), 0 deletions(-)
  create mode 100644 drivers/media/video/odif/Kconfig
  create mode 100644 drivers/media/video/odif/Makefile
  create mode 100644 drivers/media/video/odif/odif.c
  create mode 100644 drivers/media/video/odif/odif.h

 diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
 index 5684a00..8740ee9 100644
 --- a/drivers/media/video/Kconfig
 +++ b/drivers/media/video/Kconfig
 @@ -1166,3 +1166,5 @@ config VIDEO_SAMSUNG_S5P_MFC
           MFC 5.1 driver for V4L2.

  endif # V4L_MEM2MEM_DRIVERS
 +
 +source drivers/media/video/odif/Kconfig
 diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
 index bc797f2..259c8d8 100644
 --- a/drivers/media/video/Makefile
 +++ b/drivers/media/video/Makefile
 @@ -197,6 +197,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
  obj-y        += davinci/

  obj-$(CONFIG_ARCH_OMAP)      += omap/
 +obj-$(CONFIG_ODIF)   += odif/

  ccflags-y += -Idrivers/media/dvb/dvb-core
  ccflags-y += -Idrivers/media/dvb/frontends
 diff --git a/drivers/media/video/odif/Kconfig 
 b/drivers/media/video/odif/Kconfig
 new file mode 100644
 index 000..5090bd6
 --- /dev/null
 +++ b/drivers/media/video/odif/Kconfig
 @@ -0,0 +1,7 @@
 +config ODIF
 +     depends on VIDEO_DEV  VIDEO_V4L2
 +     select VIDEOBUF2_PAGE
 +     tristate Object Detection module
 +     help
 +       The ODIF is a object detection module, which can be integrated into
 +       some SoCs to detect objects in images or video.
 diff --git a/drivers/media/video/odif/Makefile 
 b/drivers/media/video/odif/Makefile
 new file mode 100644
 index 000..a55ff66
 --- /dev/null
 +++ b/drivers/media/video/odif/Makefile
 @@ -0,0 +1 @@
 +obj-$(CONFIG_ODIF)           += odif.o
 diff --git a/drivers/media/video/odif/odif.c 
 b/drivers/media/video/odif/odif.c
 new file mode 100644
 index 000..381ab9d
 --- /dev/null
 +++ b/drivers/media/video/odif/odif.c
 @@ -0,0 +1,890 @@
 +/*
 + *      odif.c  --  object detection module driver
 + *
 + *      Copyright (C) 2011  Ming Lei (ming@canonical.com)
 + *
 + *   This file is based on drivers/media/video/vivi.c.
 + *
 + *      This program is free software; you can redistribute it and/or modify
 + *      it under the terms of the GNU General Public License as published by
 + *      the Free Software Foundation; either version 2 of the License, or
 + *      (at your option) any later version.
 + *
 + *      This program is distributed in the hope that it will be useful,
 + *      but WITHOUT ANY WARRANTY; without even the implied warranty of
 + *      MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + *      GNU General Public License for more details.
 + *
 + *      You should have received a copy of the GNU General Public License
 + *      along with this program; if not, write to the Free Software
 + *      Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 + *
 + */
 +
 +/*/
 +
 +#include linux/module.h
 +#include linux/fs.h
 +#include linux/mm.h
 +#include linux/signal.h
 +#include linux/wait.h
 +#include linux/poll.h
 +#include linux/mman.h
 +#include linux/pm_runtime.h
 +#include linux/delay.h
 +#include linux/platform_device.h
 +#include linux/interrupt.h
 +#include asm/uaccess.h
 +#include asm/byteorder.h
 +#include asm/io.h
 +#include odif.h
 +
 +#define      input_from_user(dev) \
 +     (dev-input == OD_INPUT_FROM_USER_SPACE)
 +
 +#define      DEFAULT_PENDING_RESULT_CNT      8
 +
 +static unsigned debug = 0;
 +module_param(debug, uint, 0644);
 +MODULE_PARM_DESC(debug, activates debug info);
 +
 +static unsigned result_cnt_threshold = DEFAULT_PENDING_RESULT_CNT;
 

Re: [RFC PATCH v2 7/8] media: video: introduce object detection driver module

2011-12-29 Thread Sylwester Nawrocki
Hi Ming,

On 12/14/2011 03:00 PM, Ming Lei wrote:
 This patch introduces object detection generic driver.
 
 The driver is responsible for all v4l2 stuff, buffer management
 and other general things, and doesn't touch object detection hardware
 directly. Several interfaces are exported to low level drivers
 (such as the coming omap4 FD driver) which will communicate with
 object detection hw module.
 
 So the driver will make driving object detection hw modules more
 easy.
 TODO:
   - implement object detection setting interfaces with v4l2
   controls or ext controls
 
 Signed-off-by: Ming Lei ming@canonical.com
 ---
 v2:
   - extend face detection driver to object detection driver
   - introduce subdevice and media entity
   - provide support to detect object from media HW
 ---
  drivers/media/video/Kconfig   |2 +
  drivers/media/video/Makefile  |1 +
  drivers/media/video/odif/Kconfig  |7 +
  drivers/media/video/odif/Makefile |1 +
  drivers/media/video/odif/odif.c   |  890 
 +
  drivers/media/video/odif/odif.h   |  157 +++
  6 files changed, 1058 insertions(+), 0 deletions(-)
  create mode 100644 drivers/media/video/odif/Kconfig
  create mode 100644 drivers/media/video/odif/Makefile
  create mode 100644 drivers/media/video/odif/odif.c
  create mode 100644 drivers/media/video/odif/odif.h
 
 diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
 index 5684a00..8740ee9 100644
 --- a/drivers/media/video/Kconfig
 +++ b/drivers/media/video/Kconfig
 @@ -1166,3 +1166,5 @@ config VIDEO_SAMSUNG_S5P_MFC
   MFC 5.1 driver for V4L2.
  
  endif # V4L_MEM2MEM_DRIVERS
 +
 +source drivers/media/video/odif/Kconfig
 diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
 index bc797f2..259c8d8 100644
 --- a/drivers/media/video/Makefile
 +++ b/drivers/media/video/Makefile
 @@ -197,6 +197,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
  obj-y+= davinci/
  
  obj-$(CONFIG_ARCH_OMAP)  += omap/
 +obj-$(CONFIG_ODIF)   += odif/
  
  ccflags-y += -Idrivers/media/dvb/dvb-core
  ccflags-y += -Idrivers/media/dvb/frontends
 diff --git a/drivers/media/video/odif/Kconfig 
 b/drivers/media/video/odif/Kconfig
 new file mode 100644
 index 000..5090bd6
 --- /dev/null
 +++ b/drivers/media/video/odif/Kconfig
 @@ -0,0 +1,7 @@
 +config ODIF
 + depends on VIDEO_DEV  VIDEO_V4L2
 + select VIDEOBUF2_PAGE
 + tristate Object Detection module
 + help
 +   The ODIF is a object detection module, which can be integrated into
 +   some SoCs to detect objects in images or video.
 diff --git a/drivers/media/video/odif/Makefile 
 b/drivers/media/video/odif/Makefile
 new file mode 100644
 index 000..a55ff66
 --- /dev/null
 +++ b/drivers/media/video/odif/Makefile
 @@ -0,0 +1 @@
 +obj-$(CONFIG_ODIF)   += odif.o
 diff --git a/drivers/media/video/odif/odif.c b/drivers/media/video/odif/odif.c
 new file mode 100644
 index 000..381ab9d
 --- /dev/null
 +++ b/drivers/media/video/odif/odif.c
 @@ -0,0 +1,890 @@
 +/*
 + *  odif.c  --  object detection module driver
 + *
 + *  Copyright (C) 2011  Ming Lei (ming@canonical.com)
 + *
 + *   This file is based on drivers/media/video/vivi.c.
 + *
 + *  This program is free software; you can redistribute it and/or modify
 + *  it under the terms of the GNU General Public License as published by
 + *  the Free Software Foundation; either version 2 of the License, or
 + *  (at your option) any later version.
 + *
 + *  This program is distributed in the hope that it will be useful,
 + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + *  GNU General Public License for more details.
 + *
 + *  You should have received a copy of the GNU General Public License
 + *  along with this program; if not, write to the Free Software
 + *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 + *
 + */
 +
 +/*/
 +
 +#include linux/module.h
 +#include linux/fs.h
 +#include linux/mm.h
 +#include linux/signal.h
 +#include linux/wait.h
 +#include linux/poll.h
 +#include linux/mman.h
 +#include linux/pm_runtime.h
 +#include linux/delay.h
 +#include linux/platform_device.h
 +#include linux/interrupt.h
 +#include asm/uaccess.h
 +#include asm/byteorder.h
 +#include asm/io.h
 +#include odif.h
 +
 +#define  input_from_user(dev) \
 + (dev-input == OD_INPUT_FROM_USER_SPACE)
 +
 +#define  DEFAULT_PENDING_RESULT_CNT  8
 +
 +static unsigned debug = 0;
 +module_param(debug, uint, 0644);
 +MODULE_PARM_DESC(debug, activates debug info);
 +
 +static unsigned result_cnt_threshold = DEFAULT_PENDING_RESULT_CNT;
 +module_param(result_cnt_threshold, uint, 0644);
 +MODULE_PARM_DESC(result_cnt, max pending result count);
 +
 +static 

[RFC PATCH v2 7/8] media: video: introduce object detection driver module

2011-12-14 Thread Ming Lei
This patch introduces object detection generic driver.

The driver is responsible for all v4l2 stuff, buffer management
and other general things, and doesn't touch object detection hardware
directly. Several interfaces are exported to low level drivers
(such as the coming omap4 FD driver) which will communicate with
object detection hw module.

So the driver will make driving object detection hw modules more
easy.

TODO:
- implement object detection setting interfaces with v4l2
controls or ext controls

Signed-off-by: Ming Lei ming@canonical.com
---
v2:
- extend face detection driver to object detection driver
- introduce subdevice and media entity
- provide support to detect object from media HW
---
 drivers/media/video/Kconfig   |2 +
 drivers/media/video/Makefile  |1 +
 drivers/media/video/odif/Kconfig  |7 +
 drivers/media/video/odif/Makefile |1 +
 drivers/media/video/odif/odif.c   |  890 +
 drivers/media/video/odif/odif.h   |  157 +++
 6 files changed, 1058 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/odif/Kconfig
 create mode 100644 drivers/media/video/odif/Makefile
 create mode 100644 drivers/media/video/odif/odif.c
 create mode 100644 drivers/media/video/odif/odif.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 5684a00..8740ee9 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -1166,3 +1166,5 @@ config VIDEO_SAMSUNG_S5P_MFC
MFC 5.1 driver for V4L2.
 
 endif # V4L_MEM2MEM_DRIVERS
+
+source drivers/media/video/odif/Kconfig
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index bc797f2..259c8d8 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -197,6 +197,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-y  += davinci/
 
 obj-$(CONFIG_ARCH_OMAP)+= omap/
+obj-$(CONFIG_ODIF) += odif/
 
 ccflags-y += -Idrivers/media/dvb/dvb-core
 ccflags-y += -Idrivers/media/dvb/frontends
diff --git a/drivers/media/video/odif/Kconfig b/drivers/media/video/odif/Kconfig
new file mode 100644
index 000..5090bd6
--- /dev/null
+++ b/drivers/media/video/odif/Kconfig
@@ -0,0 +1,7 @@
+config ODIF
+   depends on VIDEO_DEV  VIDEO_V4L2
+   select VIDEOBUF2_PAGE
+   tristate Object Detection module
+   help
+ The ODIF is a object detection module, which can be integrated into
+ some SoCs to detect objects in images or video.
diff --git a/drivers/media/video/odif/Makefile 
b/drivers/media/video/odif/Makefile
new file mode 100644
index 000..a55ff66
--- /dev/null
+++ b/drivers/media/video/odif/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ODIF) += odif.o
diff --git a/drivers/media/video/odif/odif.c b/drivers/media/video/odif/odif.c
new file mode 100644
index 000..381ab9d
--- /dev/null
+++ b/drivers/media/video/odif/odif.c
@@ -0,0 +1,890 @@
+/*
+ *  odif.c  --  object detection module driver
+ *
+ *  Copyright (C) 2011  Ming Lei (ming@canonical.com)
+ *
+ * This file is based on drivers/media/video/vivi.c.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+/*/
+
+#include linux/module.h
+#include linux/fs.h
+#include linux/mm.h
+#include linux/signal.h
+#include linux/wait.h
+#include linux/poll.h
+#include linux/mman.h
+#include linux/pm_runtime.h
+#include linux/delay.h
+#include linux/platform_device.h
+#include linux/interrupt.h
+#include asm/uaccess.h
+#include asm/byteorder.h
+#include asm/io.h
+#include odif.h
+
+#defineinput_from_user(dev) \
+   (dev-input == OD_INPUT_FROM_USER_SPACE)
+
+#defineDEFAULT_PENDING_RESULT_CNT  8
+
+static unsigned debug = 0;
+module_param(debug, uint, 0644);
+MODULE_PARM_DESC(debug, activates debug info);
+
+static unsigned result_cnt_threshold = DEFAULT_PENDING_RESULT_CNT;
+module_param(result_cnt_threshold, uint, 0644);
+MODULE_PARM_DESC(result_cnt, max pending result count);
+
+static LIST_HEAD(odif_devlist);
+static unsigned video_nr = -1;
+
+int odif_open(struct file *file)
+{
+   struct odif_dev *dev = video_drvdata(file);
+
+