[v4,19/20] libcamera: camera_sensor: Add parameter to limit returned sensor size
diff mbox series

Message ID 20241216154124.203650-20-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • rkisp1: Fix aspect ratio and ScalerCrop
Related show

Commit Message

Stefan Klug Dec. 16, 2024, 3:40 p.m. UTC 
The getFormat function takes the aspect ratio and the area of the
requested size into account when choosing the best sensor size. In case
the sensor is connected to an rkisp1 the maximum supported frame size of
the ISP is another constraining factor for the selection of the best
format. Add a maxSize parameter to support such a constraint.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---

Changes in v4:
- Collected tags

Changes in v3:
- Added this patch
---
 include/libcamera/internal/camera_sensor.h    | 2 +-
 src/libcamera/sensor/camera_sensor.cpp        | 3 +++
 src/libcamera/sensor/camera_sensor_legacy.cpp | 9 +++++++--
 3 files changed, 11 insertions(+), 3 deletions(-)

Comments

Jacopo Mondi Dec. 16, 2024, 6:37 p.m. UTC | #1 
Hi Stefan

On Mon, Dec 16, 2024 at 04:40:59PM +0100, Stefan Klug wrote:
> The getFormat function takes the aspect ratio and the area of the
> requested size into account when choosing the best sensor size. In case
> the sensor is connected to an rkisp1 the maximum supported frame size of
> the ISP is another constraining factor for the selection of the best
> format. Add a maxSize parameter to support such a constraint.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

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

Thanks
  j

>
> ---
>
> Changes in v4:
> - Collected tags
>
> Changes in v3:
> - Added this patch
> ---
>  include/libcamera/internal/camera_sensor.h    | 2 +-
>  src/libcamera/sensor/camera_sensor.cpp        | 3 +++
>  src/libcamera/sensor/camera_sensor_legacy.cpp | 9 +++++++--
>  3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d030e254a552..605ea8136900 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -53,7 +53,7 @@ public:
>
>  	virtual V4L2SubdeviceFormat
>  	getFormat(const std::vector<unsigned int> &mbusCodes,
> -		  const Size &size) const = 0;
> +		  const Size &size, const Size maxSize = Size()) const = 0;
>  	virtual int setFormat(V4L2SubdeviceFormat *format,
>  			      Transform transform = Transform::Identity) = 0;
>  	virtual int tryFormat(V4L2SubdeviceFormat *format) const = 0;
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index 208a1603cb32..a131ac224ec0 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -116,6 +116,7 @@ CameraSensor::~CameraSensor() = default;
>   * \brief Retrieve the best sensor format for a desired output
>   * \param[in] mbusCodes The list of acceptable media bus codes
>   * \param[in] size The desired size
> + * \param[in] maxSize The maximum size
>   *
>   * Media bus codes are selected from \a mbusCodes, which lists all acceptable
>   * codes in decreasing order of preference. Media bus codes supported by the
> @@ -134,6 +135,8 @@ CameraSensor::~CameraSensor() = default;
>   *   bandwidth.
>   * - The desired \a size shall be supported by one of the media bus code listed
>   *   in \a mbusCodes.
> + * - The desired \a size shall fit into the maximum size \a maxSize if it is not
> + *   null.
>   *
>   * When multiple media bus codes can produce the same size, the code at the
>   * lowest position in \a mbusCodes is selected.
> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
> index 17d6fa680e39..32989c19c019 100644
> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
> @@ -74,7 +74,8 @@ public:
>  	Size resolution() const override;
>
>  	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
> -				      const Size &size) const override;
> +				      const Size &size,
> +				      const Size maxSize) const override;
>  	int setFormat(V4L2SubdeviceFormat *format,
>  		      Transform transform = Transform::Identity) override;
>  	int tryFormat(V4L2SubdeviceFormat *format) const override;
> @@ -699,7 +700,7 @@ Size CameraSensorLegacy::resolution() const
>
>  V4L2SubdeviceFormat
>  CameraSensorLegacy::getFormat(const std::vector<unsigned int> &mbusCodes,
> -			      const Size &size) const
> +			      const Size &size, Size maxSize) const
>  {
>  	unsigned int desiredArea = size.width * size.height;
>  	unsigned int bestArea = UINT_MAX;
> @@ -716,6 +717,10 @@ CameraSensorLegacy::getFormat(const std::vector<unsigned int> &mbusCodes,
>  		for (const SizeRange &range : formats->second) {
>  			const Size &sz = range.max;
>
> +			if (!maxSize.isNull() &&
> +			    (sz.width > maxSize.width || sz.height > maxSize.height))
> +				continue;
> +
>  			if (sz.width < size.width || sz.height < size.height)
>  				continue;
>
> --
> 2.43.0
>
Laurent Pinchart Dec. 17, 2024, 9:05 p.m. UTC | #2 
On Mon, Dec 16, 2024 at 04:40:59PM +0100, Stefan Klug wrote:
> The getFormat function takes the aspect ratio and the area of the
> requested size into account when choosing the best sensor size. In case
> the sensor is connected to an rkisp1 the maximum supported frame size of
> the ISP is another constraining factor for the selection of the best
> format. Add a maxSize parameter to support such a constraint.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> 
> Changes in v4:
> - Collected tags
> 
> Changes in v3:
> - Added this patch
> ---
>  include/libcamera/internal/camera_sensor.h    | 2 +-
>  src/libcamera/sensor/camera_sensor.cpp        | 3 +++
>  src/libcamera/sensor/camera_sensor_legacy.cpp | 9 +++++++--
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d030e254a552..605ea8136900 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -53,7 +53,7 @@ public:
>  
>  	virtual V4L2SubdeviceFormat
>  	getFormat(const std::vector<unsigned int> &mbusCodes,
> -		  const Size &size) const = 0;
> +		  const Size &size, const Size maxSize = Size()) const = 0;

This feels like a bit of a hack. That's fine for now, the CameraSensor
API is made of hacks. We'll clean it up later.

>  	virtual int setFormat(V4L2SubdeviceFormat *format,
>  			      Transform transform = Transform::Identity) = 0;
>  	virtual int tryFormat(V4L2SubdeviceFormat *format) const = 0;
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index 208a1603cb32..a131ac224ec0 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -116,6 +116,7 @@ CameraSensor::~CameraSensor() = default;
>   * \brief Retrieve the best sensor format for a desired output
>   * \param[in] mbusCodes The list of acceptable media bus codes
>   * \param[in] size The desired size
> + * \param[in] maxSize The maximum size
>   *
>   * Media bus codes are selected from \a mbusCodes, which lists all acceptable
>   * codes in decreasing order of preference. Media bus codes supported by the
> @@ -134,6 +135,8 @@ CameraSensor::~CameraSensor() = default;
>   *   bandwidth.
>   * - The desired \a size shall be supported by one of the media bus code listed
>   *   in \a mbusCodes.
> + * - The desired \a size shall fit into the maximum size \a maxSize if it is not
> + *   null.
>   *
>   * When multiple media bus codes can produce the same size, the code at the
>   * lowest position in \a mbusCodes is selected.
> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
> index 17d6fa680e39..32989c19c019 100644
> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
> @@ -74,7 +74,8 @@ public:
>  	Size resolution() const override;
>  
>  	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
> -				      const Size &size) const override;
> +				      const Size &size,
> +				      const Size maxSize) const override;
>  	int setFormat(V4L2SubdeviceFormat *format,
>  		      Transform transform = Transform::Identity) override;
>  	int tryFormat(V4L2SubdeviceFormat *format) const override;
> @@ -699,7 +700,7 @@ Size CameraSensorLegacy::resolution() const
>  
>  V4L2SubdeviceFormat
>  CameraSensorLegacy::getFormat(const std::vector<unsigned int> &mbusCodes,
> -			      const Size &size) const
> +			      const Size &size, Size maxSize) const
>  {
>  	unsigned int desiredArea = size.width * size.height;
>  	unsigned int bestArea = UINT_MAX;
> @@ -716,6 +717,10 @@ CameraSensorLegacy::getFormat(const std::vector<unsigned int> &mbusCodes,
>  		for (const SizeRange &range : formats->second) {
>  			const Size &sz = range.max;
>  
> +			if (!maxSize.isNull() &&
> +			    (sz.width > maxSize.width || sz.height > maxSize.height))
> +				continue;
> +
>  			if (sz.width < size.width || sz.height < size.height)
>  				continue;
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
index d030e254a552..605ea8136900 100644
--- a/include/libcamera/internal/camera_sensor.h
+++ b/include/libcamera/internal/camera_sensor.h
@@ -53,7 +53,7 @@  public:
 
 	virtual V4L2SubdeviceFormat
 	getFormat(const std::vector<unsigned int> &mbusCodes,
-		  const Size &size) const = 0;
+		  const Size &size, const Size maxSize = Size()) const = 0;
 	virtual int setFormat(V4L2SubdeviceFormat *format,
 			      Transform transform = Transform::Identity) = 0;
 	virtual int tryFormat(V4L2SubdeviceFormat *format) const = 0;
diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
index 208a1603cb32..a131ac224ec0 100644
--- a/src/libcamera/sensor/camera_sensor.cpp
+++ b/src/libcamera/sensor/camera_sensor.cpp
@@ -116,6 +116,7 @@  CameraSensor::~CameraSensor() = default;
  * \brief Retrieve the best sensor format for a desired output
  * \param[in] mbusCodes The list of acceptable media bus codes
  * \param[in] size The desired size
+ * \param[in] maxSize The maximum size
  *
  * Media bus codes are selected from \a mbusCodes, which lists all acceptable
  * codes in decreasing order of preference. Media bus codes supported by the
@@ -134,6 +135,8 @@  CameraSensor::~CameraSensor() = default;
  *   bandwidth.
  * - The desired \a size shall be supported by one of the media bus code listed
  *   in \a mbusCodes.
+ * - The desired \a size shall fit into the maximum size \a maxSize if it is not
+ *   null.
  *
  * When multiple media bus codes can produce the same size, the code at the
  * lowest position in \a mbusCodes is selected.
diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
index 17d6fa680e39..32989c19c019 100644
--- a/src/libcamera/sensor/camera_sensor_legacy.cpp
+++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
@@ -74,7 +74,8 @@  public:
 	Size resolution() const override;
 
 	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
-				      const Size &size) const override;
+				      const Size &size,
+				      const Size maxSize) const override;
 	int setFormat(V4L2SubdeviceFormat *format,
 		      Transform transform = Transform::Identity) override;
 	int tryFormat(V4L2SubdeviceFormat *format) const override;
@@ -699,7 +700,7 @@  Size CameraSensorLegacy::resolution() const
 
 V4L2SubdeviceFormat
 CameraSensorLegacy::getFormat(const std::vector<unsigned int> &mbusCodes,
-			      const Size &size) const
+			      const Size &size, Size maxSize) const
 {
 	unsigned int desiredArea = size.width * size.height;
 	unsigned int bestArea = UINT_MAX;
@@ -716,6 +717,10 @@  CameraSensorLegacy::getFormat(const std::vector<unsigned int> &mbusCodes,
 		for (const SizeRange &range : formats->second) {
 			const Size &sz = range.max;
 
+			if (!maxSize.isNull() &&
+			    (sz.width > maxSize.width || sz.height > maxSize.height))
+				continue;
+
 			if (sz.width < size.width || sz.height < size.height)
 				continue;