[libcamera-devel,2/4] libcamera: camera_sensor: Break out properties initialization
diff mbox series

Message ID 20201106154947.261132-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Collect pixel array properties
Related show

Commit Message

Jacopo Mondi Nov. 6, 2020, 3:49 p.m. UTC 
Break out initialization to its own function in preparation to
add more properties.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/internal/camera_sensor.h |  1 +
 src/libcamera/camera_sensor.cpp            | 79 ++++++++++++----------
 2 files changed, 45 insertions(+), 35 deletions(-)

Comments

Niklas Söderlund Nov. 9, 2020, 12:43 p.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

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

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

> ---
>  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;
>  }
>  
> -- 
> 2.29.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Dec. 1, 2020, 6:57 p.m. UTC | #2 
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@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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;
>  }
>

Patch
diff mbox series 

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;
 }