[libcamera-devel,08/17] libcamera: v4l2_subdevice: Replace FormatEnum with V4L2SubdeviceFormats
diff mbox series

Message ID 20190527001543.13593-9-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Add support for format information and validation
Related show

Commit Message

Niklas Söderlund May 27, 2019, 12:15 a.m. UTC 
Replace all usage of FormatEnum with V4L2SubdeviceFormats and completely
remove FormatEnum which is no longer needed.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/camera_sensor.cpp        | 12 +++++-------
 src/libcamera/formats.cpp              | 11 -----------
 src/libcamera/include/formats.h        |  2 --
 src/libcamera/include/v4l2_subdevice.h |  2 +-
 src/libcamera/v4l2_subdevice.cpp       |  6 +++---
 test/v4l2_subdevice/list_formats.cpp   | 10 +++++-----
 6 files changed, 14 insertions(+), 29 deletions(-)

Comments

Laurent Pinchart June 9, 2019, 12:51 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Mon, May 27, 2019 at 02:15:34AM +0200, Niklas Söderlund wrote:
> Replace all usage of FormatEnum with V4L2SubdeviceFormats and completely
> remove FormatEnum which is no longer needed.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/camera_sensor.cpp        | 12 +++++-------
>  src/libcamera/formats.cpp              | 11 -----------
>  src/libcamera/include/formats.h        |  2 --
>  src/libcamera/include/v4l2_subdevice.h |  2 +-
>  src/libcamera/v4l2_subdevice.cpp       |  6 +++---
>  test/v4l2_subdevice/list_formats.cpp   | 10 +++++-----
>  6 files changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 2b9d8fa593c13177..e46bc28c7ac4a8e1 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -90,27 +90,25 @@ int CameraSensor::init()
>  		return ret;
>  
>  	/* Enumerate and cache media bus codes and sizes. */
> -	const FormatEnum formats = subdev_->formats(0);
> +	const V4L2SubdeviceFormats formats = subdev_->formats(0);
>  	if (formats.empty()) {
>  		LOG(CameraSensor, Error) << "No image format found";
>  		return -EINVAL;
>  	}
>  
> -	std::transform(formats.begin(), formats.end(),
> -		       std::back_inserter(mbusCodes_),
> -		       [](decltype(*formats.begin()) f) { return f.first; });
> +	mbusCodes_ = formats.codes();
>  
>  	/*
>  	 * Extract the supported sizes from the first format as we only support
>  	 * sensors that offer the same frame sizes for all media bus codes.
>  	 * Verify this assumption and reject the sensor if it isn't true.
>  	 */
> -	const std::vector<SizeRange> &sizes = formats.begin()->second;
> +	const std::vector<SizeRange> &sizes = formats.sizes(mbusCodes_[0]);
>  	std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_),
>  		       [](const SizeRange &range) { return range.max; });
>  
> -	for (auto it = ++formats.begin(); it != formats.end(); ++it) {
> -		if (it->second != sizes) {
> +	for (unsigned int code : mbusCodes_) {
> +		if (formats.sizes(code) != sizes) {
>  			LOG(CameraSensor, Error)
>  				<< "Frame sizes differ between media bus codes";
>  			return -EINVAL;
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index f5841b38cea5686c..7b2097413c3b7378 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -14,17 +14,6 @@
>  
>  namespace libcamera {
>  
> -/**
> - * \typedef FormatEnum
> - * \brief Type definition for the map of image formats and sizes
> - *
> - * Type definition used to enumerate the supported pixel formats and image
> - * frame sizes. The type associates in a map a pixel format (for memory
> - * formats) or a media bus code (for bus formats), to a vector of image
> - * resolutions represented by SizeRange items.
> - */
> -
> -
>  /**
>   * \class DeviceFormats
>   * \brief Base class for V4L2Device and V4L2SubDevice Formats
> diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
> index 372f6e6d71b236dd..2e849e70c4a88aeb 100644
> --- a/src/libcamera/include/formats.h
> +++ b/src/libcamera/include/formats.h
> @@ -15,8 +15,6 @@
>  
>  namespace libcamera {
>  
> -typedef std::map<unsigned int, std::vector<SizeRange>> FormatEnum;
> -
>  class DeviceFormats
>  {
>  public:
> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> index c6fdf417b43c0423..a6d691ea540a4e5e 100644
> --- a/src/libcamera/include/v4l2_subdevice.h
> +++ b/src/libcamera/include/v4l2_subdevice.h
> @@ -45,7 +45,7 @@ public:
>  	int setCrop(unsigned int pad, Rectangle *rect);
>  	int setCompose(unsigned int pad, Rectangle *rect);
>  
> -	FormatEnum formats(unsigned int pad);
> +	V4L2SubdeviceFormats formats(unsigned int pad);
>  
>  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
>  	int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 8720013600ddb95f..fe393c66729e8511 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -200,9 +200,9 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
>   * \return A map of image formats associated with a list of image sizes, or
>   * an empty map on error or if the pad does not exist
>   */
> -FormatEnum V4L2Subdevice::formats(unsigned int pad)
> +V4L2SubdeviceFormats V4L2Subdevice::formats(unsigned int pad)
>  {
> -	FormatEnum formatMap = {};
> +	std::map<unsigned int, std::vector<SizeRange>> formatMap = {};
>  
>  	if (pad >= entity_->pads().size()) {
>  		LOG(V4L2Subdev, Error) << "Invalid pad: " << pad;
> @@ -212,7 +212,7 @@ FormatEnum V4L2Subdevice::formats(unsigned int pad)
>  	for (unsigned int code : enumPadCodes(pad))
>  		formatMap[code] = enumPadSizes(pad, code);
>  
> -	return formatMap;
> +	return V4L2SubdeviceFormats(formatMap);

Apart from the V4L2SubdeviceFormats constructor that requires building a
map first this patch looks good to me.

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

>  }
>  
>  /**
> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> index 3f0edafcdcd72d6b..1c044a969961d172 100644
> --- a/test/v4l2_subdevice/list_formats.cpp
> +++ b/test/v4l2_subdevice/list_formats.cpp
> @@ -47,7 +47,7 @@ void ListFormatsTest::printFormats(unsigned int pad,
>  int ListFormatsTest::run()
>  {
>  	/* List all formats available on existing "Scaler" pads. */
> -	std::map<unsigned int, std::vector<SizeRange>> formats;
> +	V4L2SubdeviceFormats formats;
>  
>  	formats = scaler_->formats(0);
>  	if (formats.empty()) {
> @@ -55,8 +55,8 @@ int ListFormatsTest::run()
>  		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
> -	for (auto it = formats.begin(); it != formats.end(); ++it)
> -		printFormats(0, it->first, it->second);
> +	for (unsigned int code : formats.codes())
> +		printFormats(0, code, formats.sizes(code));
>  
>  	formats = scaler_->formats(1);
>  	if (formats.empty()) {
> @@ -64,8 +64,8 @@ int ListFormatsTest::run()
>  		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
> -	for (auto it = formats.begin(); it != formats.end(); ++it)
> -		printFormats(1, it->first, it->second);
> +	for (unsigned int code : formats.codes())
> +		printFormats(1, code, formats.sizes(code));
>  
>  	/* List format on a non-existing pad, format vector shall be empty. */
>  	formats = scaler_->formats(2);

Patch
diff mbox series 

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index 2b9d8fa593c13177..e46bc28c7ac4a8e1 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -90,27 +90,25 @@  int CameraSensor::init()
 		return ret;
 
 	/* Enumerate and cache media bus codes and sizes. */
-	const FormatEnum formats = subdev_->formats(0);
+	const V4L2SubdeviceFormats formats = subdev_->formats(0);
 	if (formats.empty()) {
 		LOG(CameraSensor, Error) << "No image format found";
 		return -EINVAL;
 	}
 
-	std::transform(formats.begin(), formats.end(),
-		       std::back_inserter(mbusCodes_),
-		       [](decltype(*formats.begin()) f) { return f.first; });
+	mbusCodes_ = formats.codes();
 
 	/*
 	 * Extract the supported sizes from the first format as we only support
 	 * sensors that offer the same frame sizes for all media bus codes.
 	 * Verify this assumption and reject the sensor if it isn't true.
 	 */
-	const std::vector<SizeRange> &sizes = formats.begin()->second;
+	const std::vector<SizeRange> &sizes = formats.sizes(mbusCodes_[0]);
 	std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_),
 		       [](const SizeRange &range) { return range.max; });
 
-	for (auto it = ++formats.begin(); it != formats.end(); ++it) {
-		if (it->second != sizes) {
+	for (unsigned int code : mbusCodes_) {
+		if (formats.sizes(code) != sizes) {
 			LOG(CameraSensor, Error)
 				<< "Frame sizes differ between media bus codes";
 			return -EINVAL;
diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
index f5841b38cea5686c..7b2097413c3b7378 100644
--- a/src/libcamera/formats.cpp
+++ b/src/libcamera/formats.cpp
@@ -14,17 +14,6 @@ 
 
 namespace libcamera {
 
-/**
- * \typedef FormatEnum
- * \brief Type definition for the map of image formats and sizes
- *
- * Type definition used to enumerate the supported pixel formats and image
- * frame sizes. The type associates in a map a pixel format (for memory
- * formats) or a media bus code (for bus formats), to a vector of image
- * resolutions represented by SizeRange items.
- */
-
-
 /**
  * \class DeviceFormats
  * \brief Base class for V4L2Device and V4L2SubDevice Formats
diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
index 372f6e6d71b236dd..2e849e70c4a88aeb 100644
--- a/src/libcamera/include/formats.h
+++ b/src/libcamera/include/formats.h
@@ -15,8 +15,6 @@ 
 
 namespace libcamera {
 
-typedef std::map<unsigned int, std::vector<SizeRange>> FormatEnum;
-
 class DeviceFormats
 {
 public:
diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
index c6fdf417b43c0423..a6d691ea540a4e5e 100644
--- a/src/libcamera/include/v4l2_subdevice.h
+++ b/src/libcamera/include/v4l2_subdevice.h
@@ -45,7 +45,7 @@  public:
 	int setCrop(unsigned int pad, Rectangle *rect);
 	int setCompose(unsigned int pad, Rectangle *rect);
 
-	FormatEnum formats(unsigned int pad);
+	V4L2SubdeviceFormats formats(unsigned int pad);
 
 	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
 	int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index 8720013600ddb95f..fe393c66729e8511 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -200,9 +200,9 @@  int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
  * \return A map of image formats associated with a list of image sizes, or
  * an empty map on error or if the pad does not exist
  */
-FormatEnum V4L2Subdevice::formats(unsigned int pad)
+V4L2SubdeviceFormats V4L2Subdevice::formats(unsigned int pad)
 {
-	FormatEnum formatMap = {};
+	std::map<unsigned int, std::vector<SizeRange>> formatMap = {};
 
 	if (pad >= entity_->pads().size()) {
 		LOG(V4L2Subdev, Error) << "Invalid pad: " << pad;
@@ -212,7 +212,7 @@  FormatEnum V4L2Subdevice::formats(unsigned int pad)
 	for (unsigned int code : enumPadCodes(pad))
 		formatMap[code] = enumPadSizes(pad, code);
 
-	return formatMap;
+	return V4L2SubdeviceFormats(formatMap);
 }
 
 /**
diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
index 3f0edafcdcd72d6b..1c044a969961d172 100644
--- a/test/v4l2_subdevice/list_formats.cpp
+++ b/test/v4l2_subdevice/list_formats.cpp
@@ -47,7 +47,7 @@  void ListFormatsTest::printFormats(unsigned int pad,
 int ListFormatsTest::run()
 {
 	/* List all formats available on existing "Scaler" pads. */
-	std::map<unsigned int, std::vector<SizeRange>> formats;
+	V4L2SubdeviceFormats formats;
 
 	formats = scaler_->formats(0);
 	if (formats.empty()) {
@@ -55,8 +55,8 @@  int ListFormatsTest::run()
 		     << scaler_->entity()->name() << endl;
 		return TestFail;
 	}
-	for (auto it = formats.begin(); it != formats.end(); ++it)
-		printFormats(0, it->first, it->second);
+	for (unsigned int code : formats.codes())
+		printFormats(0, code, formats.sizes(code));
 
 	formats = scaler_->formats(1);
 	if (formats.empty()) {
@@ -64,8 +64,8 @@  int ListFormatsTest::run()
 		     << scaler_->entity()->name() << endl;
 		return TestFail;
 	}
-	for (auto it = formats.begin(); it != formats.end(); ++it)
-		printFormats(1, it->first, it->second);
+	for (unsigned int code : formats.codes())
+		printFormats(1, code, formats.sizes(code));
 
 	/* List format on a non-existing pad, format vector shall be empty. */
 	formats = scaler_->formats(2);