[libcamera-devel] [PATCH v8 2/4] libcamera: controls: Define size of array controls as a shape vector
Christian Rauch
Rauch.Christian at gmx.de
Tue Jul 5 02:14:10 CEST 2022
Am 05.07.22 um 00:50 schrieb Laurent Pinchart:
> On Tue, Jul 05, 2022 at 02:37:57AM +0300, Laurent Pinchart via libcamera-devel wrote:
>> 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.
>
> Just a quick note on this. In the review of previous versions, a syntax
> such as [,] was proposed for a 2D NxN array, and [,3] for a Nx3 array.
> This won't work correctly, as neither are valid YAML syntax.
>
> One workaround for this would be to turn the size element into a string,
> e.g.
>
> size: '[,]'
>
> I don't like this much. Another option would be to reuse 'n', e.g. [n,n]
> and [n,3]. If we go that way, we should keep [n] already to denote a
> dynamic 1D array.
>
> Any other option ?
In such a case, I would use "0" as the magic value to indicate a
variable size dimension, since a 0-size will never be used in practice.
With this syntax, "[0,3]" would translate into a "N x 3" sized array and
"[0,2,0]" into "N x 2 x M". We would need to parse this and replace all
occurrences of 0 with a dynamic-sized length, but the sum will then
still mean the "minimum total size".
I am not a fan of using a character (e.g. 'n') to indicate this, as this
is a bit arbitrary and "[n,3,n]" could mean that the first and last
dimension have to have the same length "n".
>
>> 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
>
More information about the libcamera-devel
mailing list