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

Umang Jain umang.jain at ideasonboard.com
Thu Sep 26 17:42:49 CEST 2024


Hi Jacopo,

On 26/09/24 7:41 pm, Jacopo Mondi wrote:
> 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 ?

If I try to directly populate sensorSizesMap_ with the max size filter, 
it might have duplicated sizes entries (same size for different codes).

That's why, I opted a temporary vector sensorSizes here, is used to 
collect all sizes for the formats that the sensor supports.

Then later sorted, de-duplicated and filtered, before populating the 
sensorSizeMap_.second

>
>> +			sensorSizes.push_back(sz);
>> +	}
> Can't we build this map once at populateFormats() time and solely

The map is only built once. Checkout the early return at the start of 
the function
> return the max supported size when needed ?

I don't think you can do at populateFormats() as it is called in init()

And RkISP1Path::init() is called in match() for RkISP1 pipeline handler, 
before the camera is even created.
>
>> +
>> +	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
>>



More information about the libcamera-devel mailing list