| Message ID | 20201106154947.261132-2-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. On 2020-11-06 16:49:44 +0100, Jacopo Mondi wrote: > The CameraSensorInfo::analogCrop top-left corner is defined relatively > to the sensor active area. > > The analogCrop rectangle is constucted by retrieving the V4L2 > selection target V4L2_SEL_TGT_CROP which is instead defined relatively > to the whole sensor's pixel array size. > > Adjust the the analogCrop rectangle subtracting from its top-left corner > the active area distance from the full pixel array. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > This patch depends on the upstreaming of > "media: i2c: imx219: Selection compliance fixes" in mainline Linux I assume this patch do not depend on this fix as this implements the correct behavior in libcamera. But libcamera using an IMX219 sensor will be broken without the upstream fix? > --- > src/libcamera/camera_sensor.cpp | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 935de528c496..2af0b0a8db52 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -526,6 +526,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const > << "the camera sensor does not report the crop rectangle"; > return ret; > } > + /* > + * CameraSensorInfo::analogCrop::x and CameraSensorInfo::analogCrop::y > + * are defined in respect to the active pixel area, while V4L2's > + * TGT_CROP target is defined in respect to the native pixel array. > + * > + * Compensate it subtracting the active areas offset. > + */ > + info->analogCrop.x -= rect.x; > + info->analogCrop.y -= rect.y; > > /* The bit depth and image size depend on the currently applied format. */ > V4L2SubdeviceFormat format{}; > -- > 2.29.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas,
+Naush, Dave
On Mon, Nov 09, 2020 at 01:42:42PM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-11-06 16:49:44 +0100, Jacopo Mondi wrote:
> > The CameraSensorInfo::analogCrop top-left corner is defined relatively
> > to the sensor active area.
> >
> > The analogCrop rectangle is constucted by retrieving the V4L2
> > selection target V4L2_SEL_TGT_CROP which is instead defined relatively
> > to the whole sensor's pixel array size.
> >
> > Adjust the the analogCrop rectangle subtracting from its top-left corner
> > the active area distance from the full pixel array.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>
> >
> > ---
> > This patch depends on the upstreaming of
> > "media: i2c: imx219: Selection compliance fixes" in mainline Linux
>
> I assume this patch do not depend on this fix as this implements the
> correct behavior in libcamera. But libcamera using an IMX219 sensor will
> be broken without the upstream fix?
Correct. Libcamera with this patch applied and a kernel which does not
include the above patch will report slightl incorrect sizes.
The patch has now been collected for 5.11, I'll make sure the same
hits rpi's downstream.
Thanks
j
>
> > ---
> > src/libcamera/camera_sensor.cpp | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 935de528c496..2af0b0a8db52 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -526,6 +526,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const
> > << "the camera sensor does not report the crop rectangle";
> > return ret;
> > }
> > + /*
> > + * CameraSensorInfo::analogCrop::x and CameraSensorInfo::analogCrop::y
> > + * are defined in respect to the active pixel area, while V4L2's
> > + * TGT_CROP target is defined in respect to the native pixel array.
> > + *
> > + * Compensate it subtracting the active areas offset.
> > + */
> > + info->analogCrop.x -= rect.x;
> > + info->analogCrop.y -= rect.y;
> >
> > /* The bit depth and image size depend on the currently applied format. */
> > V4L2SubdeviceFormat format{};
> > --
> > 2.29.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
Hi Jacopo, Thank you for the patch. On Fri, Nov 06, 2020 at 04:49:44PM +0100, Jacopo Mondi wrote: > The CameraSensorInfo::analogCrop top-left corner is defined relatively > to the sensor active area. > > The analogCrop rectangle is constucted by retrieving the V4L2 > selection target V4L2_SEL_TGT_CROP which is instead defined relatively > to the whole sensor's pixel array size. > > Adjust the the analogCrop rectangle subtracting from its top-left corner > the active area distance from the full pixel array. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > This patch depends on the upstreaming of > "media: i2c: imx219: Selection compliance fixes" in mainline Linux > --- > src/libcamera/camera_sensor.cpp | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 935de528c496..2af0b0a8db52 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -526,6 +526,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const > << "the camera sensor does not report the crop rectangle"; > return ret; > } I'd add a blank line here. > + /* > + * CameraSensorInfo::analogCrop::x and CameraSensorInfo::analogCrop::y > + * are defined in respect to the active pixel area, while V4L2's s/in respect/relatively/ ? (not sure, "in respect" sounds a bit weird to me here, but I could be wrong). Same on the next line. > + * TGT_CROP target is defined in respect to the native pixel array. s/native/full/ ? Not sure, I'm lost between the different terms and how they're used differently in different contexts :-) > + * > + * Compensate it subtracting the active areas offset. s/it/it by/ ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + info->analogCrop.x -= rect.x; > + info->analogCrop.y -= rect.y; > > /* The bit depth and image size depend on the currently applied format. */ > V4L2SubdeviceFormat format{};
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 935de528c496..2af0b0a8db52 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -526,6 +526,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const << "the camera sensor does not report the crop rectangle"; return ret; } + /* + * CameraSensorInfo::analogCrop::x and CameraSensorInfo::analogCrop::y + * are defined in respect to the active pixel area, while V4L2's + * TGT_CROP target is defined in respect to the native pixel array. + * + * Compensate it subtracting the active areas offset. + */ + info->analogCrop.x -= rect.x; + info->analogCrop.y -= rect.y; /* The bit depth and image size depend on the currently applied format. */ V4L2SubdeviceFormat format{};
The CameraSensorInfo::analogCrop top-left corner is defined relatively to the sensor active area. The analogCrop rectangle is constucted by retrieving the V4L2 selection target V4L2_SEL_TGT_CROP which is instead defined relatively to the whole sensor's pixel array size. Adjust the the analogCrop rectangle subtracting from its top-left corner the active area distance from the full pixel array. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- This patch depends on the upstreaming of "media: i2c: imx219: Selection compliance fixes" in mainline Linux --- src/libcamera/camera_sensor.cpp | 9 +++++++++ 1 file changed, 9 insertions(+)