[libcamera-devel] [PATCH v5 04/10] libcamera: camera_sensor: Default analogue crop rectangle
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Jan 6 11:50:41 CET 2021
Hi Jacopo,
Thanks for your work.
On 2021-01-05 13:31:22 +0100, Jacopo Mondi wrote:
> As support for the V4L2_SEL_TGT_CROP selection target, used to read the
> sensor analogue crop rectangle is schedule to become mandatory but is
> still optional, cache a default value equal to the sensor's active area
> size at sensor validation time to allow the creation of the
> CameraSensorInfo in the case the driver does not support it.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/camera_sensor.h | 1 +
> src/libcamera/camera_sensor.cpp | 27 ++++++++++++++--------
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 86902b85ada8..de6a0202d19a 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -86,6 +86,7 @@ private:
>
> Size pixelArraySize_;
> Rectangle activeArea_;
> + Rectangle analogueCrop_;
>
> ControlList properties_;
> };
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 92c90862215d..eda41980aa22 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -289,8 +289,10 @@ int CameraSensor::validateSensorDriver()
>
> ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &rect);
> if (ret) {
> + analogueCrop_ = activeArea_;
> LOG(CameraSensor, Warning)
> - << "Failed to retrieve the sensor crop rectangle";
> + << "The analogue crop rectangle has been defaulted to: "
> + << analogueCrop_.toString();
> err = -EINVAL;
> }
>
> @@ -643,13 +645,20 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const
> */
> info->activeAreaSize = { activeArea_.width, activeArea_.height };
>
> - /* It's mandatory for the subdevice to report its crop rectangle. */
> - int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);
> - if (ret) {
> - LOG(CameraSensor, Error)
> - << "Failed to construct camera sensor info: "
> - << "the camera sensor does not report the crop rectangle";
> - return ret;
> + /*
> + * \todo Support for retreiving the crop rectangle is scheduled to
> + * become mandatory. For the time being use the default value if it has
> + * been initialized at sensor driver validation time.
> + */
> + if (!analogueCrop_.isNull()) {
> + info->analogCrop = analogueCrop_;
> + } else {
> + int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);
> + if (ret) {
> + LOG(CameraSensor, Error)
> + << "Failed to retrieve the sensor crop rectangle.";
> + return -EINVAL;
> + }
As I understand 3/10 activeArea_ is static right? Would it make more
sens to use it here instead of adding and caching it in analogueCrop_?
This would align the code here to always call getSelection() and
huddling the defaulting in the if (ret) code block? I think this would
make the code cleaner as the default as you point out is a temporary
work around.
> }
>
> /*
> @@ -664,7 +673,7 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const
>
> /* The bit depth and image size depend on the currently applied format. */
> V4L2SubdeviceFormat format{};
> - ret = subdev_->getFormat(pad_, &format);
> + int ret = subdev_->getFormat(pad_, &format);
> if (ret)
> return ret;
> info->bitsPerPixel = format.bitsPerPixel();
> --
> 2.29.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list