[libcamera-devel,v2,4/5] libcamera: rkisp1: Add support for Transform
diff mbox series

Message ID 20230114194712.23272-5-jacopo.mondi@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: camera_sensor: Centralize flips handling
Related show

Commit Message

Jacopo Mondi Jan. 14, 2023, 7:47 p.m. UTC
Add support for Transform to the RkISP1 pipeline handler.

The pipeline rotates using the sensor's V/H flips, hence use the
CameraSensor helpers to handle transformation requests from
applications.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Kieran Bingham Jan. 30, 2023, 10:22 a.m. UTC | #1
Hi Jacopo,

Quoting Jacopo Mondi via libcamera-devel (2023-01-14 19:47:11)
> Add support for Transform to the RkISP1 pipeline handler.
> 
> The pipeline rotates using the sensor's V/H flips, hence use the
> CameraSensor helpers to handle transformation requests from
> applications.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c27b3ef9bd8e..b81ad647be79 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -23,6 +23,10 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/formats.h>
>  #include <libcamera/framebuffer.h>
> +#include <libcamera/request.h>
> +#include <libcamera/stream.h>
> +#include <libcamera/transform.h>
> +
>  #include <libcamera/ipa/core_ipa_interface.h>
>  #include <libcamera/ipa/rkisp1_ipa_interface.h>
>  #include <libcamera/ipa/rkisp1_ipa_proxy.h>
> @@ -469,17 +473,17 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  
>         status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);
>  
> -       if (transform != Transform::Identity) {
> -               transform = Transform::Identity;
> -               status = Adjusted;
> -       }
> -
>         /* Cap the number of entries to the available streams. */
>         if (config_.size() > pathCount) {
>                 config_.resize(pathCount);
>                 status = Adjusted;
>         }
>  
> +       Transform requestedTransform = transform;
> +       Transform combined = sensor->validateTransform(&transform);
> +       if (transform != requestedTransform)
> +               status = Adjusted;
> +

I'm so tempted to look into how we could mock up devices so we can
'fake' devices to libcamera pipeline handlers. Things like this almost
warrant having tests set up to validate every orientation of a sensor,
against every orientation asked for by an application...

Maybe one to add to the GSoC list. But Laurent isn't convinced.

Anyway, in the absence of being able to test this otherwise:


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

>         /*
>          * Simultaneous capture of raw and processed streams isn't possible. If
>          * there is any raw stream, cap the number of streams to one.
> @@ -589,6 +593,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>         if (sensorFormat_.size.isNull())
>                 sensorFormat_.size = sensor->resolution();
>  
> +       sensorFormat_.transform = combined;
> +
>         return status;
>  }
>  
> -- 
> 2.39.0
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index c27b3ef9bd8e..b81ad647be79 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -23,6 +23,10 @@ 
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
 #include <libcamera/framebuffer.h>
+#include <libcamera/request.h>
+#include <libcamera/stream.h>
+#include <libcamera/transform.h>
+
 #include <libcamera/ipa/core_ipa_interface.h>
 #include <libcamera/ipa/rkisp1_ipa_interface.h>
 #include <libcamera/ipa/rkisp1_ipa_proxy.h>
@@ -469,17 +473,17 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 
 	status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);
 
-	if (transform != Transform::Identity) {
-		transform = Transform::Identity;
-		status = Adjusted;
-	}
-
 	/* Cap the number of entries to the available streams. */
 	if (config_.size() > pathCount) {
 		config_.resize(pathCount);
 		status = Adjusted;
 	}
 
+	Transform requestedTransform = transform;
+	Transform combined = sensor->validateTransform(&transform);
+	if (transform != requestedTransform)
+		status = Adjusted;
+
 	/*
 	 * Simultaneous capture of raw and processed streams isn't possible. If
 	 * there is any raw stream, cap the number of streams to one.
@@ -589,6 +593,8 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 	if (sensorFormat_.size.isNull())
 		sensorFormat_.size = sensor->resolution();
 
+	sensorFormat_.transform = combined;
+
 	return status;
 }