[0/3] libcamera: Make ControlValue a view
mbox series

Message ID 20250806-control_storage-v1-0-2ec8424f6f7d@ideasonboard.com
Headers show
Series
  • libcamera: Make ControlValue a view
Related show

Message

Jacopo Mondi Aug. 6, 2025, 12:30 p.m. UTC
This series picks Barnabas patch

[PATCH RFC 1/3] libcamera: controls: Add `ControlValueView`

and on top of that one it makes the existing ControlValue a view
re-naming it to ControlStorage.

The final goal is to prepare for the introduction of MetadataList and
make sure it expose the same types to applications as ControlList does.

MetadataList stores control values in a serialized buffer of binary
data, while ControlList at the moment stores controls in a map of <id,
ControlValue>

For this reason MetadataList, not storing values in a map of
ControlValue, cannot use ControlValue in their interface but instead
expose a view-like interface which gives to application a read-only
access to the underlying storage. I think that's desirable for
ControlList as well, even more now that we're preparing to get towards a
C ABI.

On top of this series, it would be possible to apply Barnabas
https://patchwork.libcamera.org/project/libcamera/list/?series=5135
that gives to ControlValue (now ControlStorage) a move interface.

I would even consider going further and make it a move-only type, so
that a time there is a single storage for a ControlValue.

Sending as RFC to collect opinions, as this certainly is an ABI breaking
change.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
Barnabás Pőcze (1):
      libcamera: controls: Add `ControlValueView`

Jacopo Mondi (2):
      libcamera: Rename ControlValue to ControlStorage
      libcamera: Make ControlValue a view

 include/libcamera/control_ids.h.in                 |   2 +-
 include/libcamera/controls.h                       | 133 +++++--
 include/libcamera/internal/control_serializer.h    |   2 +-
 include/libcamera/internal/debug_controls.h        |   2 +-
 include/libcamera/internal/delayed_controls.h      |   6 +-
 src/apps/cam/capture_script.cpp                    |  16 +-
 src/apps/cam/capture_script.h                      |   8 +-
 src/ipa/libipa/agc_mean_luminance.cpp              |   4 +-
 src/ipa/libipa/awb.cpp                             |   4 +-
 src/ipa/libipa/awb.h                               |   2 +-
 src/ipa/rkisp1/algorithms/agc.cpp                  |  16 +-
 src/ipa/rkisp1/algorithms/ccm.cpp                  |   6 +-
 src/ipa/rpi/common/ipa_base.cpp                    |  18 +-
 src/ipa/rpi/pisp/pisp.cpp                          |   2 +-
 src/ipa/rpi/vc4/vc4.cpp                            |  20 +-
 src/libcamera/control_ids.cpp.in                   |   2 +-
 src/libcamera/control_serializer.cpp               |  14 +-
 src/libcamera/controls.cpp                         | 399 ++++++++++++++-------
 src/libcamera/debug_controls.cpp                   |   4 +-
 src/libcamera/ipa_controls.cpp                     |   2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp               |   4 +-
 .../pipeline/rpi/common/delayed_controls.h         |   6 +-
 .../pipeline/rpi/common/pipeline_base.cpp          |   2 +-
 src/libcamera/pipeline/rpi/vc4/vc4.cpp             |   4 +-
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       |  10 +-
 src/libcamera/pipeline/virtual/config_parser.cpp   |   2 +-
 src/libcamera/sensor/camera_sensor_legacy.cpp      |   2 +-
 src/libcamera/sensor/camera_sensor_raw.cpp         |   2 +-
 src/libcamera/v4l2_device.cpp                      |  14 +-
 src/py/libcamera/py_helpers.cpp                    |  18 +-
 src/py/libcamera/py_helpers.h                      |   2 +-
 test/controls/control_value.cpp                    |   8 +-
 test/serialization/serialization_test.cpp          |  12 +-
 test/v4l2_videodevice/controls.cpp                 |   4 +-
 34 files changed, 478 insertions(+), 274 deletions(-)
---
base-commit: 7a42f3c3d88926aa05b07d9c6a783bdbbfb73610
change-id: 20250803-control_storage-8972535fed5f

Best regards,

Comments

Kieran Bingham Aug. 7, 2025, 2:32 p.m. UTC | #1
Quoting Jacopo Mondi (2025-08-06 13:30:44)
> This series picks Barnabas patch
> 
> [PATCH RFC 1/3] libcamera: controls: Add `ControlValueView`
> 
> and on top of that one it makes the existing ControlValue a view
> re-naming it to ControlStorage.
> 
> The final goal is to prepare for the introduction of MetadataList and
> make sure it expose the same types to applications as ControlList does.
> 
> MetadataList stores control values in a serialized buffer of binary
> data, while ControlList at the moment stores controls in a map of <id,
> ControlValue>
> 
> For this reason MetadataList, not storing values in a map of
> ControlValue, cannot use ControlValue in their interface but instead
> expose a view-like interface which gives to application a read-only
> access to the underlying storage. I think that's desirable for
> ControlList as well, even more now that we're preparing to get towards a
> C ABI.
> 
> On top of this series, it would be possible to apply Barnabas
> https://patchwork.libcamera.org/project/libcamera/list/?series=5135
> that gives to ControlValue (now ControlStorage) a move interface.
> 
> I would even consider going further and make it a move-only type, so
> that a time there is a single storage for a ControlValue.
> 
> Sending as RFC to collect opinions, as this certainly is an ABI breaking
> change.

Nows a good time ;-)

https://lists.libcamera.org/pipermail/libcamera-devel/2025-August/052182.html

--
Kieran

> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
> Barnabás Pőcze (1):
>       libcamera: controls: Add `ControlValueView`
> 
> Jacopo Mondi (2):
>       libcamera: Rename ControlValue to ControlStorage
>       libcamera: Make ControlValue a view
> 
>  include/libcamera/control_ids.h.in                 |   2 +-
>  include/libcamera/controls.h                       | 133 +++++--
>  include/libcamera/internal/control_serializer.h    |   2 +-
>  include/libcamera/internal/debug_controls.h        |   2 +-
>  include/libcamera/internal/delayed_controls.h      |   6 +-
>  src/apps/cam/capture_script.cpp                    |  16 +-
>  src/apps/cam/capture_script.h                      |   8 +-
>  src/ipa/libipa/agc_mean_luminance.cpp              |   4 +-
>  src/ipa/libipa/awb.cpp                             |   4 +-
>  src/ipa/libipa/awb.h                               |   2 +-
>  src/ipa/rkisp1/algorithms/agc.cpp                  |  16 +-
>  src/ipa/rkisp1/algorithms/ccm.cpp                  |   6 +-
>  src/ipa/rpi/common/ipa_base.cpp                    |  18 +-
>  src/ipa/rpi/pisp/pisp.cpp                          |   2 +-
>  src/ipa/rpi/vc4/vc4.cpp                            |  20 +-
>  src/libcamera/control_ids.cpp.in                   |   2 +-
>  src/libcamera/control_serializer.cpp               |  14 +-
>  src/libcamera/controls.cpp                         | 399 ++++++++++++++-------
>  src/libcamera/debug_controls.cpp                   |   4 +-
>  src/libcamera/ipa_controls.cpp                     |   2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp               |   4 +-
>  .../pipeline/rpi/common/delayed_controls.h         |   6 +-
>  .../pipeline/rpi/common/pipeline_base.cpp          |   2 +-
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp             |   4 +-
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       |  10 +-
>  src/libcamera/pipeline/virtual/config_parser.cpp   |   2 +-
>  src/libcamera/sensor/camera_sensor_legacy.cpp      |   2 +-
>  src/libcamera/sensor/camera_sensor_raw.cpp         |   2 +-
>  src/libcamera/v4l2_device.cpp                      |  14 +-
>  src/py/libcamera/py_helpers.cpp                    |  18 +-
>  src/py/libcamera/py_helpers.h                      |   2 +-
>  test/controls/control_value.cpp                    |   8 +-
>  test/serialization/serialization_test.cpp          |  12 +-
>  test/v4l2_videodevice/controls.cpp                 |   4 +-
>  34 files changed, 478 insertions(+), 274 deletions(-)
> ---
> base-commit: 7a42f3c3d88926aa05b07d9c6a783bdbbfb73610
> change-id: 20250803-control_storage-8972535fed5f
> 
> Best regards,
> -- 
> Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>