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

21413 diff mbox series

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@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@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(-)

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@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Tested-by: Kieran Bingham <kieran.bingham@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@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
>

Quoting Jacopo Mondi (2024-09-30 07:08:50)
> 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@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Tested-by: Kieran Bingham <kieran.bingham@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@ideasonboard.com>

I don't know where the current output of the raw stream sizes gets
identified from, but I assume this is the same list, and should be
common somehow?

Ulitmately, we should have a way in the future to report these to the
application, along with a representation of what the field of view is
against the sensor native pixel array size.

So I think this could get reworked when we handle that ... which I hope
will be somewhere on the horizon coming into view ;-)

--
Kieran


> 
> > +{
> > +     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
> >