Re: RFC: Properties, Configuration Storage, Selections and Matrices

2013-12-07 Thread Hans Verkuil
Hi Sylwester,

Thank you for your review! Much appreciated.

On 12/07/2013 12:51 AM, Sylwester Nawrocki wrote:
 Hi Hans,
 
 Thank you for working on this, and my apologies for not reviewing it 
 earlier.
 
 On 11/23/2013 03:07 PM, Hans Verkuil wrote:
 Another update:

 While attempting to add fake 'shadow register' support to vivi I realized 
 that
 the API to set up configuration stores didn't quite work out. Supporting
 shadow registers is one of the main goals of the configuration storage idea, 
 so
 this should work smoothly.

 In the original proposal a configuration store is set by VIDIOC_S_EXT_CTRLS 
 and
 any controls not in the control list will also be absent from the 
 configuration
 store. That makes no sense for shadow registers: i.e. you will always have N
 sets of X registers and when switching from one set to another you switch 
 them
 all.

 So the API has been changed a bit: all the driver-defined configuration 
 stores
 are always there and you can enumerate, get and set all the controls in each 
 store.

 If controls are implemented as shadow registers, then the driver will tell 
 the
 framework which of the configuration stores is the current store (i.e. which
 store maps to the currently selected shadow register) and switching from one
 configuration store to another will just update the current store of a 
 control
 to the new one.

 If controls are not implemented as a shadow register and you really just want
 to apply the new values from a configuration store, then that will actually
 involve copying the values from the selected store to the current control 
 values.

 This approach made implementing fake shadow registers in vivi a trivial
 exercise and worked out much better. In addition, internal data structures
 became simpler as well (always a good sign).

 My next step will be to add selection support as properties and see if I can
 add per-frame crop/compose support to vivi.

 Regards,

  Hans

 On 11/19/2013 09:10 AM, Hans Verkuil wrote:
 Just a quick update:

 I've been working on this for the past four days and I am now in the process
 of testing this. Adding matrix support to the control framework was actually
 the hardest part, particularly since I wanted it to be efficient. The code 
 is
 still a bit too complex for my liking, but once testing verifies that 
 everything
 is working I plan to clean things up some more.

 I hope to be able to post a first version during or just after the weekend.

 Please review this RFC: other than some small changes this is pretty much 
 how
 it will turn out.

 Some of the changes are:

 - The property bit is 26, not 27. Bit 27 clashes with the PRIVATE_BASE 
 define so
 it can't be used.

 - Any control/property type can be changed into a matrix by setting bit 15 
 in
 the type. E.g. V4l2_PROP_TYPE_MATRIX | V4L2_CTRL_TYPE_INTEGER. When passing 
 the
 data of a matrix the data is always preceded by a struct v4l2_rect which 
 tells
 the driver which subset of the matrix the application wants to set/get.

 - We definitely need a V4L2_CTRL_FLAG_CAN_STORE flag. Stores take memory 
 and you
 only want to allocate that for those controls that actually need to be 
 stored.

 - Regarding implementing stores for matrices: I store it in the internal 
 format,
 but with additional information regarding the parts of the matrix that need 
 to
 be updated. Basically the best of both worlds. With some careful tweaking of
 data structure this turned out to be fairly easy to do. This systems allows
 you to update only parts of a matrix in a config store efficiently without
 having to convert from one format to another.

 Regards,

 Hans

 On 11/15/2013 03:19 PM, Hans Verkuil wrote:
 Properties, Configuration Storage, Selections and Matrices
 ==

 This proposal is an attempt to solve multiple issues with one single 
 solution.

 There have been various discussions in the past regarding a property-based
 API. Basically similar to the way controls are handled but without having
 to deal with the GUI-related aspects of controls and with more flexibility
 with regards to supported types.

 Another discussion is about how to atomically set certain pipeline 
 configurations,
 selections in particular. And for Android's camera3 library we also need to
 switch configuration on a per-buffer or per-frame basis, so we need some
 sort of configuration store.

 Also, I proposed an API extension for matrices:

 http://www.spinics.net/lists/linux-media/msg67326.html

 This would fit better in a property-based API.

 Recent discussions regarding multi-selection also ran into the same 
 atomicity
 problems and unhappiness about the impact of the API changes necessary to
 support multi-selection.

 It is quite easy to extend the control framework to support pretty much all
 of the requirements stated above. It already satisfies the atomicity 
 requirements
 (by far the hardest to implement), and extending it 

Re: RFC: Properties, Configuration Storage, Selections and Matrices

2013-12-06 Thread Sylwester Nawrocki

Hi Hans,

Thank you for working on this, and my apologies for not reviewing it 
earlier.


On 11/23/2013 03:07 PM, Hans Verkuil wrote:

Another update:

While attempting to add fake 'shadow register' support to vivi I realized that
the API to set up configuration stores didn't quite work out. Supporting
shadow registers is one of the main goals of the configuration storage idea, so
this should work smoothly.

In the original proposal a configuration store is set by VIDIOC_S_EXT_CTRLS and
any controls not in the control list will also be absent from the configuration
store. That makes no sense for shadow registers: i.e. you will always have N
sets of X registers and when switching from one set to another you switch them
all.

So the API has been changed a bit: all the driver-defined configuration stores
are always there and you can enumerate, get and set all the controls in each 
store.

If controls are implemented as shadow registers, then the driver will tell the
framework which of the configuration stores is the current store (i.e. which
store maps to the currently selected shadow register) and switching from one
configuration store to another will just update the current store of a control
to the new one.

If controls are not implemented as a shadow register and you really just want
to apply the new values from a configuration store, then that will actually
involve copying the values from the selected store to the current control 
values.

This approach made implementing fake shadow registers in vivi a trivial
exercise and worked out much better. In addition, internal data structures
became simpler as well (always a good sign).

My next step will be to add selection support as properties and see if I can
add per-frame crop/compose support to vivi.

Regards,

Hans

On 11/19/2013 09:10 AM, Hans Verkuil wrote:

Just a quick update:

I've been working on this for the past four days and I am now in the process
of testing this. Adding matrix support to the control framework was actually
the hardest part, particularly since I wanted it to be efficient. The code is
still a bit too complex for my liking, but once testing verifies that everything
is working I plan to clean things up some more.

I hope to be able to post a first version during or just after the weekend.

Please review this RFC: other than some small changes this is pretty much how
it will turn out.

Some of the changes are:

- The property bit is 26, not 27. Bit 27 clashes with the PRIVATE_BASE define so
it can't be used.

- Any control/property type can be changed into a matrix by setting bit 15 in
the type. E.g. V4l2_PROP_TYPE_MATRIX | V4L2_CTRL_TYPE_INTEGER. When passing the
data of a matrix the data is always preceded by a struct v4l2_rect which tells
the driver which subset of the matrix the application wants to set/get.

- We definitely need a V4L2_CTRL_FLAG_CAN_STORE flag. Stores take memory and you
only want to allocate that for those controls that actually need to be stored.

- Regarding implementing stores for matrices: I store it in the internal format,
but with additional information regarding the parts of the matrix that need to
be updated. Basically the best of both worlds. With some careful tweaking of
data structure this turned out to be fairly easy to do. This systems allows
you to update only parts of a matrix in a config store efficiently without
having to convert from one format to another.

Regards,

Hans

On 11/15/2013 03:19 PM, Hans Verkuil wrote:

Properties, Configuration Storage, Selections and Matrices
==

This proposal is an attempt to solve multiple issues with one single solution.

There have been various discussions in the past regarding a property-based
API. Basically similar to the way controls are handled but without having
to deal with the GUI-related aspects of controls and with more flexibility
with regards to supported types.

Another discussion is about how to atomically set certain pipeline 
configurations,
selections in particular. And for Android's camera3 library we also need to
switch configuration on a per-buffer or per-frame basis, so we need some
sort of configuration store.

Also, I proposed an API extension for matrices:

http://www.spinics.net/lists/linux-media/msg67326.html

This would fit better in a property-based API.

Recent discussions regarding multi-selection also ran into the same atomicity
problems and unhappiness about the impact of the API changes necessary to
support multi-selection.

It is quite easy to extend the control framework to support pretty much all
of the requirements stated above. It already satisfies the atomicity 
requirements
(by far the hardest to implement), and extending it to support compound,
array and matrix types isn't hard either. Neither is implementing a 
configuration
store.

One important observation regarding configuration stores: the information it
can contain should be information 

Re: RFC: Properties, Configuration Storage, Selections and Matrices

2013-11-23 Thread Hans Verkuil
Another update:

While attempting to add fake 'shadow register' support to vivi I realized that
the API to set up configuration stores didn't quite work out. Supporting
shadow registers is one of the main goals of the configuration storage idea, so
this should work smoothly.

In the original proposal a configuration store is set by VIDIOC_S_EXT_CTRLS and
any controls not in the control list will also be absent from the configuration
store. That makes no sense for shadow registers: i.e. you will always have N
sets of X registers and when switching from one set to another you switch them
all.

So the API has been changed a bit: all the driver-defined configuration stores
are always there and you can enumerate, get and set all the controls in each 
store.

If controls are implemented as shadow registers, then the driver will tell the
framework which of the configuration stores is the current store (i.e. which
store maps to the currently selected shadow register) and switching from one
configuration store to another will just update the current store of a control
to the new one.

If controls are not implemented as a shadow register and you really just want
to apply the new values from a configuration store, then that will actually
involve copying the values from the selected store to the current control 
values.

This approach made implementing fake shadow registers in vivi a trivial
exercise and worked out much better. In addition, internal data structures
became simpler as well (always a good sign).

My next step will be to add selection support as properties and see if I can
add per-frame crop/compose support to vivi.

Regards,

Hans

On 11/19/2013 09:10 AM, Hans Verkuil wrote:
 Just a quick update:
 
 I've been working on this for the past four days and I am now in the process
 of testing this. Adding matrix support to the control framework was actually
 the hardest part, particularly since I wanted it to be efficient. The code is
 still a bit too complex for my liking, but once testing verifies that 
 everything
 is working I plan to clean things up some more.
 
 I hope to be able to post a first version during or just after the weekend.
 
 Please review this RFC: other than some small changes this is pretty much how
 it will turn out.
 
 Some of the changes are:
 
 - The property bit is 26, not 27. Bit 27 clashes with the PRIVATE_BASE define 
 so
 it can't be used.
 
 - Any control/property type can be changed into a matrix by setting bit 15 in
 the type. E.g. V4l2_PROP_TYPE_MATRIX | V4L2_CTRL_TYPE_INTEGER. When passing 
 the
 data of a matrix the data is always preceded by a struct v4l2_rect which tells
 the driver which subset of the matrix the application wants to set/get.
 
 - We definitely need a V4L2_CTRL_FLAG_CAN_STORE flag. Stores take memory and 
 you
 only want to allocate that for those controls that actually need to be stored.
 
 - Regarding implementing stores for matrices: I store it in the internal 
 format,
 but with additional information regarding the parts of the matrix that need to
 be updated. Basically the best of both worlds. With some careful tweaking of
 data structure this turned out to be fairly easy to do. This systems allows
 you to update only parts of a matrix in a config store efficiently without
 having to convert from one format to another.
 
 Regards,
 
   Hans
 
 On 11/15/2013 03:19 PM, Hans Verkuil wrote:
 Properties, Configuration Storage, Selections and Matrices
 ==

 This proposal is an attempt to solve multiple issues with one single 
 solution.

 There have been various discussions in the past regarding a property-based
 API. Basically similar to the way controls are handled but without having
 to deal with the GUI-related aspects of controls and with more flexibility
 with regards to supported types.

 Another discussion is about how to atomically set certain pipeline 
 configurations,
 selections in particular. And for Android's camera3 library we also need to
 switch configuration on a per-buffer or per-frame basis, so we need some
 sort of configuration store.

 Also, I proposed an API extension for matrices:

 http://www.spinics.net/lists/linux-media/msg67326.html

 This would fit better in a property-based API.

 Recent discussions regarding multi-selection also ran into the same atomicity
 problems and unhappiness about the impact of the API changes necessary to
 support multi-selection.

 It is quite easy to extend the control framework to support pretty much all
 of the requirements stated above. It already satisfies the atomicity 
 requirements
 (by far the hardest to implement), and extending it to support compound,
 array and matrix types isn't hard either. Neither is implementing a 
 configuration
 store.

 One important observation regarding configuration stores: the information it
 can contain should be information that can change while streaming. If it
 can't be changed while 

Re: RFC: Properties, Configuration Storage, Selections and Matrices

2013-11-19 Thread Hans Verkuil
Just a quick update:

I've been working on this for the past four days and I am now in the process
of testing this. Adding matrix support to the control framework was actually
the hardest part, particularly since I wanted it to be efficient. The code is
still a bit too complex for my liking, but once testing verifies that everything
is working I plan to clean things up some more.

I hope to be able to post a first version during or just after the weekend.

Please review this RFC: other than some small changes this is pretty much how
it will turn out.

Some of the changes are:

- The property bit is 26, not 27. Bit 27 clashes with the PRIVATE_BASE define so
it can't be used.

- Any control/property type can be changed into a matrix by setting bit 15 in
the type. E.g. V4l2_PROP_TYPE_MATRIX | V4L2_CTRL_TYPE_INTEGER. When passing the
data of a matrix the data is always preceded by a struct v4l2_rect which tells
the driver which subset of the matrix the application wants to set/get.

- We definitely need a V4L2_CTRL_FLAG_CAN_STORE flag. Stores take memory and you
only want to allocate that for those controls that actually need to be stored.

- Regarding implementing stores for matrices: I store it in the internal format,
but with additional information regarding the parts of the matrix that need to
be updated. Basically the best of both worlds. With some careful tweaking of
data structure this turned out to be fairly easy to do. This systems allows
you to update only parts of a matrix in a config store efficiently without
having to convert from one format to another.

Regards,

Hans

On 11/15/2013 03:19 PM, Hans Verkuil wrote:
 Properties, Configuration Storage, Selections and Matrices
 ==
 
 This proposal is an attempt to solve multiple issues with one single solution.
 
 There have been various discussions in the past regarding a property-based
 API. Basically similar to the way controls are handled but without having
 to deal with the GUI-related aspects of controls and with more flexibility
 with regards to supported types.
 
 Another discussion is about how to atomically set certain pipeline 
 configurations,
 selections in particular. And for Android's camera3 library we also need to
 switch configuration on a per-buffer or per-frame basis, so we need some
 sort of configuration store.
 
 Also, I proposed an API extension for matrices:
 
 http://www.spinics.net/lists/linux-media/msg67326.html
 
 This would fit better in a property-based API.
 
 Recent discussions regarding multi-selection also ran into the same atomicity
 problems and unhappiness about the impact of the API changes necessary to
 support multi-selection.
 
 It is quite easy to extend the control framework to support pretty much all
 of the requirements stated above. It already satisfies the atomicity 
 requirements
 (by far the hardest to implement), and extending it to support compound,
 array and matrix types isn't hard either. Neither is implementing a 
 configuration
 store.
 
 One important observation regarding configuration stores: the information it
 can contain should be information that can change while streaming. If it
 can't be changed while streaming, then there is no reason to put it in a
 config store. Looking at all the ioctls we have there are really only a few
 types of ioctls for which this requirement is true: controls and selections
 being the primary ones.
 
 S_PARM for setting the timeperframe is another. S_INPUT/OUTPUT might be
 useful as well for surveillance apps to switch input/output between frames.
 
 That's not many and it wouldn't be hard to implement that functionality as
 properties.
 
 
 Property API Proposal
 =
 
 Step 1: Property IDs
 
 All properties have bit 27 (0x0800) set. So V4L2_CTRL_ID2CLASS(propid)
 will always return a value between 0x0800 and 0x0fff.
 
 For controls that range is 0x0098 - 0x07ff. It starts at 0x0098
 due to historical reasons.
 
 The existing V4L2_CTRL_FLAG_NEXT_CTRL flag will only enumerate controls.
 A new flag V4L2_CTRL_FLAG_NEXT_PROP (0x4000) is added to enumerate
 properties. If you want to enumerate both, then both flags need to be set.
 
 Question: should we still use classes to group properties as we do with
 controls? Personally I like classes as it enforces a system on what
 otherwise will be a big pile of random IDs.
 
 If we keep using classes, then we can decide on making a 1-to-1 mapping
 between the classes used for controls and the classes used for properties:
 e.g. you have V4L2_CTRL_CLASS_MPEG (0x0099) and V4L2_PROP_CLASS_MPEG
 (0x0899).
 
 Properties have a V4L2_PID_ prefix instead of _CID_.
 
 
 Step 2: Property Types
 
 The enum v4l2_ctrl_type can be extended with new property types. The matrix
 proposal referred to above requires u8 and u16 matrix types in order to
 implement the motion detection API:
 
 struct v4l2_prop_type_matrix_u8 {

RFC: Properties, Configuration Storage, Selections and Matrices

2013-11-15 Thread Hans Verkuil
Properties, Configuration Storage, Selections and Matrices
==

This proposal is an attempt to solve multiple issues with one single solution.

There have been various discussions in the past regarding a property-based
API. Basically similar to the way controls are handled but without having
to deal with the GUI-related aspects of controls and with more flexibility
with regards to supported types.

Another discussion is about how to atomically set certain pipeline 
configurations,
selections in particular. And for Android's camera3 library we also need to
switch configuration on a per-buffer or per-frame basis, so we need some
sort of configuration store.

Also, I proposed an API extension for matrices:

http://www.spinics.net/lists/linux-media/msg67326.html

This would fit better in a property-based API.

Recent discussions regarding multi-selection also ran into the same atomicity
problems and unhappiness about the impact of the API changes necessary to
support multi-selection.

It is quite easy to extend the control framework to support pretty much all
of the requirements stated above. It already satisfies the atomicity 
requirements
(by far the hardest to implement), and extending it to support compound,
array and matrix types isn't hard either. Neither is implementing a 
configuration
store.

One important observation regarding configuration stores: the information it
can contain should be information that can change while streaming. If it
can't be changed while streaming, then there is no reason to put it in a
config store. Looking at all the ioctls we have there are really only a few
types of ioctls for which this requirement is true: controls and selections
being the primary ones.

S_PARM for setting the timeperframe is another. S_INPUT/OUTPUT might be
useful as well for surveillance apps to switch input/output between frames.

That's not many and it wouldn't be hard to implement that functionality as
properties.


Property API Proposal
=

Step 1: Property IDs

All properties have bit 27 (0x0800) set. So V4L2_CTRL_ID2CLASS(propid)
will always return a value between 0x0800 and 0x0fff.

For controls that range is 0x0098 - 0x07ff. It starts at 0x0098
due to historical reasons.

The existing V4L2_CTRL_FLAG_NEXT_CTRL flag will only enumerate controls.
A new flag V4L2_CTRL_FLAG_NEXT_PROP (0x4000) is added to enumerate
properties. If you want to enumerate both, then both flags need to be set.

Question: should we still use classes to group properties as we do with
controls? Personally I like classes as it enforces a system on what
otherwise will be a big pile of random IDs.

If we keep using classes, then we can decide on making a 1-to-1 mapping
between the classes used for controls and the classes used for properties:
e.g. you have V4L2_CTRL_CLASS_MPEG (0x0099) and V4L2_PROP_CLASS_MPEG
(0x0899).

Properties have a V4L2_PID_ prefix instead of _CID_.


Step 2: Property Types

The enum v4l2_ctrl_type can be extended with new property types. The matrix
proposal referred to above requires u8 and u16 matrix types in order to
implement the motion detection API:

struct v4l2_prop_type_matrix_u8 {
struct v4l2_rect r;
__u8 data[];
};

struct v4l2_prop_type_matrix_u16 {
struct v4l2_rect r;
__u16 data[];
};

V4l2_PROP_TYPE_MATRIX_U8  = 0x100,
V4l2_PROP_TYPE_MATRIX_U16,

All property types start at 0x100, 0x01-0xff is reserved for control types.

The v4l2_rect in the matrix type allows you to set only a subset of a matrix.
Note that an array is a one-dimensional matrix, so I do not see any need to
add specific array support. Yes, higher-dimensions are possible but I have
never seen it in any hardware related to video capture. Should we ever see
that, then we can add a new property type for that.

In order to support compound or matrix types the v4l2_ext_control struct
has to be extended:

struct v4l2_ext_control {
__u32 id;
__u32 size;
__u32 reserved2[1];
union {
__s32 value;
__s64 value64;
char *string;
void *prop;
};
} __attribute__ ((packed));

All types = 0x100 are assumed to use the prop field and the 'size' field 
contains
the total size of the data prop points to. If a property fits one of the 
existing
control types, then those should be used of course.


Step 3: Querying Properties

You need to be able to query the properties as well. v4l2_queryctrl is no
longer sufficient, so we add a struct v4l2_query_ext_ctrl and 
VIDIOC_QUERY_EXT_CTRL
instead.

I thought about naming it v4l2_queryprop, but it can be used with extended
controls as well, and since all the existing naming is centered around _ext_ctrl
I think this is more consistent.

struct v4l2_query_ext_ctrl {
__u32config_store;
__u32id;