[libcamera-devel] [PATCH v2 7/7] libcamera: camera_sensor: Retrieve sensor sizes

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-08-27 11:50:07 +0200, Jacopo Mondi wrote:
> Retrieve the camera sensor pixel array sizes and the active pixel area
> using the V4L2Subdevice selection API.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/geometry.h          |  1 +
>  src/libcamera/camera_sensor.cpp       | 51 +++++++++++++++++++++++++++
>  src/libcamera/geometry.cpp            | 24 +++++++++++++
>  src/libcamera/include/camera_sensor.h |  4 +++
>  4 files changed, 80 insertions(+)

I think you should split the changes to geometry.{cpp,h} into its own 
patch.

> 
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index 52f4d010de76..b91fcfa1dd17 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -42,6 +42,7 @@ struct Size {
>  	unsigned int height;
>  
>  	const std::string toString() const;
> +	bool contains(const Rectangle &rectangle) const;
>  };
>  
>  bool operator==(const Size &lhs, const Size &rhs);
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index fc7fdcdcaf5b..b6ccaae0930b 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -128,6 +128,28 @@ int CameraSensor::init()
>  	}
>  	rotation_ = value;
>  
> +	/*
> +	 * Retrieve and store the sensor pixel array size and active area
> +	 * rectangle.
> +	 */
> +	Rectangle rect;
> +	ret = subdev_->getSelection(0, V4L2_SEL_TGT_NATIVE_SIZE, &rect);
> +	if (ret)
> +		return ret;
> +	pixelArray_.width = rect.w;
> +	pixelArray_.height = rect.h;
> +
> +	ret = subdev_->getSelection(0, V4L2_SEL_TGT_CROP_BOUNDS, &rect);
> +	if (ret)
> +		return ret;
> +
> +	if (!pixelArray_.contains(rect)) {
> +		LOG(CameraSensor, Error)
> +			<< "Invalid camera sensor pixel array dimension";
> +		return -EINVAL;
> +	}
> +	activeArea_ = rect;
> +
>  	/* Enumerate and cache media bus codes and sizes. */
>  	const ImageFormats formats = subdev_->formats(0);
>  	if (formats.isEmpty()) {
> @@ -192,6 +214,35 @@ const Size &CameraSensor::resolution() const
>  	return sizes_.back();
>  }
>  
> +/**
> + * \fn CameraSensor::pixelArray()
> + * \brief Retrieve the camera sensor pixel array dimension
> + * \return The number of horizontal and vertical pixel units installed on the
> + * camera sensor
> + *
> + * The returned pixel array size is the number of pixels units physically
> + * installed on the sensor, including non-active ones, such as pixels used
> + * for calibration or testing.
> + */
> +
> +/**
> + * \fn CameraSensor::activeArea()
> + * \brief Retrieve the active pixels area
> + * \return A rectangle describing the active pixel area
> + *
> + * The returned rectangle is contained in the larger pixel array area,
> + * returned by the CameraSensor::pixelArray() operation.
> + *
> + * \todo This operation collides with CameraSensor::resolution() as they
> + * both reports the same information (bugs in the driver code apart).
> + * Also, for some sensors, the maximum available resolution might depend on
> + * the image size ratio (ie the maximumx resolution for images in 4:3 format
> + * is different from the maximum available resolution for 16:9 format). This
> + * has to be sorted out as well.

Should we remove CameraSensor::resolution() in favor if new more 
"correct" way of finding out the sensor resolution?

> + *
> + * \sa Size::contains()
> + */
> +
>  /**
>   * \brief Retrieve the best sensor format for a desired output
>   * \param[in] mbusCodes The list of acceptable media bus codes
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index 32b0faeadc63..9b60768a57a7 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -116,6 +116,30 @@ const std::string Size::toString() const
>  	return std::to_string(width) + "x" + std::to_string(height);
>  }
>  
> +/**
> + * \fn Size::contains()
> + * \param rectangle The rectangle to verify
> + * \return True if \a rectangle is contained in the area represented by this
> + * Size, false otherwise

Return comes at the end of the documentation block.

> + *
> + * Return true if \a rectangle is completely contained in the area represented
> + * by the width and height of this Size, with its top left corner assumed to be
> + * at coordinates (0, 0). The rectangle 'x' and 'y' fields represent the
> + * horizontal and vertical positive displacement from the Size top left corner,
> + * from where the rectangle's horizontal and vertical sizes are calculated.
> + */
> +bool Size::contains(const Rectangle &rectangle) const
> +{
> +	if (rectangle.x < 0 || rectangle.y < 0)
> +		return false;
> +
> +	if (rectangle.x + rectangle.w > width ||
> +	    rectangle.y + rectangle.h > height)
> +		return false;
> +
> +	return true;
> +}
> +
>  /**
>   * \brief Compare sizes for equality
>   * \return True if the two sizes are equal, false otherwise
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index 32d39127b275..abf5344cf9d8 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -38,6 +38,8 @@ public:
>  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
>  	const std::vector<Size> &sizes() const { return sizes_; }
>  	const Size &resolution() const;
> +	const Size pixelArray() const { return pixelArray_; }
> +	const Rectangle activeArea() const { return activeArea_; }
>  
>  	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
>  				      const Size &size) const;
> @@ -59,6 +61,8 @@ private:
>  
>  	CameraLocation location_;
>  	unsigned int rotation_;
> +	Size pixelArray_;
> +	Rectangle activeArea_;
>  };
>  
>  } /* namespace libcamera */
> -- 
> 2.23.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund