[libcamera-devel,v4,1/7] libcamera: pipeline: raspberrypi: Revert "Set sensor default orientation before configure()"

Message ID 20200828144110.17303-2-david.plowman@raspberrypi.com
State Accepted
Headers show
Series
  • 2D transforms
Related show

Commit Message

David Plowman Aug. 28, 2020, 2:41 p.m. UTC
This reverts commit 1e8c91b65695449c5246d17ba7dc439c8058b781.

Now that we shall be implementing application-defined 2D transforms
it's no longer possible to set the sensor orientation so early on. We
have to wait until we have the CameraConfiguration object as that's
where the application puts its choice of transform.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Kieran Bingham Aug. 28, 2020, 2:57 p.m. UTC | #1
Hi David,

On 28/08/2020 15:41, David Plowman wrote:
> This reverts commit 1e8c91b65695449c5246d17ba7dc439c8058b781.
> 
> Now that we shall be implementing application-defined 2D transforms
> it's no longer possible to set the sensor orientation so early on. We
> have to wait until we have the CameraConfiguration object as that's
> where the application puts its choice of transform.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

Well that didn't last long ;-S

The only reason I can think of that this 'might' be better to get
squashed with the next patch is if we consider that this now introduces
a regression - but I don't think that's too much of an issue.

So, if we don't decide to squash:

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


> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index c1451e7..42c9caa 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -960,13 +960,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  	/* Initialize the camera properties. */
>  	data->properties_ = data->sensor_->properties();
>  
> -	/* Configure the H/V flip controls based on the sensor rotation. */
> -	ControlList ctrls(data->unicam_[Unicam::Image].dev()->controls());
> -	int32_t rotation = data->properties_.get(properties::Rotation);
> -	ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
> -	ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));
> -	data->unicam_[Unicam::Image].dev()->setControls(&ctrls);
> -
>  	/*
>  	 * List the available output streams.
>  	 * Currently cannot do Unicam streams!
> @@ -1171,6 +1164,13 @@ int RPiCameraData::configureIPA()
>  					      { V4L2_CID_EXPOSURE, result.data[1] } });
>  			sensorMetadata_ = result.data[2];
>  		}
> +
> +		/* Configure the H/V flip controls based on the sensor rotation. */
> +		ControlList ctrls(unicam_[Unicam::Image].dev()->controls());
> +		int32_t rotation = sensor_->properties().get(properties::Rotation);
> +		ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
> +		ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));
> +		unicam_[Unicam::Image].dev()->setControls(&ctrls);
>  	}
>  
>  	if (result.operation & RPI_IPA_CONFIG_SENSOR) {
>

Patch

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index c1451e7..42c9caa 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -960,13 +960,6 @@  bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
 	/* Initialize the camera properties. */
 	data->properties_ = data->sensor_->properties();
 
-	/* Configure the H/V flip controls based on the sensor rotation. */
-	ControlList ctrls(data->unicam_[Unicam::Image].dev()->controls());
-	int32_t rotation = data->properties_.get(properties::Rotation);
-	ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
-	ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));
-	data->unicam_[Unicam::Image].dev()->setControls(&ctrls);
-
 	/*
 	 * List the available output streams.
 	 * Currently cannot do Unicam streams!
@@ -1171,6 +1164,13 @@  int RPiCameraData::configureIPA()
 					      { V4L2_CID_EXPOSURE, result.data[1] } });
 			sensorMetadata_ = result.data[2];
 		}
+
+		/* Configure the H/V flip controls based on the sensor rotation. */
+		ControlList ctrls(unicam_[Unicam::Image].dev()->controls());
+		int32_t rotation = sensor_->properties().get(properties::Rotation);
+		ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
+		ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));
+		unicam_[Unicam::Image].dev()->setControls(&ctrls);
 	}
 
 	if (result.operation & RPI_IPA_CONFIG_SENSOR) {