lkml.org 
[lkml]   [2011]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
    Hi Ming,

    (I've pruned the Cc list, leaving just the mailing lists)

    On 12/02/2011 04:02 PM, Ming Lei wrote:
    > This patch introduces one driver for face detection purpose.
    >
    > The driver is responsible for all v4l2 stuff, buffer management
    > and other general things, and doesn't touch face detection hardware
    > directly. Several interfaces are exported to low level drivers
    > (such as the coming omap4 FD driver)which will communicate with
    > face detection hw module.
    >
    > So the driver will make driving face detection hw modules more
    > easy.


    I would hold on for a moment on implementing generic face detection
    module which is based on the V4L2 video device interface. We need to
    first define an API that would be also usable at sub-device interface
    level (http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html).
    AFAICS OMAP4 FDIF processes only data stored in memory, thus it seems
    reasonable to use the videodev interface for passing data to the kernel
    from user space.

    But there might be face detection devices that accept data from other
    H/W modules, e.g. transferred through SoC internal data buses between
    image processing pipeline blocks. Thus any new interfaces need to be
    designed with such devices in mind.

    Also the face detection hardware block might now have an input DMA
    engine in it, the data could be fed from memory through some other
    subsystem (e.g. resize/colour converter). Then the driver for that
    subsystem would implement a video node.

    I'm for leaving the buffer handling details for individual drivers
    and focusing on a standard interface for applications, i.e. new
    ioctl(s) and controls.

    >
    > TODO:
    > - implement FD setting interfaces with v4l2 controls or
    > ext controls
    >
    > Signed-off-by: Ming Lei <ming.lei@canonical.com>
    > ---
    > drivers/media/video/Kconfig | 2 +
    > drivers/media/video/Makefile | 1 +
    > drivers/media/video/fdif/Kconfig | 7 +
    > drivers/media/video/fdif/Makefile | 1 +
    > drivers/media/video/fdif/fdif.c | 645 +++++++++++++++++++++++++++++++++++++
    > drivers/media/video/fdif/fdif.h | 114 +++++++
    > 6 files changed, 770 insertions(+), 0 deletions(-)
    > create mode 100644 drivers/media/video/fdif/Kconfig
    > create mode 100644 drivers/media/video/fdif/Makefile
    > create mode 100644 drivers/media/video/fdif/fdif.c
    > create mode 100644 drivers/media/video/fdif/fdif.h

    [...]

    > diff --git a/drivers/media/video/fdif/fdif.h b/drivers/media/video/fdif/fdif.h
    > new file mode 100644
    > index 0000000..ae37ab8
    > --- /dev/null
    > +++ b/drivers/media/video/fdif/fdif.h
    > @@ -0,0 +1,114 @@
    > +#ifndef _LINUX_FDIF_H
    > +#define _LINUX_FDIF_H
    > +
    > +#include <linux/types.h>
    > +#include <linux/magic.h>
    > +#include <linux/errno.h>
    > +#include <linux/kref.h>
    > +#include <linux/kernel.h>
    > +#include <linux/videodev2.h>
    > +#include <media/videobuf2-page.h>
    > +#include <media/v4l2-device.h>
    > +#include <media/v4l2-ioctl.h>
    > +#include <media/v4l2-ctrls.h>
    > +#include <media/v4l2-fh.h>
    > +#include <media/v4l2-event.h>
    > +#include <media/v4l2-common.h>
    > +
    > +#define MAX_FACE_COUNT 40
    > +
    > +#define FACE_SIZE_20_PIXELS 0
    > +#define FACE_SIZE_25_PIXELS 1
    > +#define FACE_SIZE_32_PIXELS 2
    > +#define FACE_SIZE_40_PIXELS 3

    This is still OMAP4 FDIF specific, we need to think about v4l2 controls
    for this. An ideal would be a menu control type that supports pixel size
    (width/height), but unfortunately something like this isn't available
    in v4l2 yet.

    > +
    > +#define FACE_DIR_UP 0
    > +#define FACE_DIR_RIGHT 1
    > +#define FACE_DIR_LIFT 2
    > +
    > +struct fdif_fmt {
    > + char *name;
    > + u32 fourcc; /* v4l2 format id */
    > + int depth;
    > + int width, height;

    Could width/height be negative ? I don't think it's the case for pixel
    resolution. The more proper data type would be u32.

    Please refer to struct v4l2_pix_format or struct v4l2_rect.

    > +};
    > +
    > +struct fdif_setting {
    > + struct fdif_fmt *fmt;
    > + enum v4l2_field field;
    > +
    > + int min_face_size;
    > + int face_dir;
    > +
    > + int startx, starty;

    s32

    > + int sizex, sizey;

    u32

    > + int lhit;
    > +
    > + int width, height;

    u32

    > +};
    > +
    > +/* buffer for one video frame */
    > +struct fdif_buffer {
    > + /* common v4l buffer stuff -- must be first */
    > + struct vb2_buffer vb;
    > + struct list_head list;
    > +};
    > +
    > +
    > +struct v4l2_fdif_result {
    > + struct list_head list;
    > + unsigned int face_cnt;
    > + struct v4l2_fd_detection *faces;
    > +
    > + /*v4l2 buffer index*/
    > + __u32 index;
    > +};
    > +
    > +struct fdif_dmaqueue {
    > + struct list_head complete;
    > + struct list_head active;
    > + wait_queue_head_t wq;
    > +};
    > +
    > +
    > +struct fdif_dev {
    > + struct kref ref;
    > + struct device *dev;
    > +
    > + struct list_head fdif_devlist;
    > + struct v4l2_device v4l2_dev;
    > + struct vb2_queue vbq;
    > + struct mutex mutex;
    > + spinlock_t lock;
    > +
    > + struct video_device *vfd;
    > + struct fdif_dmaqueue fdif_dq;
    > +
    > + /*setting*/
    > + struct fdif_setting s;

    yy, please make it more descriptive. e.g.

    struct fdif_config config;

    > +
    > + struct fdif_ops *ops;
    > +
    > + unsigned long priv[0];
    > +};
    > +
    [...]

    --

    Regards,
    Sylwester


    \
     
     \ /
      Last update: 2011-12-05 22:59    [W:3.399 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site