Hans Verkuil | 3 Jun 14:14 2013
Picon
Picon

[RFC] Add query/get/set matrix ioctls

Hi all,

When working on the Motion Detection API, I realized that my proposal for
two new G/S_MD_BLOCKS ioctls was too specific for the motion detection use
case.

The Motion Detection RFC can be found here:

http://www.mail-archive.com/linux-media <at> vger.kernel.org/msg62085.html

What I was really attempting to do was to create an API to pass a matrix
to the hardware.

This is also related to a long-standing request to add support for
arrays to the control API. Adding such support to the control API is in my
opinion a very bad idea since the control API is meant to provide all
meta data necessary in order to create e.g. control panels. Adding array
support to the control API would make that very difficult, particularly
with respect to GUI design.

So instead this proposal creates a new API to query, get and set matrices:

/* Define to which motion detection region each element belongs.
 * Each element is a __u8. */
#define V4L2_MATRIX_TYPE_MD_REGION     (1)
/* Define the motion detection threshold for each element.
 * Each element is a __u16. */
#define V4L2_MATRIX_TYPE_MD_THRESHOLD  (2)

/**
 * struct v4l2_query_matrix - VIDIOC_QUERY_MATRIX argument
 *  <at> type:       matrix type
 *  <at> index:      matrix index of the given type
 *  <at> columns:    number of columns in the matrix
 *  <at> rows:       number of rows in the matrix
 *  <at> elem_min:   minimum matrix element value
 *  <at> elem_max:   maximum matrix element value
 *  <at> elem_size:  size in bytes each matrix element
 *  <at> reserved:   future extensions, applications and drivers must zero this.
 */
struct v4l2_query_matrix {
        __u32 type;
        __u32 index;
        __u32 columns;
        __u32 rows;
        __s64 elem_min;
        __s64 elem_max;
        __u32 elem_size;
        __u32 reserved[23];
} __attribute__ ((packed));

/**
 * struct v4l2_matrix - VIDIOC_G/S_MATRIX argument
 *  <at> type:       matrix type
 *  <at> index:      matrix index of the given type
 *  <at> rect:       which part of the matrix to get/set
 *  <at> matrix:     pointer to the matrix of size (in bytes):
 *              elem_size * rect.width * rect.height
 *  <at> reserved:   future extensions, applications and drivers must zero this.
 */
struct v4l2_matrix {
        __u32 type;
        __u32 index;
        struct v4l2_rect rect;
        void __user *matrix;
        __u32 reserved[12];
} __attribute__ ((packed));

/* Experimental, these three ioctls may change over the next couple of kernel
   versions. */
#define VIDIOC_QUERY_MATRIX     _IORW('V', 103, struct v4l2_query_matrix)
#define VIDIOC_G_MATRIX         _IORW('V', 104, struct v4l2_matrix)
#define VIDIOC_S_MATRIX         _IORW('V', 105, struct v4l2_matrix)

Each matrix has a type (which describes the meaning of the matrix) and an
index (allowing for multiple matrices of the same type).

QUERY_MATRIX will return the number of columns and rows in the full matrix,
the size (in bytes) of each element and the minimum and maximum value of
each element. Some matrix types may have non-integer elements, in which case
the minimum and maximum values are ignored.

With S_MATRIX and G_MATRIX you can get/set a (part of a) matrix. The rect
struct will give the part of the matrix that you want to set or retrieve, and
the matrix pointer points to the matrix data.

Currently only two matrix types are defined, see the motion detection RFC for
details.

This approach is basically the same as proposed in the motion detection RFC,
but it is much more general.

Discussion points:

1) I'm using elem_size to allow for any element size. An alternative would be to
define specific element types (e.g. U8, S8, U16, S16, etc.), but I feel that
that is overkill. It is easier to associate each matrix type with a specific
element type in the documentation for each type. For allocation purposes it
is more useful to know the element size than the element type. But perhaps
elem_size can be dropped altogether, or, alternatively, both an elem_size and
elem_type should be defined.

2) Per-driver matrix types are probably necessary as well: for example while
colorspace conversion matrices are in principle generic, in practice the
precise format of the elements is hardware specific. This isn't a problem
as long as it is a well-defined private matrix type.

Comments? Questions?

	Hans

Gmane