[PATCH v4] pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline

[Jacopo Mondi]

On Mon, Sep 30, 2024 at 11:10:25AM GMT, Umang Jain wrote:
> It is possible that the maximum sensor size (returned by
> CameraSensor::resolution()) is not supported by the pipeline. In such
> cases, a filter function is required to filter out resolutions of the
> camera sensor, which cannot be supported by the pipeline.
>
> Introduce the filter function filterSensorResolution() in RkISP1Path
> class and use it for validate() and generateConfiguration(). The
> maximum sensor resolution is picked from that filtered set of
> resolutions instead of CameraSensor::resolution().
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> Changes in v4:
> - Include Jacopo's suggestion(v3) for sorting in-place.
>
> Changes v3:
> - None, just a resent over latest master
> - Split out from [v2,0/2] pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline
>   so that, this can make independent progress.
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 42 ++++++++++++++++++-
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  8 ++++
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index c49017d1..90c49d99 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -126,12 +126,50 @@ void RkISP1Path::populateFormats()
>  	}
>  }
>
> +/**
> + * \brief Filter the sensor resolutions that can be supported
> + * \param[in] sensor The camera sensor
> + *
> + * This function retrieves all the sizes supported by the sensor and
> + * filters all the resolutions that can be supported on the pipeline.
> + * It is possible that the sensor's maximum output resolution is higher
> + * than the ISP maximum input. In that case, this function filters out all
> + * the resolution incapable of being supported and returns the maximum
> + * sensor resolution that can be supported by the pipeline.
> + *
> + * \return Maximum sensor size supported on the pipeline
> + */
> +Size RkISP1Path::filterSensorResolution(const CameraSensor *sensor)

Still no sure about the function name and use cases for storing all
resolutions.

Not a big deal though, let's land this one
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

> +{
> +	auto iter = sensorSizesMap_.find(sensor);
> +	if (iter != sensorSizesMap_.end())
> +		return iter->second.back();
> +
> +	std::vector<Size> &sizes = sensorSizesMap_[sensor];
> +	for (unsigned int code : sensor->mbusCodes()) {
> +		for (const Size &size : sensor->sizes(code)) {
> +			if (size.width > maxResolution_.width ||
> +			    size.height > maxResolution_.height)
> +				continue;
> +
> +			sizes.push_back(size);
> +		}
> +	}
> +
> +	/* Sort in increasing order and remove duplicates. */
> +	std::sort(sizes.begin(), sizes.end());
> +	auto last = std::unique(sizes.begin(), sizes.end());
> +	sizes.erase(last, sizes.end());
> +
> +	return sizes.back();
> +}
> +
>  StreamConfiguration
>  RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,
>  				  StreamRole role)
>  {
>  	const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> -	const Size &resolution = sensor->resolution();
> +	Size resolution = filterSensorResolution(sensor);
>
>  	/* Min and max resolutions to populate the available stream formats. */
>  	Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
> @@ -220,7 +258,7 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,
>  						 StreamConfiguration *cfg)
>  {
>  	const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> -	const Size &resolution = sensor->resolution();
> +	Size resolution = filterSensorResolution(sensor);
>
>  	const StreamConfiguration reqCfg = *cfg;
>  	CameraConfiguration::Status status = CameraConfiguration::Valid;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index 08edefec..9f75fe1f 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -7,6 +7,7 @@
>
>  #pragma once
>
> +#include <map>
>  #include <memory>
>  #include <set>
>  #include <vector>
> @@ -63,6 +64,7 @@ public:
>
>  private:
>  	void populateFormats();
> +	Size filterSensorResolution(const CameraSensor *sensor);
>
>  	static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;
>
> @@ -77,6 +79,12 @@ private:
>  	std::unique_ptr<V4L2Subdevice> resizer_;
>  	std::unique_ptr<V4L2VideoDevice> video_;
>  	MediaLink *link_;
> +
> +	/*
> +	 * Map from camera sensors to the sizes (in increasing order),
> +	 * which are guaranteed to be supported by the pipeline.
> +	 */
> +	std::map<const CameraSensor *, std::vector<Size>> sensorSizesMap_;
>  };
>
>  class RkISP1MainPath : public RkISP1Path
> --
> 2.45.0
>