[libcamera-devel] [PATCH v6 4/6] libcamera: CameraSensor: Enable retrieving supported test pattern modes

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Fri, May 28, 2021 at 12:05:29PM +0900, Hirokazu Honda wrote:
> This enables retrieving supported test pattern modes through
> CameraSensorInfo.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/internal/camera_sensor.h |  7 +++++
>  src/libcamera/camera_sensor.cpp            | 32 ++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index cf6c1c1e..dcde647d 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -39,6 +39,10 @@ public:
>  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
>  	const std::vector<Size> &sizes() const { return sizes_; }
>  	Size resolution() const;
> +	const std::vector<uint8_t> &testPatternModes() const
> +	{
> +		return testPatternModes_;
> +	}
>  
>  	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
>  				      const Size &size) const;
> @@ -66,6 +70,8 @@ private:
>  	void initVimcDefaultProperties();
>  	void initStaticProperties();
>  	int initProperties();
> +	void initTestPatternModes(
> +		const std::map<uint8_t, uint8_t> &testPatternModeMap);
>  
>  	const MediaEntity *entity_;
>  	std::unique_ptr<V4L2Subdevice> subdev_;
> @@ -77,6 +83,7 @@ private:
>  	V4L2Subdevice::Formats formats_;
>  	std::vector<unsigned int> mbusCodes_;
>  	std::vector<Size> sizes_;
> +	std::vector<uint8_t> testPatternModes_;
>  
>  	Size pixelArraySize_;
>  	Rectangle activeArea_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 0fb8a258..44dd5988 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -297,6 +297,30 @@ void CameraSensor::initVimcDefaultProperties()
>  	activeArea_ = Rectangle(pixelArraySize_);
>  }
>  
> +void CameraSensor::initTestPatternModes(
> +	const std::map<uint8_t, uint8_t> &testPatternModeMap)
> +{
> +	const auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);
> +	if (v4l2TestPattern == controls().end()) {
> +		LOG(CameraSensor, Debug) << "No static test pattern map for \'"
> +					 << model() << "\'";
> +		return;
> +	}
> +
> +	for (const ControlValue &value : v4l2TestPattern->second.values()) {
> +		const int32_t index = value.get<int32_t>();
> +
> +		const auto it = testPatternModeMap.find(index);
> +		if (it == testPatternModeMap.end()) {
> +			LOG(CameraSensor, Debug)
> +				<< "Test pattern mode " << index << " ignored";
> +			continue;
> +		}
> +
> +		testPatternModes_.push_back(it->second);
> +	}
> +}

Could you please move this function below initProperties(), to match the
declaration order in the class ? I would actually move it right after
initStaticProperties() and update the class definition accordingly.

> +
>  void CameraSensor::initStaticProperties()
>  {
>  	const CameraSensorProperties *props = CameraSensorProperties::get(model_);
> @@ -305,6 +329,8 @@ void CameraSensor::initStaticProperties()
>  
>  	/* Register the properties retrieved from the sensor database. */
>  	properties_.set(properties::UnitCellSize, props->unitCellSize);
> +
> +	initTestPatternModes(props->testPatternModeMap);
>  }
>  
>  int CameraSensor::initProperties()
> @@ -469,6 +495,12 @@ Size CameraSensor::resolution() const
>  	return std::min(sizes_.back(), activeArea_.size());
>  }
>  
> +/**
> + * \fn CameraSensor::testPatternModes()
> + * \brief Retrieve all the supported test pattern modes of the camera sensor

I'd add

 *
 * The test pattern mode values correspond to the controls::TestPattern control.
 *

as I had to read the code to see if it was V4L2 or libcamera control
values.

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

> + * \return The list of test pattern modes
> + */
> +
>  /**
>   * \brief Retrieve the best sensor format for a desired output
>   * \param[in] mbusCodes The list of acceptable media bus codes

-- 
Regards,

Laurent Pinchart