[libcamera-devel] [PATCH 2/4] libcamera: camera_sensor: Break out properties initialization

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Nov 06, 2020 at 04:49:45PM +0100, Jacopo Mondi wrote:
> Break out initialization to its own function in preparation to
> add more properties.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  include/libcamera/internal/camera_sensor.h |  1 +
>  src/libcamera/camera_sensor.cpp            | 79 ++++++++++++----------
>  2 files changed, 45 insertions(+), 35 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index b9eba89f00fb..f80d836161a5 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -69,6 +69,7 @@ protected:
>  
>  private:
>  	int generateId();
> +	int initProperties();
>  
>  	const MediaEntity *entity_;
>  	std::unique_ptr<V4L2Subdevice> subdev_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 2af0b0a8db52..49b0a026125c 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -172,6 +172,49 @@ int CameraSensor::init()
>  		return -EINVAL;
>  	}
>  
> +	/* Create and open the subdev. */
> +	subdev_ = std::make_unique<V4L2Subdevice>(entity_);
> +	int ret = subdev_->open();
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = initProperties();
> +	if (ret)
> +		return ret;
> +
> +	/* Enumerate, sort and cache media bus codes and sizes. */
> +	formats_ = subdev_->formats(pad_);
> +	if (formats_.empty()) {
> +		LOG(CameraSensor, Error) << "No image format found";
> +		return -EINVAL;
> +	}
> +
> +	mbusCodes_ = utils::map_keys(formats_);
> +	std::sort(mbusCodes_.begin(), mbusCodes_.end());
> +
> +	for (const auto &format : formats_) {
> +		const std::vector<SizeRange> &ranges = format.second;
> +		std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),
> +			       [](const SizeRange &range) { return range.max; });
> +	}
> +
> +	std::sort(sizes_.begin(), sizes_.end());
> +
> +	/* Remove duplicates. */
> +	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();
> +
> +	return 0;
> +}
> +
> +int CameraSensor::initProperties()
> +{
>  	/*
>  	 * Extract the camera sensor model name from the media entity name.
>  	 *
> @@ -202,14 +245,8 @@ int CameraSensor::init()
>  
>  	properties_.set(properties::Model, utils::toAscii(model_));
>  
> -	/* Create and open the subdev. */
> -	subdev_ = std::make_unique<V4L2Subdevice>(entity_);
> -	int ret = subdev_->open();
> -	if (ret < 0)
> -		return ret;
> -
>  	/* Generate a unique ID for the sensor. */
> -	ret = generateId();
> +	int ret = generateId();
>  	if (ret)
>  		return ret;
>  
> @@ -251,34 +288,6 @@ int CameraSensor::init()
>  		propertyValue = 0;
>  	properties_.set(properties::Rotation, propertyValue);
>  
> -	/* Enumerate, sort and cache media bus codes and sizes. */
> -	formats_ = subdev_->formats(pad_);
> -	if (formats_.empty()) {
> -		LOG(CameraSensor, Error) << "No image format found";
> -		return -EINVAL;
> -	}
> -
> -	mbusCodes_ = utils::map_keys(formats_);
> -	std::sort(mbusCodes_.begin(), mbusCodes_.end());
> -
> -	for (const auto &format : formats_) {
> -		const std::vector<SizeRange> &ranges = format.second;
> -		std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),
> -			       [](const SizeRange &range) { return range.max; });
> -	}
> -
> -	std::sort(sizes_.begin(), sizes_.end());
> -
> -	/* Remove duplicates. */
> -	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();
> -
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart