[libcamera-devel] [PATCH v2] libcamera: camera_sensor: Use active area size as resolution

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2021-03-02 21:49:48 +0200, Laurent Pinchart wrote:
> When a sensor can upscale the image, the native sensor resolution isn't
> equal to the largest size reported by the sensor. Use the active area
> size instead, and default it to the largest enumerated size if the crop
> rectangle targets are not supported.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
> Changes since v1:
> 
> - Don't set default pixelArraySize_ to override it later
> 
> Note how this causes the pixelArraySize_ = sizes_.back() assignment to
> be duplicated in CameraSensor::validateSensorDriver() and
> CameraSensor::initVimcDefaultProperties(). Jacopo, do you prefer v1 or
> v2 ?
> 
> ---
>  include/libcamera/internal/camera_sensor.h |  3 +--
>  src/libcamera/camera_sensor.cpp            | 23 +++++++++++-----------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index f22ffbfe9f97..71d012f795fe 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -53,7 +53,7 @@ public:
>  	const MediaEntity *entity() const { return entity_; }
>  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
>  	const std::vector<Size> &sizes() const { return sizes_; }
> -	const Size &resolution() const { return resolution_; }
> +	Size resolution() const { return activeArea_.size(); }
>  
>  	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
>  				      const Size &size) const;
> @@ -87,7 +87,6 @@ private:
>  	std::string id_;
>  
>  	V4L2Subdevice::Formats formats_;
> -	Size resolution_;
>  	std::vector<unsigned int> mbusCodes_;
>  	std::vector<Size> sizes_;
>  
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 8a1b9bd277df..8db6e8974a8d 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -233,12 +233,6 @@ int CameraSensor::init()
>  	auto last = std::unique(sizes_.begin(), sizes_.end());
>  	sizes_.erase(last, sizes_.end());
>  
> -	/*
> -	 * The sizes_ vector is sorted in ascending order, the resolution is
> -	 * thus the last element of the vector.
> -	 */
> -	resolution_ = sizes_.back();
> -
>  	/*
>  	 * VIMC is a bit special, as it does not yet support all the mandatory
>  	 * requirements regular sensors have to respect.
> @@ -324,14 +318,20 @@ int CameraSensor::validateSensorDriver()
>  	Rectangle rect;
>  	int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);
>  	if (ret) {
> -		rect = Rectangle(resolution());
> +		/*
> +		 * Default the pixel array size to the largest size supported
> +		 * by the sensor. The sizes_ vector is sorted in ascending
> +		 * order, the largest size is thus the last element.
> +		 */
> +		pixelArraySize_ = sizes_.back();
> +
>  		LOG(CameraSensor, Warning)
>  			<< "The PixelArraySize property has been defaulted to "
> -			<< rect.toString();
> +			<< pixelArraySize_.toString();
>  		err = -EINVAL;
> +	} else {
> +		pixelArraySize_ = rect.size();
>  	}
> -	pixelArraySize_.width = rect.width;
> -	pixelArraySize_.height = rect.height;
>  
>  	ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_);
>  	if (ret) {
> @@ -397,7 +397,8 @@ int CameraSensor::validateSensorDriver()
>   */
>  void CameraSensor::initVimcDefaultProperties()
>  {
> -	pixelArraySize_ = resolution();
> +	/* Use the largest supported size. */
> +	pixelArraySize_ = sizes_.back();
>  	activeArea_ = Rectangle(pixelArraySize_);
>  }
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund