| Message ID | 20260819141541.221617-4-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Quoting Jacopo Mondi (2026-08-20 12:20:25) > Hi Stefan > > On Wed, Aug 19, 2026 at 04:15:16PM +0200, Stefan Klug wrote: > > Add a IPACameraSensorInfo::pixelArraySize property to communicate the > > full size of the pixel array to the IPA. This property is not used today > > but will be useful to do sanity checks on coordinates handled in the > > algorithms. > > Do you already have a use case in mind ? I'd like to migrate the LSC algorithm to read the reference image crop (that was used during tuning) directly from the tuning file instead of assuming the active area. In that case I'd like to do a validity check that the rect is inside pixelArraySize. But that's all. Best regards, Stefan > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > include/libcamera/ipa/core.mojom | 6 ++++++ > > src/libcamera/sensor/camera_sensor_legacy.cpp | 1 + > > src/libcamera/sensor/camera_sensor_raw.cpp | 1 + > > 3 files changed, 8 insertions(+) > > > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom > > index d3375333baa0..8c0fc2165baa 100644 > > --- a/include/libcamera/ipa/core.mojom > > +++ b/include/libcamera/ipa/core.mojom > > @@ -152,6 +152,11 @@ module libcamera; > > * RGB for sensors that don't have a CFA. > > */ > > > > +/** > > + * \var IPACameraSensorInfo::pixelArraySize > > + * \brief The size of the physical pixel array of the sensor in pixels > > + */ > > + > > /** > > * \var IPACameraSensorInfo::activeArea > > * \brief The active pixel area of the sensor, relative to the physical pixel > > @@ -250,6 +255,7 @@ struct IPACameraSensorInfo { > > uint32 bitsPerPixel; > > uint32 cfaPattern; > > > > + Size pixelArraySize; > > Rectangle activeArea; > > Rectangle analogCrop; > > Size outputSize; > > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > > index 9e32f798c2dd..18941666fb4c 100644 > > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > > @@ -881,6 +881,7 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const > > return -EINVAL; > > > > info->model = model(); > > + info->pixelArraySize = pixelArraySize_; > > > > /* > > * The active area is a static property, while the crop rectangle needs > > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > > index a1f48b5ff557..4b0b8ff6fc2b 100644 > > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > > @@ -1007,6 +1007,7 @@ int CameraSensorRaw::setEmbeddedDataEnabled(bool enable) > > int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const > > { > > info->model = model(); > > + info->pixelArraySize = pixelArraySize_; > > > > /* > > * The active area is a static property, while the crop rectangle needs > > -- > > 2.53.0 > >
Hi Stefan On Thu, Aug 20, 2026 at 01:51:35PM +0200, Stefan Klug wrote: > Hi Jacopo, > > Quoting Jacopo Mondi (2026-08-20 12:20:25) > > Hi Stefan > > > > On Wed, Aug 19, 2026 at 04:15:16PM +0200, Stefan Klug wrote: > > > Add a IPACameraSensorInfo::pixelArraySize property to communicate the > > > full size of the pixel array to the IPA. This property is not used today > > > but will be useful to do sanity checks on coordinates handled in the > > > algorithms. > > > > Do you already have a use case in mind ? > > I'd like to migrate the LSC algorithm to read the reference image crop > (that was used during tuning) directly from the tuning file instead of > assuming the active area. In that case I'd like to do a validity check > that the rect is inside pixelArraySize. But that's all. > I see.. would you prefer to add this one right away, or when you'll implement the above use case ? > Best regards, > Stefan > > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > include/libcamera/ipa/core.mojom | 6 ++++++ > > > src/libcamera/sensor/camera_sensor_legacy.cpp | 1 + > > > src/libcamera/sensor/camera_sensor_raw.cpp | 1 + > > > 3 files changed, 8 insertions(+) > > > > > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom > > > index d3375333baa0..8c0fc2165baa 100644 > > > --- a/include/libcamera/ipa/core.mojom > > > +++ b/include/libcamera/ipa/core.mojom > > > @@ -152,6 +152,11 @@ module libcamera; > > > * RGB for sensors that don't have a CFA. > > > */ > > > > > > +/** > > > + * \var IPACameraSensorInfo::pixelArraySize > > > + * \brief The size of the physical pixel array of the sensor in pixels > > > + */ > > > + > > > /** > > > * \var IPACameraSensorInfo::activeArea > > > * \brief The active pixel area of the sensor, relative to the physical pixel > > > @@ -250,6 +255,7 @@ struct IPACameraSensorInfo { > > > uint32 bitsPerPixel; > > > uint32 cfaPattern; > > > > > > + Size pixelArraySize; > > > Rectangle activeArea; > > > Rectangle analogCrop; > > > Size outputSize; > > > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > > > index 9e32f798c2dd..18941666fb4c 100644 > > > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > > > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > > > @@ -881,6 +881,7 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const > > > return -EINVAL; > > > > > > info->model = model(); > > > + info->pixelArraySize = pixelArraySize_; > > > > > > /* > > > * The active area is a static property, while the crop rectangle needs > > > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > > > index a1f48b5ff557..4b0b8ff6fc2b 100644 > > > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > > > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > > > @@ -1007,6 +1007,7 @@ int CameraSensorRaw::setEmbeddedDataEnabled(bool enable) > > > int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const > > > { > > > info->model = model(); > > > + info->pixelArraySize = pixelArraySize_; > > > > > > /* > > > * The active area is a static property, while the crop rectangle needs > > > -- > > > 2.53.0 > > >
Hi Jacopo, Quoting Jacopo Mondi (2026-08-20 14:03:16) > Hi Stefan > > On Thu, Aug 20, 2026 at 01:51:35PM +0200, Stefan Klug wrote: > > Hi Jacopo, > > > > Quoting Jacopo Mondi (2026-08-20 12:20:25) > > > Hi Stefan > > > > > > On Wed, Aug 19, 2026 at 04:15:16PM +0200, Stefan Klug wrote: > > > > Add a IPACameraSensorInfo::pixelArraySize property to communicate the > > > > full size of the pixel array to the IPA. This property is not used today > > > > but will be useful to do sanity checks on coordinates handled in the > > > > algorithms. > > > > > > Do you already have a use case in mind ? > > > > I'd like to migrate the LSC algorithm to read the reference image crop > > (that was used during tuning) directly from the tuning file instead of > > assuming the active area. In that case I'd like to do a validity check > > that the rect is inside pixelArraySize. But that's all. > > > > I see.. would you prefer to add this one right away, or when you'll > implement the above use case ? I don't really care too much. It was in the first version, so I didn't want to drop it silently. As I'm advertising against using activeArea it feels like it should be there to make IPACameraSensorInfo complete. I would keep it, but if there are objections, we can delay it. Best regards, Stefan > > > Best regards, > > Stefan > > > > > > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > --- > > > > include/libcamera/ipa/core.mojom | 6 ++++++ > > > > src/libcamera/sensor/camera_sensor_legacy.cpp | 1 + > > > > src/libcamera/sensor/camera_sensor_raw.cpp | 1 + > > > > 3 files changed, 8 insertions(+) > > > > > > > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom > > > > index d3375333baa0..8c0fc2165baa 100644 > > > > --- a/include/libcamera/ipa/core.mojom > > > > +++ b/include/libcamera/ipa/core.mojom > > > > @@ -152,6 +152,11 @@ module libcamera; > > > > * RGB for sensors that don't have a CFA. > > > > */ > > > > > > > > +/** > > > > + * \var IPACameraSensorInfo::pixelArraySize > > > > + * \brief The size of the physical pixel array of the sensor in pixels > > > > + */ > > > > + > > > > /** > > > > * \var IPACameraSensorInfo::activeArea > > > > * \brief The active pixel area of the sensor, relative to the physical pixel > > > > @@ -250,6 +255,7 @@ struct IPACameraSensorInfo { > > > > uint32 bitsPerPixel; > > > > uint32 cfaPattern; > > > > > > > > + Size pixelArraySize; > > > > Rectangle activeArea; > > > > Rectangle analogCrop; > > > > Size outputSize; > > > > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > > > > index 9e32f798c2dd..18941666fb4c 100644 > > > > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > > > > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > > > > @@ -881,6 +881,7 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const > > > > return -EINVAL; > > > > > > > > info->model = model(); > > > > + info->pixelArraySize = pixelArraySize_; > > > > > > > > /* > > > > * The active area is a static property, while the crop rectangle needs > > > > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > > > > index a1f48b5ff557..4b0b8ff6fc2b 100644 > > > > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > > > > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > > > > @@ -1007,6 +1007,7 @@ int CameraSensorRaw::setEmbeddedDataEnabled(bool enable) > > > > int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const > > > > { > > > > info->model = model(); > > > > + info->pixelArraySize = pixelArraySize_; > > > > > > > > /* > > > > * The active area is a static property, while the crop rectangle needs > > > > -- > > > > 2.53.0 > > > >
Hi Stefan On Thu, Aug 20, 2026 at 03:24:07PM +0200, Stefan Klug wrote: > Hi Jacopo, > > Quoting Jacopo Mondi (2026-08-20 14:03:16) > > Hi Stefan > > > > On Thu, Aug 20, 2026 at 01:51:35PM +0200, Stefan Klug wrote: > > > Hi Jacopo, > > > > > > Quoting Jacopo Mondi (2026-08-20 12:20:25) > > > > Hi Stefan > > > > > > > > On Wed, Aug 19, 2026 at 04:15:16PM +0200, Stefan Klug wrote: > > > > > Add a IPACameraSensorInfo::pixelArraySize property to communicate the > > > > > full size of the pixel array to the IPA. This property is not used today > > > > > but will be useful to do sanity checks on coordinates handled in the > > > > > algorithms. > > > > > > > > Do you already have a use case in mind ? > > > > > > I'd like to migrate the LSC algorithm to read the reference image crop > > > (that was used during tuning) directly from the tuning file instead of > > > assuming the active area. In that case I'd like to do a validity check > > > that the rect is inside pixelArraySize. But that's all. > > > > > > > I see.. would you prefer to add this one right away, or when you'll > > implement the above use case ? > > I don't really care too much. It was in the first version, so I didn't > want to drop it silently. As I'm advertising against using activeArea it > feels like it should be there to make IPACameraSensorInfo complete. > > I would keep it, but if there are objections, we can delay it. > I don't know, I won't push back even if I would have introduced this only with a first user. Up to you Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Best regards, > Stefan > > > > > > Best regards, > > > Stefan > > > > > > > > > > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > > --- > > > > > include/libcamera/ipa/core.mojom | 6 ++++++ > > > > > src/libcamera/sensor/camera_sensor_legacy.cpp | 1 + > > > > > src/libcamera/sensor/camera_sensor_raw.cpp | 1 + > > > > > 3 files changed, 8 insertions(+) > > > > > > > > > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom > > > > > index d3375333baa0..8c0fc2165baa 100644 > > > > > --- a/include/libcamera/ipa/core.mojom > > > > > +++ b/include/libcamera/ipa/core.mojom > > > > > @@ -152,6 +152,11 @@ module libcamera; > > > > > * RGB for sensors that don't have a CFA. > > > > > */ > > > > > > > > > > +/** > > > > > + * \var IPACameraSensorInfo::pixelArraySize > > > > > + * \brief The size of the physical pixel array of the sensor in pixels > > > > > + */ > > > > > + > > > > > /** > > > > > * \var IPACameraSensorInfo::activeArea > > > > > * \brief The active pixel area of the sensor, relative to the physical pixel > > > > > @@ -250,6 +255,7 @@ struct IPACameraSensorInfo { > > > > > uint32 bitsPerPixel; > > > > > uint32 cfaPattern; > > > > > > > > > > + Size pixelArraySize; > > > > > Rectangle activeArea; > > > > > Rectangle analogCrop; > > > > > Size outputSize; > > > > > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > > > > > index 9e32f798c2dd..18941666fb4c 100644 > > > > > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > > > > > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > > > > > @@ -881,6 +881,7 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const > > > > > return -EINVAL; > > > > > > > > > > info->model = model(); > > > > > + info->pixelArraySize = pixelArraySize_; > > > > > > > > > > /* > > > > > * The active area is a static property, while the crop rectangle needs > > > > > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > > > > > index a1f48b5ff557..4b0b8ff6fc2b 100644 > > > > > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > > > > > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > > > > > @@ -1007,6 +1007,7 @@ int CameraSensorRaw::setEmbeddedDataEnabled(bool enable) > > > > > int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const > > > > > { > > > > > info->model = model(); > > > > > + info->pixelArraySize = pixelArraySize_; > > > > > > > > > > /* > > > > > * The active area is a static property, while the crop rectangle needs > > > > > -- > > > > > 2.53.0 > > > > >
diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom index d3375333baa0..8c0fc2165baa 100644 --- a/include/libcamera/ipa/core.mojom +++ b/include/libcamera/ipa/core.mojom @@ -152,6 +152,11 @@ module libcamera; * RGB for sensors that don't have a CFA. */ +/** + * \var IPACameraSensorInfo::pixelArraySize + * \brief The size of the physical pixel array of the sensor in pixels + */ + /** * \var IPACameraSensorInfo::activeArea * \brief The active pixel area of the sensor, relative to the physical pixel @@ -250,6 +255,7 @@ struct IPACameraSensorInfo { uint32 bitsPerPixel; uint32 cfaPattern; + Size pixelArraySize; Rectangle activeArea; Rectangle analogCrop; Size outputSize; diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp index 9e32f798c2dd..18941666fb4c 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -881,6 +881,7 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const return -EINVAL; info->model = model(); + info->pixelArraySize = pixelArraySize_; /* * The active area is a static property, while the crop rectangle needs diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp index a1f48b5ff557..4b0b8ff6fc2b 100644 --- a/src/libcamera/sensor/camera_sensor_raw.cpp +++ b/src/libcamera/sensor/camera_sensor_raw.cpp @@ -1007,6 +1007,7 @@ int CameraSensorRaw::setEmbeddedDataEnabled(bool enable) int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const { info->model = model(); + info->pixelArraySize = pixelArraySize_; /* * The active area is a static property, while the crop rectangle needs
Add a IPACameraSensorInfo::pixelArraySize property to communicate the full size of the pixel array to the IPA. This property is not used today but will be useful to do sanity checks on coordinates handled in the algorithms. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- include/libcamera/ipa/core.mojom | 6 ++++++ src/libcamera/sensor/camera_sensor_legacy.cpp | 1 + src/libcamera/sensor/camera_sensor_raw.cpp | 1 + 3 files changed, 8 insertions(+)