[libcamera-devel] [PATCH v2 1/2] libcamera: camera_sensor: Enable to set a test pattern mode

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Tue, Oct 26, 2021 at 07:39:13PM +0900, Hirokazu Honda wrote:
> This adds a function to set a camera sensor driver a test pattern
> mode.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/internal/camera_sensor.h |  6 +++
>  src/libcamera/camera_sensor.cpp            | 50 +++++++++++++++++++---
>  2 files changed, 50 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d25a1165..d08cfec5 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -26,6 +26,8 @@ namespace libcamera {
>  class BayerFormat;
>  class MediaEntity;
>  
> +struct CameraSensorProperties;
> +
>  class CameraSensor : protected Loggable
>  {
>  public:
> @@ -44,6 +46,7 @@ public:
>  	{
>  		return testPatternModes_;
>  	}
> +	int setTestPatternMode(int32_t testPatternMode);

Could the function take a TestPatternModeEnum instead of an int32_t ?
That's the proper type of the libcamera TestPatternMode control.

>  
>  	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
>  				      const Size &size) const;
> @@ -78,6 +81,8 @@ private:
>  	std::unique_ptr<V4L2Subdevice> subdev_;
>  	unsigned int pad_;
>  
> +	const CameraSensorProperties *props_;
> +
>  	std::string model_;
>  	std::string id_;
>  
> @@ -85,6 +90,7 @@ private:
>  	std::vector<unsigned int> mbusCodes_;
>  	std::vector<Size> sizes_;
>  	std::vector<int32_t> testPatternModes_;
> +	int32_t testPatternMode_;

Same here.

>  
>  	Size pixelArraySize_;
>  	Rectangle activeArea_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9fdb8c09..47bb13f9 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -54,8 +54,8 @@ LOG_DEFINE_CATEGORY(CameraSensor)
>   * Once constructed the instance must be initialized with init().
>   */
>  CameraSensor::CameraSensor(const MediaEntity *entity)
> -	: entity_(entity), pad_(UINT_MAX), bayerFormat_(nullptr),
> -	  properties_(properties::properties)
> +	: entity_(entity), pad_(UINT_MAX), testPatternMode_(-1),

Shouldn't this be initialized to TestPatternModeOff ?

> +	  bayerFormat_(nullptr), properties_(properties::properties)
>  {
>  }
>  
> @@ -300,14 +300,16 @@ void CameraSensor::initVimcDefaultProperties()
>  
>  void CameraSensor::initStaticProperties()
>  {
> -	const CameraSensorProperties *props = CameraSensorProperties::get(model_);
> -	if (!props)
> +	props_ = CameraSensorProperties::get(model_);
> +	if (!props_) {
> +		LOG(CameraSensor, Debug) << "No properties for " << model_;
>  		return;
> +	}
>  
>  	/* Register the properties retrieved from the sensor database. */
> -	properties_.set(properties::UnitCellSize, props->unitCellSize);
> +	properties_.set(properties::UnitCellSize, props_->unitCellSize);
>  
> -	initTestPatternModes(props->testPatternModes);
> +	initTestPatternModes(props_->testPatternModes);
>  }
>  
>  void CameraSensor::initTestPatternModes(
> @@ -531,6 +533,42 @@ Size CameraSensor::resolution() const
>   * \return The list of test pattern modes
>   */
>  
> +/**
> + * \brief Set the camera sensor a specified controls::TestPatternMode

 * \brief Set the test pattern mode for the camera sensor

> + * \param[in] testPatternMode test pattern mode control value to set the camera

s/test pattern/Test pattern/

> + * sensor
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CameraSensor::setTestPatternMode(int32_t testPatternMode)
> +{
> +	if (testPatternMode_ == testPatternMode)
> +		return 0;
> +
> +	if (!props_) {
> +		LOG(CameraSensor, Error) << "No property is found";

s/property is found/properties are found/

> +		return -EINVAL;
> +	}
> +
> +	auto it = props_->testPatternModes.find(testPatternMode);
> +	if (it == props_->testPatternModes.end()) {
> +		LOG(CameraSensor, Error) << "Unsupported test pattern mode: "

s/mode:/mode/

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

> +					 << testPatternMode;
> +		return -EINVAL;
> +	}
> +
> +	ControlList ctrls{ controls() };
> +	ctrls.set(V4L2_CID_TEST_PATTERN, it->second);
> +
> +	int ret = setControls(&ctrls);
> +	if (ret)
> +		return ret;
> +
> +	testPatternMode_ = testPatternMode;
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Retrieve the best sensor format for a desired output
>   * \param[in] mbusCodes The list of acceptable media bus codes

-- 
Regards,

Laurent Pinchart