| Message ID | 20220408014231.231083-2-Rauch.Christian@gmx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Christian, Thank you for the patch. On Fri, Apr 08, 2022 at 02:42:28AM +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 this, but it causes a compilation failure: 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:402: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:177: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:189: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 think you'll have to update the gen-controls.py script in the same patch to take the change into account. Patch 2/4 can then move to fixed-extent spans. > Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de> > --- > src/libcamera/control_ids.yaml | 2 +- > src/libcamera/property_ids.yaml | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > index 9d4638ae..c3f593a1 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 > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml > index 12ecbce5..47c350ed 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
Hi Christian, On Fri, Apr 08, 2022 at 02:42:28AM +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 see, so you can just chain comma-separated values to add more dimensions, neat. So if you have a variable-length array of objects of size 3 then you'll have [3,] ? I suppose it's not that bad. > > Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de> By the way, you have a Reviewed-by tag from Jacopo on v1. Paul > --- > src/libcamera/control_ids.yaml | 2 +- > src/libcamera/property_ids.yaml | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > index 9d4638ae..c3f593a1 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 > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml > index 12ecbce5..47c350ed 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 > -- > 2.25.1 >
Hi Paul, Am 20.04.22 um 03:07 schrieb paul.elder@ideasonboard.com: > Hi Christian, > > On Fri, Apr 08, 2022 at 02:42:28AM +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 see, so you can just chain comma-separated values to add more > dimensions, neat. > > So if you have a variable-length array of objects of size 3 then you'll > have [3,] ? I suppose it's not that bad. Spans are 1-dimensional for now and all documented dimensions are "flattened". Having multiple dimensions is really only useful for documentation purposes. Once multidimensional Spans or tensors are supported, their dimensionality definition could be implemented that way. E.g. "[3,,]" could be interpreted as a tensor with fixed-size first dimension and variable-sized second and third dimension. > >> >> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de> > > By the way, you have a Reviewed-by tag from Jacopo on v1. I am afraid, I don't know how these things work. Do I just copy & paste this into my commit message or do I have to "import" this reviewed commit into my branch again? Best, Christian > > > Paul > >> --- >> src/libcamera/control_ids.yaml | 2 +- >> src/libcamera/property_ids.yaml | 4 ++-- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml >> index 9d4638ae..c3f593a1 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 >> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml >> index 12ecbce5..47c350ed 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 >> -- >> 2.25.1 >>
Hi Christian On Tue, Apr 26, 2022 at 12:50:17AM +0100, Christian Rauch via libcamera-devel wrote: > Hi Paul, > > Am 20.04.22 um 03:07 schrieb paul.elder@ideasonboard.com: > > Hi Christian, > > > > On Fri, Apr 08, 2022 at 02:42:28AM +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 see, so you can just chain comma-separated values to add more > > dimensions, neat. > > > > So if you have a variable-length array of objects of size 3 then you'll > > have [3,] ? I suppose it's not that bad. > > Spans are 1-dimensional for now and all documented dimensions are > "flattened". Having multiple dimensions is really only useful for > documentation purposes. > Once multidimensional Spans or tensors are supported, their > dimensionality definition could be implemented that way. E.g. "[3,,]" > could be interpreted as a tensor with fixed-size first dimension and > variable-sized second and third dimension. > > > >> > >> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de> > > > > By the way, you have a Reviewed-by tag from Jacopo on v1. > > I am afraid, I don't know how these things work. Do I just copy & paste > this into my commit message or do I have to "import" this reviewed > commit into my branch again? > Yes, just copy and paste tags you have received in the new version of the patch file. If changes between versions are considerable, you might decide to drop the tags. Thanks j > Best, > Christian > > > > > > Paul > > > >> --- > >> src/libcamera/control_ids.yaml | 2 +- > >> src/libcamera/property_ids.yaml | 4 ++-- > >> 2 files changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > >> index 9d4638ae..c3f593a1 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 > >> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml > >> index 12ecbce5..47c350ed 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 > >> -- > >> 2.25.1 > >>
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml index 9d4638ae..c3f593a1 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 diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml index 12ecbce5..47c350ed 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
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. Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de> --- src/libcamera/control_ids.yaml | 2 +- src/libcamera/property_ids.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.25.1