Re: RFC: Properties, Configuration Storage, Selections and Matrices
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
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
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
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
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;