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

[Kieran Bingham]

Quoting Umang Jain (2024-09-24 08:13:41)
> The resizer limits are dynamically queried in populateFormats() and
> 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.
> 

This looks reasonable to me.

I can see that the sizes are set correctly at PopulateFormats, and as
the types are a Size, there is no possibility of use of uninitialized
data, so this all looks good to me.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> 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);
>  
>         bool init(MediaDevice *media);
>  
> -- 
> 2.45.2
>