[libcamera-devel] [PATCH v6 2/4] libcamera: camera_sensor: Reference test pattern modes by enum type

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Wed, Nov 10, 2021 at 04:22:05PM +0900, Hirokazu Honda wrote:
> The CameraSensor stores TestPatternModes as an int32_t. This prevents
> the compiler from verifying the usage against the defined enum types.
> 
> Fix references to the TestPatternMode to store the value as the
> TestPatternModeEnum type which is defined by the control generator.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

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

> ---
>  include/libcamera/internal/camera_sensor.h            | 10 +++++++---
>  include/libcamera/internal/camera_sensor_properties.h |  3 ++-
>  src/libcamera/camera_sensor.cpp                       |  4 ++--
>  src/libcamera/pipeline/ipu3/ipu3.cpp                  |  7 ++++---
>  4 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d25a1165..edef2220 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -14,8 +14,10 @@
>  #include <libcamera/base/class.h>
>  #include <libcamera/base/log.h>
>  
> +#include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
> +
>  #include <libcamera/ipa/core_ipa_interface.h>
>  
>  #include "libcamera/internal/formats.h"
> @@ -40,7 +42,8 @@ public:
>  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
>  	const std::vector<Size> sizes(unsigned int mbusCode) const;
>  	Size resolution() const;
> -	const std::vector<int32_t> &testPatternModes() const
> +	const std::vector<controls::draft::TestPatternModeEnum>
> +		&testPatternModes() const
>  	{
>  		return testPatternModes_;
>  	}
> @@ -71,7 +74,8 @@ private:
>  	void initVimcDefaultProperties();
>  	void initStaticProperties();
>  	void initTestPatternModes(
> -		const std::map<int32_t, int32_t> &testPatternModeMap);
> +		const std::map<controls::draft::TestPatternModeEnum, int32_t>
> +			&testPatternModeMap);
>  	int initProperties();
>  
>  	const MediaEntity *entity_;
> @@ -84,7 +88,7 @@ private:
>  	V4L2Subdevice::Formats formats_;
>  	std::vector<unsigned int> mbusCodes_;
>  	std::vector<Size> sizes_;
> -	std::vector<int32_t> testPatternModes_;
> +	std::vector<controls::draft::TestPatternModeEnum> testPatternModes_;
>  
>  	Size pixelArraySize_;
>  	Rectangle activeArea_;
> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h
> index 67c77920..5c7e5e87 100644
> --- a/include/libcamera/internal/camera_sensor_properties.h
> +++ b/include/libcamera/internal/camera_sensor_properties.h
> @@ -10,6 +10,7 @@
>  #include <map>
>  #include <string>
>  
> +#include <libcamera/control_ids.h>
>  #include <libcamera/geometry.h>
>  
>  namespace libcamera {
> @@ -18,7 +19,7 @@ struct CameraSensorProperties {
>  	static const CameraSensorProperties *get(const std::string &sensor);
>  
>  	Size unitCellSize;
> -	std::map<int32_t, int32_t> testPatternModes;
> +	std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9fdb8c09..f0aa9f24 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -311,7 +311,7 @@ void CameraSensor::initStaticProperties()
>  }
>  
>  void CameraSensor::initTestPatternModes(
> -	const std::map<int32_t, int32_t> &testPatternModes)
> +	const std::map<controls::draft::TestPatternModeEnum, int32_t> &testPatternModes)
>  {
>  	const auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);
>  	if (v4l2TestPattern == controls().end()) {
> @@ -327,7 +327,7 @@ void CameraSensor::initTestPatternModes(
>  	 * control index is supported in the below for loop that creates the
>  	 * list of supported test patterns.
>  	 */
> -	std::map<int32_t, int32_t> indexToTestPatternMode;
> +	std::map<int32_t, controls::draft::TestPatternModeEnum> indexToTestPatternMode;
>  	for (const auto &it : testPatternModes)
>  		indexToTestPatternMode[it.second] = it.first;
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index eb714aa6..63cb7f11 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -981,13 +981,14 @@ int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)
>  		return ret;
>  
>  	ControlInfoMap::Map controls = IPU3Controls;
> -	const std::vector<int32_t> &testPatternModes = sensor->testPatternModes();
> +	const std::vector<controls::draft::TestPatternModeEnum>
> +		&testPatternModes = sensor->testPatternModes();
>  	if (!testPatternModes.empty()) {
>  		std::vector<ControlValue> values;
>  		values.reserve(testPatternModes.size());
>  
> -		for (int32_t pattern : testPatternModes)
> -			values.emplace_back(pattern);
> +		for (auto pattern : testPatternModes)
> +			values.emplace_back(static_cast<int32_t>(pattern));
>  
>  		controls[&controls::draft::TestPatternMode] = ControlInfo(values);
>  	}

-- 
Regards,

Laurent Pinchart