Re: [RFCv2 PATCH 06/21] v4l2-ctrls: add support for complex types.

[Hans Verkuil <hverkuil@xs4all.nl>]

Hi Sakari,

On 01/24/2014 04:44 PM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Jan 20, 2014 at 01:45:59PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> This patch implements initial support for complex types.
>>
>> For the most part the changes are fairly obvious (basic support for is_ptr
>> types, the type_is_int function is replaced by a is_int bitfield, and
>> v4l2_query_ext_ctrl is added), but one change needs more explanation:
>>
>> The v4l2_ctrl struct adds a 'new' field and a 'stores' array at the end
>> of the struct. This is in preparation for future patches where each control
>> can have multiple configuration stores. The idea is that stores[0] is the current
>> control value, stores[1] etc. are the control values for each configuration store
>> and the 'new' value can be accessed through 'stores[-1]', i.e. the 'new' field.
>> However, for now only stores[-1] and stores[0] is used.
> 
> Could we use zero or positive indices only, e.g. the new being zero and
> current 1, or the other way? Or make the "new" value special, i.e. using a
> different field name.
> 
> I think accessing the previous struct member by index -1 looks a little bit
> hackish.

I should document this better. Drivers are not expected to use the stores array,
it is for internal use in the control framework only as it allows me to refer to
either the new or the current value by just an array index and later also
configuration stores which will start at index 1 and up, which is where this really
becomes important.

I am not yet adding configuration stores to the control framework as there is not
yet a driver that needs it, and it is for the most part a separate issue anyway.
But this generalization of how values can be accessed makes it much easier to
later add support for configuration stores.

>> These new fields use the v4l2_ctrl_ptr union, which is a pointer to a control
>> value.
>>
>> Note that these two new fields are not yet actually used.
> 
> Should they be then added yet in the first place? :-)

Well, they are used a few patches later, but I will see if it makes sense to only
introduce them when they are actually needed.

Regards,

	Hans