Re: [RFC PATCH v2 7/8] media: video: introduce object detection driver module
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
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
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
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); + +