[libcamera-devel,0/5] libcamera: Control framework backend rework

1978 mbox series

Hello,
   this series is the first half of the currently in progress serialization
support work.

The basic idea is that to be able to serialize Controls and V4L2Controls
using the same API, their back-end storage should be unified to use the same
polymorphic data storage class. This was the initial plan to be honest, but it
didn't match the urgent need to get controls support in at the time, but it's
now required for serialization, so it's now a good time to fix this long
standing issue.

The series addresses the issue taking the following steps.
1) Break out the DataValue class from the ControlValue class and associate it
   with a DataInfo class, that is meant to support constant validation
   information associated with a value.

   The mapping will be
   V4L2Control = Control = DataValue
   V4L2ControlInfo = ControlInfo = DataInfo

   With V4L2ControlList being a list of V4L2Controls
   and ControlList being a map of Controls associated to a map of
       ControlInfo both indexed by ControlId

   [ later considerations: when writing below about the API of ControlList and
     V4L2ControlList I realized they're effectively maps of ids to values, and
     naming them "Map" would make the syntax "ControlMap[id] = value" more
     natural. What do you think? ]

   All of this in patches 1, 2, 3

2) Serialization implies de-serialization, and to construct a ControlList we
   need a camera. Break this dependency in patch 4/5 to be able to reconstruct
   a ControlList from serialized data later

3) Bikeshedders heaven: rewording of comments and rename of data types in
   Control framework. As explained in the commit message of 5/5, the rewording
   tries to enforce the following concepts: control metadata, control info,
   and control values.

On top of this I have patches and tests for Serialization, but not yet for
de-serialization. I hope not, but it might be that implementing it
require changes in this series, so be prepared.

All of this for the back-end storage.

The user APIs: there is work there too, as both V4L2Controls and Controls might
need re-thinking of their user API. The fundamental difference between the two
(and it took me a while to realize the implication of this in the API
definition) it's where validation information are kept.

ControlList is created with a reference to a ControlInfoMap used for validation
and produced by pipeline handlers at camera registration time. This allows to
validate a control at the moment it is added to the list, as doing so when
queuing requests is indeed too late for a safe recovery.

V4L2Control list only contains values. The validation informations are kept in
the device the list is applied on. I think this still makes sense, as V4L2
controls are meant to be validate at the time they are immediately applied to
the device by the driver exposing them, and they will not be exposed to
applications. Anyway, creating a V4L2ControlList from a device might make
serialization easier because both values and information would then be kept
in the same class, as it happens now for Controls.

The other difference in the API is a matter of tastes mostly. Controls use the
overloaded operator[] to set/get controls, V4L2Controls have explicit get/set.
Having implemented them, it's clear that my preference goes toward explicit
get/set, as operator overloading makes me always a bit unconfortable and I
prefer to type in 3 letters more for set/get but have an explicit indication of
what's happening. Anyway, the use of [] made in ControlList is pretty natural
and does what you would expect it to do (maybe if it was a ControlMap it would
feel more natural to do controls[x] = y ? Now that I wrote it here, I'll suggest
it in the above comment too) and seems to be also in line with
https://google.github.io/styleguide/cppguide.html "Operator Overloading"
section. So I'll go with popular preference, but in any case I think the two
should be unified.

As said, serialization will come on top of this work, so please comment to avoid
me too many re-bases :)

Thanks
   j

Jacopo Mondi (5):
  libcamera: Create DataValue and DataInfo
  libcamera: controls: Use DataType and DataValue
  libcamera: v4l2_controls: Use DataValue and DataInfo
  libcamera: controls: De-couple Controls from Camera
  libcamera: controls: Control framework refresh

 include/libcamera/controls.h                  |  94 +----
 include/libcamera/data_value.h                |  84 ++++
 include/libcamera/meson.build                 |   1 +
 src/libcamera/controls.cpp                    | 398 +++++-------------
 src/libcamera/data_value.cpp                  | 317 ++++++++++++++
 src/libcamera/gen-controls.awk                |   4 +-
 src/libcamera/include/v4l2_controls.h         |  22 +-
 src/libcamera/include/v4l2_device.h           |   1 -
 src/libcamera/meson.build                     |   7 +-
 src/libcamera/pipeline/uvcvideo.cpp           |   2 +-
 src/libcamera/pipeline/vimc.cpp               |   2 +-
 src/libcamera/request.cpp                     |   4 +-
 src/libcamera/v4l2_controls.cpp               |  49 +--
 src/libcamera/v4l2_device.cpp                 |  25 +-
 test/controls/control_info.cpp                |   4 +-
 test/controls/control_list.cpp                |   6 +-
 test/controls/meson.build                     |   1 -
 .../data_value.cpp}                           |  24 +-
 test/data_value/meson.build                   |  12 +
 test/meson.build                              |   1 +
 20 files changed, 593 insertions(+), 465 deletions(-)
 create mode 100644 include/libcamera/data_value.h
 create mode 100644 src/libcamera/data_value.cpp
 rename test/{controls/control_value.cpp => data_value/data_value.cpp} (68%)
 create mode 100644 test/data_value/meson.build

--
2.23.0