[PATCH] libcamera: rkisp1: Eliminate hard-coded resizer limits

[Jacopo Mondi]

Hi Umang

On Tue, Sep 24, 2024 at 12:43:41PM GMT, Umang Jain wrote:
> The resizer limits are dynamically queried in populateFormats() and

Is this the resizer ?

minResolution_ and maxResolution_ are populated from the list of
formats from video_ and if I recall correctly RkISP1 has an explicit
resizer entity, right ?

> assigned to minResolution_ and maxResolution_. Therefore, initializing
> these values in the constructor is unnecessary.
>
> This change allows us to remove the hard-coded max/min resolution limits
> from RkISP1Path, simplifying its constructor further.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 21 ++++++-------------
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  3 +--
>  2 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index c49017d1..8015c58c 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -54,11 +54,8 @@ const std::map<PixelFormat, uint32_t> formatToMediaBus = {
>
>  } /* namespace */
>
> -RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
> -		       const Size &minResolution, const Size &maxResolution)
> -	: name_(name), running_(false), formats_(formats),
> -	  minResolution_(minResolution), maxResolution_(maxResolution),
> -	  link_(nullptr)
> +RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats)
> +	: name_(name), running_(false), formats_(formats), link_(nullptr)
>  {
>  }
>
> @@ -435,12 +432,10 @@ void RkISP1Path::stop()
>  }
>
>  /*
> - * \todo Remove the hardcoded resolutions and formats once all users will have
> - * migrated to a recent enough kernel.
> + * \todo Remove the hardcoded formats once all users will have migrated to a
> + * recent enough kernel.
>   */
>  namespace {
> -constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
> -constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
>  constexpr std::array<PixelFormat, 18> RKISP1_RSZ_MP_FORMATS{
>  	formats::YUYV,
>  	formats::NV16,
> @@ -462,8 +457,6 @@ constexpr std::array<PixelFormat, 18> RKISP1_RSZ_MP_FORMATS{
>  	formats::SRGGB12,
>  };
>
> -constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
> -constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };
>  constexpr std::array<PixelFormat, 8> RKISP1_RSZ_SP_FORMATS{
>  	formats::YUYV,
>  	formats::NV16,
> @@ -477,14 +470,12 @@ constexpr std::array<PixelFormat, 8> RKISP1_RSZ_SP_FORMATS{
>  } /* namespace */
>
>  RkISP1MainPath::RkISP1MainPath()
> -	: RkISP1Path("main", RKISP1_RSZ_MP_FORMATS,
> -		     RKISP1_RSZ_MP_SRC_MIN, RKISP1_RSZ_MP_SRC_MAX)
> +	: RkISP1Path("main", RKISP1_RSZ_MP_FORMATS)
>  {
>  }
>
>  RkISP1SelfPath::RkISP1SelfPath()
> -	: RkISP1Path("self", RKISP1_RSZ_SP_FORMATS,
> -		     RKISP1_RSZ_SP_SRC_MIN, RKISP1_RSZ_SP_SRC_MAX)
> +	: RkISP1Path("self", RKISP1_RSZ_SP_FORMATS)
>  {
>  }
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index 08edefec..d33471e4 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -32,8 +32,7 @@ struct V4L2SubdeviceFormat;
>  class RkISP1Path
>  {
>  public:
> -	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
> -		   const Size &minResolution, const Size &maxResolution);
> +	RkISP1Path(const char *name, const Span<const PixelFormat> &formats);

However I think this is safe. minResolution_ and maxResolution_ are
populated at ::init() time and the class is not usable before that
function has been called.

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>
>  	bool init(MediaDevice *media);
>
> --
> 2.45.2
>