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

[Jacopo Mondi]

Hi Umang

On Thu, Sep 26, 2024 at 07:02:36PM 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 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 | 51 ++++++++++++++++++-
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  8 +++
>  2 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index c49017d1..2421d06c 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -126,12 +126,59 @@ 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)
> +{
> +	auto iter = sensorSizesMap_.find(sensor);
> +	if (iter != sensorSizesMap_.end() && !iter->second.empty())
> +		return iter->second.back();
> +
> +	sensorSizesMap_.emplace(sensor, std::vector<Size>());
> +
> +	std::vector<Size> sensorSizes;
> +	const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> +	for (const auto it : mbusCodes) {
> +		std::vector<Size> sizes = sensor->sizes(it);
> +		for (Size sz : sizes)

Can't you filter out sizes that are larger than max sizes ?

So you can populate sensorSizesMap_.second directly avoiding copies ?

> +			sensorSizes.push_back(sz);
> +	}

Can't we build this map once at populateFormats() time and solely
return the max supported size when needed ?

> +
> +	std::sort(sensorSizes.begin(), sensorSizes.end());
> +
> +	/* Remove duplicates. */
> +	auto last = std::unique(sensorSizes.begin(), sensorSizes.end());
> +	sensorSizes.erase(last, sensorSizes.end());
> +
> +	/* Discard any sizes that the pipeline is unable to support. */
> +	for (auto sz : sensorSizes) {
> +		if (sz.width > maxResolution_.width ||
> +		    sz.height > maxResolution_.height)
> +			continue;
> +
> +		sensorSizesMap_[sensor].push_back(sz);
> +	}
> +
> +	return sensorSizesMap_[sensor].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 +267,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
>