[libcamera-devel] [PATCH v8 2/4] libcamera: controls: Define size of array controls as a shape vector

[Laurent Pinchart]

Hi Christian,

Thank you for the patch.

On Fri, Jun 10, 2022 at 01:03:36PM +0100, Christian Rauch via libcamera-devel wrote:
> This follows the convention in other Tensor APIs. Since all tensors are
> represented as a Span with a single dimension, values provided in 'size'
> are interpreted as fixed-size Spans, while an empty array ("[]") will be
> interpreted as variable-sized Span.

I like the new syntax, even if it doesn't make it clear how we'll handle
multi-dimensional arrays with variable numbers of elements in one or
multiple dimensions, but we can worry about that later when/if needed.

This patch by itself introduces compilation errors, for instance, with
clang-14,

In file included from ../../src/libcamera/camera_sensor.cpp:8:
In file included from ../../include/libcamera/internal/camera_sensor.h:17:
In file included from include/libcamera/control_ids.h:15:
../../include/libcamera/controls.h:403:8: error: no matching member function for call to 'set'
                val->set<T>(Span<const typename std::remove_cv_t<V>>{ value.begin(), value.size() });
                ~~~~~^~~~~~
../../src/libcamera/camera_sensor.cpp:421:14: note: in instantiation of function template specialization 'libcamera::ControlList::set<libcamera::Rectangle, libcamera::Rectangle>' requested here
        properties_.set(properties::PixelArrayActiveAreas, { activeArea_ });
                    ^
../../include/libcamera/controls.h:178:7: note: candidate function template not viable: no known conversion from 'Span<const typename std::remove_cv_t<Rectangle>>' (aka 'Span<const libcamera::Rectangle>') to 'const libcamera::Rectangle' for 1st argument
        void set(const T &value)
             ^
../../include/libcamera/controls.h:190:7: note: candidate template ignored: requirement 'details::is_span<libcamera::Rectangle>::value || std::is_same<std::string, libcamera::Rectangle>::value' was not satisfied [with T = libcamera::Rectangle]
        void set(const T &value)
             ^
1 error generated.

I'll update the Python generator script here to fix this, it will be
part of v9.

> Signed-off-by: Christian Rauch <Rauch.Christian at gmx.de>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/control_ids.yaml  | 4 ++--
>  src/libcamera/property_ids.yaml | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index cd1d4512..f707c1f5 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -291,7 +291,7 @@ controls:
>          transformation. The 3x3 matrix is stored in conventional reading
>          order in an array of 9 floating point values.
> 
> -      size: [3x3]
> +      size: [3,3]
> 
>    - ScalerCrop:
>        type: Rectangle
> @@ -515,7 +515,7 @@ controls:
>          the window where the focal distance for the objects shown in that part
>          of the image are closest to the camera.
> 
> -      size: [n]
> +      size: []
> 
>    - AfTrigger:
>        type: int32_t
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 11b7ebdc..a87485d7 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -497,7 +497,7 @@ controls:
> 
>    - PixelArrayOpticalBlackRectangles:
>        type: Rectangle
> -      size: [n]
> +      size: []
>        description: |
>          The pixel array region(s) which contain optical black pixels
>          considered valid for calibration purposes.
> @@ -592,7 +592,7 @@ controls:
> 
>    - PixelArrayActiveAreas:
>        type: Rectangle
> -      size: [n]
> +      size: []
>        description: |
>          The PixelArrayActiveAreas property defines the (possibly multiple and
>          overlapping) portions of the camera sensor readable pixel matrix

-- 
Regards,

Laurent Pinchart